From 7a8232d773d746cd8d3d391d6dd625dd0b74e9f5 Mon Sep 17 00:00:00 2001
From: Craig Tiller <ctiller@google.com>
Date: Mon, 3 Apr 2017 10:59:42 -0700
Subject: [PATCH] review feedback
---
src/core/lib/surface/call.c | 6 +++++-
src/cpp/common/channel_filter.cc | 6 +++---
src/cpp/common/channel_filter.h | 24 ++++++++++++------------
test/cpp/end2end/filter_end2end_test.cc | 5 +++--
4 files changed, 23 insertions(+), 18 deletions(-)
diff --git a/src/core/lib/surface/call.c b/src/core/lib/surface/call.c
index 966d89451d9..87787b3eea7 100644
--- a/src/core/lib/surface/call.c
+++ b/src/core/lib/surface/call.c
@@ -119,7 +119,11 @@ typedef struct batch_control {
grpc_call *call;
/* Share memory for cq_completion and notify_tag as they are never needed
simultaneously. Each byte used in this data structure count as six bytes
- per call, so any savings we can make are worthwhile */
+ per call, so any savings we can make are worthwhile,
+
+ We use notify_tag to determine whether or not to send notification to the
+ completion queue. Once we've made that determination, we can reuse the
+ memory for cq_completion. */
union {
grpc_cq_completion cq_completion;
struct {
diff --git a/src/cpp/common/channel_filter.cc b/src/cpp/common/channel_filter.cc
index 253614ca9b4..a7b3c2c0dac 100644
--- a/src/cpp/common/channel_filter.cc
+++ b/src/cpp/common/channel_filter.cc
@@ -69,9 +69,9 @@ void ChannelData::GetInfo(grpc_exec_ctx *exec_ctx, grpc_channel_element *elem,
// CallData
-void CallData::StartTransportStreamOp(grpc_exec_ctx *exec_ctx,
- grpc_call_element *elem,
- TransportStreamOp *op) {
+void CallData::StartTransportStreamOpBatch(grpc_exec_ctx *exec_ctx,
+ grpc_call_element *elem,
+ TransportStreamOpBatch *op) {
grpc_call_next_op(exec_ctx, elem, op->op());
}
diff --git a/src/cpp/common/channel_filter.h b/src/cpp/common/channel_filter.h
index 726e5abf8d3..8d800b87d9a 100644
--- a/src/cpp/common/channel_filter.h
+++ b/src/cpp/common/channel_filter.h
@@ -141,12 +141,12 @@ class TransportOp {
};
/// A C++ wrapper for the \c grpc_transport_stream_op_batch struct.
-class TransportStreamOp {
+class TransportStreamOpBatch {
public:
/// Borrows a pointer to \a op, but does NOT take ownership.
/// The caller must ensure that \a op continues to exist for as
- /// long as the TransportStreamOp object does.
- explicit TransportStreamOp(grpc_transport_stream_op_batch *op)
+ /// long as the TransportStreamOpBatch object does.
+ explicit TransportStreamOpBatch(grpc_transport_stream_op_batch *op)
: op_(op),
send_initial_metadata_(
op->send_initial_metadata
@@ -257,9 +257,9 @@ class CallData {
// TODO(roth): Find a way to avoid passing elem into these methods.
/// Starts a new stream operation.
- virtual void StartTransportStreamOp(grpc_exec_ctx *exec_ctx,
- grpc_call_element *elem,
- TransportStreamOp *op);
+ virtual void StartTransportStreamOpBatch(grpc_exec_ctx *exec_ctx,
+ grpc_call_element *elem,
+ TransportStreamOpBatch *op);
/// Sets a pollset or pollset set.
virtual void SetPollsetOrPollsetSet(grpc_exec_ctx *exec_ctx,
@@ -329,12 +329,12 @@ class ChannelFilter final {
reinterpret_cast<CallDataType *>(elem->call_data)->~CallDataType();
}
- static void StartTransportStreamOp(grpc_exec_ctx *exec_ctx,
- grpc_call_element *elem,
- grpc_transport_stream_op_batch *op) {
+ static void StartTransportStreamOpBatch(grpc_exec_ctx *exec_ctx,
+ grpc_call_element *elem,
+ grpc_transport_stream_op_batch *op) {
CallDataType *call_data = (CallDataType *)elem->call_data;
- TransportStreamOp op_wrapper(op);
- call_data->StartTransportStreamOp(exec_ctx, elem, &op_wrapper);
+ TransportStreamOpBatch op_wrapper(op);
+ call_data->StartTransportStreamOpBatch(exec_ctx, elem, &op_wrapper);
}
static void SetPollsetOrPollsetSet(grpc_exec_ctx *exec_ctx,
@@ -386,7 +386,7 @@ void RegisterChannelFilter(
stack_type,
priority,
include_filter,
- {FilterType::StartTransportStreamOp, FilterType::StartTransportOp,
+ {FilterType::StartTransportStreamOpBatch, FilterType::StartTransportOp,
FilterType::call_data_size, FilterType::InitCallElement,
FilterType::SetPollsetOrPollsetSet, FilterType::DestroyCallElement,
FilterType::channel_data_size, FilterType::InitChannelElement,
diff --git a/test/cpp/end2end/filter_end2end_test.cc b/test/cpp/end2end/filter_end2end_test.cc
index bd384f68b40..2f873eeaa8e 100644
--- a/test/cpp/end2end/filter_end2end_test.cc
+++ b/test/cpp/end2end/filter_end2end_test.cc
@@ -122,8 +122,9 @@ class ChannelDataImpl : public ChannelData {
class CallDataImpl : public CallData {
public:
- void StartTransportStreamOp(grpc_exec_ctx* exec_ctx, grpc_call_element* elem,
- TransportStreamOp* op) override {
+ void StartTransportStreamOpBatch(grpc_exec_ctx* exec_ctx,
+ grpc_call_element* elem,
+ TransportStreamOpBatch* op) override {
// Incrementing the counter could be done from Init(), but we want
// to test that the individual methods are actually called correctly.
if (op->recv_initial_metadata() != nullptr) IncrementCallCounter();