From d1cecad651306dac1c7b56d3186a96f545dfc47f Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Fri, 24 Sep 2021 15:23:02 -0700 Subject: [PATCH] ExecCtx based Activity wakeup mechanism (#27454) * wakeup scheduler * wakeup scheduler * Automated change: Fix sanity tests * Update exec_ctx_wakeup_scheduler_test.cc * Remove unused name * fix_build Co-authored-by: ctiller --- BUILD | 12 ++++ CMakeLists.txt | 49 +++++++++++++ build_autogenerated.yaml | 47 +++++++++++++ .../lib/promise/exec_ctx_wakeup_scheduler.h | 45 ++++++++++++ test/core/promise/BUILD | 13 ++++ .../promise/exec_ctx_wakeup_scheduler_test.cc | 69 +++++++++++++++++++ tools/run_tests/generated/tests.json | 24 +++++++ 7 files changed, 259 insertions(+) create mode 100644 src/core/lib/promise/exec_ctx_wakeup_scheduler.h create mode 100644 test/core/promise/exec_ctx_wakeup_scheduler_test.cc diff --git a/BUILD b/BUILD index b7186c7479f..4a99663b6a6 100644 --- a/BUILD +++ b/BUILD @@ -1121,6 +1121,18 @@ grpc_cc_library( ], ) +grpc_cc_library( + name = "exec_ctx_wakeup_scheduler", + hdrs = [ + "src/core/lib/promise/exec_ctx_wakeup_scheduler.h", + ], + language = "c++", + deps = [ + "exec_ctx", + "gpr_base", + ], +) + grpc_cc_library( name = "wait_set", external_deps = [ diff --git a/CMakeLists.txt b/CMakeLists.txt index a452071d466..6f0e917a6d1 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -863,6 +863,7 @@ if(gRPC_BUILD_TESTS) add_dependencies(buildtests_cxx examine_stack_test) endif() add_dependencies(buildtests_cxx exception_test) + add_dependencies(buildtests_cxx exec_ctx_wakeup_scheduler_test) add_dependencies(buildtests_cxx fake_binder_test) add_dependencies(buildtests_cxx file_watcher_certificate_provider_factory_test) add_dependencies(buildtests_cxx filter_end2end_test) @@ -11044,6 +11045,54 @@ target_link_libraries(exception_test ) +endif() +if(gRPC_BUILD_TESTS) + +add_executable(exec_ctx_wakeup_scheduler_test + src/core/lib/debug/trace.cc + src/core/lib/iomgr/combiner.cc + src/core/lib/iomgr/error.cc + src/core/lib/iomgr/exec_ctx.cc + src/core/lib/iomgr/executor.cc + src/core/lib/iomgr/iomgr_internal.cc + src/core/lib/promise/activity.cc + src/core/lib/slice/slice.cc + src/core/lib/slice/slice_refcount.cc + src/core/lib/slice/slice_string_helpers.cc + src/core/lib/slice/static_slice.cc + test/core/promise/exec_ctx_wakeup_scheduler_test.cc + third_party/googletest/googletest/src/gtest-all.cc + third_party/googletest/googlemock/src/gmock-all.cc +) + +target_include_directories(exec_ctx_wakeup_scheduler_test + PRIVATE + ${CMAKE_CURRENT_SOURCE_DIR} + ${CMAKE_CURRENT_SOURCE_DIR}/include + ${_gRPC_ADDRESS_SORTING_INCLUDE_DIR} + ${_gRPC_RE2_INCLUDE_DIR} + ${_gRPC_SSL_INCLUDE_DIR} + ${_gRPC_UPB_GENERATED_DIR} + ${_gRPC_UPB_GRPC_GENERATED_DIR} + ${_gRPC_UPB_INCLUDE_DIR} + ${_gRPC_XXHASH_INCLUDE_DIR} + ${_gRPC_ZLIB_INCLUDE_DIR} + third_party/googletest/googletest/include + third_party/googletest/googletest + third_party/googletest/googlemock/include + third_party/googletest/googlemock + ${_gRPC_PROTO_GENS_DIR} +) + +target_link_libraries(exec_ctx_wakeup_scheduler_test + ${_gRPC_PROTOBUF_LIBRARIES} + ${_gRPC_ALLTARGETS_LIBRARIES} + absl::statusor + absl::variant + gpr +) + + endif() if(gRPC_BUILD_TESTS) diff --git a/build_autogenerated.yaml b/build_autogenerated.yaml index 7f8344833fb..90b559e9cab 100644 --- a/build_autogenerated.yaml +++ b/build_autogenerated.yaml @@ -5636,6 +5636,53 @@ targets: - test/cpp/end2end/exception_test.cc deps: - grpc++_test_util +- name: exec_ctx_wakeup_scheduler_test + gtest: true + build: test + language: c++ + headers: + - src/core/lib/debug/trace.h + - src/core/lib/gprpp/atomic_utils.h + - src/core/lib/gprpp/ref_counted.h + - src/core/lib/gprpp/ref_counted_ptr.h + - src/core/lib/iomgr/closure.h + - src/core/lib/iomgr/combiner.h + - src/core/lib/iomgr/error.h + - src/core/lib/iomgr/error_internal.h + - src/core/lib/iomgr/exec_ctx.h + - src/core/lib/iomgr/executor.h + - src/core/lib/iomgr/iomgr_internal.h + - src/core/lib/promise/activity.h + - src/core/lib/promise/context.h + - src/core/lib/promise/detail/promise_factory.h + - src/core/lib/promise/detail/promise_like.h + - src/core/lib/promise/detail/status.h + - src/core/lib/promise/exec_ctx_wakeup_scheduler.h + - src/core/lib/promise/poll.h + - src/core/lib/slice/slice_internal.h + - src/core/lib/slice/slice_refcount.h + - src/core/lib/slice/slice_refcount_base.h + - src/core/lib/slice/slice_string_helpers.h + - src/core/lib/slice/slice_utils.h + - src/core/lib/slice/static_slice.h + src: + - src/core/lib/debug/trace.cc + - src/core/lib/iomgr/combiner.cc + - src/core/lib/iomgr/error.cc + - src/core/lib/iomgr/exec_ctx.cc + - src/core/lib/iomgr/executor.cc + - src/core/lib/iomgr/iomgr_internal.cc + - src/core/lib/promise/activity.cc + - src/core/lib/slice/slice.cc + - src/core/lib/slice/slice_refcount.cc + - src/core/lib/slice/slice_string_helpers.cc + - src/core/lib/slice/static_slice.cc + - test/core/promise/exec_ctx_wakeup_scheduler_test.cc + deps: + - absl/status:statusor + - absl/types:variant + - gpr + uses_polling: false - name: fake_binder_test gtest: true build: test diff --git a/src/core/lib/promise/exec_ctx_wakeup_scheduler.h b/src/core/lib/promise/exec_ctx_wakeup_scheduler.h new file mode 100644 index 00000000000..f67db49e746 --- /dev/null +++ b/src/core/lib/promise/exec_ctx_wakeup_scheduler.h @@ -0,0 +1,45 @@ +// Copyright 2021 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_PROMISE_EXEC_CTX_WAKEUP_SCHEDULER_H +#define GRPC_CORE_LIB_PROMISE_EXEC_CTX_WAKEUP_SCHEDULER_H + +#include + +#include "src/core/lib/iomgr/exec_ctx.h" + +namespace grpc_core { + +// A callback scheduler for activities that works by scheduling callbacks on the +// exec ctx. +class ExecCtxWakeupScheduler { + public: + template + void ScheduleWakeup(ActivityType* activity) { + GRPC_CLOSURE_INIT( + &closure_, + [](void* arg, grpc_error_handle) { + static_cast(arg)->RunScheduledWakeup(); + }, + activity, grpc_schedule_on_exec_ctx); + ExecCtx::Run(DEBUG_LOCATION, &closure_, GRPC_ERROR_NONE); + } + + private: + grpc_closure closure_; +}; + +} // namespace grpc_core + +#endif // GRPC_CORE_LIB_PROMISE_EXEC_CTX_WAKEUP_SCHEDULER_H diff --git a/test/core/promise/BUILD b/test/core/promise/BUILD index e0db806d219..aabfe02d4a8 100644 --- a/test/core/promise/BUILD +++ b/test/core/promise/BUILD @@ -264,3 +264,16 @@ grpc_proto_fuzzer( "//test/core/util:grpc_test_util", ], ) + +grpc_cc_test( + name = "exec_ctx_wakeup_scheduler_test", + srcs = ["exec_ctx_wakeup_scheduler_test.cc"], + external_deps = ["gtest"], + language = "c++", + uses_polling = False, + deps = [ + "//:activity", + "//:exec_ctx_wakeup_scheduler", + "//test/core/util:grpc_suppressions", + ], +) diff --git a/test/core/promise/exec_ctx_wakeup_scheduler_test.cc b/test/core/promise/exec_ctx_wakeup_scheduler_test.cc new file mode 100644 index 00000000000..d432f437565 --- /dev/null +++ b/test/core/promise/exec_ctx_wakeup_scheduler_test.cc @@ -0,0 +1,69 @@ +// Copyright 2021 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 "src/core/lib/promise/exec_ctx_wakeup_scheduler.h" + +#include + +#include "src/core/lib/promise/activity.h" + +namespace grpc_core { + +TEST(ExecCtxWakeupSchedulerTest, Works) { + int state = 0; + bool done = false; + auto activity = MakeActivity( + [&state]() mutable -> Poll { + ++state; + switch (state) { + case 1: + return Pending(); + case 2: + return absl::OkStatus(); + default: + abort(); + } + }, + ExecCtxWakeupScheduler(), + [&done](absl::Status status) { + EXPECT_EQ(status, absl::OkStatus()); + done = true; + }); + + EXPECT_EQ(state, 1); + EXPECT_FALSE(done); + { + ExecCtx exec_ctx; + EXPECT_FALSE(exec_ctx.HasWork()); + activity->ForceWakeup(); + EXPECT_TRUE(exec_ctx.HasWork()); + EXPECT_EQ(state, 1); + EXPECT_FALSE(done); + } + EXPECT_EQ(state, 2); + EXPECT_TRUE(done); +} + +} // namespace grpc_core + +// Hook needed to run ExecCtx outside of iomgr. +void grpc_set_default_iomgr_platform() {} + +int main(int argc, char** argv) { + ::testing::InitGoogleTest(&argc, argv); + grpc_core::ExecCtx::GlobalInit(); + int r = RUN_ALL_TESTS(); + grpc_core::ExecCtx::GlobalShutdown(); + return r; +} diff --git a/tools/run_tests/generated/tests.json b/tools/run_tests/generated/tests.json index d72d3623073..2373978317a 100644 --- a/tools/run_tests/generated/tests.json +++ b/tools/run_tests/generated/tests.json @@ -4727,6 +4727,30 @@ ], "uses_polling": true }, + { + "args": [], + "benchmark": false, + "ci_platforms": [ + "linux", + "mac", + "posix", + "windows" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [], + "flaky": false, + "gtest": true, + "language": "c++", + "name": "exec_ctx_wakeup_scheduler_test", + "platforms": [ + "linux", + "mac", + "posix", + "windows" + ], + "uses_polling": false + }, { "args": [], "benchmark": false,