From 2a80f0edc72a444b726ac96e7312488920bed8d1 Mon Sep 17 00:00:00 2001 From: Vijay Pai Date: Mon, 25 Feb 2019 16:28:59 -0800 Subject: [PATCH 1/4] Support use of ByteBuffer for request-side of code-gen unary --- src/compiler/cpp_generator.cc | 18 ++++++++++ .../end2end/client_callback_end2end_test.cc | 36 +++++++++++++++++++ 2 files changed, 54 insertions(+) diff --git a/src/compiler/cpp_generator.cc b/src/compiler/cpp_generator.cc index b0046872502..96e9ab8dcfb 100644 --- a/src/compiler/cpp_generator.cc +++ b/src/compiler/cpp_generator.cc @@ -580,6 +580,10 @@ void PrintHeaderClientMethodCallbackInterfaces( "virtual void $Method$(::grpc::ClientContext* context, " "const $Request$* request, $Response$* response, " "std::function) = 0;\n"); + printer->Print(*vars, + "virtual void $Method$(::grpc::ClientContext* context, " + "const ::grpc::ByteBuffer* request, $Response$* response, " + "std::function) = 0;\n"); } else if (ClientOnlyStreaming(method)) { printer->Print(*vars, "virtual void $Method$(::grpc::ClientContext* context, " @@ -642,6 +646,10 @@ void PrintHeaderClientMethodCallback(grpc_generator::Printer* printer, "void $Method$(::grpc::ClientContext* context, " "const $Request$* request, $Response$* response, " "std::function) override;\n"); + printer->Print(*vars, + "void $Method$(::grpc::ClientContext* context, " + "const ::grpc::ByteBuffer* request, $Response$* response, " + "std::function) override;\n"); } else if (ClientOnlyStreaming(method)) { printer->Print(*vars, "void $Method$(::grpc::ClientContext* context, " @@ -1643,6 +1651,16 @@ void PrintSourceClientMethod(grpc_generator::Printer* printer, "(stub_->channel_.get(), stub_->rpcmethod_$Method$_, " "context, request, response, std::move(f));\n}\n\n"); + printer->Print(*vars, + "void $ns$$Service$::Stub::experimental_async::$Method$(" + "::grpc::ClientContext* context, " + "const ::grpc::ByteBuffer* request, $Response$* response, " + "std::function f) {\n"); + printer->Print(*vars, + " return ::grpc::internal::CallbackUnaryCall" + "(stub_->channel_.get(), stub_->rpcmethod_$Method$_, " + "context, request, response, std::move(f));\n}\n\n"); + for (auto async_prefix : async_prefixes) { (*vars)["AsyncPrefix"] = async_prefix.prefix; (*vars)["AsyncStart"] = async_prefix.start; diff --git a/test/cpp/end2end/client_callback_end2end_test.cc b/test/cpp/end2end/client_callback_end2end_test.cc index 7ed15beabe1..893d009392d 100644 --- a/test/cpp/end2end/client_callback_end2end_test.cc +++ b/test/cpp/end2end/client_callback_end2end_test.cc @@ -220,6 +220,36 @@ class ClientCallbackEnd2endTest } } + void SendRpcsRawReq(int num_rpcs) { + grpc::string test_string("Hello raw world."); + EchoRequest request; + request.set_message(test_string); + std::unique_ptr send_buf = SerializeToByteBuffer(&request); + + for (int i = 0; i < num_rpcs; i++) { + EchoResponse response; + ClientContext cli_ctx; + + std::mutex mu; + std::condition_variable cv; + bool done = false; + stub_->experimental_async()->Echo( + &cli_ctx, send_buf.get(), &response, + [&request, &response, &done, &mu, &cv](Status s) { + GPR_ASSERT(s.ok()); + + EXPECT_EQ(request.message(), response.message()); + std::lock_guard l(mu); + done = true; + cv.notify_one(); + }); + std::unique_lock l(mu); + while (!done) { + cv.wait(l); + } + } + } + void SendRpcsGeneric(int num_rpcs, bool maybe_except) { const grpc::string kMethodName("/grpc.testing.EchoTestService/Echo"); grpc::string test_string(""); @@ -347,6 +377,12 @@ TEST_P(ClientCallbackEnd2endTest, SequentialRpcs) { SendRpcs(10, false); } +TEST_P(ClientCallbackEnd2endTest, SequentialRpcsRawReq) { + MAYBE_SKIP_TEST; + ResetStub(); + SendRpcsRawReq(10); +} + TEST_P(ClientCallbackEnd2endTest, SendClientInitialMetadata) { MAYBE_SKIP_TEST; ResetStub(); From de29ab752ebb2714a114463302e3796b75fea330 Mon Sep 17 00:00:00 2001 From: Vijay Pai Date: Mon, 25 Feb 2019 23:29:02 -0800 Subject: [PATCH 2/4] Fix golden-file test --- test/cpp/codegen/compiler_test_golden | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/cpp/codegen/compiler_test_golden b/test/cpp/codegen/compiler_test_golden index 1871e1375ed..7f9fd29026e 100644 --- a/test/cpp/codegen/compiler_test_golden +++ b/test/cpp/codegen/compiler_test_golden @@ -113,6 +113,7 @@ class ServiceA final { virtual ~experimental_async_interface() {} // MethodA1 leading comment 1 virtual void MethodA1(::grpc::ClientContext* context, const ::grpc::testing::Request* request, ::grpc::testing::Response* response, std::function) = 0; + virtual void MethodA1(::grpc::ClientContext* context, const ::grpc::ByteBuffer* request, ::grpc::testing::Response* response, std::function) = 0; // MethodA1 trailing comment 1 // MethodA2 detached leading comment 1 // @@ -182,6 +183,7 @@ class ServiceA final { public StubInterface::experimental_async_interface { public: void MethodA1(::grpc::ClientContext* context, const ::grpc::testing::Request* request, ::grpc::testing::Response* response, std::function) override; + void MethodA1(::grpc::ClientContext* context, const ::grpc::ByteBuffer* request, ::grpc::testing::Response* response, std::function) override; void MethodA2(::grpc::ClientContext* context, ::grpc::testing::Response* response, ::grpc::experimental::ClientWriteReactor< ::grpc::testing::Request>* reactor) override; void MethodA3(::grpc::ClientContext* context, ::grpc::testing::Request* request, ::grpc::experimental::ClientReadReactor< ::grpc::testing::Response>* reactor) override; void MethodA4(::grpc::ClientContext* context, ::grpc::experimental::ClientBidiReactor< ::grpc::testing::Request,::grpc::testing::Response>* reactor) override; @@ -714,6 +716,7 @@ class ServiceB final { virtual ~experimental_async_interface() {} // MethodB1 leading comment 1 virtual void MethodB1(::grpc::ClientContext* context, const ::grpc::testing::Request* request, ::grpc::testing::Response* response, std::function) = 0; + virtual void MethodB1(::grpc::ClientContext* context, const ::grpc::ByteBuffer* request, ::grpc::testing::Response* response, std::function) = 0; // MethodB1 trailing comment 1 }; virtual class experimental_async_interface* experimental_async() { return nullptr; } @@ -735,6 +738,7 @@ class ServiceB final { public StubInterface::experimental_async_interface { public: void MethodB1(::grpc::ClientContext* context, const ::grpc::testing::Request* request, ::grpc::testing::Response* response, std::function) override; + void MethodB1(::grpc::ClientContext* context, const ::grpc::ByteBuffer* request, ::grpc::testing::Response* response, std::function) override; private: friend class Stub; explicit experimental_async(Stub* stub): stub_(stub) { } From 2eb25c871e41b730e44360a8055b3d4a2e3cffb9 Mon Sep 17 00:00:00 2001 From: Vijay Pai Date: Tue, 26 Feb 2019 03:08:06 -0800 Subject: [PATCH 3/4] Avoid build errors --- include/grpcpp/impl/codegen/byte_buffer.h | 15 ++++++++++++--- src/cpp/util/byte_buffer_cc.cc | 14 -------------- 2 files changed, 12 insertions(+), 17 deletions(-) diff --git a/include/grpcpp/impl/codegen/byte_buffer.h b/include/grpcpp/impl/codegen/byte_buffer.h index a77e36dfc50..7b82f49a84e 100644 --- a/include/grpcpp/impl/codegen/byte_buffer.h +++ b/include/grpcpp/impl/codegen/byte_buffer.h @@ -96,7 +96,7 @@ class ByteBuffer final { /// \a buf. Wrapper of core function grpc_byte_buffer_copy . This is not /// a deep copy; it is just a referencing. As a result, its performance is /// size-independent. - ByteBuffer(const ByteBuffer& buf); + ByteBuffer(const ByteBuffer& buf) : buffer_(nullptr) { operator=(buf); } ~ByteBuffer() { if (buffer_) { @@ -107,7 +107,16 @@ class ByteBuffer final { /// Wrapper of core function grpc_byte_buffer_copy . This is not /// a deep copy; it is just a referencing. As a result, its performance is /// size-independent. - ByteBuffer& operator=(const ByteBuffer&); + ByteBuffer& operator=(const ByteBuffer& buf) { + if (this != &buf) { + Clear(); // first remove existing data + } + if (buf.buffer_) { + // then copy + buffer_ = g_core_codegen_interface->grpc_byte_buffer_copy(buf.buffer_); + } + return *this; + } /// Dump (read) the buffer contents into \a slices. Status Dump(std::vector* slices) const; @@ -215,7 +224,7 @@ class SerializationTraits { bool* own_buffer) { *buffer = source; *own_buffer = true; - return Status::OK; + return g_core_codegen_interface->ok(); } }; diff --git a/src/cpp/util/byte_buffer_cc.cc b/src/cpp/util/byte_buffer_cc.cc index a7e16454352..fb705906455 100644 --- a/src/cpp/util/byte_buffer_cc.cc +++ b/src/cpp/util/byte_buffer_cc.cc @@ -43,18 +43,4 @@ Status ByteBuffer::Dump(std::vector* slices) const { return Status::OK; } -ByteBuffer::ByteBuffer(const ByteBuffer& buf) : buffer_(nullptr) { - operator=(buf); -} - -ByteBuffer& ByteBuffer::operator=(const ByteBuffer& buf) { - if (this != &buf) { - Clear(); // first remove existing data - } - if (buf.buffer_) { - buffer_ = grpc_byte_buffer_copy(buf.buffer_); // then copy - } - return *this; -} - } // namespace grpc From 29191d5eda70a81f6d3e83130562e4407a6bdf07 Mon Sep 17 00:00:00 2001 From: Vijay Pai Date: Tue, 26 Feb 2019 12:44:13 -0800 Subject: [PATCH 4/4] Need to properly init library for microbenchmarks --- test/cpp/microbenchmarks/helpers.cc | 11 +++++++++++ test/cpp/microbenchmarks/helpers.h | 8 +------- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/test/cpp/microbenchmarks/helpers.cc b/test/cpp/microbenchmarks/helpers.cc index bce72985dc2..d4070de7481 100644 --- a/test/cpp/microbenchmarks/helpers.cc +++ b/test/cpp/microbenchmarks/helpers.cc @@ -20,6 +20,17 @@ #include "test/cpp/microbenchmarks/helpers.h" +static grpc::internal::GrpcLibraryInitializer g_gli_initializer; + +Library::Library() { + g_gli_initializer.summon(); +#ifdef GPR_LOW_LEVEL_COUNTERS + grpc_memory_counters_init(); +#endif + init_lib_.init(); + rq_ = grpc_resource_quota_create("bm"); +} + void TrackCounters::Finish(benchmark::State& state) { std::ostringstream out; for (const auto& l : labels_) { diff --git a/test/cpp/microbenchmarks/helpers.h b/test/cpp/microbenchmarks/helpers.h index 25d34b5f871..770966aa189 100644 --- a/test/cpp/microbenchmarks/helpers.h +++ b/test/cpp/microbenchmarks/helpers.h @@ -39,13 +39,7 @@ class Library { grpc_resource_quota* rq() { return rq_; } private: - Library() { -#ifdef GPR_LOW_LEVEL_COUNTERS - grpc_memory_counters_init(); -#endif - init_lib_.init(); - rq_ = grpc_resource_quota_create("bm"); - } + Library(); ~Library() { init_lib_.shutdown(); }