review feedback

reviewable/pr9949/r7
Craig Tiller 8 years ago
parent e434d1d60b
commit 7a8232d773
  1. 6
      src/core/lib/surface/call.c
  2. 6
      src/cpp/common/channel_filter.cc
  3. 24
      src/cpp/common/channel_filter.h
  4. 5
      test/cpp/end2end/filter_end2end_test.cc

@ -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 {

@ -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());
}

@ -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,

@ -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();

Loading…
Cancel
Save