[call-v3] Remove CallFactory (#36226)

This was a concept that I thought we'd need, but as work has continued it's clear that the right api is CallDestination (or variants thereof)

Closes #36226

COPYBARA_INTEGRATE_REVIEW=https://github.com/grpc/grpc/pull/36226 from ctiller:call_factory c30a893f3a
PiperOrigin-RevId: 621255162
pull/36234/head
Craig Tiller 12 months ago committed by Copybara-Service
parent f99bc55ffe
commit e78ec82a2c
  1. 3
      BUILD
  2. 2
      CMakeLists.txt
  3. 1
      Makefile
  4. 2
      Package.swift
  5. 4
      build_autogenerated.yaml
  6. 1
      config.m4
  7. 1
      config.w32
  8. 2
      gRPC-C++.podspec
  9. 3
      gRPC-Core.podspec
  10. 2
      grpc.gemspec
  11. 2
      package.xml
  12. 20
      src/core/BUILD
  13. 18
      src/core/lib/surface/legacy_channel.cc
  14. 16
      src/core/lib/surface/legacy_channel.h
  15. 41
      src/core/lib/transport/call_factory.cc
  16. 56
      src/core/lib/transport/call_factory.h
  17. 1
      src/python/grpcio/grpc_core_dependencies.py
  18. 2
      tools/doxygen/Doxyfile.c++.internal
  19. 2
      tools/doxygen/Doxyfile.core.internal

@ -1779,7 +1779,7 @@ grpc_cc_library(
"ref_counted_ptr",
"stats",
"//src/core:arena",
"//src/core:call_factory",
"//src/core:call_size_estimator",
"//src/core:channel_args",
"//src/core:channel_fwd",
"//src/core:channel_init",
@ -1790,6 +1790,7 @@ grpc_cc_library(
"//src/core:init_internally",
"//src/core:iomgr_fwd",
"//src/core:metrics",
"//src/core:resource_quota",
"//src/core:slice",
"//src/core:stats_data",
"//src/core:time",

2
CMakeLists.txt generated

@ -2510,7 +2510,6 @@ add_library(grpc
src/core/lib/surface/wait_for_cq_end_op.cc
src/core/lib/transport/batch_builder.cc
src/core/lib/transport/bdp_estimator.cc
src/core/lib/transport/call_factory.cc
src/core/lib/transport/call_filters.cc
src/core/lib/transport/call_final_info.cc
src/core/lib/transport/call_size_estimator.cc
@ -3236,7 +3235,6 @@ add_library(grpc_unsecure
src/core/lib/surface/wait_for_cq_end_op.cc
src/core/lib/transport/batch_builder.cc
src/core/lib/transport/bdp_estimator.cc
src/core/lib/transport/call_factory.cc
src/core/lib/transport/call_filters.cc
src/core/lib/transport/call_final_info.cc
src/core/lib/transport/call_size_estimator.cc

1
Makefile generated

@ -1407,7 +1407,6 @@ LIBGRPC_SRC = \
src/core/lib/surface/wait_for_cq_end_op.cc \
src/core/lib/transport/batch_builder.cc \
src/core/lib/transport/bdp_estimator.cc \
src/core/lib/transport/call_factory.cc \
src/core/lib/transport/call_filters.cc \
src/core/lib/transport/call_final_info.cc \
src/core/lib/transport/call_size_estimator.cc \

2
Package.swift generated

@ -1804,8 +1804,6 @@ let package = Package(
"src/core/lib/transport/batch_builder.h",
"src/core/lib/transport/bdp_estimator.cc",
"src/core/lib/transport/bdp_estimator.h",
"src/core/lib/transport/call_factory.cc",
"src/core/lib/transport/call_factory.h",
"src/core/lib/transport/call_filters.cc",
"src/core/lib/transport/call_filters.h",
"src/core/lib/transport/call_final_info.cc",

@ -1138,7 +1138,6 @@ libs:
- src/core/lib/surface/wait_for_cq_end_op.h
- src/core/lib/transport/batch_builder.h
- src/core/lib/transport/bdp_estimator.h
- src/core/lib/transport/call_factory.h
- src/core/lib/transport/call_filters.h
- src/core/lib/transport/call_final_info.h
- src/core/lib/transport/call_size_estimator.h
@ -1940,7 +1939,6 @@ libs:
- src/core/lib/surface/wait_for_cq_end_op.cc
- src/core/lib/transport/batch_builder.cc
- src/core/lib/transport/bdp_estimator.cc
- src/core/lib/transport/call_factory.cc
- src/core/lib/transport/call_filters.cc
- src/core/lib/transport/call_final_info.cc
- src/core/lib/transport/call_size_estimator.cc
@ -2606,7 +2604,6 @@ libs:
- src/core/lib/surface/wait_for_cq_end_op.h
- src/core/lib/transport/batch_builder.h
- src/core/lib/transport/bdp_estimator.h
- src/core/lib/transport/call_factory.h
- src/core/lib/transport/call_filters.h
- src/core/lib/transport/call_final_info.h
- src/core/lib/transport/call_size_estimator.h
@ -3026,7 +3023,6 @@ libs:
- src/core/lib/surface/wait_for_cq_end_op.cc
- src/core/lib/transport/batch_builder.cc
- src/core/lib/transport/bdp_estimator.cc
- src/core/lib/transport/call_factory.cc
- src/core/lib/transport/call_filters.cc
- src/core/lib/transport/call_final_info.cc
- src/core/lib/transport/call_size_estimator.cc

1
config.m4 generated

@ -782,7 +782,6 @@ if test "$PHP_GRPC" != "no"; then
src/core/lib/surface/wait_for_cq_end_op.cc \
src/core/lib/transport/batch_builder.cc \
src/core/lib/transport/bdp_estimator.cc \
src/core/lib/transport/call_factory.cc \
src/core/lib/transport/call_filters.cc \
src/core/lib/transport/call_final_info.cc \
src/core/lib/transport/call_size_estimator.cc \

1
config.w32 generated

@ -747,7 +747,6 @@ if (PHP_GRPC != "no") {
"src\\core\\lib\\surface\\wait_for_cq_end_op.cc " +
"src\\core\\lib\\transport\\batch_builder.cc " +
"src\\core\\lib\\transport\\bdp_estimator.cc " +
"src\\core\\lib\\transport\\call_factory.cc " +
"src\\core\\lib\\transport\\call_filters.cc " +
"src\\core\\lib\\transport\\call_final_info.cc " +
"src\\core\\lib\\transport\\call_size_estimator.cc " +

2
gRPC-C++.podspec generated

@ -1242,7 +1242,6 @@ Pod::Spec.new do |s|
'src/core/lib/surface/wait_for_cq_end_op.h',
'src/core/lib/transport/batch_builder.h',
'src/core/lib/transport/bdp_estimator.h',
'src/core/lib/transport/call_factory.h',
'src/core/lib/transport/call_filters.h',
'src/core/lib/transport/call_final_info.h',
'src/core/lib/transport/call_size_estimator.h',
@ -2512,7 +2511,6 @@ Pod::Spec.new do |s|
'src/core/lib/surface/wait_for_cq_end_op.h',
'src/core/lib/transport/batch_builder.h',
'src/core/lib/transport/bdp_estimator.h',
'src/core/lib/transport/call_factory.h',
'src/core/lib/transport/call_filters.h',
'src/core/lib/transport/call_final_info.h',
'src/core/lib/transport/call_size_estimator.h',

3
gRPC-Core.podspec generated

@ -1916,8 +1916,6 @@ Pod::Spec.new do |s|
'src/core/lib/transport/batch_builder.h',
'src/core/lib/transport/bdp_estimator.cc',
'src/core/lib/transport/bdp_estimator.h',
'src/core/lib/transport/call_factory.cc',
'src/core/lib/transport/call_factory.h',
'src/core/lib/transport/call_filters.cc',
'src/core/lib/transport/call_filters.h',
'src/core/lib/transport/call_final_info.cc',
@ -3293,7 +3291,6 @@ Pod::Spec.new do |s|
'src/core/lib/surface/wait_for_cq_end_op.h',
'src/core/lib/transport/batch_builder.h',
'src/core/lib/transport/bdp_estimator.h',
'src/core/lib/transport/call_factory.h',
'src/core/lib/transport/call_filters.h',
'src/core/lib/transport/call_final_info.h',
'src/core/lib/transport/call_size_estimator.h',

2
grpc.gemspec generated

@ -1806,8 +1806,6 @@ Gem::Specification.new do |s|
s.files += %w( src/core/lib/transport/batch_builder.h )
s.files += %w( src/core/lib/transport/bdp_estimator.cc )
s.files += %w( src/core/lib/transport/bdp_estimator.h )
s.files += %w( src/core/lib/transport/call_factory.cc )
s.files += %w( src/core/lib/transport/call_factory.h )
s.files += %w( src/core/lib/transport/call_filters.cc )
s.files += %w( src/core/lib/transport/call_filters.h )
s.files += %w( src/core/lib/transport/call_final_info.cc )

2
package.xml generated

@ -1788,8 +1788,6 @@
<file baseinstalldir="/" name="src/core/lib/transport/batch_builder.h" role="src" />
<file baseinstalldir="/" name="src/core/lib/transport/bdp_estimator.cc" role="src" />
<file baseinstalldir="/" name="src/core/lib/transport/bdp_estimator.h" role="src" />
<file baseinstalldir="/" name="src/core/lib/transport/call_factory.cc" role="src" />
<file baseinstalldir="/" name="src/core/lib/transport/call_factory.h" role="src" />
<file baseinstalldir="/" name="src/core/lib/transport/call_filters.cc" role="src" />
<file baseinstalldir="/" name="src/core/lib/transport/call_filters.h" role="src" />
<file baseinstalldir="/" name="src/core/lib/transport/call_final_info.cc" role="src" />

@ -7227,26 +7227,6 @@ grpc_cc_library(
],
)
grpc_cc_library(
name = "call_factory",
srcs = [
"lib/transport/call_factory.cc",
],
hdrs = [
"lib/transport/call_factory.h",
],
deps = [
"arena",
"call_size_estimator",
"call_spine",
"channel_args",
"ref_counted",
"resource_quota",
"//:gpr_platform",
"//:stats",
],
)
grpc_cc_library(
name = "call_destination",
hdrs = [

@ -48,6 +48,7 @@
#include "src/core/lib/iomgr/closure.h"
#include "src/core/lib/iomgr/error.h"
#include "src/core/lib/iomgr/exec_ctx.h"
#include "src/core/lib/resource_quota/resource_quota.h"
#include "src/core/lib/surface/call.h"
#include "src/core/lib/surface/channel.h"
#include "src/core/lib/surface/channel_init.h"
@ -55,7 +56,6 @@
#include "src/core/lib/surface/completion_queue.h"
#include "src/core/lib/surface/init_internally.h"
#include "src/core/lib/surface/lame_client.h"
#include "src/core/lib/transport/call_factory.h"
#include "src/core/lib/transport/transport.h"
namespace grpc_core {
@ -106,18 +106,6 @@ absl::StatusOr<OrphanablePtr<Channel>> LegacyChannel::Create(
builder.IsPromising(), std::move(target), args, std::move(*r));
}
namespace {
class NotReallyACallFactory final : public CallFactory {
public:
using CallFactory::CallFactory;
CallInitiator CreateCall(ClientMetadataHandle, Arena*) override {
Crash("NotReallyACallFactory::CreateCall should never be called");
}
};
} // namespace
LegacyChannel::LegacyChannel(bool is_client, bool is_promising,
std::string target,
const ChannelArgs& channel_args,
@ -126,7 +114,9 @@ LegacyChannel::LegacyChannel(bool is_client, bool is_promising,
is_client_(is_client),
is_promising_(is_promising),
channel_stack_(std::move(channel_stack)),
call_factory_(MakeRefCounted<NotReallyACallFactory>(channel_args)) {
allocator_(channel_args.GetObject<ResourceQuota>()
->memory_quota()
->CreateMemoryOwner()) {
// We need to make sure that grpc_shutdown() does not shut things down
// until after the channel is destroyed. However, the channel may not
// actually be destroyed by the time grpc_channel_destroy() returns,

@ -33,14 +33,14 @@
#include "src/core/lib/channel/channel_args.h"
#include "src/core/lib/channel/channel_fwd.h"
#include "src/core/lib/channel/channel_stack.h" // IWYU pragma: keep
#include "src/core/lib/debug/stats.h"
#include "src/core/lib/gprpp/ref_counted_ptr.h"
#include "src/core/lib/gprpp/time.h"
#include "src/core/lib/iomgr/iomgr_fwd.h"
#include "src/core/lib/slice/slice.h"
#include "src/core/lib/surface/channel.h"
#include "src/core/lib/surface/channel_stack_type.h"
#include "src/core/lib/transport/call_factory.h"
#include "src/core/lib/transport/transport.h"
#include "src/core/lib/transport/call_size_estimator.h"
namespace grpc_core {
@ -57,9 +57,14 @@ class LegacyChannel final : public Channel {
void Orphan() override;
Arena* CreateArena() override { return call_factory_->CreateArena(); }
Arena* CreateArena() override {
const size_t initial_size = call_size_estimator_.CallSizeEstimate();
global_stats().IncrementCallInitialSize(initial_size);
return Arena::Create(initial_size, &allocator_);
}
void DestroyArena(Arena* arena) override {
return call_factory_->DestroyArena(arena);
call_size_estimator_.UpdateCallSizeEstimate(arena->TotalUsedBytes());
arena->Destroy();
}
bool IsLame() const override;
@ -110,7 +115,8 @@ class LegacyChannel final : public Channel {
const bool is_client_;
const bool is_promising_;
RefCountedPtr<grpc_channel_stack> channel_stack_;
const RefCountedPtr<CallFactory> call_factory_;
CallSizeEstimator call_size_estimator_{1024};
grpc_event_engine::experimental::MemoryAllocator allocator_;
};
} // namespace grpc_core

@ -1,41 +0,0 @@
// Copyright 2024 gRPC authors.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
#include <grpc/support/port_platform.h>
#include "src/core/lib/transport/call_factory.h"
#include "src/core/lib/debug/stats.h"
#include "src/core/lib/resource_quota/resource_quota.h"
namespace grpc_core {
CallFactory::CallFactory(const ChannelArgs& args)
: call_size_estimator_(1024),
allocator_(args.GetObject<ResourceQuota>()
->memory_quota()
->CreateMemoryOwner()) {}
Arena* CallFactory::CreateArena() {
const size_t initial_size = call_size_estimator_.CallSizeEstimate();
global_stats().IncrementCallInitialSize(initial_size);
return Arena::Create(initial_size, &allocator_);
}
void CallFactory::DestroyArena(Arena* arena) {
call_size_estimator_.UpdateCallSizeEstimate(arena->TotalUsedBytes());
arena->Destroy();
}
} // namespace grpc_core

@ -1,56 +0,0 @@
// Copyright 2024 gRPC authors.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
#ifndef GRPC_SRC_CORE_LIB_TRANSPORT_CALL_FACTORY_H
#define GRPC_SRC_CORE_LIB_TRANSPORT_CALL_FACTORY_H
#include <grpc/support/port_platform.h>
#include "src/core/lib/channel/channel_args.h"
#include "src/core/lib/gprpp/ref_counted.h"
#include "src/core/lib/resource_quota/arena.h"
#include "src/core/lib/transport/call_size_estimator.h"
#include "src/core/lib/transport/call_spine.h"
namespace grpc_core {
// CallFactory creates calls.
class CallFactory : public RefCounted<CallFactory> {
public:
explicit CallFactory(const ChannelArgs& args);
// Create an arena for a call.
// We do this as a separate step so that servers can create arenas without
// creating the call into it - in the case that we have a HTTP/2 rapid reset
// like attack this saves a lot of cpu time.
Arena* CreateArena();
// Destroy an arena created by CreateArena.
// Updates the call size estimator so that we always create arenas of about
// the right size.
void DestroyArena(Arena* arena);
// Create a call. The call will be created in the given arena.
// It is the CallFactory's responsibility to ensure that the CallHandler
// associated with the call is eventually handled by something (typically a
// CallDestination, but this is not strictly required).
virtual CallInitiator CreateCall(ClientMetadataHandle md, Arena* arena) = 0;
private:
CallSizeEstimator call_size_estimator_;
MemoryAllocator allocator_;
};
} // namespace grpc_core
#endif // GRPC_SRC_CORE_LIB_TRANSPORT_CALL_FACTORY_H

@ -756,7 +756,6 @@ CORE_SOURCE_FILES = [
'src/core/lib/surface/wait_for_cq_end_op.cc',
'src/core/lib/transport/batch_builder.cc',
'src/core/lib/transport/bdp_estimator.cc',
'src/core/lib/transport/call_factory.cc',
'src/core/lib/transport/call_filters.cc',
'src/core/lib/transport/call_final_info.cc',
'src/core/lib/transport/call_size_estimator.cc',

@ -2805,8 +2805,6 @@ src/core/lib/transport/batch_builder.cc \
src/core/lib/transport/batch_builder.h \
src/core/lib/transport/bdp_estimator.cc \
src/core/lib/transport/bdp_estimator.h \
src/core/lib/transport/call_factory.cc \
src/core/lib/transport/call_factory.h \
src/core/lib/transport/call_filters.cc \
src/core/lib/transport/call_filters.h \
src/core/lib/transport/call_final_info.cc \

@ -2582,8 +2582,6 @@ src/core/lib/transport/batch_builder.cc \
src/core/lib/transport/batch_builder.h \
src/core/lib/transport/bdp_estimator.cc \
src/core/lib/transport/bdp_estimator.h \
src/core/lib/transport/call_factory.cc \
src/core/lib/transport/call_factory.h \
src/core/lib/transport/call_filters.cc \
src/core/lib/transport/call_filters.h \
src/core/lib/transport/call_final_info.cc \

Loading…
Cancel
Save