From a21ab2ba29bd5f84c861476871cf811131449ec8 Mon Sep 17 00:00:00 2001 From: David Garcia Quintas Date: Thu, 1 Feb 2018 16:24:00 -0800 Subject: Fix compression algorithm accessors --- include/grpc++/impl/codegen/client_context.h | 4 +++- include/grpc++/impl/codegen/server_context.h | 5 ++++- 2 files changed, 7 insertions(+), 2 deletions(-) (limited to 'include/grpc++') diff --git a/include/grpc++/impl/codegen/client_context.h b/include/grpc++/impl/codegen/client_context.h index 61d97ce818..ff6b107452 100644 --- a/include/grpc++/impl/codegen/client_context.h +++ b/include/grpc++/impl/codegen/client_context.h @@ -289,7 +289,9 @@ class ClientContext { creds_ = creds; } - /// Return the compression algorithm to be used by the client call. + /// Return the compression algorithm the client call will request be used. + /// Note that the gRPC runtime may decide to ignore this request, for example, + /// due to resource constraints. grpc_compression_algorithm compression_algorithm() const { return compression_algorithm_; } diff --git a/include/grpc++/impl/codegen/server_context.h b/include/grpc++/impl/codegen/server_context.h index a2d6967bf8..c36975860e 100644 --- a/include/grpc++/impl/codegen/server_context.h +++ b/include/grpc++/impl/codegen/server_context.h @@ -185,7 +185,10 @@ class ServerContext { /// \a set_compression_level. bool compression_level_set() const { return compression_level_set_; } - /// Return the compression algorithm to be used by the server call. + /// Return the compression algorithm the server call will request be used. + /// Note that the gRPC runtime may decide to ignore this request, for example, + /// due to resource constraints, or if the server is aware the client doesn't + /// support the requested algorithm. grpc_compression_algorithm compression_algorithm() const { return compression_algorithm_; } -- cgit v1.2.3 From dcd5ef3ad8b964ec487c96aeea4f1323193815ed Mon Sep 17 00:00:00 2001 From: Vijay Pai Date: Mon, 5 Feb 2018 13:08:44 -0800 Subject: Deprecate GenericStub()::Call for multi-threaded use --- include/grpc++/generic/generic_stub.h | 19 ++++++++++--------- test/cpp/end2end/generic_end2end_test.cc | 6 ++++-- 2 files changed, 14 insertions(+), 11 deletions(-) (limited to 'include/grpc++') diff --git a/include/grpc++/generic/generic_stub.h b/include/grpc++/generic/generic_stub.h index d5064318cf..e72826bdc1 100644 --- a/include/grpc++/generic/generic_stub.h +++ b/include/grpc++/generic/generic_stub.h @@ -37,15 +37,6 @@ class GenericStub final { explicit GenericStub(std::shared_ptr channel) : channel_(channel) {} - /// Begin a call to a named method \a method using \a context. - /// A tag \a tag will be delivered to \a cq when the call has been started - /// (i.e, initial metadata has been sent). - /// The return value only indicates whether or not registration of the call - /// succeeded (i.e. the call won't proceed if the return value is nullptr). - std::unique_ptr Call( - ClientContext* context, const grpc::string& method, CompletionQueue* cq, - void* tag); - /// Setup a call to a named method \a method using \a context, but don't /// start it. Let it be started explicitly with StartCall and a tag. /// The return value only indicates whether or not registration of the call @@ -61,6 +52,16 @@ class GenericStub final { ClientContext* context, const grpc::string& method, const ByteBuffer& request, CompletionQueue* cq); + /// DEPRECATED for multi-threaded use + /// Begin a call to a named method \a method using \a context. + /// A tag \a tag will be delivered to \a cq when the call has been started + /// (i.e, initial metadata has been sent). + /// The return value only indicates whether or not registration of the call + /// succeeded (i.e. the call won't proceed if the return value is nullptr). + std::unique_ptr Call( + ClientContext* context, const grpc::string& method, CompletionQueue* cq, + void* tag); + private: std::shared_ptr channel_; }; diff --git a/test/cpp/end2end/generic_end2end_test.cc b/test/cpp/end2end/generic_end2end_test.cc index bf432844cb..ba2608982a 100644 --- a/test/cpp/end2end/generic_end2end_test.cc +++ b/test/cpp/end2end/generic_end2end_test.cc @@ -125,7 +125,8 @@ class GenericEnd2endTest : public ::testing::Test { } std::unique_ptr call = - generic_stub_->Call(&cli_ctx, kMethodName, &cli_cq_, tag(1)); + generic_stub_->PrepareCall(&cli_ctx, kMethodName, &cli_cq_); + call->StartCall(tag(1)); client_ok(1); std::unique_ptr send_buffer = SerializeToByteBuffer(&send_request); @@ -271,7 +272,8 @@ TEST_F(GenericEnd2endTest, SimpleBidiStreaming) { cli_ctx.set_compression_algorithm(GRPC_COMPRESS_MESSAGE_GZIP); send_request.set_message("Hello"); std::unique_ptr cli_stream = - generic_stub_->Call(&cli_ctx, kMethodName, &cli_cq_, tag(1)); + generic_stub_->PrepareCall(&cli_ctx, kMethodName, &cli_cq_); + cli_stream->StartCall(tag(1)); client_ok(1); generic_service_.RequestCall(&srv_ctx, &srv_stream, srv_cq_.get(), -- cgit v1.2.3 From e81ead58fb9ab5685b6931fa28de4f28da694669 Mon Sep 17 00:00:00 2001 From: Vijay Pai Date: Mon, 5 Feb 2018 13:46:14 -0800 Subject: Improve CQ and cancellation comments --- include/grpc++/impl/codegen/client_context.h | 9 +- include/grpc++/impl/codegen/completion_queue.h | 115 +++++++++++++++++-------- include/grpc++/impl/codegen/server_context.h | 4 + 3 files changed, 91 insertions(+), 37 deletions(-) (limited to 'include/grpc++') diff --git a/include/grpc++/impl/codegen/client_context.h b/include/grpc++/impl/codegen/client_context.h index ff6b107452..38cce27b99 100644 --- a/include/grpc++/impl/codegen/client_context.h +++ b/include/grpc++/impl/codegen/client_context.h @@ -304,7 +304,10 @@ class ClientContext { /// Flag whether the initial metadata should be \a corked /// /// If \a corked is true, then the initial metadata will be coalesced with the - /// write of first message in the stream. + /// write of first message in the stream. As a result, any tag set for the + /// initial metadata operation (starting a client-streaming or bidi-streaming + /// RPC) will not actually be sent to the completion queue or delivered + /// via Next. /// /// \param corked The flag indicating whether the initial metadata is to be /// corked or not. @@ -332,6 +335,10 @@ class ClientContext { /// already finished, it may still return success. /// /// There is no guarantee the call will be cancelled. + /// + /// Note that TryCancel() does not change any of the tags that are pending + /// on the completion queue. All pending tags will still be delivered + /// (though their ok result may reflect the effect of cancellation). void TryCancel(); /// Global Callbacks diff --git a/include/grpc++/impl/codegen/completion_queue.h b/include/grpc++/impl/codegen/completion_queue.h index b8a7862578..83477d0489 100644 --- a/include/grpc++/impl/codegen/completion_queue.h +++ b/include/grpc++/impl/codegen/completion_queue.h @@ -111,12 +111,83 @@ class CompletionQueue : private GrpcLibraryCodegen { /// Tri-state return for AsyncNext: SHUTDOWN, GOT_EVENT, TIMEOUT. enum NextStatus { - SHUTDOWN, ///< The completion queue has been shutdown. + SHUTDOWN, ///< The completion queue has been shutdown and fully-drained GOT_EVENT, ///< Got a new event; \a tag will be filled in with its ///< associated value; \a ok indicating its success. TIMEOUT ///< deadline was reached. }; + /// Read from the queue, blocking until an event is available or the queue is + /// shutting down. + /// + /// \param tag[out] Updated to point to the read event's tag. + /// \param ok[out] true if read a successful event, false otherwise. + /// + /// Note that each tag sent to the completion queue (through RPC operations + /// or alarms) will be delivered out of the completion queue by a call to + /// Next (or a related method), regardless of whether the operation succeeded + /// or not. Success here means that this operation completed in the normal + /// valid manner. + /// + /// Server-side RPC request: \a ok indicates that the RPC has indeed + /// been started. If it is false, the server has been Shutdown + /// before this particular call got matched to an incoming RPC. + /// + /// Client-side StartCall/RPC invocation: \a ok indicates that the RPC is + /// going to go to the wire. If it is false, it not going to the wire. This + /// would happen if the channel is either permanently broken or + /// transiently broken but with the fail-fast option. (Note that async unary + /// RPCs don't post a CQ tag at this point, nor do client-streaming + /// or bidi-streaming RPCs that have the initial metadata corked option set.) + /// + /// Client-side Write, Client-side WritesDone, Server-side Write, + /// Server-side Finish, Server-side SendInitialMetadata (which is + /// typically included in Write or Finish when not done explicitly): + /// \a ok means that the data/metadata/status/etc is going to go to the + /// wire. If it is false, it not going to the wire because the call + /// is already dead (i.e., canceled, deadline expired, other side + /// dropped the channel, etc). + /// + /// Client-side Read, Server-side Read, Client-side + /// RecvInitialMetadata (which is typically included in Read if not + /// done explicitly): \a ok indicates whether there is a valid message + /// that got read. If not, you know that there are certainly no more + /// messages that can ever be read from this stream. For the client-side + /// operations, this only happens because the call is dead. For the + /// server-sider operation, though, this could happen because the client + /// has done a WritesDone already. + /// + /// Client-side Finish: \a ok should always be true + /// + /// Server-side AsyncNotifyWhenDone: \a ok should always be true + /// + /// Alarm: \a ok is true if it expired, false if it was canceled + /// + /// \return true if got an event, false if the queue is fully drained and + /// shut down. + bool Next(void** tag, bool* ok) { + return (AsyncNextInternal(tag, ok, + g_core_codegen_interface->gpr_inf_future( + GPR_CLOCK_REALTIME)) != SHUTDOWN); + } + + /// Read from the queue, blocking up to \a deadline (or the queue's shutdown). + /// Both \a tag and \a ok are updated upon success (if an event is available + /// within the \a deadline). A \a tag points to an arbitrary location usually + /// employed to uniquely identify an event. + /// + /// \param tag[out] Upon sucess, updated to point to the event's tag. + /// \param ok[out] Upon sucess, true if a successful event, false otherwise + /// See documentation for CompletionQueue::Next for explanation of ok + /// \param deadline[in] How long to block in wait for an event. + /// + /// \return The type of event read. + template + NextStatus AsyncNext(void** tag, bool* ok, const T& deadline) { + TimePoint deadline_tp(deadline); + return AsyncNextInternal(tag, ok, deadline_tp.raw_time()); + } + /// EXPERIMENTAL /// First executes \a F, then reads from the queue, blocking up to /// \a deadline (or the queue's shutdown). @@ -141,44 +212,16 @@ class CompletionQueue : private GrpcLibraryCodegen { } } - /// Read from the queue, blocking up to \a deadline (or the queue's shutdown). - /// Both \a tag and \a ok are updated upon success (if an event is available - /// within the \a deadline). A \a tag points to an arbitrary location usually - /// employed to uniquely identify an event. - /// - /// \param tag[out] Upon sucess, updated to point to the event's tag. - /// \param ok[out] Upon sucess, true if read a regular event, false otherwise. - /// \param deadline[in] How long to block in wait for an event. - /// - /// \return The type of event read. - template - NextStatus AsyncNext(void** tag, bool* ok, const T& deadline) { - TimePoint deadline_tp(deadline); - return AsyncNextInternal(tag, ok, deadline_tp.raw_time()); - } - - /// Read from the queue, blocking until an event is available or the queue is - /// shutting down. - /// - /// \param tag[out] Updated to point to the read event's tag. - /// \param ok[out] true if read a regular event, false otherwise. - /// - /// \return true if read a regular event, false if the queue is shutting down. - bool Next(void** tag, bool* ok) { - return (AsyncNextInternal(tag, ok, - g_core_codegen_interface->gpr_inf_future( - GPR_CLOCK_REALTIME)) != SHUTDOWN); - } - /// Request the shutdown of the queue. /// /// \warning This method must be called at some point if this completion queue - /// is accessed with Next or AsyncNext. Once invoked, \a Next - /// will start to return false and \a AsyncNext will return \a - /// NextStatus::SHUTDOWN. Only once either one of these methods does that - /// (that is, once the queue has been \em drained) can an instance of this - /// class be destroyed. Also note that applications must ensure that - /// no work is enqueued on this completion queue after this method is called. + /// is accessed with Next or AsyncNext. \a Next will not return false + /// until this method has been called and all pending tags have been drained. + /// (Likewise for \a AsyncNext returning \a NextStatus::SHUTDOWN .) + /// Only once either one of these methods does that (that is, once the queue + /// has been \em drained) can an instance of this class be destroyed. + /// Also note that applications must ensure that no work is enqueued on this + /// completion queue after this method is called. void Shutdown(); /// Returns a \em raw pointer to the underlying \a grpc_completion_queue diff --git a/include/grpc++/impl/codegen/server_context.h b/include/grpc++/impl/codegen/server_context.h index c36975860e..57347f4fcd 100644 --- a/include/grpc++/impl/codegen/server_context.h +++ b/include/grpc++/impl/codegen/server_context.h @@ -151,6 +151,10 @@ class ServerContext { /// The only exception is that if the serverhandler is already returning an /// error status code, it is ok to not return Status::CANCELLED even if /// TryCancel() was called. + /// + /// Note that TryCancel() does not change any of the tags that are pending + /// on the completion queue. All pending tags will still be delivered + /// (though their ok result may reflect the effect of cancellation). void TryCancel() const; /// Return a collection of initial metadata key-value pairs sent from the -- cgit v1.2.3 From 89464de6f04860b661d736c5645951be0b69357b Mon Sep 17 00:00:00 2001 From: yang-g Date: Wed, 7 Feb 2018 15:56:51 -0800 Subject: Trim backup slice when it is larger than needed --- include/grpc++/impl/codegen/proto_utils.h | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) (limited to 'include/grpc++') diff --git a/include/grpc++/impl/codegen/proto_utils.h b/include/grpc++/impl/codegen/proto_utils.h index b7636034d4..209250bba2 100644 --- a/include/grpc++/impl/codegen/proto_utils.h +++ b/include/grpc++/impl/codegen/proto_utils.h @@ -59,18 +59,22 @@ class GrpcBufferWriter : public ::grpc::protobuf::io::ZeroCopyOutputStream { bool Next(void** data, int* size) override { // Protobuf should not ask for more memory than total_size_. GPR_CODEGEN_ASSERT(byte_count_ < total_size_); + size_t remain = total_size_ - byte_count_; if (have_backup_) { slice_ = backup_slice_; have_backup_ = false; + if (GRPC_SLICE_LENGTH(slice_) > remain) { + GRPC_SLICE_SET_LENGTH(slice_, remain); + } } else { // When less than a whole block is needed, only allocate that much. // But make sure the allocated slice is not inlined. - size_t remain = total_size_ - byte_count_ > block_size_ - ? block_size_ - : total_size_ - byte_count_; + size_t allocate_length = + remain > static_cast(block_size_) ? block_size_ : remain; slice_ = g_core_codegen_interface->grpc_slice_malloc( - remain > GRPC_SLICE_INLINED_SIZE ? remain - : GRPC_SLICE_INLINED_SIZE + 1); + allocate_length > GRPC_SLICE_INLINED_SIZE + ? allocate_length + : GRPC_SLICE_INLINED_SIZE + 1); } *data = GRPC_SLICE_START_PTR(slice_); // On win x64, int is only 32bit -- cgit v1.2.3