C++: Experimental client callback streaming API by vjpai · Pull Request #17104 · grpc/grpc

vjpai · 2018-11-05T21:13:17Z

Add C++ callback streaming API based on "Reactor" model

Detailed review request to @yashykt

Words of wisdom request to @yang-g and @wcevans

grpc-testing · 2018-11-30T10:58:24Z

[trickle] No significant performance differences

grpc-testing · 2018-11-30T11:03:00Z

Objective-C binary sizes
*****************STATIC******************
  New size                      Old size
 2,020,410      Total (=)      2,020,410

 No significant differences in binary sizes

***************FRAMEWORKS****************
  New size                      Old size
11,174,077      Total (<)     11,174,080

 No significant differences in binary sizes

grpc-testing · 2018-11-30T11:07:37Z

Objective-C binary sizes
*****************STATIC******************
  New size                      Old size
 2,020,410      Total (=)      2,020,410

 No significant differences in binary sizes

***************FRAMEWORKS****************
  New size                      Old size
11,174,085      Total (>)     11,174,079

 No significant differences in binary sizes

grpc-testing · 2018-11-30T11:13:58Z

Corrupt JSON data (indicates timeout or crash): 
    bm_call_create.BM_IsolatedFilter_ClientChannelFilter_NoOp_.counters.new: 10
    bm_call_create.BM_IsolatedFilter_ClientChannelFilter_NoOp_.counters.old: 10


[microbenchmarks] No significant performance differences

grpc-testing · 2018-11-30T11:14:24Z

Corrupt JSON data (indicates timeout or crash): 
    bm_call_create.BM_IsolatedFilter_ClientChannelFilter_NoOp_.counters.new: 10
    bm_call_create.BM_IsolatedFilter_ClientChannelFilter_NoOp_.counters.old: 10


[microbenchmarks] No significant performance differences

grpc-testing · 2018-11-30T11:14:48Z

Corrupt JSON data (indicates timeout or crash): 
    bm_call_create.BM_IsolatedFilter_ClientChannelFilter_NoOp_.counters.new: 10
    bm_call_create.BM_IsolatedFilter_ClientChannelFilter_NoOp_.counters.old: 10


[microbenchmarks] No significant performance differences

grpc-testing · 2018-11-30T11:31:06Z

Objective-C binary sizes
*****************STATIC******************
  New size                      Old size
 2,020,410      Total (=)      2,020,410

 No significant differences in binary sizes

***************FRAMEWORKS****************
  New size                      Old size
11,174,077      Total (>)     11,174,075

 No significant differences in binary sizes

grpc-testing · 2018-11-30T11:40:58Z

Corrupt JSON data (indicates timeout or crash): 
    bm_call_create.BM_IsolatedFilter_ClientChannelFilter_NoOp_.counters.new: 10
    bm_call_create.BM_IsolatedFilter_ClientChannelFilter_NoOp_.counters.old: 10


[microbenchmarks] No significant performance differences

grpc-testing · 2018-11-30T12:20:50Z

****************************************************************

libgrpc.so

     VM SIZE        FILE SIZE
 ++++++++++++++  ++++++++++++++

  [ = ]       0        0  [ = ]


****************************************************************

libgrpc++.so

     VM SIZE                                                                                            FILE SIZE
 ++++++++++++++ GROWING                                                                              ++++++++++++++
  +6.4% +19.3Ki [None]                                                                                +153Ki  +1.9%
      +6.3% +17.5Ki [Unmapped]                                                                            +151Ki  +1.9%
      [NEW]    +146 typeinfo name for grpc::internal::CallOpSet<grpc::internal::CallOpSendInitialMetadat    +146  [NEW]
      [NEW]    +142 typeinfo name for grpc::internal::CallOpSet<grpc::internal::CallOpSendInitialMetadat    +142  [NEW]
      [NEW]    +136 typeinfo for grpc::internal::CallOpSet<grpc::internal::CallOpClientRecvStatus, grpc:    +136  [NEW]
      [NEW]    +136 typeinfo for grpc::internal::CallOpSet<grpc::internal::CallOpSendInitialMetadata, gr    +136  [NEW]
      [NEW]    +136 typeinfo for grpc::internal::CallOpSet<grpc::internal::CallOpSendInitialMetadata, gr    +136  [NEW]
      [NEW]    +122 typeinfo name for grpc::internal::CallOpSet<grpc::internal::CallOpClientRecvStatus,     +122  [NEW]
      [NEW]     +90 typeinfo name for grpc::internal::ClientCallbackReaderWriterImpl<grpc::ByteBuffer, g     +90  [NEW]
      [NEW]     +89 typeinfo name for grpc::internal::ClientCallbackReaderWriterImpl<grpc::ByteBuffer, g     +89  [NEW]
      [NEW]     +89 typeinfo name for grpc::internal::ClientCallbackReaderWriterImpl<grpc::ByteBuffer, g     +89  [NEW]
      [NEW]     +89 typeinfo name for grpc::internal::ClientCallbackReaderWriterImpl<grpc::ByteBuffer, g     +89  [NEW]
      [NEW]     +88 typeinfo name for grpc::internal::ClientCallbackReaderWriterImpl<grpc::ByteBuffer, g     +88  [NEW]
      [NEW]     +80 [Other]                                                                                  +80  [NEW]
      [NEW]     +80 vtable for grpc::internal::CallOpSet<grpc::internal::CallOpClientRecvStatus, grpc::i     +80  [NEW]
      [NEW]     +80 vtable for grpc::internal::CallOpSet<grpc::internal::CallOpSendInitialMetadata, grpc     +80  [NEW]
      [NEW]     +80 vtable for grpc::internal::CallOpSet<grpc::internal::CallOpSendInitialMetadata, grpc     +80  [NEW]
      [NEW]     +71 typeinfo name for grpc::experimental::ClientCallbackReaderWriter<grpc::ByteBuffer, g     +71  [NEW]
      [NEW]     +70 typeinfo name for grpc::internal::ClientCallbackReaderWriterImpl<grpc::ByteBuffer, g     +70  [NEW]
      [NEW]     +64 vtable for grpc::internal::ClientCallbackReaderWriterImpl<grpc::ByteBuffer, grpc::By     +64  [NEW]
      [NEW]     +24 typeinfo for grpc::internal::ClientCallbackReaderWriterImpl<grpc::ByteBuffer, grpc::     +24  [NEW]
      [NEW]     +16 typeinfo for grpc::experimental::ClientCallbackReaderWriter<grpc::ByteBuffer, grpc::     +16  [NEW]
   +29% +12.0Ki src/cpp/client/generic_stub.cc                                                       +12.0Ki   +29%
     +11e3% +3.07Ki [Other]                                                                              +3.07Ki +11e3%
      [NEW] +2.72Ki grpc::GenericStub::experimental_type::PrepareBidiStreamingCall                       +2.72Ki  [NEW]
      [NEW] +1.78Ki grpc::internal::ClientCallbackReaderWriterImpl<grpc::ByteBuffer, grpc::ByteBuffer>:: +1.78Ki  [NEW]
      [NEW] +1.58Ki grpc::internal::CallOpClientRecvStatus::FinishOp                                     +1.58Ki  [NEW]
      [NEW] +1.27Ki grpc::internal::ClientCallbackReaderWriterImpl<grpc::ByteBuffer, grpc::ByteBuffer>:: +1.27Ki  [NEW]
      [NEW] +1.27Ki grpc::internal::ClientCallbackReaderWriterImpl<grpc::ByteBuffer, grpc::ByteBuffer>:: +1.27Ki  [NEW]
      [NEW]    +941 grpc::internal::CallOpSet<grpc::internal::CallOpSendInitialMetadata, grpc::internal:    +941  [NEW]
      [NEW]    +925 grpc::internal::CallOpSet<grpc::internal::CallOpSendInitialMetadata, grpc::internal:    +925  [NEW]
      [NEW]    +694 grpc::internal::CallOpSet<grpc::internal::CallOpSendInitialMetadata, grpc::internal:    +694  [NEW]
      [NEW]    +662 grpc::internal::CallOpSet<grpc::internal::CallOpSendInitialMetadata, grpc::internal:    +662  [NEW]
      [NEW]    +590 grpc::internal::ClientCallbackReaderWriterImpl<grpc::ByteBuffer, grpc::ByteBuffer>::    +590  [NEW]
      [NEW]    +565 grpc::internal::CallOpSet<grpc::internal::CallOpRecvMessage<grpc::ByteBuffer>, grpc:    +565  [NEW]
      [NEW]    +466 grpc::internal::ClientCallbackReaderWriterImpl<grpc::ByteBuffer, grpc::ByteBuffer>::    +466  [NEW]
      [NEW]    +464 grpc::internal::CallOpSet<grpc::internal::CallOpSendInitialMetadata, grpc::internal:    +464  [NEW]
      [NEW]    +464 grpc::internal::CallOpSet<grpc::internal::CallOpSendInitialMetadata, grpc::internal:    +464  [NEW]
      [NEW]    +448 grpc::internal::CallOpSet<grpc::internal::CallOpClientRecvStatus, grpc::internal::Ca    +448  [NEW]
      [NEW]    +432 grpc::internal::CallOpSet<grpc::internal::CallOpRecvMessage<grpc::ByteBuffer>, grpc:    +432  [NEW]
      [NEW]    +359 grpc::internal::InterceptorBatchMethodsImpl::RunInterceptors                            +359  [NEW]
      [NEW]    +300 grpc::internal::CallOpSet<grpc::internal::CallOpClientRecvStatus, grpc::internal::Ca    +300  [NEW]
      [NEW]    +268 grpc::internal::CallOpSet<grpc::internal::CallOpRecvMessage<grpc::ByteBuffer>, grpc:    +268  [NEW]
      [NEW]    +210 grpc::internal::CallOpSet<grpc::internal::CallOpClientRecvStatus, grpc::internal::Ca    +210  [NEW]

 -------------- SHRINKING                                                                            --------------
  -3.7% -1.80Ki src/cpp/server/server_cc.cc                                                          -1.80Ki  -3.7%
      [DEL]    -565 grpc::internal::CallOpSet<grpc::internal::CallOpRecvMessage<grpc::ByteBuffer>, grpc:    -565  [DEL]
      [DEL]    -359 grpc::internal::InterceptorBatchMethodsImpl::RunInterceptors                            -359  [DEL]
      [DEL]    -268 grpc::internal::CallOpSet<grpc::internal::CallOpRecvMessage<grpc::ByteBuffer>, grpc:    -268  [DEL]
      [DEL]    -178 grpc::internal::CallOpSet<grpc::internal::CallOpRecvMessage<grpc::ByteBuffer>, grpc:    -178  [DEL]
      [DEL]    -151 grpc::internal::CallOpSet<grpc::internal::CallOpRecvMessage<grpc::ByteBuffer>, grpc:    -151  [DEL]
      [DEL]    -147 grpc::internal::CallOpSet<grpc::internal::CallOpRecvMessage<grpc::ByteBuffer>, grpc:    -147  [DEL]
      [DEL]    -146 grpc::internal::CallOpSet<grpc::internal::CallOpRecvMessage<grpc::ByteBuffer>, grpc:    -146  [DEL]
      [DEL]     -21 grpc::internal::CallOpSet<grpc::internal::CallOpRecvMessage<grpc::ByteBuffer>, grpc:     -21  [DEL]
      [DEL]      -5 grpc::internal::CallOpSet<grpc::internal::CallOpRecvMessage<grpc::ByteBuffer>, grpc:      -5  [DEL]
  -1.0%    -118 src/cpp/server/server_context.cc                                                        -118  -1.0%
      [DEL]    -195 grpc::internal::CallbackWithSuccessTag::StaticRun                                       -195  [DEL]

  +5.5% +29.3Ki TOTAL                                                                                 +163Ki  +2.0%

grpc-testing · 2018-11-30T12:27:51Z

[trickle] No significant performance differences

vjpai · 2018-11-30T12:31:35Z

This is reviewable

grpc-testing · 2018-11-30T13:02:33Z

Objective-C binary sizes
*****************STATIC******************
  New size                      Old size
 2,020,410      Total (=)      2,020,410

 No significant differences in binary sizes

***************FRAMEWORKS****************
  New size                      Old size
11,174,081      Total (>)     11,174,077

 No significant differences in binary sizes

grpc-testing · 2018-11-30T13:13:38Z

Corrupt JSON data (indicates timeout or crash): 
    bm_call_create.BM_IsolatedFilter_ClientChannelFilter_NoOp_.counters.new: 10
    bm_call_create.BM_IsolatedFilter_ClientChannelFilter_NoOp_.counters.old: 10


[microbenchmarks] No significant performance differences

vjpai · 2018-11-30T16:26:03Z

Remaining failures unrelated

include/grpcpp/impl/codegen/client_callback.h

test/cpp/end2end/client_callback_end2end_test.cc

vjpai force-pushed the callback_streaming branch from 834e749 to 318a7a5 Compare November 5, 2018 21:18

vjpai added the release notes: no Indicates if PR should not be in release notes label Nov 5, 2018

vjpai force-pushed the callback_streaming branch 3 times, most recently from ad03010 to 944bb93 Compare November 10, 2018 21:16

vjpai changed the title ~~WIP C++: Experimental client callback streaming API~~ C++: Experimental client callback streaming API Nov 10, 2018

vjpai requested a review from yashykt November 10, 2018 21:17

grpc deleted a comment from grpc-testing Nov 11, 2018

vjpai added 9 commits November 30, 2018 04:13

Client callback streaming

d7eb266

Improve test readability at suggestion of @ghemawat

85128c7

Address reviewer comments

fa45ffd

More review comments

5dcfc8b

Revert an order change

60949a8

Make StartCall() a releasing operation so that you can pile up ops

dac2066

Stop exposing streaming object class

ea1156d

clang-format

28dd798

Fix clang-tidy and golden file issues

2b5d45a

vjpai force-pushed the callback_streaming branch from 9d46fd0 to 2b5d45a Compare November 30, 2018 12:13

mhaidrygoog reviewed Nov 30, 2018

View reviewed changes

mhaidrygoog approved these changes Nov 30, 2018

View reviewed changes

vjpai merged commit 53949b7 into grpc:master Nov 30, 2018

vjpai deleted the callback_streaming branch November 30, 2018 23:24

jtattermusch mentioned this pull request Dec 3, 2018

//test/cpp/end2end:client_callback_end2end_test broken on MSAN #17375

Closed

lock bot locked as resolved and limited conversation to collaborators Feb 28, 2019

Conversation

vjpai commented Nov 5, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

grpc-testing commented Nov 30, 2018

Uh oh!

grpc-testing commented Nov 30, 2018

Uh oh!

grpc-testing commented Nov 30, 2018

Uh oh!

grpc-testing commented Nov 30, 2018

Uh oh!

grpc-testing commented Nov 30, 2018

Uh oh!

grpc-testing commented Nov 30, 2018

Uh oh!

grpc-testing commented Nov 30, 2018

Uh oh!

grpc-testing commented Nov 30, 2018

Uh oh!

grpc-testing commented Nov 30, 2018

Uh oh!

grpc-testing commented Nov 30, 2018

Uh oh!

vjpai commented Nov 30, 2018

Uh oh!

grpc-testing commented Nov 30, 2018

Uh oh!

grpc-testing commented Nov 30, 2018

Uh oh!

vjpai commented Nov 30, 2018

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

vjpai commented Nov 5, 2018 •

edited

Loading