From 0ea1eeb4e75f63f3b0df9c5d33a1c1fd9ab32b29 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Tue, 15 Mar 2022 21:33:32 -0700 Subject: [PATCH] Expose channel stack type to builder (#29088) * split builder * expose channel stack type to builder * Automated change: Fix sanity tests * Update channel_stack_builder_impl.h * Update channel_init.h Co-authored-by: ctiller --- BUILD | 26 ++++- CMakeLists.txt | 2 + Makefile | 2 + build_autogenerated.yaml | 4 + config.m4 | 1 + config.w32 | 1 + gRPC-C++.podspec | 2 + gRPC-Core.podspec | 3 + grpc.gemspec | 2 + grpc.gyp | 2 + package.xml | 2 + .../ext/filters/client_channel/subchannel.cc | 6 +- src/core/lib/channel/channel_stack_builder.cc | 65 ----------- src/core/lib/channel/channel_stack_builder.h | 33 ++++-- .../lib/channel/channel_stack_builder_impl.cc | 102 ++++++++++++++++++ .../lib/channel/channel_stack_builder_impl.h | 48 +++++++++ src/core/lib/channel/connected_channel.h | 1 + src/core/lib/surface/channel.cc | 7 +- src/core/lib/surface/channel_init.cc | 5 +- src/core/lib/surface/channel_init.h | 8 +- src/python/grpcio/grpc_core_dependencies.py | 1 + .../channel/channel_stack_builder_test.cc | 4 +- .../channel/minimal_stack_is_minimal_test.cc | 7 +- .../core/end2end/tests/filter_causes_close.cc | 1 + test/core/end2end/tests/filter_context.cc | 1 + test/core/end2end/tests/filter_init_fails.cc | 1 + test/core/end2end/tests/filter_latency.cc | 1 + .../xds/xds_channel_stack_modifier_test.cc | 11 +- test/cpp/microbenchmarks/bm_call_create.cc | 3 +- tools/doxygen/Doxyfile.c++.internal | 2 + tools/doxygen/Doxyfile.core.internal | 2 + 31 files changed, 257 insertions(+), 99 deletions(-) create mode 100644 src/core/lib/channel/channel_stack_builder_impl.cc create mode 100644 src/core/lib/channel/channel_stack_builder_impl.h diff --git a/BUILD b/BUILD index 57a812031ba..fcd3f6cc596 100644 --- a/BUILD +++ b/BUILD @@ -1875,7 +1875,7 @@ grpc_cc_library( "src/core/lib/address_utils/parse_address.cc", "src/core/lib/backoff/backoff.cc", "src/core/lib/channel/channel_stack.cc", - "src/core/lib/channel/channel_stack_builder.cc", + "src/core/lib/channel/channel_stack_builder_impl.cc", "src/core/lib/channel/channel_trace.cc", "src/core/lib/channel/channelz.cc", "src/core/lib/channel/channelz_registry.cc", @@ -2017,7 +2017,7 @@ grpc_cc_library( "src/core/lib/channel/call_tracer.h", "src/core/lib/channel/channel_stack.h", "src/core/lib/channel/promise_based_filter.h", - "src/core/lib/channel/channel_stack_builder.h", + "src/core/lib/channel/channel_stack_builder_impl.h", "src/core/lib/channel/channel_trace.h", "src/core/lib/channel/channelz.h", "src/core/lib/channel/channelz_registry.h", @@ -2132,6 +2132,7 @@ grpc_cc_library( "src/core/lib/iomgr/combiner.h", "src/core/lib/iomgr/iomgr_internal.h", "src/core/lib/channel/channel_args.h", + "src/core/lib/channel/channel_stack_builder.h", ] + # TODO(hork): delete the iomgr glue code when EventEngine is fully # integrated, or when it becomes obvious the glue code is unnecessary. @@ -2164,6 +2165,7 @@ grpc_cc_library( "avl", "bitset", "channel_args", + "channel_stack_builder", "channel_stack_type", "chunked_vector", "closure", @@ -2224,11 +2226,31 @@ grpc_cc_library( ], language = "c++", deps = [ + "channel_stack_builder", "channel_stack_type", "gpr_base", ], ) +grpc_cc_library( + name = "channel_stack_builder", + srcs = [ + "src/core/lib/channel/channel_stack_builder.cc", + ], + hdrs = [ + "src/core/lib/channel/channel_stack_builder.h", + ], + language = "c++", + visibility = ["@grpc:alt_grpc_base_legacy"], + deps = [ + "channel_args", + "channel_stack_type", + "closure", + "error", + "gpr_base", + ], +) + grpc_cc_library( name = "grpc_common", defines = select({ diff --git a/CMakeLists.txt b/CMakeLists.txt index 58f1a6a700f..ec5384416af 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1838,6 +1838,7 @@ add_library(grpc src/core/lib/channel/channel_args_preconditioning.cc src/core/lib/channel/channel_stack.cc src/core/lib/channel/channel_stack_builder.cc + src/core/lib/channel/channel_stack_builder_impl.cc src/core/lib/channel/channel_trace.cc src/core/lib/channel/channelz.cc src/core/lib/channel/channelz_registry.cc @@ -2484,6 +2485,7 @@ add_library(grpc_unsecure src/core/lib/channel/channel_args_preconditioning.cc src/core/lib/channel/channel_stack.cc src/core/lib/channel/channel_stack_builder.cc + src/core/lib/channel/channel_stack_builder_impl.cc src/core/lib/channel/channel_trace.cc src/core/lib/channel/channelz.cc src/core/lib/channel/channelz_registry.cc diff --git a/Makefile b/Makefile index 25b4b4e6eb0..1d8d625c37c 100644 --- a/Makefile +++ b/Makefile @@ -1425,6 +1425,7 @@ LIBGRPC_SRC = \ src/core/lib/channel/channel_args_preconditioning.cc \ src/core/lib/channel/channel_stack.cc \ src/core/lib/channel/channel_stack_builder.cc \ + src/core/lib/channel/channel_stack_builder_impl.cc \ src/core/lib/channel/channel_trace.cc \ src/core/lib/channel/channelz.cc \ src/core/lib/channel/channelz_registry.cc \ @@ -1920,6 +1921,7 @@ LIBGRPC_UNSECURE_SRC = \ src/core/lib/channel/channel_args_preconditioning.cc \ src/core/lib/channel/channel_stack.cc \ src/core/lib/channel/channel_stack_builder.cc \ + src/core/lib/channel/channel_stack_builder_impl.cc \ src/core/lib/channel/channel_trace.cc \ src/core/lib/channel/channelz.cc \ src/core/lib/channel/channelz_registry.cc \ diff --git a/build_autogenerated.yaml b/build_autogenerated.yaml index 0d2bb6f05a9..8ebf1d4ccbe 100644 --- a/build_autogenerated.yaml +++ b/build_autogenerated.yaml @@ -702,6 +702,7 @@ libs: - src/core/lib/channel/channel_args_preconditioning.h - src/core/lib/channel/channel_stack.h - src/core/lib/channel/channel_stack_builder.h + - src/core/lib/channel/channel_stack_builder_impl.h - src/core/lib/channel/channel_trace.h - src/core/lib/channel/channelz.h - src/core/lib/channel/channelz_registry.h @@ -1367,6 +1368,7 @@ libs: - src/core/lib/channel/channel_args_preconditioning.cc - src/core/lib/channel/channel_stack.cc - src/core/lib/channel/channel_stack_builder.cc + - src/core/lib/channel/channel_stack_builder_impl.cc - src/core/lib/channel/channel_trace.cc - src/core/lib/channel/channelz.cc - src/core/lib/channel/channelz_registry.cc @@ -1879,6 +1881,7 @@ libs: - src/core/lib/channel/channel_args_preconditioning.h - src/core/lib/channel/channel_stack.h - src/core/lib/channel/channel_stack_builder.h + - src/core/lib/channel/channel_stack_builder_impl.h - src/core/lib/channel/channel_trace.h - src/core/lib/channel/channelz.h - src/core/lib/channel/channelz_registry.h @@ -2198,6 +2201,7 @@ libs: - src/core/lib/channel/channel_args_preconditioning.cc - src/core/lib/channel/channel_stack.cc - src/core/lib/channel/channel_stack_builder.cc + - src/core/lib/channel/channel_stack_builder_impl.cc - src/core/lib/channel/channel_trace.cc - src/core/lib/channel/channelz.cc - src/core/lib/channel/channelz_registry.cc diff --git a/config.m4 b/config.m4 index 705bb715137..fca3d6c8680 100644 --- a/config.m4 +++ b/config.m4 @@ -443,6 +443,7 @@ if test "$PHP_GRPC" != "no"; then src/core/lib/channel/channel_args_preconditioning.cc \ src/core/lib/channel/channel_stack.cc \ src/core/lib/channel/channel_stack_builder.cc \ + src/core/lib/channel/channel_stack_builder_impl.cc \ src/core/lib/channel/channel_trace.cc \ src/core/lib/channel/channelz.cc \ src/core/lib/channel/channelz_registry.cc \ diff --git a/config.w32 b/config.w32 index 18cfe1aaf74..05a623c7683 100644 --- a/config.w32 +++ b/config.w32 @@ -409,6 +409,7 @@ if (PHP_GRPC != "no") { "src\\core\\lib\\channel\\channel_args_preconditioning.cc " + "src\\core\\lib\\channel\\channel_stack.cc " + "src\\core\\lib\\channel\\channel_stack_builder.cc " + + "src\\core\\lib\\channel\\channel_stack_builder_impl.cc " + "src\\core\\lib\\channel\\channel_trace.cc " + "src\\core\\lib\\channel\\channelz.cc " + "src\\core\\lib\\channel\\channelz_registry.cc " + diff --git a/gRPC-C++.podspec b/gRPC-C++.podspec index 15ed9eca191..fc123d5ed7d 100644 --- a/gRPC-C++.podspec +++ b/gRPC-C++.podspec @@ -642,6 +642,7 @@ Pod::Spec.new do |s| 'src/core/lib/channel/channel_args_preconditioning.h', 'src/core/lib/channel/channel_stack.h', 'src/core/lib/channel/channel_stack_builder.h', + 'src/core/lib/channel/channel_stack_builder_impl.h', 'src/core/lib/channel/channel_trace.h', 'src/core/lib/channel/channelz.h', 'src/core/lib/channel/channelz_registry.h', @@ -1446,6 +1447,7 @@ Pod::Spec.new do |s| 'src/core/lib/channel/channel_args_preconditioning.h', 'src/core/lib/channel/channel_stack.h', 'src/core/lib/channel/channel_stack_builder.h', + 'src/core/lib/channel/channel_stack_builder_impl.h', 'src/core/lib/channel/channel_trace.h', 'src/core/lib/channel/channelz.h', 'src/core/lib/channel/channelz_registry.h', diff --git a/gRPC-Core.podspec b/gRPC-Core.podspec index 470d41337bd..42a3e29e62a 100644 --- a/gRPC-Core.podspec +++ b/gRPC-Core.podspec @@ -984,6 +984,8 @@ Pod::Spec.new do |s| 'src/core/lib/channel/channel_stack.h', 'src/core/lib/channel/channel_stack_builder.cc', 'src/core/lib/channel/channel_stack_builder.h', + 'src/core/lib/channel/channel_stack_builder_impl.cc', + 'src/core/lib/channel/channel_stack_builder_impl.h', 'src/core/lib/channel/channel_trace.cc', 'src/core/lib/channel/channel_trace.h', 'src/core/lib/channel/channelz.cc', @@ -2045,6 +2047,7 @@ Pod::Spec.new do |s| 'src/core/lib/channel/channel_args_preconditioning.h', 'src/core/lib/channel/channel_stack.h', 'src/core/lib/channel/channel_stack_builder.h', + 'src/core/lib/channel/channel_stack_builder_impl.h', 'src/core/lib/channel/channel_trace.h', 'src/core/lib/channel/channelz.h', 'src/core/lib/channel/channelz_registry.h', diff --git a/grpc.gemspec b/grpc.gemspec index 4416790add4..89a3c7bca58 100644 --- a/grpc.gemspec +++ b/grpc.gemspec @@ -903,6 +903,8 @@ Gem::Specification.new do |s| s.files += %w( src/core/lib/channel/channel_stack.h ) s.files += %w( src/core/lib/channel/channel_stack_builder.cc ) s.files += %w( src/core/lib/channel/channel_stack_builder.h ) + s.files += %w( src/core/lib/channel/channel_stack_builder_impl.cc ) + s.files += %w( src/core/lib/channel/channel_stack_builder_impl.h ) s.files += %w( src/core/lib/channel/channel_trace.cc ) s.files += %w( src/core/lib/channel/channel_trace.h ) s.files += %w( src/core/lib/channel/channelz.cc ) diff --git a/grpc.gyp b/grpc.gyp index d67e931b15d..5535d553247 100644 --- a/grpc.gyp +++ b/grpc.gyp @@ -773,6 +773,7 @@ 'src/core/lib/channel/channel_args_preconditioning.cc', 'src/core/lib/channel/channel_stack.cc', 'src/core/lib/channel/channel_stack_builder.cc', + 'src/core/lib/channel/channel_stack_builder_impl.cc', 'src/core/lib/channel/channel_trace.cc', 'src/core/lib/channel/channelz.cc', 'src/core/lib/channel/channelz_registry.cc', @@ -1239,6 +1240,7 @@ 'src/core/lib/channel/channel_args_preconditioning.cc', 'src/core/lib/channel/channel_stack.cc', 'src/core/lib/channel/channel_stack_builder.cc', + 'src/core/lib/channel/channel_stack_builder_impl.cc', 'src/core/lib/channel/channel_trace.cc', 'src/core/lib/channel/channelz.cc', 'src/core/lib/channel/channelz_registry.cc', diff --git a/package.xml b/package.xml index b0f233e99d1..12a93079e37 100644 --- a/package.xml +++ b/package.xml @@ -883,6 +883,8 @@ + + diff --git a/src/core/ext/filters/client_channel/subchannel.cc b/src/core/ext/filters/client_channel/subchannel.cc index 7520b183f73..f64041790c0 100644 --- a/src/core/ext/filters/client_channel/subchannel.cc +++ b/src/core/ext/filters/client_channel/subchannel.cc @@ -38,6 +38,7 @@ #include "src/core/lib/address_utils/sockaddr_utils.h" #include "src/core/lib/backoff/backoff.h" #include "src/core/lib/channel/channel_args.h" +#include "src/core/lib/channel/channel_stack_builder_impl.h" #include "src/core/lib/channel/connected_channel.h" #include "src/core/lib/config/core_configuration.h" #include "src/core/lib/debug/stats.h" @@ -966,11 +967,10 @@ void ConnectionDestroy(void* arg, grpc_error_handle /*error*/) { bool Subchannel::PublishTransportLocked() { // Construct channel stack. - ChannelStackBuilder builder("subchannel"); + ChannelStackBuilderImpl builder("subchannel", GRPC_CLIENT_SUBCHANNEL); builder.SetChannelArgs(connecting_result_.channel_args) .SetTransport(connecting_result_.transport); - if (!CoreConfiguration::Get().channel_init().CreateStack( - &builder, GRPC_CLIENT_SUBCHANNEL)) { + if (!CoreConfiguration::Get().channel_init().CreateStack(&builder)) { return false; } grpc_channel_stack* stk; diff --git a/src/core/lib/channel/channel_stack_builder.cc b/src/core/lib/channel/channel_stack_builder.cc index bcfd022d72b..3022efefac5 100644 --- a/src/core/lib/channel/channel_stack_builder.cc +++ b/src/core/lib/channel/channel_stack_builder.cc @@ -61,69 +61,4 @@ void ChannelStackBuilder::AppendFilter(const grpc_channel_filter* filter, stack_.push_back({filter, std::move(post_init)}); } -grpc_error_handle ChannelStackBuilder::Build(size_t prefix_bytes, - int initial_refs, - grpc_iomgr_cb_func destroy, - void* destroy_arg, void** result) { - // create an array of filters - std::vector filters; - filters.reserve(stack_.size()); - for (const auto& elem : stack_) { - filters.push_back(elem.filter); - } - - // calculate the size of the channel stack - size_t channel_stack_size = - grpc_channel_stack_size(filters.data(), filters.size()); - - // allocate memory, with prefix_bytes followed by channel_stack_size - *result = gpr_zalloc(prefix_bytes + channel_stack_size); - // fetch a pointer to the channel stack - grpc_channel_stack* channel_stack = reinterpret_cast( - static_cast(*result) + prefix_bytes); - - const grpc_channel_args* final_args; - if (transport_ != nullptr) { - static const grpc_arg_pointer_vtable vtable = { - // copy - [](void* p) { return p; }, - // destroy - [](void*) {}, - // cmp - [](void* a, void* b) { return QsortCompare(a, b); }, - }; - grpc_arg arg = grpc_channel_arg_pointer_create( - const_cast(GRPC_ARG_TRANSPORT), transport_, &vtable); - final_args = grpc_channel_args_copy_and_add(args_, &arg, 1); - } else { - final_args = args_; - } - - // and initialize it - grpc_error_handle error = grpc_channel_stack_init( - initial_refs, destroy, destroy_arg == nullptr ? *result : destroy_arg, - filters.data(), filters.size(), final_args, name_, channel_stack); - - if (final_args != args_) { - grpc_channel_args_destroy(final_args); - } - - if (error != GRPC_ERROR_NONE) { - grpc_channel_stack_destroy(channel_stack); - gpr_free(*result); - *result = nullptr; - return error; - } - - // run post-initialization functions - for (size_t i = 0; i < filters.size(); i++) { - if (stack_[i].post_init != nullptr) { - stack_[i].post_init(channel_stack, - grpc_channel_stack_element(channel_stack, i)); - } - } - - return GRPC_ERROR_NONE; -} - } // namespace grpc_core diff --git a/src/core/lib/channel/channel_stack_builder.h b/src/core/lib/channel/channel_stack_builder.h index fa64f7c42d1..41049d712e4 100644 --- a/src/core/lib/channel/channel_stack_builder.h +++ b/src/core/lib/channel/channel_stack_builder.h @@ -19,8 +19,20 @@ #include +#include +#include + +#include "absl/strings/string_view.h" + #include "src/core/lib/channel/channel_args.h" -#include "src/core/lib/channel/channel_stack.h" +#include "src/core/lib/iomgr/closure.h" +#include "src/core/lib/iomgr/error.h" +#include "src/core/lib/surface/channel_stack_type.h" + +typedef struct grpc_channel_stack grpc_channel_stack; +typedef struct grpc_channel_element grpc_channel_element; +typedef struct grpc_channel_filter grpc_channel_filter; +typedef struct grpc_transport grpc_transport; namespace grpc_core { @@ -43,9 +55,10 @@ class ChannelStackBuilder { }; // Initialize with a name. - explicit ChannelStackBuilder(const char* name) : name_(name) {} + ChannelStackBuilder(const char* name, grpc_channel_stack_type type) + : name_(name), type_(type) {} - ~ChannelStackBuilder(); + const char* name() const { return name_; } // Set the target string. ChannelStackBuilder& SetTarget(const char* target); @@ -72,6 +85,9 @@ class ChannelStackBuilder { // Mutable vector of proposed stack entries. std::vector* mutable_stack() { return &stack_; } + // The type of channel stack being built. + grpc_channel_stack_type channel_stack_type() const { return type_; } + // Helper to add a filter to the front of the stack. void PrependFilter(const grpc_channel_filter* filter, PostInitFunc post_init); @@ -83,15 +99,20 @@ class ChannelStackBuilder { // prefix_bytes are allocated before the channel stack, // initial_refs, destroy, destroy_arg are as per grpc_channel_stack_init // On failure, *result is nullptr. - grpc_error_handle Build(size_t prefix_bytes, int initial_refs, - grpc_iomgr_cb_func destroy, void* destroy_arg, - void** result); + virtual grpc_error_handle Build(size_t prefix_bytes, int initial_refs, + grpc_iomgr_cb_func destroy, void* destroy_arg, + void** result) = 0; + + protected: + ~ChannelStackBuilder(); private: static std::string unknown_target() { return "unknown"; } // The name of the stack const char* const name_; + // The type of stack being built + const grpc_channel_stack_type type_; // The target std::string target_{unknown_target()}; // The transport diff --git a/src/core/lib/channel/channel_stack_builder_impl.cc b/src/core/lib/channel/channel_stack_builder_impl.cc new file mode 100644 index 00000000000..39c9bd36b80 --- /dev/null +++ b/src/core/lib/channel/channel_stack_builder_impl.cc @@ -0,0 +1,102 @@ +/* + * + * Copyright 2016 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 + +#include "src/core/lib/channel/channel_stack_builder_impl.h" + +#include + +#include +#include + +#include "src/core/lib/channel/channel_args.h" +#include "src/core/lib/channel/channel_stack.h" +#include "src/core/lib/gprpp/memory.h" + +namespace grpc_core { + +grpc_error_handle ChannelStackBuilderImpl::Build(size_t prefix_bytes, + int initial_refs, + grpc_iomgr_cb_func destroy, + void* destroy_arg, + void** result) { + auto* stack = mutable_stack(); + + // create an array of filters + std::vector filters; + filters.reserve(stack->size()); + for (const auto& elem : *stack) { + filters.push_back(elem.filter); + } + + // calculate the size of the channel stack + size_t channel_stack_size = + grpc_channel_stack_size(filters.data(), filters.size()); + + // allocate memory, with prefix_bytes followed by channel_stack_size + *result = gpr_zalloc(prefix_bytes + channel_stack_size); + // fetch a pointer to the channel stack + grpc_channel_stack* channel_stack = reinterpret_cast( + static_cast(*result) + prefix_bytes); + + const grpc_channel_args* final_args; + if (transport() != nullptr) { + static const grpc_arg_pointer_vtable vtable = { + // copy + [](void* p) { return p; }, + // destroy + [](void*) {}, + // cmp + [](void* a, void* b) { return QsortCompare(a, b); }, + }; + grpc_arg arg = grpc_channel_arg_pointer_create( + const_cast(GRPC_ARG_TRANSPORT), transport(), &vtable); + final_args = grpc_channel_args_copy_and_add(channel_args(), &arg, 1); + } else { + final_args = channel_args(); + } + + // and initialize it + grpc_error_handle error = grpc_channel_stack_init( + initial_refs, destroy, destroy_arg == nullptr ? *result : destroy_arg, + filters.data(), filters.size(), final_args, name(), channel_stack); + + if (final_args != channel_args()) { + grpc_channel_args_destroy(final_args); + } + + if (error != GRPC_ERROR_NONE) { + grpc_channel_stack_destroy(channel_stack); + gpr_free(*result); + *result = nullptr; + return error; + } + + // run post-initialization functions + for (size_t i = 0; i < filters.size(); i++) { + if ((*stack)[i].post_init != nullptr) { + (*stack)[i].post_init(channel_stack, + grpc_channel_stack_element(channel_stack, i)); + } + } + + return GRPC_ERROR_NONE; +} + +} // namespace grpc_core diff --git a/src/core/lib/channel/channel_stack_builder_impl.h b/src/core/lib/channel/channel_stack_builder_impl.h new file mode 100644 index 00000000000..4290f2f56e9 --- /dev/null +++ b/src/core/lib/channel/channel_stack_builder_impl.h @@ -0,0 +1,48 @@ +// Copyright 2016 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_CORE_LIB_CHANNEL_CHANNEL_STACK_BUILDER_IMPL_H +#define GRPC_CORE_LIB_CHANNEL_CHANNEL_STACK_BUILDER_IMPL_H + +#include + +#include + +#include "src/core/lib/channel/channel_args.h" +#include "src/core/lib/channel/channel_stack.h" +#include "src/core/lib/channel/channel_stack_builder.h" + +namespace grpc_core { + +// Build a channel stack. +// Allows interested parties to add filters to the stack, and to query an +// in-progress build. +// Carries some useful context for the channel stack, such as a target string +// and a transport. +class ChannelStackBuilderImpl final : public ChannelStackBuilder { + public: + using ChannelStackBuilder::ChannelStackBuilder; + + // Build the channel stack. + // After success, *result holds the new channel stack, + // prefix_bytes are allocated before the channel stack, + // initial_refs, destroy, destroy_arg are as per grpc_channel_stack_init + // On failure, *result is nullptr. + grpc_error_handle Build(size_t prefix_bytes, int initial_refs, + grpc_iomgr_cb_func destroy, void* destroy_arg, + void** result) override; +}; +} // namespace grpc_core + +#endif // GRPC_CORE_LIB_CHANNEL_CHANNEL_STACK_BUILDER_IMPL_H diff --git a/src/core/lib/channel/connected_channel.h b/src/core/lib/channel/connected_channel.h index 7da9b21353d..71bdd5a5a76 100644 --- a/src/core/lib/channel/connected_channel.h +++ b/src/core/lib/channel/connected_channel.h @@ -21,6 +21,7 @@ #include +#include "src/core/lib/channel/channel_stack.h" #include "src/core/lib/channel/channel_stack_builder.h" extern const grpc_channel_filter grpc_connected_filter; diff --git a/src/core/lib/surface/channel.cc b/src/core/lib/surface/channel.cc index a9ef675212e..4dab2827fcc 100644 --- a/src/core/lib/surface/channel.cc +++ b/src/core/lib/surface/channel.cc @@ -31,6 +31,7 @@ #include #include "src/core/lib/channel/channel_args.h" +#include "src/core/lib/channel/channel_stack_builder_impl.h" #include "src/core/lib/channel/channel_trace.h" #include "src/core/lib/channel/channelz.h" #include "src/core/lib/channel/channelz_registry.h" @@ -241,8 +242,8 @@ grpc_channel* grpc_channel_create_internal( // grpc_shutdown() when the channel is actually destroyed, thus // ensuring that shutdown is deferred until that point. grpc_init(); - grpc_core::ChannelStackBuilder builder( - grpc_channel_stack_type_string(channel_stack_type)); + grpc_core::ChannelStackBuilderImpl builder( + grpc_channel_stack_type_string(channel_stack_type), channel_stack_type); const grpc_core::UniquePtr default_authority = get_default_authority(input_args); grpc_channel_args* args = @@ -258,7 +259,7 @@ grpc_channel* grpc_channel_create_internal( optional_transport); grpc_channel_args_destroy(args); if (!grpc_core::CoreConfiguration::Get().channel_init().CreateStack( - &builder, channel_stack_type)) { + &builder)) { grpc_shutdown(); // Since we won't call destroy_channel(). return nullptr; } diff --git a/src/core/lib/surface/channel_init.cc b/src/core/lib/surface/channel_init.cc index 8725991357c..459cbdea9b3 100644 --- a/src/core/lib/surface/channel_init.cc +++ b/src/core/lib/surface/channel_init.cc @@ -45,9 +45,8 @@ ChannelInit ChannelInit::Builder::Build() { return result; } -bool ChannelInit::CreateStack(ChannelStackBuilder* builder, - grpc_channel_stack_type type) const { - for (const auto& stage : slots_[type]) { +bool ChannelInit::CreateStack(ChannelStackBuilder* builder) const { + for (const auto& stage : slots_[builder->channel_stack_type()]) { if (!stage(builder)) return false; } return true; diff --git a/src/core/lib/surface/channel_init.h b/src/core/lib/surface/channel_init.h index ca58086df9d..9f4bb6fd50f 100644 --- a/src/core/lib/surface/channel_init.h +++ b/src/core/lib/surface/channel_init.h @@ -24,7 +24,7 @@ #include #include -#include "src/core/lib/surface/channel_stack_type.h" +#include "src/core/lib/channel/channel_stack_builder.h" #define GRPC_CHANNEL_INIT_BUILTIN_PRIORITY 10000 @@ -35,8 +35,6 @@ namespace grpc_core { -class ChannelStackBuilder; - class ChannelInit { public: /// One stage of mutation: call functions against \a builder to influence the @@ -72,10 +70,8 @@ class ChannelInit { }; /// Construct a channel stack of some sort: see channel_stack.h for details - /// \a type is the type of channel stack to create /// \a builder is the channel stack builder to build into. - bool CreateStack(ChannelStackBuilder* builder, - grpc_channel_stack_type type) const; + bool CreateStack(ChannelStackBuilder* builder) const; private: std::vector slots_[GRPC_NUM_CHANNEL_STACK_TYPES]; diff --git a/src/python/grpcio/grpc_core_dependencies.py b/src/python/grpcio/grpc_core_dependencies.py index 6508877eafc..f80acb34d33 100644 --- a/src/python/grpcio/grpc_core_dependencies.py +++ b/src/python/grpcio/grpc_core_dependencies.py @@ -418,6 +418,7 @@ CORE_SOURCE_FILES = [ 'src/core/lib/channel/channel_args_preconditioning.cc', 'src/core/lib/channel/channel_stack.cc', 'src/core/lib/channel/channel_stack_builder.cc', + 'src/core/lib/channel/channel_stack_builder_impl.cc', 'src/core/lib/channel/channel_trace.cc', 'src/core/lib/channel/channelz.cc', 'src/core/lib/channel/channelz_registry.cc', diff --git a/test/core/channel/channel_stack_builder_test.cc b/test/core/channel/channel_stack_builder_test.cc index d4b0dd956eb..aee08223bbe 100644 --- a/test/core/channel/channel_stack_builder_test.cc +++ b/test/core/channel/channel_stack_builder_test.cc @@ -28,6 +28,8 @@ #include #include +#include "src/core/lib/channel/channel_stack.h" +#include "src/core/lib/channel/channel_stack_builder_impl.h" #include "src/core/lib/config/core_configuration.h" #include "src/core/lib/slice/slice_internal.h" #include "src/core/lib/surface/channel_init.h" @@ -123,7 +125,7 @@ bool AddOriginalFilter(ChannelStackBuilder* builder) { } TEST(ChannelStackBuilder, UnknownTarget) { - ChannelStackBuilder builder("alpha-beta-gamma"); + ChannelStackBuilderImpl builder("alpha-beta-gamma", GRPC_CLIENT_CHANNEL); EXPECT_EQ(builder.target(), "unknown"); } diff --git a/test/core/channel/minimal_stack_is_minimal_test.cc b/test/core/channel/minimal_stack_is_minimal_test.cc index c5cbd86870d..7801b9b23a5 100644 --- a/test/core/channel/minimal_stack_is_minimal_test.cc +++ b/test/core/channel/minimal_stack_is_minimal_test.cc @@ -39,7 +39,7 @@ #include #include -#include "src/core/lib/channel/channel_stack_builder.h" +#include "src/core/lib/channel/channel_stack_builder_impl.h" #include "src/core/lib/config/core_configuration.h" #include "src/core/lib/gpr/string.h" #include "src/core/lib/surface/channel_init.h" @@ -124,7 +124,8 @@ static int check_stack(const char* file, int line, const char* transport_name, grpc_channel_args* init_args, unsigned channel_stack_type, ...) { // create phony channel stack - grpc_core::ChannelStackBuilder builder("test"); + grpc_core::ChannelStackBuilderImpl builder( + "test", static_cast(channel_stack_type)); grpc_transport_vtable fake_transport_vtable; memset(&fake_transport_vtable, 0, sizeof(grpc_transport_vtable)); fake_transport_vtable.name = transport_name; @@ -137,7 +138,7 @@ static int check_stack(const char* file, int line, const char* transport_name, { grpc_core::ExecCtx exec_ctx; GPR_ASSERT(grpc_core::CoreConfiguration::Get().channel_init().CreateStack( - &builder, (grpc_channel_stack_type)channel_stack_type)); + &builder)); } // build up our expectation list diff --git a/test/core/end2end/tests/filter_causes_close.cc b/test/core/end2end/tests/filter_causes_close.cc index eabed9bbfa3..4e34c9bbdbd 100644 --- a/test/core/end2end/tests/filter_causes_close.cc +++ b/test/core/end2end/tests/filter_causes_close.cc @@ -25,6 +25,7 @@ #include #include +#include "src/core/lib/channel/channel_stack.h" #include "src/core/lib/channel/channel_stack_builder.h" #include "src/core/lib/config/core_configuration.h" #include "src/core/lib/surface/channel_init.h" diff --git a/test/core/end2end/tests/filter_context.cc b/test/core/end2end/tests/filter_context.cc index 9cd6a82cc9b..c498dc27d98 100644 --- a/test/core/end2end/tests/filter_context.cc +++ b/test/core/end2end/tests/filter_context.cc @@ -26,6 +26,7 @@ #include #include +#include "src/core/lib/channel/channel_stack.h" #include "src/core/lib/channel/channel_stack_builder.h" #include "src/core/lib/config/core_configuration.h" #include "src/core/lib/surface/channel_init.h" diff --git a/test/core/end2end/tests/filter_init_fails.cc b/test/core/end2end/tests/filter_init_fails.cc index cc10a65691f..403b7c55036 100644 --- a/test/core/end2end/tests/filter_init_fails.cc +++ b/test/core/end2end/tests/filter_init_fails.cc @@ -26,6 +26,7 @@ #include #include +#include "src/core/lib/channel/channel_stack.h" #include "src/core/lib/channel/channel_stack_builder.h" #include "src/core/lib/config/core_configuration.h" #include "src/core/lib/surface/channel_init.h" diff --git a/test/core/end2end/tests/filter_latency.cc b/test/core/end2end/tests/filter_latency.cc index 16a0b48fa42..6170c747014 100644 --- a/test/core/end2end/tests/filter_latency.cc +++ b/test/core/end2end/tests/filter_latency.cc @@ -26,6 +26,7 @@ #include #include +#include "src/core/lib/channel/channel_stack.h" #include "src/core/lib/channel/channel_stack_builder.h" #include "src/core/lib/config/core_configuration.h" #include "src/core/lib/surface/channel_init.h" diff --git a/test/core/xds/xds_channel_stack_modifier_test.cc b/test/core/xds/xds_channel_stack_modifier_test.cc index b83184d53d8..2726c8623c7 100644 --- a/test/core/xds/xds_channel_stack_modifier_test.cc +++ b/test/core/xds/xds_channel_stack_modifier_test.cc @@ -22,6 +22,7 @@ #include +#include "src/core/lib/channel/channel_stack_builder_impl.h" #include "src/core/lib/config/core_configuration.h" #include "src/core/lib/surface/channel_init.h" #include "src/core/lib/transport/transport_impl.h" @@ -79,7 +80,7 @@ TEST(XdsChannelStackModifierTest, XdsHttpFiltersInsertion) { grpc_arg arg = channel_stack_modifier->MakeChannelArg(); // Create a phony ChannelStackBuilder object grpc_channel_args* args = grpc_channel_args_copy_and_add(nullptr, &arg, 1); - ChannelStackBuilder builder("test"); + ChannelStackBuilderImpl builder("test", GRPC_SERVER_CHANNEL); builder.SetChannelArgs(args); grpc_channel_args_destroy(args); grpc_transport_vtable fake_transport_vtable; @@ -89,8 +90,7 @@ TEST(XdsChannelStackModifierTest, XdsHttpFiltersInsertion) { builder.SetTransport(&fake_transport); // Construct channel stack and verify that the test filters were successfully // added - ASSERT_TRUE(CoreConfiguration::Get().channel_init().CreateStack( - &builder, GRPC_SERVER_CHANNEL)); + ASSERT_TRUE(CoreConfiguration::Get().channel_init().CreateStack(&builder)); std::vector filters; for (const auto& entry : *builder.mutable_stack()) { filters.push_back(entry.filter->name); @@ -118,7 +118,7 @@ TEST(XdsChannelStackModifierTest, XdsHttpFiltersInsertionAfterCensus) { grpc_arg arg = channel_stack_modifier->MakeChannelArg(); // Create a phony ChannelStackBuilder object grpc_channel_args* args = grpc_channel_args_copy_and_add(nullptr, &arg, 1); - ChannelStackBuilder builder("test"); + ChannelStackBuilderImpl builder("test", GRPC_SERVER_CHANNEL); builder.SetChannelArgs(args); grpc_channel_args_destroy(args); grpc_transport_vtable fake_transport_vtable; @@ -128,8 +128,7 @@ TEST(XdsChannelStackModifierTest, XdsHttpFiltersInsertionAfterCensus) { builder.SetTransport(&fake_transport); // Construct channel stack and verify that the test filters were successfully // added after the census filter - ASSERT_TRUE(CoreConfiguration::Get().channel_init().CreateStack( - &builder, GRPC_SERVER_CHANNEL)); + ASSERT_TRUE(CoreConfiguration::Get().channel_init().CreateStack(&builder)); std::vector filters; for (const auto& entry : *builder.mutable_stack()) { filters.push_back(entry.filter->name); diff --git a/test/cpp/microbenchmarks/bm_call_create.cc b/test/cpp/microbenchmarks/bm_call_create.cc index ed81152c597..137e14e0a5b 100644 --- a/test/cpp/microbenchmarks/bm_call_create.cc +++ b/test/cpp/microbenchmarks/bm_call_create.cc @@ -39,6 +39,7 @@ #include "src/core/ext/filters/http/server/http_server_filter.h" #include "src/core/ext/filters/message_size/message_size_filter.h" #include "src/core/lib/channel/channel_stack.h" +#include "src/core/lib/channel/channel_stack_builder_impl.h" #include "src/core/lib/channel/connected_channel.h" #include "src/core/lib/config/core_configuration.h" #include "src/core/lib/iomgr/call_combiner.h" @@ -723,7 +724,7 @@ class IsolatedCallFixture : public TrackCounters { const grpc_channel_args* args = grpc_core::CoreConfiguration::Get() .channel_args_preconditioning() .PreconditionChannelArgs(nullptr); - grpc_core::ChannelStackBuilder builder("phony"); + grpc_core::ChannelStackBuilderImpl builder("phony", GRPC_CLIENT_CHANNEL); builder.SetTarget("phony_target"); builder.SetChannelArgs(args); builder.AppendFilter(&isolated_call_filter::isolated_call_filter, nullptr); diff --git a/tools/doxygen/Doxyfile.c++.internal b/tools/doxygen/Doxyfile.c++.internal index 954b4dfc637..3f558f77388 100644 --- a/tools/doxygen/Doxyfile.c++.internal +++ b/tools/doxygen/Doxyfile.c++.internal @@ -1882,6 +1882,8 @@ src/core/lib/channel/channel_stack.cc \ src/core/lib/channel/channel_stack.h \ src/core/lib/channel/channel_stack_builder.cc \ src/core/lib/channel/channel_stack_builder.h \ +src/core/lib/channel/channel_stack_builder_impl.cc \ +src/core/lib/channel/channel_stack_builder_impl.h \ src/core/lib/channel/channel_trace.cc \ src/core/lib/channel/channel_trace.h \ src/core/lib/channel/channelz.cc \ diff --git a/tools/doxygen/Doxyfile.core.internal b/tools/doxygen/Doxyfile.core.internal index 01478efe38a..ea597fb1f74 100644 --- a/tools/doxygen/Doxyfile.core.internal +++ b/tools/doxygen/Doxyfile.core.internal @@ -1674,6 +1674,8 @@ src/core/lib/channel/channel_stack.cc \ src/core/lib/channel/channel_stack.h \ src/core/lib/channel/channel_stack_builder.cc \ src/core/lib/channel/channel_stack_builder.h \ +src/core/lib/channel/channel_stack_builder_impl.cc \ +src/core/lib/channel/channel_stack_builder_impl.h \ src/core/lib/channel/channel_trace.cc \ src/core/lib/channel/channel_trace.h \ src/core/lib/channel/channelz.cc \