From a40ccd858085bfd5148e4e018b62d3f82430ebc6 Mon Sep 17 00:00:00 2001 From: Muxi Yan Date: Sat, 5 Nov 2016 21:39:44 -0700 Subject: [PATCH 01/44] Packet coalescing Objc layer and interop tests --- .../cronet/transport/cronet_api_dummy.c | 3 +- src/objective-c/GRPCClient/GRPCCall.m | 46 +++++-- .../RxLibrary/GRXImmediateSingleWriter.h | 50 ++++++++ .../RxLibrary/GRXImmediateSingleWriter.m | 83 +++++++++++++ .../RxLibrary/GRXWriter+Immediate.m | 3 +- src/objective-c/tests/InteropTests.m | 64 ++++++++-- .../InteropTestsRemoteWithCronet.m | 11 ++ src/objective-c/tests/Podfile | 1 + .../tests/Tests.xcodeproj/project.pbxproj | 1 + .../objective_c/Cronet/cronet_c_for_grpc.h | 115 ++++++++++++------ 10 files changed, 320 insertions(+), 57 deletions(-) create mode 100644 src/objective-c/RxLibrary/GRXImmediateSingleWriter.h create mode 100644 src/objective-c/RxLibrary/GRXImmediateSingleWriter.m diff --git a/src/core/ext/transport/cronet/transport/cronet_api_dummy.c b/src/core/ext/transport/cronet/transport/cronet_api_dummy.c index 687026c9fde..38755604b97 100644 --- a/src/core/ext/transport/cronet/transport/cronet_api_dummy.c +++ b/src/core/ext/transport/cronet/transport/cronet_api_dummy.c @@ -77,9 +77,8 @@ int cronet_bidirectional_stream_write(cronet_bidirectional_stream* stream, return 0; } -int cronet_bidirectional_stream_cancel(cronet_bidirectional_stream* stream) { +void cronet_bidirectional_stream_cancel(cronet_bidirectional_stream* stream) { GPR_ASSERT(0); - return 0; } #endif /* GRPC_COMPILE_WITH_CRONET */ diff --git a/src/objective-c/GRPCClient/GRPCCall.m b/src/objective-c/GRPCClient/GRPCCall.m index 44393f6b999..0a103223675 100644 --- a/src/objective-c/GRPCClient/GRPCCall.m +++ b/src/objective-c/GRPCClient/GRPCCall.m @@ -36,6 +36,7 @@ #include #include #import +#import #import "private/GRPCConnectivityMonitor.h" #import "private/GRPCHost.h" @@ -100,6 +101,10 @@ static NSMutableDictionary *callFlags; GRPCCall *_retainSelf; GRPCRequestHeaders *_requestHeaders; + + BOOL _unaryCall; + + NSMutableArray *_unaryOpBatch; } @synthesize state = _state; @@ -157,6 +162,11 @@ static NSMutableDictionary *callFlags; _requestWriter = requestWriter; _requestHeaders = [[GRPCRequestHeaders alloc] initWithCall:self]; + + if ([requestWriter isKindOfClass:[GRXImmediateSingleWriter class]]) { + _unaryCall = true; + _unaryOpBatch = [[NSMutableArray alloc] init]; + } } return self; } @@ -165,6 +175,9 @@ static NSMutableDictionary *callFlags; - (void)finishWithError:(NSError *)errorOrNil { @synchronized(self) { + if (_state == GRXWriterStateFinished) { + return; + } _state = GRXWriterStateFinished; } @@ -254,9 +267,15 @@ static NSMutableDictionary *callFlags; - (void)sendHeaders:(NSDictionary *)headers { // TODO(jcanizales): Add error handlers for async failures - [_wrappedCall startBatchWithOperations:@[[[GRPCOpSendMetadata alloc] initWithMetadata:headers - flags:[GRPCCall callFlagsForHost:_host path:_path] - handler:nil]]]; + if (!_unaryCall) { + [_wrappedCall startBatchWithOperations:@[[[GRPCOpSendMetadata alloc] initWithMetadata:headers + flags:[GRPCCall callFlagsForHost:_host path:_path] + handler:nil]]]; + } else { + [_unaryOpBatch addObject:[[GRPCOpSendMetadata alloc] initWithMetadata:headers + flags:[GRPCCall callFlagsForHost:_host path:_path] + handler:nil]]; + } } #pragma mark GRXWriteable implementation @@ -275,9 +294,14 @@ static NSMutableDictionary *callFlags; } } }; - [_wrappedCall startBatchWithOperations:@[[[GRPCOpSendMessage alloc] initWithMessage:message - handler:resumingHandler]] - errorHandler:errorHandler]; + if (!_unaryCall) { + [_wrappedCall startBatchWithOperations:@[[[GRPCOpSendMessage alloc] initWithMessage:message + handler:resumingHandler]] + errorHandler:errorHandler]; + } else { + [_unaryOpBatch addObject:[[GRPCOpSendMessage alloc] initWithMessage:message + handler:resumingHandler]]; + } } - (void)writeValue:(id)value { @@ -302,8 +326,14 @@ static NSMutableDictionary *callFlags; // Only called from the call queue. The error handler will be called from the // network queue if the requests stream couldn't be closed successfully. - (void)finishRequestWithErrorHandler:(void (^)())errorHandler { - [_wrappedCall startBatchWithOperations:@[[[GRPCOpSendClose alloc] init]] - errorHandler:errorHandler]; + if (!_unaryOpBatch) { + [_wrappedCall startBatchWithOperations:@[[[GRPCOpSendClose alloc] init]] + errorHandler:errorHandler]; + } else { + [_unaryOpBatch addObject:[[GRPCOpSendClose alloc] init]]; + [_wrappedCall startBatchWithOperations:_unaryOpBatch + errorHandler:errorHandler]; + } } - (void)writesFinishedWithError:(NSError *)errorOrNil { diff --git a/src/objective-c/RxLibrary/GRXImmediateSingleWriter.h b/src/objective-c/RxLibrary/GRXImmediateSingleWriter.h new file mode 100644 index 00000000000..0ec788f756b --- /dev/null +++ b/src/objective-c/RxLibrary/GRXImmediateSingleWriter.h @@ -0,0 +1,50 @@ +/* + * + * Copyright 2016, Google Inc. + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are + * met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following disclaimer + * in the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Google Inc. nor the names of its + * contributors may be used to endorse or promote products derived from + * this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + * + */ + +#import + +#import "GRXImmediateWriter.h" + +/** + * Utility to construct GRXWriter instances from values that are immediately available when + * required. + */ +@interface GRXImmediateSingleWriter : GRXImmediateWriter + +/** + * Returns a writer that sends the passed value to its writeable and then finishes (releasing the + * value). + */ ++ (GRXWriter *)writerWithValue:(id)value; + +@end diff --git a/src/objective-c/RxLibrary/GRXImmediateSingleWriter.m b/src/objective-c/RxLibrary/GRXImmediateSingleWriter.m new file mode 100644 index 00000000000..a0d3b771e8d --- /dev/null +++ b/src/objective-c/RxLibrary/GRXImmediateSingleWriter.m @@ -0,0 +1,83 @@ +/* + * + * Copyright 2016, Google Inc. + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are + * met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following disclaimer + * in the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Google Inc. nor the names of its + * contributors may be used to endorse or promote products derived from + * this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + * + */ + +#import "GRXImmediateSingleWriter.h" + +@implementation GRXImmediateSingleWriter { + id _value; + NSError *_errorOrNil; + id _writeable; +} + +@synthesize state = _state; + +- (instancetype)initWithValue:(id)value error:(NSError *)errorOrNil { + if (self = [super init]) { + _value = value; + _errorOrNil = errorOrNil; + _state = GRXWriterStateNotStarted; + } + return self; +} + ++ (GRXWriter *)writerWithValue:(id)value { + return [[self alloc] initWithValue:value error:nil]; +} + +- (void)startWithWriteable:(id)writeable { + _state = GRXWriterStateStarted; + _writeable = writeable; + [writeable writeValue:_value]; + [self finishWithError:_errorOrNil]; +} + +- (void)finishWithError:(NSError *)errorOrNil { + _state = GRXWriterStateFinished; + _errorOrNil = nil; + _value = nil; + id writeable = _writeable; + _writeable = nil; + [writeable writesFinishedWithError:errorOrNil]; +} + +- (void)setState:(GRXWriterState)newState { + // Manual state transition is not allowed + return; +} + +- (GRXWriter *)map:(id (^)(id))map { + _value = map(_value); + return self; +} + +@end \ No newline at end of file diff --git a/src/objective-c/RxLibrary/GRXWriter+Immediate.m b/src/objective-c/RxLibrary/GRXWriter+Immediate.m index 1d55eb35293..ea6e6814063 100644 --- a/src/objective-c/RxLibrary/GRXWriter+Immediate.m +++ b/src/objective-c/RxLibrary/GRXWriter+Immediate.m @@ -34,6 +34,7 @@ #import "GRXWriter+Immediate.h" #import "GRXImmediateWriter.h" +#import "GRXImmediateSingleWriter.h" @implementation GRXWriter (Immediate) @@ -50,7 +51,7 @@ } + (instancetype)writerWithValue:(id)value { - return [GRXImmediateWriter writerWithValue:value]; + return [GRXImmediateSingleWriter writerWithValue:value]; } + (instancetype)writerWithError:(NSError *)error { diff --git a/src/objective-c/tests/InteropTests.m b/src/objective-c/tests/InteropTests.m index c3935ce1e09..8adf0a6164c 100644 --- a/src/objective-c/tests/InteropTests.m +++ b/src/objective-c/tests/InteropTests.m @@ -45,6 +45,8 @@ #import #import #import +#import +#import #define TEST_TIMEOUT 32 @@ -94,15 +96,6 @@ return 0; } -+ (void)setUp { -#ifdef GRPC_COMPILE_WITH_CRONET - // Cronet setup - [Cronet setHttp2Enabled:YES]; - [Cronet start]; - [GRPCCall useCronetWithEngine:[Cronet getGlobalEngine]]; -#endif -} - - (void)setUp { self.continueAfterFailure = NO; @@ -152,6 +145,59 @@ [self waitForExpectationsWithTimeout:TEST_TIMEOUT handler:nil]; } +// TODO (mxyan): Do the same test for chttp2 +#ifdef GRPC_COMPILE_WITH_CRONET +#ifdef GRPC_CRONET_WITH_PACKET_COALESCING + +static bool coalesced_message_and_eos; + +static void log_processor(gpr_log_func_args *args) { + unsigned long file_len = strlen(args->file); + const char suffix[] = "call.c"; + const int suffix_len = sizeof(suffix) - 1; + const char nops[] = "nops=3"; + + if (file_len > suffix_len && + 0 == strcmp(suffix, &args->file[file_len - suffix_len]) && + strstr(args->message, nops)) { + fprintf(stderr, "%s, %s\n", args->file, args->message); + coalesced_message_and_eos = true; + } +} + +- (void)testPacketCoalescing { + gpr_set_log_verbosity(GPR_LOG_SEVERITY_DEBUG); + grpc_tracer_set_enabled("all", 1); + gpr_set_log_function(log_processor); + coalesced_message_and_eos = false; + + XCTAssertNotNil(self.class.host); + __weak XCTestExpectation *expectation = [self expectationWithDescription:@"LargeUnary"]; + + RMTSimpleRequest *request = [RMTSimpleRequest message]; + request.responseType = RMTPayloadType_Compressable; + request.responseSize = 10; + request.payload.body = [NSMutableData dataWithLength:10]; + + [_service unaryCallWithRequest:request handler:^(RMTSimpleResponse *response, NSError *error) { + XCTAssertNil(error, @"Finished with unexpected error: %@", error); + + RMTSimpleResponse *expectedResponse = [RMTSimpleResponse message]; + expectedResponse.payload.type = RMTPayloadType_Compressable; + expectedResponse.payload.body = [NSMutableData dataWithLength:10]; + XCTAssertEqualObjects(response, expectedResponse); + + XCTAssert(coalesced_message_and_eos); + + [expectation fulfill]; + }]; + + [self waitForExpectationsWithTimeout:16 handler:nil]; +} + +#endif +#endif + - (void)test4MBResponsesAreAccepted { XCTAssertNotNil(self.class.host); __weak XCTestExpectation *expectation = [self expectationWithDescription:@"MaxResponseSize"]; diff --git a/src/objective-c/tests/InteropTestsRemoteWithCronet/InteropTestsRemoteWithCronet.m b/src/objective-c/tests/InteropTestsRemoteWithCronet/InteropTestsRemoteWithCronet.m index fab8ad8d25f..793b71a9bee 100644 --- a/src/objective-c/tests/InteropTestsRemoteWithCronet/InteropTestsRemoteWithCronet.m +++ b/src/objective-c/tests/InteropTestsRemoteWithCronet/InteropTestsRemoteWithCronet.m @@ -33,6 +33,9 @@ #import +#import +#import + #import "InteropTests.h" static NSString * const kRemoteSSLHost = @"grpc-test.sandbox.googleapis.com"; @@ -43,6 +46,14 @@ static NSString * const kRemoteSSLHost = @"grpc-test.sandbox.googleapis.com"; @implementation InteropTestsRemoteWithCronet ++ (void)setUp { + // Cronet setup + [Cronet setHttp2Enabled:YES]; + [Cronet start]; + [GRPCCall useCronetWithEngine:[Cronet getGlobalEngine]]; + [Cronet startNetLogToFile:@"Documents/cronet_netlog.json" logBytes:YES]; +} + + (NSString *)host { return kRemoteSSLHost; } diff --git a/src/objective-c/tests/Podfile b/src/objective-c/tests/Podfile index 5785b976f2d..d1ef0886fe4 100644 --- a/src/objective-c/tests/Podfile +++ b/src/objective-c/tests/Podfile @@ -92,6 +92,7 @@ post_install do |installer| # GPR_UNREACHABLE_CODE causes "Control may reach end of non-void # function" warning config.build_settings['GCC_WARN_ABOUT_RETURN_TYPE'] = 'NO' + config.build_settings['GCC_PREPROCESSOR_DEFINITIONS'] = '$(inherited) COCOAPODS=1 GRPC_CRONET_WITH_PACKET_COALESCING=1' end end diff --git a/src/objective-c/tests/Tests.xcodeproj/project.pbxproj b/src/objective-c/tests/Tests.xcodeproj/project.pbxproj index c4a6567ae0e..8455e71b026 100644 --- a/src/objective-c/tests/Tests.xcodeproj/project.pbxproj +++ b/src/objective-c/tests/Tests.xcodeproj/project.pbxproj @@ -1296,6 +1296,7 @@ "$(inherited)", "GPB_USE_PROTOBUF_FRAMEWORK_IMPORTS=1", "GRPC_COMPILE_WITH_CRONET=1", + "GRPC_CRONET_WITH_PACKET_COALESCING=1", ); INFOPLIST_FILE = InteropTestsRemoteWithCronet/Info.plist; IPHONEOS_DEPLOYMENT_TARGET = 9.3; diff --git a/third_party/objective_c/Cronet/cronet_c_for_grpc.h b/third_party/objective_c/Cronet/cronet_c_for_grpc.h index 15a511aebd0..3d58a8370ef 100644 --- a/third_party/objective_c/Cronet/cronet_c_for_grpc.h +++ b/third_party/objective_c/Cronet/cronet_c_for_grpc.h @@ -5,6 +5,8 @@ #ifndef COMPONENTS_CRONET_IOS_CRONET_C_FOR_GRPC_H_ #define COMPONENTS_CRONET_IOS_CRONET_C_FOR_GRPC_H_ +#define CRONET_EXPORT __attribute__((visibility("default"))) + #ifdef __cplusplus extern "C" { #endif @@ -15,12 +17,10 @@ extern "C" { /* Opaque object representing Cronet Engine. Created and configured outside * of this API to facilitate sharing with other components */ -typedef struct cronet_engine { void* obj; } cronet_engine; - -void cronet_engine_add_quic_hint(cronet_engine* engine, - const char* host, - int port, - int alternate_port); +typedef struct cronet_engine { + void* obj; + void* annotation; +} cronet_engine; /* Cronet Bidirectional Stream API */ @@ -45,11 +45,12 @@ typedef struct cronet_bidirectional_stream_header_array { /* Set of callbacks used to receive callbacks from bidirectional stream. */ typedef struct cronet_bidirectional_stream_callback { - /* Invoked when request headers are sent. Indicates that stream has initiated - * the request. Consumer may call cronet_bidirectional_stream_write() to start - * writing data. + /* Invoked when the stream is ready for reading and writing. + * Consumer may call cronet_bidirectional_stream_read() to start reading data. + * Consumer may call cronet_bidirectional_stream_write() to start writing + * data. */ - void (*on_request_headers_sent)(cronet_bidirectional_stream* stream); + void (*on_stream_ready)(cronet_bidirectional_stream* stream); /* Invoked when initial response headers are received. * Consumer must call cronet_bidirectional_stream_read() to start reading. @@ -67,20 +68,19 @@ typedef struct cronet_bidirectional_stream_callback { * It may be invoked after on_response_trailers_received()}, if there was * pending read data before trailers were received. * - * If count is 0, it means the remote side has signaled that it will send no - * more data; future calls to cronet_bidirectional_stream_read() will result - * in the on_data_read() callback or on_succeded() callback if + * If |bytes_read| is 0, it means the remote side has signaled that it will + * send no more data; future calls to cronet_bidirectional_stream_read() + * will result in the on_data_read() callback or on_succeded() callback if * cronet_bidirectional_stream_write() was invoked with end_of_stream set to * true. */ void (*on_read_completed)(cronet_bidirectional_stream* stream, char* data, - int count); + int bytes_read); /** * Invoked when all data passed to cronet_bidirectional_stream_write() is - * sent. - * To continue writing, call cronet_bidirectional_stream_write(). + * sent. To continue writing, call cronet_bidirectional_stream_write(). */ void (*on_write_completed)(cronet_bidirectional_stream* stream, const char* data); @@ -117,7 +117,7 @@ typedef struct cronet_bidirectional_stream_callback { void (*on_canceled)(cronet_bidirectional_stream* stream); } cronet_bidirectional_stream_callback; -/* Create a new stream object that uses |engine| and |callback|. All stream +/* Creates a new stream object that uses |engine| and |callback|. All stream * tasks are performed asynchronously on the |engine| network thread. |callback| * methods are invoked synchronously on the |engine| network thread, but must * not run tasks on the current thread to prevent blocking networking operations @@ -129,6 +129,7 @@ typedef struct cronet_bidirectional_stream_callback { * * Both |calback| and |engine| must remain valid until stream is destroyed. */ +CRONET_EXPORT cronet_bidirectional_stream* cronet_bidirectional_stream_create( cronet_engine* engine, void* annotation, @@ -136,15 +137,40 @@ cronet_bidirectional_stream* cronet_bidirectional_stream_create( /* TBD: The following methods return int. Should it be a custom type? */ -/* Destroy stream object. Destroy could be called from any thread, including +/* Destroys stream object. Destroy could be called from any thread, including * network thread, but is posted, so |stream| is valid until calling task is * complete. */ +CRONET_EXPORT int cronet_bidirectional_stream_destroy(cronet_bidirectional_stream* stream); -/* Start the stream by sending request to |url| using |method| and |headers|. If - * |end_of_stream| is true, then no data is expected to be written. +/** + * Disables or enables auto flush. By default, data is flushed after + * every cronet_bidirectional_stream_write(). If the auto flush is disabled, + * the client should explicitly call cronet_bidirectional_stream_flush to flush + * the data. + */ +CRONET_EXPORT void cronet_bidirectional_stream_disable_auto_flush( + cronet_bidirectional_stream* stream, + bool disable_auto_flush); + +/** + * Delays sending request headers until cronet_bidirectional_stream_flush() + * is called. This flag is currently only respected when QUIC is negotiated. + * When true, QUIC will send request header frame along with data frame(s) + * as a single packet when possible. + */ +CRONET_EXPORT +void cronet_bidirectional_stream_delay_request_headers_until_flush( + cronet_bidirectional_stream* stream, + bool delay_headers_until_flush); + +/* Starts the stream by sending request to |url| using |method| and |headers|. + * If |end_of_stream| is true, then no data is expected to be written. The + * |method| is HTTP verb, with PUT having a special meaning to mark idempotent + * request, which could use QUIC 0-RTT. */ +CRONET_EXPORT int cronet_bidirectional_stream_start( cronet_bidirectional_stream* stream, const char* url, @@ -153,46 +179,61 @@ int cronet_bidirectional_stream_start( const cronet_bidirectional_stream_header_array* headers, bool end_of_stream); -/* Read response data into |buffer| of |capacity| length. Must only be called at - * most once in response to each invocation of the - * on_response_headers_received() and on_read_completed() methods of the - * cronet_bidirectional_stream_callback. - * Each call will result in an invocation of one of the callback's - * on_read_completed method if data is read, its on_succeeded() method if - * the stream is closed, or its on_failed() method if there's an error. +/* Reads response data into |buffer| of |capacity| length. Must only be called + * at most once in response to each invocation of the + * on_stream_ready()/on_response_headers_received() and on_read_completed() + * methods of the cronet_bidirectional_stream_callback. + * Each call will result in an invocation of the callback's + * on_read_completed() method if data is read, or its on_failed() method if + * there's an error. The callback's on_succeeded() method is also invoked if + * there is no more data to read and |end_of_stream| was previously sent. */ +CRONET_EXPORT int cronet_bidirectional_stream_read(cronet_bidirectional_stream* stream, char* buffer, int capacity); -/* Read response data into |buffer| of |capacity| length. Must only be called at - * most once in response to each invocation of the - * on_response_headers_received() and on_read_completed() methods of the - * cronet_bidirectional_stream_callback. - * Each call will result in an invocation of one of the callback's - * on_read_completed method if data is read, its on_succeeded() method if - * the stream is closed, or its on_failed() method if there's an error. +/* Writes request data from |buffer| of |buffer_length| length. If auto flush is + * disabled, data will be sent only after cronet_bidirectional_stream_flush() is + * called. + * Each call will result in an invocation the callback's on_write_completed() + * method if data is sent, or its on_failed() method if there's an error. + * The callback's on_succeeded() method is also invoked if |end_of_stream| is + * set and all response data has been read. */ +CRONET_EXPORT int cronet_bidirectional_stream_write(cronet_bidirectional_stream* stream, const char* buffer, - int count, + int buffer_length, bool end_of_stream); +/** + * Flushes pending writes. This method should not be called before invocation of + * on_stream_ready() method of the cronet_bidirectional_stream_callback. + * For each previously called cronet_bidirectional_stream_write() + * a corresponding on_write_completed() callback will be invoked when the buffer + * is sent. + */ +CRONET_EXPORT +void cronet_bidirectional_stream_flush(cronet_bidirectional_stream* stream); + /* Cancels the stream. Can be called at any time after * cronet_bidirectional_stream_start(). The on_canceled() method of * cronet_bidirectional_stream_callback will be invoked when cancelation * is complete and no further callback methods will be invoked. If the * stream has completed or has not started, calling * cronet_bidirectional_stream_cancel() has no effect and on_canceled() will not - * be invoked. At most one callback method may be invoked after + * be invoked. At most one callback method may be invoked after * cronet_bidirectional_stream_cancel() has completed. */ -int cronet_bidirectional_stream_cancel(cronet_bidirectional_stream* stream); +CRONET_EXPORT +void cronet_bidirectional_stream_cancel(cronet_bidirectional_stream* stream); /* Returns true if the |stream| was successfully started and is now done * (succeeded, canceled, or failed). * Returns false if the |stream| stream is not yet started or is in progress. */ +CRONET_EXPORT bool cronet_bidirectional_stream_is_done(cronet_bidirectional_stream* stream); #ifdef __cplusplus From bf803b95638753230eb57bd4f3178b7d49ff2018 Mon Sep 17 00:00:00 2001 From: Muxi Yan Date: Wed, 8 Feb 2017 11:31:26 -0800 Subject: [PATCH 02/44] Nit fixes --- src/objective-c/GRPCClient/GRPCCall.m | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/objective-c/GRPCClient/GRPCCall.m b/src/objective-c/GRPCClient/GRPCCall.m index 0a103223675..1a8fc2e2ef6 100644 --- a/src/objective-c/GRPCClient/GRPCCall.m +++ b/src/objective-c/GRPCClient/GRPCCall.m @@ -164,8 +164,8 @@ static NSMutableDictionary *callFlags; _requestHeaders = [[GRPCRequestHeaders alloc] initWithCall:self]; if ([requestWriter isKindOfClass:[GRXImmediateSingleWriter class]]) { - _unaryCall = true; - _unaryOpBatch = [[NSMutableArray alloc] init]; + _unaryCall = YES; + _unaryOpBatch = [NSMutableArray arrayWithCapacity:6]; } } return self; @@ -267,14 +267,13 @@ static NSMutableDictionary *callFlags; - (void)sendHeaders:(NSDictionary *)headers { // TODO(jcanizales): Add error handlers for async failures + GRPCOpSendMetadata *op = [[GRPCOpSendMetadata alloc] initWithMetadata:headers + flags:[GRPCCall callFlagsForHost:_host path:_path] + handler:nil]; if (!_unaryCall) { - [_wrappedCall startBatchWithOperations:@[[[GRPCOpSendMetadata alloc] initWithMetadata:headers - flags:[GRPCCall callFlagsForHost:_host path:_path] - handler:nil]]]; + [_wrappedCall startBatchWithOperations:@[op]]; } else { - [_unaryOpBatch addObject:[[GRPCOpSendMetadata alloc] initWithMetadata:headers - flags:[GRPCCall callFlagsForHost:_host path:_path] - handler:nil]]; + [_unaryOpBatch addObject:op]; } } @@ -294,13 +293,14 @@ static NSMutableDictionary *callFlags; } } }; + + GRPCOpSendMessage *op = [[GRPCOpSendMessage alloc] initWithMessage:message + handler:resumingHandler]; if (!_unaryCall) { - [_wrappedCall startBatchWithOperations:@[[[GRPCOpSendMessage alloc] initWithMessage:message - handler:resumingHandler]] + [_wrappedCall startBatchWithOperations:@[op] errorHandler:errorHandler]; } else { - [_unaryOpBatch addObject:[[GRPCOpSendMessage alloc] initWithMessage:message - handler:resumingHandler]]; + [_unaryOpBatch addObject:op]; } } From 40d7c627bde4c08d40568f62e6f284a6a615fb71 Mon Sep 17 00:00:00 2001 From: Muxi Yan Date: Wed, 8 Feb 2017 14:41:45 -0800 Subject: [PATCH 03/44] Only test the ObjC layer --- src/objective-c/GRPCClient/GRPCCall+Tests.h | 15 +++++++ src/objective-c/GRPCClient/GRPCCall+Tests.m | 37 ++++++++++++++++ .../GRPCClient/private/GRPCWrappedCall.m | 4 ++ src/objective-c/tests/InteropTests.m | 43 ++++++------------- 4 files changed, 68 insertions(+), 31 deletions(-) diff --git a/src/objective-c/GRPCClient/GRPCCall+Tests.h b/src/objective-c/GRPCClient/GRPCCall+Tests.h index 184ad09c5c8..f1618afdbf4 100644 --- a/src/objective-c/GRPCClient/GRPCCall+Tests.h +++ b/src/objective-c/GRPCClient/GRPCCall+Tests.h @@ -63,4 +63,19 @@ * cache. */ + (void)resetHostSettings; + +/** + * Enables logging of op batches. Memory consumption increases as more ops are logged. + */ ++ (void)enableOpBatchLog:(BOOL)enabled; + +/** + * Add an op batch to log. + */ ++ (void)addOpBatchToLog:(NSArray *)batch; + +/** + * Obtain the logged op batches. Invoking this method will clean the log. + */ ++ (NSArray *)obtainAndCleanOpBatchLog; @end diff --git a/src/objective-c/GRPCClient/GRPCCall+Tests.m b/src/objective-c/GRPCClient/GRPCCall+Tests.m index 656cba8fec6..fe864707e0a 100644 --- a/src/objective-c/GRPCClient/GRPCCall+Tests.m +++ b/src/objective-c/GRPCClient/GRPCCall+Tests.m @@ -64,4 +64,41 @@ + (void)resetHostSettings { [GRPCHost resetAllHostSettings]; } + +static NSMutableArray *opBatchLog = nil; + ++ (void)enableOpBatchLog:(BOOL)enabled { + @synchronized (opBatchLog) { + if (enabled) { + if (!opBatchLog) { + opBatchLog = [NSMutableArray array]; + } + } else { + if (opBatchLog) { + opBatchLog = nil; + } + } + } +} + ++ (void)addOpBatchToLog:(NSArray *)batch { + @synchronized (opBatchLog) { + if (opBatchLog) { + [opBatchLog addObject:batch]; + } + } +} + ++ (NSArray *)obtainAndCleanOpBatchLog { + @synchronized (opBatchLog) { + if (opBatchLog) { + NSArray *out = opBatchLog; + opBatchLog = [NSMutableArray array]; + return out; + } else { + return nil; + } + } +} + @end diff --git a/src/objective-c/GRPCClient/private/GRPCWrappedCall.m b/src/objective-c/GRPCClient/private/GRPCWrappedCall.m index 38fcae0299d..fd624a716ff 100644 --- a/src/objective-c/GRPCClient/private/GRPCWrappedCall.m +++ b/src/objective-c/GRPCClient/private/GRPCWrappedCall.m @@ -44,6 +44,8 @@ #import "NSData+GRPC.h" #import "NSError+GRPC.h" +#import "GRPCCall+Tests.h" + @implementation GRPCOperation { @protected // Most operation subclasses don't set any flags in the grpc_op, and rely on the flag member being @@ -271,6 +273,8 @@ } - (void)startBatchWithOperations:(NSArray *)operations errorHandler:(void (^)())errorHandler { + [GRPCCall addOpBatchToLog:operations]; + size_t nops = operations.count; grpc_op *ops_array = gpr_malloc(nops * sizeof(grpc_op)); size_t i = 0; diff --git a/src/objective-c/tests/InteropTests.m b/src/objective-c/tests/InteropTests.m index 8adf0a6164c..2bd54e17142 100644 --- a/src/objective-c/tests/InteropTests.m +++ b/src/objective-c/tests/InteropTests.m @@ -145,32 +145,7 @@ [self waitForExpectationsWithTimeout:TEST_TIMEOUT handler:nil]; } -// TODO (mxyan): Do the same test for chttp2 -#ifdef GRPC_COMPILE_WITH_CRONET -#ifdef GRPC_CRONET_WITH_PACKET_COALESCING - -static bool coalesced_message_and_eos; - -static void log_processor(gpr_log_func_args *args) { - unsigned long file_len = strlen(args->file); - const char suffix[] = "call.c"; - const int suffix_len = sizeof(suffix) - 1; - const char nops[] = "nops=3"; - - if (file_len > suffix_len && - 0 == strcmp(suffix, &args->file[file_len - suffix_len]) && - strstr(args->message, nops)) { - fprintf(stderr, "%s, %s\n", args->file, args->message); - coalesced_message_and_eos = true; - } -} - - (void)testPacketCoalescing { - gpr_set_log_verbosity(GPR_LOG_SEVERITY_DEBUG); - grpc_tracer_set_enabled("all", 1); - gpr_set_log_function(log_processor); - coalesced_message_and_eos = false; - XCTAssertNotNil(self.class.host); __weak XCTestExpectation *expectation = [self expectationWithDescription:@"LargeUnary"]; @@ -179,6 +154,7 @@ static void log_processor(gpr_log_func_args *args) { request.responseSize = 10; request.payload.body = [NSMutableData dataWithLength:10]; + [GRPCCall enableOpBatchLog:YES]; [_service unaryCallWithRequest:request handler:^(RMTSimpleResponse *response, NSError *error) { XCTAssertNil(error, @"Finished with unexpected error: %@", error); @@ -187,17 +163,22 @@ static void log_processor(gpr_log_func_args *args) { expectedResponse.payload.body = [NSMutableData dataWithLength:10]; XCTAssertEqualObjects(response, expectedResponse); - XCTAssert(coalesced_message_and_eos); - - [expectation fulfill]; + NSArray *opBatches = [GRPCCall obtainAndCleanOpBatchLog]; + for (NSObject *o in opBatches) { + if ([o isKindOfClass:[NSArray class]]) { + NSArray *batch = (NSArray *)o; + if ([batch count] == 3) { + [expectation fulfill]; + break; + } + } + } }]; [self waitForExpectationsWithTimeout:16 handler:nil]; + [GRPCCall enableOpBatchLog:NO]; } -#endif -#endif - - (void)test4MBResponsesAreAccepted { XCTAssertNotNil(self.class.host); __weak XCTestExpectation *expectation = [self expectationWithDescription:@"MaxResponseSize"]; From 5bd16b70133ba3901ed39eb867c34522fdf5778f Mon Sep 17 00:00:00 2001 From: Muxi Yan Date: Thu, 9 Feb 2017 16:54:30 -0800 Subject: [PATCH 04/44] Warnings and macro guards for op batch log --- .../{ => internal_testing}/GRPCCall+Tests.h | 13 +- .../{ => internal_testing}/GRPCCall+Tests.m | 48 ++-- .../GRPCClient/private/GRPCOpBatchLog.h | 54 +++++ .../GRPCClient/private/GRPCOpBatchLog.m | 74 ++++++ .../GRPCClient/private/GRPCWrappedCall.m | 6 +- src/objective-c/tests/GRPCClientTests.m | 2 +- src/objective-c/tests/InteropTests.m | 2 +- .../tests/InteropTestsLocalCleartext.m | 2 +- src/objective-c/tests/InteropTestsLocalSSL.m | 2 +- src/objective-c/tests/InteropTestsRemote.m | 2 +- .../InteropTestsRemoteWithCronet.m | 2 +- src/objective-c/tests/Podfile | 6 +- .../tests/Tests.xcodeproj/project.pbxproj | 211 ++++++++++++++++++ .../xcshareddata/xcschemes/AllTests.xcscheme | 6 +- .../InteropTestsLocalCleartext.xcscheme | 6 +- .../xcschemes/InteropTestsLocalSSL.xcscheme | 6 +- .../xcschemes/InteropTestsRemote.xcscheme | 6 +- 17 files changed, 390 insertions(+), 58 deletions(-) rename src/objective-c/GRPCClient/{ => internal_testing}/GRPCCall+Tests.h (88%) rename src/objective-c/GRPCClient/{ => internal_testing}/GRPCCall+Tests.m (80%) create mode 100644 src/objective-c/GRPCClient/private/GRPCOpBatchLog.h create mode 100644 src/objective-c/GRPCClient/private/GRPCOpBatchLog.m diff --git a/src/objective-c/GRPCClient/GRPCCall+Tests.h b/src/objective-c/GRPCClient/internal_testing/GRPCCall+Tests.h similarity index 88% rename from src/objective-c/GRPCClient/GRPCCall+Tests.h rename to src/objective-c/GRPCClient/internal_testing/GRPCCall+Tests.h index f1618afdbf4..0325f5a192f 100644 --- a/src/objective-c/GRPCClient/GRPCCall+Tests.h +++ b/src/objective-c/GRPCClient/internal_testing/GRPCCall+Tests.h @@ -31,7 +31,7 @@ * */ -#import "GRPCCall.h" +#import "../GRPCCall.h" /** * Methods to let tune down the security of gRPC connections for specific hosts. These shouldn't be @@ -66,16 +66,17 @@ /** * Enables logging of op batches. Memory consumption increases as more ops are logged. + * + * This function is for internal testing of gRPC only. It is not part of gRPC's public interface. + * Do not use in production. To enable, set the preprocessor flag GRPC_TEST_OBJC. */ + (void)enableOpBatchLog:(BOOL)enabled; -/** - * Add an op batch to log. - */ -+ (void)addOpBatchToLog:(NSArray *)batch; - /** * Obtain the logged op batches. Invoking this method will clean the log. + * + * This function is for internal testing of gRPC only. It is not part of gRPC's public interface. + * Do not use in production. To enable, set the preprocessor flag GRPC_TEST_OBJC. */ + (NSArray *)obtainAndCleanOpBatchLog; @end diff --git a/src/objective-c/GRPCClient/GRPCCall+Tests.m b/src/objective-c/GRPCClient/internal_testing/GRPCCall+Tests.m similarity index 80% rename from src/objective-c/GRPCClient/GRPCCall+Tests.m rename to src/objective-c/GRPCClient/internal_testing/GRPCCall+Tests.m index fe864707e0a..5109480bef8 100644 --- a/src/objective-c/GRPCClient/GRPCCall+Tests.m +++ b/src/objective-c/GRPCClient/internal_testing/GRPCCall+Tests.m @@ -33,7 +33,8 @@ #import "GRPCCall+Tests.h" -#import "private/GRPCHost.h" +#import "../private/GRPCHost.h" +#import "../private/GRPCOpBatchLog.h" @implementation GRPCCall (Tests) @@ -65,40 +66,25 @@ [GRPCHost resetAllHostSettings]; } -static NSMutableArray *opBatchLog = nil; - + (void)enableOpBatchLog:(BOOL)enabled { - @synchronized (opBatchLog) { - if (enabled) { - if (!opBatchLog) { - opBatchLog = [NSMutableArray array]; - } - } else { - if (opBatchLog) { - opBatchLog = nil; - } - } - } -} - -+ (void)addOpBatchToLog:(NSArray *)batch { - @synchronized (opBatchLog) { - if (opBatchLog) { - [opBatchLog addObject:batch]; - } - } +#ifdef GRPC_TEST_OBJC + [GRPCOpBatchLog enableOpBatchLog:enabled]; +#else + NSLog(@"This function is for internal testing of gRPC only. " + "It is not part of gRPC's public interface. Do not use in production. " + "To enable, set the preprocessor flag GRPC_TEST_OBJC."); +#endif } + (NSArray *)obtainAndCleanOpBatchLog { - @synchronized (opBatchLog) { - if (opBatchLog) { - NSArray *out = opBatchLog; - opBatchLog = [NSMutableArray array]; - return out; - } else { - return nil; - } - } +#ifdef GRPC_TEST_OBJC + return [GRPCOpBatchLog obtainAndCleanOpBatchLog]; +#else + NSLog(@"This function is for internal testing of gRPC only. " + "It is not part of gRPC's public interface. Do not use in production. " + "To enable, set the preprocessor flag GRPC_TEST_OBJC."); + return nil; +#endif } @end diff --git a/src/objective-c/GRPCClient/private/GRPCOpBatchLog.h b/src/objective-c/GRPCClient/private/GRPCOpBatchLog.h new file mode 100644 index 00000000000..dbc3417acf7 --- /dev/null +++ b/src/objective-c/GRPCClient/private/GRPCOpBatchLog.h @@ -0,0 +1,54 @@ +/* + * + * Copyright 2016, Google Inc. + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are + * met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following disclaimer + * in the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Google Inc. nor the names of its + * contributors may be used to endorse or promote products derived from + * this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + * + */ + +/** + * Logs the op batches of a client. Used for testing. + */ +@interface GRPCOpBatchLog : NSObject + +/** + * Enables logging of op batches. Memory consumption increases as more ops are logged. + */ ++ (void)enableOpBatchLog:(BOOL)enabled; + +/** + * Add an op batch to log. + */ ++ (void)addOpBatchToLog:(NSArray *)batch; + +/** + * Obtain the logged op batches. Invoking this method will clean the log. + */ ++ (NSArray *)obtainAndCleanOpBatchLog; + +@end diff --git a/src/objective-c/GRPCClient/private/GRPCOpBatchLog.m b/src/objective-c/GRPCClient/private/GRPCOpBatchLog.m new file mode 100644 index 00000000000..e5e7100ddf0 --- /dev/null +++ b/src/objective-c/GRPCClient/private/GRPCOpBatchLog.m @@ -0,0 +1,74 @@ +/* + * + * Copyright 2016, Google Inc. + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are + * met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following disclaimer + * in the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Google Inc. nor the names of its + * contributors may be used to endorse or promote products derived from + * this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + * + */ + +#import "GRPCOpBatchLog.h" + +@implementation GRPCOpBatchLog + +NSMutableArray *opBatchLog = nil; + ++ (void)enableOpBatchLog:(BOOL)enabled { + @synchronized (opBatchLog) { + if (enabled) { + if (!opBatchLog) { + opBatchLog = [NSMutableArray array]; + } + } else { + if (opBatchLog) { + opBatchLog = nil; + } + } + } +} + ++ (void)addOpBatchToLog:(NSArray *)batch { + @synchronized (opBatchLog) { + if (opBatchLog) { + [opBatchLog addObject:batch]; + } + } +} + ++ (NSArray *)obtainAndCleanOpBatchLog { + @synchronized (opBatchLog) { + if (opBatchLog) { + NSArray *out = opBatchLog; + opBatchLog = [NSMutableArray array]; + return out; + } else { + return nil; + } + } +} + +@end \ No newline at end of file diff --git a/src/objective-c/GRPCClient/private/GRPCWrappedCall.m b/src/objective-c/GRPCClient/private/GRPCWrappedCall.m index 9d493e1e8a0..4476a41bd97 100644 --- a/src/objective-c/GRPCClient/private/GRPCWrappedCall.m +++ b/src/objective-c/GRPCClient/private/GRPCWrappedCall.m @@ -44,7 +44,7 @@ #import "NSData+GRPC.h" #import "NSError+GRPC.h" -#import "GRPCCall+Tests.h" +#import "GRPCOpBatchLog.h" @implementation GRPCOperation { @protected @@ -276,7 +276,9 @@ } - (void)startBatchWithOperations:(NSArray *)operations errorHandler:(void (^)())errorHandler { - [GRPCCall addOpBatchToLog:operations]; +#ifdef GRPC_TEST_OBJC + [GRPCOpBatchLog addOpBatchToLog:operations]; +#endif size_t nops = operations.count; grpc_op *ops_array = gpr_malloc(nops * sizeof(grpc_op)); diff --git a/src/objective-c/tests/GRPCClientTests.m b/src/objective-c/tests/GRPCClientTests.m index 0b72a75f3d5..a1bb835e5d4 100644 --- a/src/objective-c/tests/GRPCClientTests.m +++ b/src/objective-c/tests/GRPCClientTests.m @@ -37,7 +37,7 @@ #import #import #import -#import +#import #import #import #import diff --git a/src/objective-c/tests/InteropTests.m b/src/objective-c/tests/InteropTests.m index 2bd54e17142..592dca9dde5 100644 --- a/src/objective-c/tests/InteropTests.m +++ b/src/objective-c/tests/InteropTests.m @@ -37,7 +37,7 @@ #import #import -#import +#import #import #import #import diff --git a/src/objective-c/tests/InteropTestsLocalCleartext.m b/src/objective-c/tests/InteropTestsLocalCleartext.m index b41210f50f8..6e7624e2046 100644 --- a/src/objective-c/tests/InteropTestsLocalCleartext.m +++ b/src/objective-c/tests/InteropTestsLocalCleartext.m @@ -31,7 +31,7 @@ * */ -#import +#import #import "InteropTests.h" diff --git a/src/objective-c/tests/InteropTestsLocalSSL.m b/src/objective-c/tests/InteropTestsLocalSSL.m index 1479c5896c3..12007bbd681 100644 --- a/src/objective-c/tests/InteropTestsLocalSSL.m +++ b/src/objective-c/tests/InteropTestsLocalSSL.m @@ -31,7 +31,7 @@ * */ -#import +#import #import "InteropTests.h" diff --git a/src/objective-c/tests/InteropTestsRemote.m b/src/objective-c/tests/InteropTestsRemote.m index 70f84753bb6..b0a64a3caed 100644 --- a/src/objective-c/tests/InteropTestsRemote.m +++ b/src/objective-c/tests/InteropTestsRemote.m @@ -31,7 +31,7 @@ * */ -#import +#import #import "InteropTests.h" diff --git a/src/objective-c/tests/InteropTestsRemoteWithCronet/InteropTestsRemoteWithCronet.m b/src/objective-c/tests/InteropTestsRemoteWithCronet/InteropTestsRemoteWithCronet.m index 793b71a9bee..c8742b7fe8c 100644 --- a/src/objective-c/tests/InteropTestsRemoteWithCronet/InteropTestsRemoteWithCronet.m +++ b/src/objective-c/tests/InteropTestsRemoteWithCronet/InteropTestsRemoteWithCronet.m @@ -31,7 +31,7 @@ * */ -#import +#import #import #import diff --git a/src/objective-c/tests/Podfile b/src/objective-c/tests/Podfile index 462c6a8e0eb..5ada61c3d40 100644 --- a/src/objective-c/tests/Podfile +++ b/src/objective-c/tests/Podfile @@ -103,10 +103,14 @@ post_install do |installer| # Activate Cronet for the dedicated build configuration 'Cronet', which will be used solely by # the test target 'InteropTestsRemoteWithCronet' + # Activate GRPCCall+Tests functions for the dedicated build configuration 'Test', which will + # be used by all test targets using it. if target.name == 'gRPC' target.build_configurations.each do |config| if config.name == 'Cronet' - config.build_settings['GCC_PREPROCESSOR_DEFINITIONS'] = '$(inherited) COCOAPODS=1 GRPC_COMPILE_WITH_CRONET=1' + config.build_settings['GCC_PREPROCESSOR_DEFINITIONS'] = '$(inherited) COCOAPODS=1 GRPC_COMPILE_WITH_CRONET=1 GRPC_TEST_OBJC' + elsif config.name == 'Test' + config.build_settings['GCC_PREPROCESSOR_DEFINITIONS'] = '$(inherited) COCOAPODS=1 GRPC_TEST_OBJC' end end end diff --git a/src/objective-c/tests/Tests.xcodeproj/project.pbxproj b/src/objective-c/tests/Tests.xcodeproj/project.pbxproj index 32b35ef333a..57e417c2a8a 100644 --- a/src/objective-c/tests/Tests.xcodeproj/project.pbxproj +++ b/src/objective-c/tests/Tests.xcodeproj/project.pbxproj @@ -125,8 +125,10 @@ 0A4F89D9C90E9C30990218F0 /* Pods.release.xcconfig */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = text.xcconfig; name = Pods.release.xcconfig; path = "Pods/Target Support Files/Pods/Pods.release.xcconfig"; sourceTree = ""; }; 0D2284C3DF7E57F0ED504E39 /* Pods-CoreCronetEnd2EndTests.debug.xcconfig */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = text.xcconfig; name = "Pods-CoreCronetEnd2EndTests.debug.xcconfig"; path = "Pods/Target Support Files/Pods-CoreCronetEnd2EndTests/Pods-CoreCronetEnd2EndTests.debug.xcconfig"; sourceTree = ""; }; 14B09A58FEE53A7A6B838920 /* Pods-InteropTestsLocalSSL.cronet.xcconfig */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = text.xcconfig; name = "Pods-InteropTestsLocalSSL.cronet.xcconfig"; path = "Pods/Target Support Files/Pods-InteropTestsLocalSSL/Pods-InteropTestsLocalSSL.cronet.xcconfig"; sourceTree = ""; }; + 1588C85DEAF7FC0ACDEA4C02 /* Pods-InteropTestsLocalCleartext.test.xcconfig */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = text.xcconfig; name = "Pods-InteropTestsLocalCleartext.test.xcconfig"; path = "Pods/Target Support Files/Pods-InteropTestsLocalCleartext/Pods-InteropTestsLocalCleartext.test.xcconfig"; sourceTree = ""; }; 17F60BF2871F6AF85FB3FA12 /* Pods-InteropTestsRemoteWithCronet.debug.xcconfig */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = text.xcconfig; name = "Pods-InteropTestsRemoteWithCronet.debug.xcconfig"; path = "Pods/Target Support Files/Pods-InteropTestsRemoteWithCronet/Pods-InteropTestsRemoteWithCronet.debug.xcconfig"; sourceTree = ""; }; 20DFF2F3C97EF098FE5A3171 /* libPods-Tests.a */ = {isa = PBXFileReference; explicitFileType = archive.ar; includeInIndex = 0; path = "libPods-Tests.a"; sourceTree = BUILT_PRODUCTS_DIR; }; + 2B89F3037963E6EDDD48D8C3 /* Pods-InteropTestsRemoteWithCronet.test.xcconfig */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = text.xcconfig; name = "Pods-InteropTestsRemoteWithCronet.test.xcconfig"; path = "Pods/Target Support Files/Pods-InteropTestsRemoteWithCronet/Pods-InteropTestsRemoteWithCronet.test.xcconfig"; sourceTree = ""; }; 35F2B6BF3BAE8F0DC4AFD76E /* libPods.a */ = {isa = PBXFileReference; explicitFileType = archive.ar; includeInIndex = 0; path = libPods.a; sourceTree = BUILT_PRODUCTS_DIR; }; 386712AEACF7C2190C4B8B3F /* Pods-CronetUnitTests.cronet.xcconfig */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = text.xcconfig; name = "Pods-CronetUnitTests.cronet.xcconfig"; path = "Pods/Target Support Files/Pods-CronetUnitTests/Pods-CronetUnitTests.cronet.xcconfig"; sourceTree = ""; }; 3B0861FC805389C52DB260D4 /* Pods-RxLibraryUnitTests.release.xcconfig */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = text.xcconfig; name = "Pods-RxLibraryUnitTests.release.xcconfig"; path = "Pods/Target Support Files/Pods-RxLibraryUnitTests/Pods-RxLibraryUnitTests.release.xcconfig"; sourceTree = ""; }; @@ -162,15 +164,22 @@ 63E240CD1B6C4E2B005F3B0E /* InteropTestsLocalSSL.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = InteropTestsLocalSSL.m; sourceTree = ""; }; 63E240CF1B6C63DC005F3B0E /* TestCertificates.bundle */ = {isa = PBXFileReference; lastKnownFileType = "wrapper.plug-in"; path = TestCertificates.bundle; sourceTree = ""; }; 64F68A9A6A63CC930DD30A6E /* Pods-CronetUnitTests.debug.xcconfig */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = text.xcconfig; name = "Pods-CronetUnitTests.debug.xcconfig"; path = "Pods/Target Support Files/Pods-CronetUnitTests/Pods-CronetUnitTests.debug.xcconfig"; sourceTree = ""; }; + 6793C9D019CB268C5BB491A2 /* Pods-CoreCronetEnd2EndTests.test.xcconfig */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = text.xcconfig; name = "Pods-CoreCronetEnd2EndTests.test.xcconfig"; path = "Pods/Target Support Files/Pods-CoreCronetEnd2EndTests/Pods-CoreCronetEnd2EndTests.test.xcconfig"; sourceTree = ""; }; + 781089FAE980F51F88A3BE0B /* Pods-RxLibraryUnitTests.test.xcconfig */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = text.xcconfig; name = "Pods-RxLibraryUnitTests.test.xcconfig"; path = "Pods/Target Support Files/Pods-RxLibraryUnitTests/Pods-RxLibraryUnitTests.test.xcconfig"; sourceTree = ""; }; 79C68EFFCB5533475D810B79 /* Pods-RxLibraryUnitTests.cronet.xcconfig */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = text.xcconfig; name = "Pods-RxLibraryUnitTests.cronet.xcconfig"; path = "Pods/Target Support Files/Pods-RxLibraryUnitTests/Pods-RxLibraryUnitTests.cronet.xcconfig"; sourceTree = ""; }; 7A2E97E3F469CC2A758D77DE /* Pods-InteropTestsLocalSSL.release.xcconfig */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = text.xcconfig; name = "Pods-InteropTestsLocalSSL.release.xcconfig"; path = "Pods/Target Support Files/Pods-InteropTestsLocalSSL/Pods-InteropTestsLocalSSL.release.xcconfig"; sourceTree = ""; }; 9E9444C764F0FFF64A7EB58E /* libPods-InteropTestsRemoteWithCronet.a */ = {isa = PBXFileReference; explicitFileType = archive.ar; includeInIndex = 0; path = "libPods-InteropTestsRemoteWithCronet.a"; sourceTree = BUILT_PRODUCTS_DIR; }; + A0361771A855917162911180 /* Pods-Tests.test.xcconfig */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = text.xcconfig; name = "Pods-Tests.test.xcconfig"; path = "Pods/Target Support Files/Pods-Tests/Pods-Tests.test.xcconfig"; sourceTree = ""; }; A58BE6DF1C62D1739EBB2C78 /* libPods-RxLibraryUnitTests.a */ = {isa = PBXFileReference; explicitFileType = archive.ar; includeInIndex = 0; path = "libPods-RxLibraryUnitTests.a"; sourceTree = BUILT_PRODUCTS_DIR; }; + A6F832FCEFA6F6881E620F12 /* Pods-InteropTestsRemote.test.xcconfig */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = text.xcconfig; name = "Pods-InteropTestsRemote.test.xcconfig"; path = "Pods/Target Support Files/Pods-InteropTestsRemote/Pods-InteropTestsRemote.test.xcconfig"; sourceTree = ""; }; AA7CB64B4DD9915AE7C03163 /* Pods-InteropTestsLocalCleartext.cronet.xcconfig */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = text.xcconfig; name = "Pods-InteropTestsLocalCleartext.cronet.xcconfig"; path = "Pods/Target Support Files/Pods-InteropTestsLocalCleartext/Pods-InteropTestsLocalCleartext.cronet.xcconfig"; sourceTree = ""; }; AC414EF7A6BF76ED02B6E480 /* Pods-InteropTestsRemoteWithCronet.release.xcconfig */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = text.xcconfig; name = "Pods-InteropTestsRemoteWithCronet.release.xcconfig"; path = "Pods/Target Support Files/Pods-InteropTestsRemoteWithCronet/Pods-InteropTestsRemoteWithCronet.release.xcconfig"; sourceTree = ""; }; + B226619DC4E709E0FFFF94B8 /* Pods-CronetUnitTests.test.xcconfig */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = text.xcconfig; name = "Pods-CronetUnitTests.test.xcconfig"; path = "Pods/Target Support Files/Pods-CronetUnitTests/Pods-CronetUnitTests.test.xcconfig"; sourceTree = ""; }; B94C27C06733CF98CE1B2757 /* Pods-AllTests.debug.xcconfig */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = text.xcconfig; name = "Pods-AllTests.debug.xcconfig"; path = "Pods/Target Support Files/Pods-AllTests/Pods-AllTests.debug.xcconfig"; sourceTree = ""; }; C6134277D2EB8B380862A03F /* libPods-CronetUnitTests.a */ = {isa = PBXFileReference; explicitFileType = archive.ar; includeInIndex = 0; path = "libPods-CronetUnitTests.a"; sourceTree = BUILT_PRODUCTS_DIR; }; CAE086D5B470DA367D415AB0 /* libPods-AllTests.a */ = {isa = PBXFileReference; explicitFileType = archive.ar; includeInIndex = 0; path = "libPods-AllTests.a"; sourceTree = BUILT_PRODUCTS_DIR; }; + D13BEC8181B8E678A1B52F54 /* Pods-InteropTestsLocalSSL.test.xcconfig */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = text.xcconfig; name = "Pods-InteropTestsLocalSSL.test.xcconfig"; path = "Pods/Target Support Files/Pods-InteropTestsLocalSSL/Pods-InteropTestsLocalSSL.test.xcconfig"; sourceTree = ""; }; + DB1F4391AF69D20D38D74B67 /* Pods-AllTests.test.xcconfig */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = text.xcconfig; name = "Pods-AllTests.test.xcconfig"; path = "Pods/Target Support Files/Pods-AllTests/Pods-AllTests.test.xcconfig"; sourceTree = ""; }; DBE059B4AC7A51919467EEC0 /* libPods-InteropTestsRemote.a */ = {isa = PBXFileReference; explicitFileType = archive.ar; includeInIndex = 0; path = "libPods-InteropTestsRemote.a"; sourceTree = BUILT_PRODUCTS_DIR; }; DBEDE45BDA60DF1E1C8950C0 /* libPods-InteropTestsLocalSSL.a */ = {isa = PBXFileReference; explicitFileType = archive.ar; includeInIndex = 0; path = "libPods-InteropTestsLocalSSL.a"; sourceTree = BUILT_PRODUCTS_DIR; }; DC3CA1D948F068E76957A861 /* Pods-InteropTestsRemote.debug.xcconfig */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = text.xcconfig; name = "Pods-InteropTestsRemote.debug.xcconfig"; path = "Pods/Target Support Files/Pods-InteropTestsRemote/Pods-InteropTestsRemote.debug.xcconfig"; sourceTree = ""; }; @@ -317,6 +326,15 @@ 64F68A9A6A63CC930DD30A6E /* Pods-CronetUnitTests.debug.xcconfig */, 386712AEACF7C2190C4B8B3F /* Pods-CronetUnitTests.cronet.xcconfig */, 02192CF1FF9534E3D18C65FC /* Pods-CronetUnitTests.release.xcconfig */, + DB1F4391AF69D20D38D74B67 /* Pods-AllTests.test.xcconfig */, + 6793C9D019CB268C5BB491A2 /* Pods-CoreCronetEnd2EndTests.test.xcconfig */, + B226619DC4E709E0FFFF94B8 /* Pods-CronetUnitTests.test.xcconfig */, + 1588C85DEAF7FC0ACDEA4C02 /* Pods-InteropTestsLocalCleartext.test.xcconfig */, + D13BEC8181B8E678A1B52F54 /* Pods-InteropTestsLocalSSL.test.xcconfig */, + A6F832FCEFA6F6881E620F12 /* Pods-InteropTestsRemote.test.xcconfig */, + 2B89F3037963E6EDDD48D8C3 /* Pods-InteropTestsRemoteWithCronet.test.xcconfig */, + 781089FAE980F51F88A3BE0B /* Pods-RxLibraryUnitTests.test.xcconfig */, + A0361771A855917162911180 /* Pods-Tests.test.xcconfig */, ); name = Pods; sourceTree = ""; @@ -1237,6 +1255,189 @@ /* End PBXTargetDependency section */ /* Begin XCBuildConfiguration section */ + 5E1228981E4D400F00E8504F /* Test */ = { + isa = XCBuildConfiguration; + buildSettings = { + ALWAYS_SEARCH_USER_PATHS = NO; + CLANG_CXX_LANGUAGE_STANDARD = "gnu++0x"; + CLANG_CXX_LIBRARY = "libc++"; + CLANG_ENABLE_MODULES = YES; + CLANG_ENABLE_OBJC_ARC = YES; + CLANG_WARN_BOOL_CONVERSION = YES; + CLANG_WARN_CONSTANT_CONVERSION = YES; + CLANG_WARN_DIRECT_OBJC_ISA_USAGE = YES_ERROR; + CLANG_WARN_EMPTY_BODY = YES; + CLANG_WARN_ENUM_CONVERSION = YES; + CLANG_WARN_INT_CONVERSION = YES; + CLANG_WARN_OBJC_ROOT_CLASS = YES_ERROR; + CLANG_WARN_UNREACHABLE_CODE = YES; + CLANG_WARN__DUPLICATE_METHOD_MATCH = YES; + COPY_PHASE_STRIP = NO; + DEBUG_INFORMATION_FORMAT = "dwarf-with-dsym"; + ENABLE_STRICT_OBJC_MSGSEND = YES; + GCC_C_LANGUAGE_STANDARD = gnu99; + GCC_DYNAMIC_NO_PIC = NO; + GCC_NO_COMMON_BLOCKS = YES; + GCC_OPTIMIZATION_LEVEL = 0; + GCC_PREPROCESSOR_DEFINITIONS = ( + "DEBUG=1", + "$(inherited)", + ); + GCC_SYMBOLS_PRIVATE_EXTERN = NO; + GCC_TREAT_WARNINGS_AS_ERRORS = YES; + GCC_WARN_64_TO_32_BIT_CONVERSION = YES; + GCC_WARN_ABOUT_RETURN_TYPE = YES_ERROR; + GCC_WARN_UNDECLARED_SELECTOR = YES; + GCC_WARN_UNINITIALIZED_AUTOS = YES_AGGRESSIVE; + GCC_WARN_UNUSED_FUNCTION = YES; + GCC_WARN_UNUSED_VARIABLE = YES; + IPHONEOS_DEPLOYMENT_TARGET = 8.3; + MTL_ENABLE_DEBUG_INFO = YES; + ONLY_ACTIVE_ARCH = YES; + SDKROOT = iphoneos; + }; + name = Test; + }; + 5E1228991E4D400F00E8504F /* Test */ = { + isa = XCBuildConfiguration; + baseConfigurationReference = A0361771A855917162911180 /* Pods-Tests.test.xcconfig */; + buildSettings = { + GCC_TREAT_WARNINGS_AS_ERRORS = YES; + PRODUCT_NAME = "$(TARGET_NAME)"; + SKIP_INSTALL = YES; + }; + name = Test; + }; + 5E12289A1E4D400F00E8504F /* Test */ = { + isa = XCBuildConfiguration; + baseConfigurationReference = DB1F4391AF69D20D38D74B67 /* Pods-AllTests.test.xcconfig */; + buildSettings = { + FRAMEWORK_SEARCH_PATHS = ( + "$(SDKROOT)/Developer/Library/Frameworks", + "$(inherited)", + ); + GCC_PREPROCESSOR_DEFINITIONS = ( + "DEBUG=1", + "$(inherited)", + ); + INFOPLIST_FILE = Info.plist; + LD_RUNPATH_SEARCH_PATHS = "$(inherited) @executable_path/Frameworks @loader_path/Frameworks"; + PRODUCT_NAME = "$(TARGET_NAME)"; + }; + name = Test; + }; + 5E12289B1E4D400F00E8504F /* Test */ = { + isa = XCBuildConfiguration; + baseConfigurationReference = 781089FAE980F51F88A3BE0B /* Pods-RxLibraryUnitTests.test.xcconfig */; + buildSettings = { + DEBUG_INFORMATION_FORMAT = dwarf; + ENABLE_TESTABILITY = YES; + INFOPLIST_FILE = Info.plist; + IPHONEOS_DEPLOYMENT_TARGET = 9.0; + LD_RUNPATH_SEARCH_PATHS = "$(inherited) @executable_path/Frameworks @loader_path/Frameworks"; + PRODUCT_BUNDLE_IDENTIFIER = io.grpc.RxLibraryUnitTests; + PRODUCT_NAME = "$(TARGET_NAME)"; + }; + name = Test; + }; + 5E12289C1E4D400F00E8504F /* Test */ = { + isa = XCBuildConfiguration; + baseConfigurationReference = A6F832FCEFA6F6881E620F12 /* Pods-InteropTestsRemote.test.xcconfig */; + buildSettings = { + DEBUG_INFORMATION_FORMAT = dwarf; + ENABLE_TESTABILITY = YES; + INFOPLIST_FILE = Info.plist; + IPHONEOS_DEPLOYMENT_TARGET = 9.0; + LD_RUNPATH_SEARCH_PATHS = "$(inherited) @executable_path/Frameworks @loader_path/Frameworks"; + PRODUCT_BUNDLE_IDENTIFIER = io.grpc.InteropTests; + PRODUCT_NAME = "$(TARGET_NAME)"; + }; + name = Test; + }; + 5E12289D1E4D400F00E8504F /* Test */ = { + isa = XCBuildConfiguration; + baseConfigurationReference = D13BEC8181B8E678A1B52F54 /* Pods-InteropTestsLocalSSL.test.xcconfig */; + buildSettings = { + DEBUG_INFORMATION_FORMAT = dwarf; + ENABLE_TESTABILITY = YES; + INFOPLIST_FILE = Info.plist; + IPHONEOS_DEPLOYMENT_TARGET = 9.0; + LD_RUNPATH_SEARCH_PATHS = "$(inherited) @executable_path/Frameworks @loader_path/Frameworks"; + PRODUCT_BUNDLE_IDENTIFIER = io.grpc.InteropTestsLocalSSL; + PRODUCT_NAME = "$(TARGET_NAME)"; + }; + name = Test; + }; + 5E12289E1E4D400F00E8504F /* Test */ = { + isa = XCBuildConfiguration; + baseConfigurationReference = 1588C85DEAF7FC0ACDEA4C02 /* Pods-InteropTestsLocalCleartext.test.xcconfig */; + buildSettings = { + DEBUG_INFORMATION_FORMAT = dwarf; + ENABLE_TESTABILITY = YES; + INFOPLIST_FILE = Info.plist; + IPHONEOS_DEPLOYMENT_TARGET = 9.0; + LD_RUNPATH_SEARCH_PATHS = "$(inherited) @executable_path/Frameworks @loader_path/Frameworks"; + PRODUCT_BUNDLE_IDENTIFIER = io.grpc.InteropTestsLocalCleartext; + PRODUCT_NAME = "$(TARGET_NAME)"; + }; + name = Test; + }; + 5E12289F1E4D400F00E8504F /* Test */ = { + isa = XCBuildConfiguration; + baseConfigurationReference = 6793C9D019CB268C5BB491A2 /* Pods-CoreCronetEnd2EndTests.test.xcconfig */; + buildSettings = { + CLANG_ANALYZER_NONNULL = YES; + "CODE_SIGN_IDENTITY[sdk=iphoneos*]" = "iPhone Developer"; + DEBUG_INFORMATION_FORMAT = dwarf; + ENABLE_TESTABILITY = YES; + INFOPLIST_FILE = CoreCronetEnd2EndTests/Info.plist; + IPHONEOS_DEPLOYMENT_TARGET = 9.3; + LD_RUNPATH_SEARCH_PATHS = "$(inherited) @executable_path/Frameworks @loader_path/Frameworks"; + PRODUCT_BUNDLE_IDENTIFIER = io.grpc.CoreCronetEnd2EndTests; + PRODUCT_NAME = "$(TARGET_NAME)"; + USER_HEADER_SEARCH_PATHS = "$(inherited) \"${PODS_ROOT}/../../../..\""; + }; + name = Test; + }; + 5E1228A01E4D400F00E8504F /* Test */ = { + isa = XCBuildConfiguration; + baseConfigurationReference = 2B89F3037963E6EDDD48D8C3 /* Pods-InteropTestsRemoteWithCronet.test.xcconfig */; + buildSettings = { + CLANG_ANALYZER_NONNULL = YES; + "CODE_SIGN_IDENTITY[sdk=iphoneos*]" = "iPhone Developer"; + DEBUG_INFORMATION_FORMAT = dwarf; + ENABLE_TESTABILITY = YES; + GCC_PREPROCESSOR_DEFINITIONS = ( + "$(inherited)", + "COCOAPODS=1", + "$(inherited)", + "GPB_USE_PROTOBUF_FRAMEWORK_IMPORTS=1", + ); + INFOPLIST_FILE = InteropTestsRemoteWithCronet/Info.plist; + IPHONEOS_DEPLOYMENT_TARGET = 9.3; + LD_RUNPATH_SEARCH_PATHS = "$(inherited) @executable_path/Frameworks @loader_path/Frameworks"; + PRODUCT_BUNDLE_IDENTIFIER = io.grpc.InteropTestsRemoteWithCronet; + PRODUCT_NAME = "$(TARGET_NAME)"; + }; + name = Test; + }; + 5E1228A11E4D400F00E8504F /* Test */ = { + isa = XCBuildConfiguration; + baseConfigurationReference = B226619DC4E709E0FFFF94B8 /* Pods-CronetUnitTests.test.xcconfig */; + buildSettings = { + CLANG_ANALYZER_NONNULL = YES; + "CODE_SIGN_IDENTITY[sdk=iphoneos*]" = "iPhone Developer"; + DEBUG_INFORMATION_FORMAT = dwarf; + ENABLE_TESTABILITY = YES; + INFOPLIST_FILE = CronetUnitTests/Info.plist; + IPHONEOS_DEPLOYMENT_TARGET = 9.3; + LD_RUNPATH_SEARCH_PATHS = "$(inherited) @executable_path/Frameworks @loader_path/Frameworks"; + PRODUCT_BUNDLE_IDENTIFIER = io.grpc.CronetUnitTests; + PRODUCT_NAME = "$(TARGET_NAME)"; + USER_HEADER_SEARCH_PATHS = "\"${PODS_ROOT}/../../../..\" $(inherited)"; + }; + name = Test; + }; 5E8A5DAC1D3840B4000F8BC4 /* Debug */ = { isa = XCBuildConfiguration; baseConfigurationReference = 0D2284C3DF7E57F0ED504E39 /* Pods-CoreCronetEnd2EndTests.debug.xcconfig */; @@ -1770,6 +1971,7 @@ isa = XCConfigurationList; buildConfigurations = ( 5E8A5DAC1D3840B4000F8BC4 /* Debug */, + 5E12289F1E4D400F00E8504F /* Test */, 5EC3C7A71D4FC18C000330E2 /* Cronet */, 5E8A5DAD1D3840B4000F8BC4 /* Release */, ); @@ -1780,6 +1982,7 @@ isa = XCConfigurationList; buildConfigurations = ( 5EAD6D2C1E27047400002378 /* Debug */, + 5E1228A11E4D400F00E8504F /* Test */, 5EAD6D2D1E27047400002378 /* Cronet */, 5EAD6D2E1E27047400002378 /* Release */, ); @@ -1790,6 +1993,7 @@ isa = XCConfigurationList; buildConfigurations = ( 5EE84BF91D4717E40050C6CC /* Debug */, + 5E1228A01E4D400F00E8504F /* Test */, 5EC3C7A81D4FC18C000330E2 /* Cronet */, 5EE84BFA1D4717E40050C6CC /* Release */, ); @@ -1800,6 +2004,7 @@ isa = XCConfigurationList; buildConfigurations = ( 63423F4E1B150A5F006CF63C /* Debug */, + 5E12289A1E4D400F00E8504F /* Test */, 5EC3C7A21D4FC18C000330E2 /* Cronet */, 63423F4F1B150A5F006CF63C /* Release */, ); @@ -1810,6 +2015,7 @@ isa = XCConfigurationList; buildConfigurations = ( 635697D91B14FC11007A7283 /* Debug */, + 5E1228981E4D400F00E8504F /* Test */, 5EC3C7A01D4FC18C000330E2 /* Cronet */, 635697DA1B14FC11007A7283 /* Release */, ); @@ -1820,6 +2026,7 @@ isa = XCConfigurationList; buildConfigurations = ( 635697DC1B14FC11007A7283 /* Debug */, + 5E1228991E4D400F00E8504F /* Test */, 5EC3C7A11D4FC18C000330E2 /* Cronet */, 635697DD1B14FC11007A7283 /* Release */, ); @@ -1830,6 +2037,7 @@ isa = XCConfigurationList; buildConfigurations = ( 63DC841C1BE15179000708E8 /* Debug */, + 5E12289B1E4D400F00E8504F /* Test */, 5EC3C7A31D4FC18C000330E2 /* Cronet */, 63DC841D1BE15179000708E8 /* Release */, ); @@ -1840,6 +2048,7 @@ isa = XCConfigurationList; buildConfigurations = ( 63DC842C1BE15267000708E8 /* Debug */, + 5E12289C1E4D400F00E8504F /* Test */, 5EC3C7A41D4FC18C000330E2 /* Cronet */, 63DC842D1BE15267000708E8 /* Release */, ); @@ -1850,6 +2059,7 @@ isa = XCConfigurationList; buildConfigurations = ( 63DC843D1BE15294000708E8 /* Debug */, + 5E12289D1E4D400F00E8504F /* Test */, 5EC3C7A51D4FC18C000330E2 /* Cronet */, 63DC843E1BE15294000708E8 /* Release */, ); @@ -1860,6 +2070,7 @@ isa = XCConfigurationList; buildConfigurations = ( 63DC844C1BE152B5000708E8 /* Debug */, + 5E12289E1E4D400F00E8504F /* Test */, 5EC3C7A61D4FC18C000330E2 /* Cronet */, 63DC844D1BE152B5000708E8 /* Release */, ); diff --git a/src/objective-c/tests/Tests.xcodeproj/xcshareddata/xcschemes/AllTests.xcscheme b/src/objective-c/tests/Tests.xcodeproj/xcshareddata/xcschemes/AllTests.xcscheme index 5524a27ffde..7575609dd74 100644 --- a/src/objective-c/tests/Tests.xcodeproj/xcshareddata/xcschemes/AllTests.xcscheme +++ b/src/objective-c/tests/Tests.xcodeproj/xcshareddata/xcschemes/AllTests.xcscheme @@ -23,7 +23,7 @@ @@ -73,7 +73,7 @@ + buildConfiguration = "Test"> @@ -54,7 +54,7 @@ + buildConfiguration = "Test"> @@ -57,7 +57,7 @@ + buildConfiguration = "Test"> @@ -57,7 +57,7 @@ + buildConfiguration = "Test"> Date: Thu, 9 Feb 2017 16:55:51 -0800 Subject: [PATCH 05/44] Warning and control the rest of interfaces in GRPCCall+Tests as well --- .../internal_testing/GRPCCall+Tests.h | 9 +++++++++ .../internal_testing/GRPCCall+Tests.m | 18 ++++++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/src/objective-c/GRPCClient/internal_testing/GRPCCall+Tests.h b/src/objective-c/GRPCClient/internal_testing/GRPCCall+Tests.h index 0325f5a192f..92c718a7493 100644 --- a/src/objective-c/GRPCClient/internal_testing/GRPCCall+Tests.h +++ b/src/objective-c/GRPCClient/internal_testing/GRPCCall+Tests.h @@ -45,6 +45,9 @@ * * Must be called before any gRPC call to that host is made. It's illegal to pass the same host to * more than one invocation of the methods of this category. + * + * This function is for internal testing of gRPC only. It is not part of gRPC's public interface. + * Do not use in production. To enable, set the preprocessor flag GRPC_TEST_OBJC. */ + (void)useTestCertsPath:(NSString *)certsPath testName:(NSString *)testName @@ -55,12 +58,18 @@ * * Must be called before any gRPC call to that host is made. It's illegal to pass the same host to * more than one invocation of the methods of this category. + * + * This function is for internal testing of gRPC only. It is not part of gRPC's public interface. + * Do not use in production. To enable, set the preprocessor flag GRPC_TEST_OBJC. */ + (void)useInsecureConnectionsForHost:(NSString *)host; /** * Resets all host configurations to their default values, and flushes all connections from the * cache. + * + * This function is for internal testing of gRPC only. It is not part of gRPC's public interface. + * Do not use in production. To enable, set the preprocessor flag GRPC_TEST_OBJC. */ + (void)resetHostSettings; diff --git a/src/objective-c/GRPCClient/internal_testing/GRPCCall+Tests.m b/src/objective-c/GRPCClient/internal_testing/GRPCCall+Tests.m index 5109480bef8..40bd2a9c803 100644 --- a/src/objective-c/GRPCClient/internal_testing/GRPCCall+Tests.m +++ b/src/objective-c/GRPCClient/internal_testing/GRPCCall+Tests.m @@ -41,6 +41,7 @@ + (void)useTestCertsPath:(NSString *)certsPath testName:(NSString *)testName forHost:(NSString *)host { +#ifdef GRPC_TEST_OBJC if (!host || !certsPath || !testName) { [NSException raise:NSInvalidArgumentException format:@"host, path and name must be provided."]; } @@ -55,15 +56,32 @@ GRPCHost *hostConfig = [GRPCHost hostWithAddress:host]; [hostConfig setTLSPEMRootCerts:certs withPrivateKey:nil withCertChain:nil error:nil]; hostConfig.hostNameOverride = testName; +#else + NSLog(@"This function is for internal testing of gRPC only. " + "It is not part of gRPC's public interface. Do not use in production. " + "To enable, set the preprocessor flag GRPC_TEST_OBJC."); +#endif } + (void)useInsecureConnectionsForHost:(NSString *)host { +#ifdef GRPC_TEST_OBJC GRPCHost *hostConfig = [GRPCHost hostWithAddress:host]; hostConfig.secure = NO; +#else + NSLog(@"This function is for internal testing of gRPC only. " + "It is not part of gRPC's public interface. Do not use in production. " + "To enable, set the preprocessor flag GRPC_TEST_OBJC."); +#endif } + (void)resetHostSettings { +#ifdef GRPC_TEST_OBJC [GRPCHost resetAllHostSettings]; +#else + NSLog(@"This function is for internal testing of gRPC only. " + "It is not part of gRPC's public interface. Do not use in production. " + "To enable, set the preprocessor flag GRPC_TEST_OBJC."); +#endif } + (void)enableOpBatchLog:(BOOL)enabled { From 8e6aec5ed7d04a19311e8c0d260a02ed5c99dce2 Mon Sep 17 00:00:00 2001 From: Muxi Yan Date: Fri, 10 Feb 2017 09:40:18 -0800 Subject: [PATCH 06/44] Separate/Restore public test interfaces and internal ones --- .../{internal_testing => }/GRPCCall+Tests.h | 27 +------- .../{internal_testing => }/GRPCCall+Tests.m | 43 +------------ .../internal_testing/GRPCCall+InternalTests.h | 57 +++++++++++++++++ .../internal_testing/GRPCCall+InternalTests.m | 61 +++++++++++++++++++ src/objective-c/tests/GRPCClientTests.m | 3 +- src/objective-c/tests/InteropTests.m | 3 +- .../tests/InteropTestsLocalCleartext.m | 3 +- src/objective-c/tests/InteropTestsLocalSSL.m | 3 +- src/objective-c/tests/InteropTestsRemote.m | 3 +- .../InteropTestsRemoteWithCronet.m | 3 +- src/objective-c/tests/Podfile | 2 +- 11 files changed, 133 insertions(+), 75 deletions(-) rename src/objective-c/GRPCClient/{internal_testing => }/GRPCCall+Tests.h (69%) rename src/objective-c/GRPCClient/{internal_testing => }/GRPCCall+Tests.m (64%) create mode 100644 src/objective-c/GRPCClient/internal_testing/GRPCCall+InternalTests.h create mode 100644 src/objective-c/GRPCClient/internal_testing/GRPCCall+InternalTests.m diff --git a/src/objective-c/GRPCClient/internal_testing/GRPCCall+Tests.h b/src/objective-c/GRPCClient/GRPCCall+Tests.h similarity index 69% rename from src/objective-c/GRPCClient/internal_testing/GRPCCall+Tests.h rename to src/objective-c/GRPCClient/GRPCCall+Tests.h index 92c718a7493..184ad09c5c8 100644 --- a/src/objective-c/GRPCClient/internal_testing/GRPCCall+Tests.h +++ b/src/objective-c/GRPCClient/GRPCCall+Tests.h @@ -31,7 +31,7 @@ * */ -#import "../GRPCCall.h" +#import "GRPCCall.h" /** * Methods to let tune down the security of gRPC connections for specific hosts. These shouldn't be @@ -45,9 +45,6 @@ * * Must be called before any gRPC call to that host is made. It's illegal to pass the same host to * more than one invocation of the methods of this category. - * - * This function is for internal testing of gRPC only. It is not part of gRPC's public interface. - * Do not use in production. To enable, set the preprocessor flag GRPC_TEST_OBJC. */ + (void)useTestCertsPath:(NSString *)certsPath testName:(NSString *)testName @@ -58,34 +55,12 @@ * * Must be called before any gRPC call to that host is made. It's illegal to pass the same host to * more than one invocation of the methods of this category. - * - * This function is for internal testing of gRPC only. It is not part of gRPC's public interface. - * Do not use in production. To enable, set the preprocessor flag GRPC_TEST_OBJC. */ + (void)useInsecureConnectionsForHost:(NSString *)host; /** * Resets all host configurations to their default values, and flushes all connections from the * cache. - * - * This function is for internal testing of gRPC only. It is not part of gRPC's public interface. - * Do not use in production. To enable, set the preprocessor flag GRPC_TEST_OBJC. */ + (void)resetHostSettings; - -/** - * Enables logging of op batches. Memory consumption increases as more ops are logged. - * - * This function is for internal testing of gRPC only. It is not part of gRPC's public interface. - * Do not use in production. To enable, set the preprocessor flag GRPC_TEST_OBJC. - */ -+ (void)enableOpBatchLog:(BOOL)enabled; - -/** - * Obtain the logged op batches. Invoking this method will clean the log. - * - * This function is for internal testing of gRPC only. It is not part of gRPC's public interface. - * Do not use in production. To enable, set the preprocessor flag GRPC_TEST_OBJC. - */ -+ (NSArray *)obtainAndCleanOpBatchLog; @end diff --git a/src/objective-c/GRPCClient/internal_testing/GRPCCall+Tests.m b/src/objective-c/GRPCClient/GRPCCall+Tests.m similarity index 64% rename from src/objective-c/GRPCClient/internal_testing/GRPCCall+Tests.m rename to src/objective-c/GRPCClient/GRPCCall+Tests.m index 40bd2a9c803..656cba8fec6 100644 --- a/src/objective-c/GRPCClient/internal_testing/GRPCCall+Tests.m +++ b/src/objective-c/GRPCClient/GRPCCall+Tests.m @@ -33,15 +33,13 @@ #import "GRPCCall+Tests.h" -#import "../private/GRPCHost.h" -#import "../private/GRPCOpBatchLog.h" +#import "private/GRPCHost.h" @implementation GRPCCall (Tests) + (void)useTestCertsPath:(NSString *)certsPath testName:(NSString *)testName forHost:(NSString *)host { -#ifdef GRPC_TEST_OBJC if (!host || !certsPath || !testName) { [NSException raise:NSInvalidArgumentException format:@"host, path and name must be provided."]; } @@ -56,53 +54,14 @@ GRPCHost *hostConfig = [GRPCHost hostWithAddress:host]; [hostConfig setTLSPEMRootCerts:certs withPrivateKey:nil withCertChain:nil error:nil]; hostConfig.hostNameOverride = testName; -#else - NSLog(@"This function is for internal testing of gRPC only. " - "It is not part of gRPC's public interface. Do not use in production. " - "To enable, set the preprocessor flag GRPC_TEST_OBJC."); -#endif } + (void)useInsecureConnectionsForHost:(NSString *)host { -#ifdef GRPC_TEST_OBJC GRPCHost *hostConfig = [GRPCHost hostWithAddress:host]; hostConfig.secure = NO; -#else - NSLog(@"This function is for internal testing of gRPC only. " - "It is not part of gRPC's public interface. Do not use in production. " - "To enable, set the preprocessor flag GRPC_TEST_OBJC."); -#endif } + (void)resetHostSettings { -#ifdef GRPC_TEST_OBJC [GRPCHost resetAllHostSettings]; -#else - NSLog(@"This function is for internal testing of gRPC only. " - "It is not part of gRPC's public interface. Do not use in production. " - "To enable, set the preprocessor flag GRPC_TEST_OBJC."); -#endif } - -+ (void)enableOpBatchLog:(BOOL)enabled { -#ifdef GRPC_TEST_OBJC - [GRPCOpBatchLog enableOpBatchLog:enabled]; -#else - NSLog(@"This function is for internal testing of gRPC only. " - "It is not part of gRPC's public interface. Do not use in production. " - "To enable, set the preprocessor flag GRPC_TEST_OBJC."); -#endif -} - -+ (NSArray *)obtainAndCleanOpBatchLog { -#ifdef GRPC_TEST_OBJC - return [GRPCOpBatchLog obtainAndCleanOpBatchLog]; -#else - NSLog(@"This function is for internal testing of gRPC only. " - "It is not part of gRPC's public interface. Do not use in production. " - "To enable, set the preprocessor flag GRPC_TEST_OBJC."); - return nil; -#endif -} - @end diff --git a/src/objective-c/GRPCClient/internal_testing/GRPCCall+InternalTests.h b/src/objective-c/GRPCClient/internal_testing/GRPCCall+InternalTests.h new file mode 100644 index 00000000000..6c12ce6355e --- /dev/null +++ b/src/objective-c/GRPCClient/internal_testing/GRPCCall+InternalTests.h @@ -0,0 +1,57 @@ +/* + * + * Copyright 2017, Google Inc. + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are + * met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following disclaimer + * in the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Google Inc. nor the names of its + * contributors may be used to endorse or promote products derived from + * this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + * + */ + +#import "../GRPCCall.h" + +/** + * Methods used for gRPC internal tests. DO NOT USE. + */ +@interface GRPCCall (InternalTests) + +/** + * Enables logging of op batches. Memory consumption increases as more ops are logged. + * + * This function is for internal testing of gRPC only. It is not part of gRPC's public interface. + * Do not use in production. To enable, set the preprocessor flag GRPC_TEST_OBJC. + */ ++ (void)enableOpBatchLog:(BOOL)enabled; + +/** + * Obtain the logged op batches. Invoking this method will clean the log. + * + * This function is for internal testing of gRPC only. It is not part of gRPC's public interface. + * Do not use in production. To enable, set the preprocessor flag GRPC_TEST_OBJC. + */ ++ (NSArray *)obtainAndCleanOpBatchLog; + +@end diff --git a/src/objective-c/GRPCClient/internal_testing/GRPCCall+InternalTests.m b/src/objective-c/GRPCClient/internal_testing/GRPCCall+InternalTests.m new file mode 100644 index 00000000000..43c1c078316 --- /dev/null +++ b/src/objective-c/GRPCClient/internal_testing/GRPCCall+InternalTests.m @@ -0,0 +1,61 @@ +/* + * + * Copyright 2017, Google Inc. + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are + * met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following disclaimer + * in the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Google Inc. nor the names of its + * contributors may be used to endorse or promote products derived from + * this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + * + */ + +#import "GRPCCall+InternalTests.h" + +#import "../private/GRPCOpBatchLog.h" + +@implementation GRPCCall (InternalTests) + ++ (void)enableOpBatchLog:(BOOL)enabled { +#ifdef GRPC_TEST_OBJC + [GRPCOpBatchLog enableOpBatchLog:enabled]; +#else + NSLog(@"This function is for internal testing of gRPC only. " + "It is not part of gRPC's public interface. Do not use in production. " + "To enable, set the preprocessor flag GRPC_TEST_OBJC."); +#endif +} + ++ (NSArray *)obtainAndCleanOpBatchLog { +#ifdef GRPC_TEST_OBJC + return [GRPCOpBatchLog obtainAndCleanOpBatchLog]; +#else + NSLog(@"This function is for internal testing of gRPC only. " + "It is not part of gRPC's public interface. Do not use in production. " + "To enable, set the preprocessor flag GRPC_TEST_OBJC."); + return nil; +#endif +} + +@end diff --git a/src/objective-c/tests/GRPCClientTests.m b/src/objective-c/tests/GRPCClientTests.m index a1bb835e5d4..76c15003f60 100644 --- a/src/objective-c/tests/GRPCClientTests.m +++ b/src/objective-c/tests/GRPCClientTests.m @@ -37,7 +37,8 @@ #import #import #import -#import +#import +#import #import #import #import diff --git a/src/objective-c/tests/InteropTests.m b/src/objective-c/tests/InteropTests.m index 592dca9dde5..4e5a5287c2f 100644 --- a/src/objective-c/tests/InteropTests.m +++ b/src/objective-c/tests/InteropTests.m @@ -37,7 +37,8 @@ #import #import -#import +#import +#import #import #import #import diff --git a/src/objective-c/tests/InteropTestsLocalCleartext.m b/src/objective-c/tests/InteropTestsLocalCleartext.m index 6e7624e2046..0b8a0e47756 100644 --- a/src/objective-c/tests/InteropTestsLocalCleartext.m +++ b/src/objective-c/tests/InteropTestsLocalCleartext.m @@ -31,7 +31,8 @@ * */ -#import +#import +#import #import "InteropTests.h" diff --git a/src/objective-c/tests/InteropTestsLocalSSL.m b/src/objective-c/tests/InteropTestsLocalSSL.m index 12007bbd681..d974c2db27b 100644 --- a/src/objective-c/tests/InteropTestsLocalSSL.m +++ b/src/objective-c/tests/InteropTestsLocalSSL.m @@ -31,7 +31,8 @@ * */ -#import +#import +#import #import "InteropTests.h" diff --git a/src/objective-c/tests/InteropTestsRemote.m b/src/objective-c/tests/InteropTestsRemote.m index b0a64a3caed..ad7efebc81c 100644 --- a/src/objective-c/tests/InteropTestsRemote.m +++ b/src/objective-c/tests/InteropTestsRemote.m @@ -31,7 +31,8 @@ * */ -#import +#import +#import #import "InteropTests.h" diff --git a/src/objective-c/tests/InteropTestsRemoteWithCronet/InteropTestsRemoteWithCronet.m b/src/objective-c/tests/InteropTestsRemoteWithCronet/InteropTestsRemoteWithCronet.m index c8742b7fe8c..660865abb44 100644 --- a/src/objective-c/tests/InteropTestsRemoteWithCronet/InteropTestsRemoteWithCronet.m +++ b/src/objective-c/tests/InteropTestsRemoteWithCronet/InteropTestsRemoteWithCronet.m @@ -31,7 +31,8 @@ * */ -#import +#import +#import #import #import diff --git a/src/objective-c/tests/Podfile b/src/objective-c/tests/Podfile index 5ada61c3d40..941031ff88e 100644 --- a/src/objective-c/tests/Podfile +++ b/src/objective-c/tests/Podfile @@ -103,7 +103,7 @@ post_install do |installer| # Activate Cronet for the dedicated build configuration 'Cronet', which will be used solely by # the test target 'InteropTestsRemoteWithCronet' - # Activate GRPCCall+Tests functions for the dedicated build configuration 'Test', which will + # Activate GRPCCall+InternalTests functions for the dedicated build configuration 'Test', which will # be used by all test targets using it. if target.name == 'gRPC' target.build_configurations.each do |config| From 003092a2020c927408fb173dfd9dcdf2dfc35775 Mon Sep 17 00:00:00 2001 From: Makarand Dharmapurikar Date: Tue, 21 Feb 2017 14:08:01 -0800 Subject: [PATCH 07/44] code and BUILD file for creating a grpcz client The client binary can be built with 'bazel build //tools/grpcz:grpcz_client' and can be invoked with bazel-bin/tools/grpcz/grpcz_client --server SERVER_ADDR:PORT You can see the stats page at http://localhost:8000/grpcz --- WORKSPACE | 26 +- tools/grpcz/BUILD | 73 +++++ tools/grpcz/any.proto | 139 +++++++++ tools/grpcz/census.proto | 318 +++++++++++++++++++++ tools/grpcz/empty.proto | 52 ++++ tools/grpcz/grpcz_client.cc | 174 +++++++++++ tools/grpcz/monitoring.proto | 131 +++++++++ tools/run_tests/sanity/check_submodules.sh | 2 +- 8 files changed, 910 insertions(+), 5 deletions(-) create mode 100644 tools/grpcz/BUILD create mode 100644 tools/grpcz/any.proto create mode 100644 tools/grpcz/census.proto create mode 100644 tools/grpcz/empty.proto create mode 100644 tools/grpcz/grpcz_client.cc create mode 100644 tools/grpcz/monitoring.proto diff --git a/WORKSPACE b/WORKSPACE index 4f90f06d881..5d163f78e81 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -33,26 +33,44 @@ bind( actual = "@submodule_gtest//:gtest", ) +bind( + name = "gflags", + actual = "@com_github_gflags_gflags//:gflags", +) + new_local_repository( name = "submodule_boringssl", - path = "third_party/boringssl-with-bazel", build_file = "third_party/boringssl-with-bazel/BUILD", + path = "third_party/boringssl-with-bazel", ) new_local_repository( name = "submodule_zlib", - path = "third_party/zlib", build_file = "third_party/zlib.BUILD", + path = "third_party/zlib", ) new_local_repository( name = "submodule_protobuf", - path = "third_party/protobuf", build_file = "third_party/protobuf/BUILD", + path = "third_party/protobuf", ) new_local_repository( name = "submodule_gtest", - path = "third_party/googletest", build_file = "third_party/gtest.BUILD", + path = "third_party/googletest", +) + +local_repository( + name = "com_github_gflags_gflags", + path = "third_party/gflags", +) +# used for tools/grpcz/grpcz_client +git_repository( + name = "mongoose_repo", + commit = "21b9ddd490783e3afaa0fa9b45d6c1133eb922dc", + remote = "https://github.com/makdharma/mongoose.git" ) + + diff --git a/tools/grpcz/BUILD b/tools/grpcz/BUILD new file mode 100644 index 00000000000..34bd072d004 --- /dev/null +++ b/tools/grpcz/BUILD @@ -0,0 +1,73 @@ +# Copyright 2017, Google Inc. +# All rights reserved. +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are +# met: +# +# * Redistributions of source code must retain the above copyright +# notice, this list of conditions and the following disclaimer. +# * Redistributions in binary form must reproduce the above +# copyright notice, this list of conditions and the following disclaimer +# in the documentation and/or other materials provided with the +# distribution. +# * Neither the name of Google Inc. nor the names of its +# contributors may be used to endorse or promote products derived from +# this software without specific prior written permission. +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +licenses(["notice"]) # 3-clause BSD + +package(default_visibility = ["//visibility:public"]) + +load("//:bazel/generate_cc.bzl", "generate_cc") + +proto_library ( + name = "monitoring_proto_local_copy", + srcs = [ + # TODO (erikgribkoff) : remove the local copies of these protos + "monitoring.proto", + "empty.proto", + "any.proto", + "census.proto", + ], +) + +generate_cc( + name = "monitoring_codegen", + srcs = [":monitoring_proto_local_copy"], +) + +generate_cc( + name = "monitoring_grpc_codegen", + srcs = [":monitoring_proto_local_copy"], + plugin = "//:grpc_cpp_plugin", +) + +cc_library( + name = "proto_lib", + srcs = [":monitoring_codegen", ":monitoring_grpc_codegen"], + hdrs = [":monitoring_codegen", ":monitoring_grpc_codegen"], + deps = ["//:grpc++", "//:grpc++_codegen_proto", "//external:protobuf"], +) + +cc_binary( + name = "grpcz_client", + srcs = ["grpcz_client.cc",], + deps = [ + "proto_lib", + "//external:gflags", + "@mongoose_repo//:mongoose_lib", + ], +) diff --git a/tools/grpcz/any.proto b/tools/grpcz/any.proto new file mode 100644 index 00000000000..a224a2b596e --- /dev/null +++ b/tools/grpcz/any.proto @@ -0,0 +1,139 @@ +// Protocol Buffers - Google's data interchange format +// Copyright 2008 Google Inc. All rights reserved. +// https://developers.google.com/protocol-buffers/ +// +// Redistribution and use in source and binary forms, with or without +// modification, are permitted provided that the following conditions are +// met: +// +// * Redistributions of source code must retain the above copyright +// notice, this list of conditions and the following disclaimer. +// * Redistributions in binary form must reproduce the above +// copyright notice, this list of conditions and the following disclaimer +// in the documentation and/or other materials provided with the +// distribution. +// * Neither the name of Google Inc. nor the names of its +// contributors may be used to endorse or promote products derived from +// this software without specific prior written permission. +// +// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +syntax = "proto3"; + +//package google.protobuf; + +option csharp_namespace = "Google.Protobuf.WellKnownTypes"; +option go_package = "github.com/golang/protobuf/ptypes/any"; +option java_package = "com.google.protobuf"; +option java_outer_classname = "AnyProto"; +option java_multiple_files = true; +option objc_class_prefix = "GPB"; + +// `Any` contains an arbitrary serialized protocol buffer message along with a +// URL that describes the type of the serialized message. +// +// Protobuf library provides support to pack/unpack Any values in the form +// of utility functions or additional generated methods of the Any type. +// +// Example 1: Pack and unpack a message in C++. +// +// Foo foo = ...; +// Any any; +// any.PackFrom(foo); +// ... +// if (any.UnpackTo(&foo)) { +// ... +// } +// +// Example 2: Pack and unpack a message in Java. +// +// Foo foo = ...; +// Any any = Any.pack(foo); +// ... +// if (any.is(Foo.class)) { +// foo = any.unpack(Foo.class); +// } +// +// Example 3: Pack and unpack a message in Python. +// +// foo = Foo(...) +// any = Any() +// any.Pack(foo) +// ... +// if any.Is(Foo.DESCRIPTOR): +// any.Unpack(foo) +// ... +// +// The pack methods provided by protobuf library will by default use +// 'type.googleapis.com/full.type.name' as the type URL and the unpack +// methods only use the fully qualified type name after the last '/' +// in the type URL, for example "foo.bar.com/x/y.z" will yield type +// name "y.z". +// +// +// JSON +// ==== +// The JSON representation of an `Any` value uses the regular +// representation of the deserialized, embedded message, with an +// additional field `@type` which contains the type URL. Example: +// +// package google.profile; +// message Person { +// string first_name = 1; +// string last_name = 2; +// } +// +// { +// "@type": "type.googleapis.com/google.profile.Person", +// "firstName": , +// "lastName": +// } +// +// If the embedded message type is well-known and has a custom JSON +// representation, that representation will be embedded adding a field +// `value` which holds the custom JSON in addition to the `@type` +// field. Example (for message [google.protobuf.Duration][]): +// +// { +// "@type": "type.googleapis.com/google.protobuf.Duration", +// "value": "1.212s" +// } +// +message Any { + // A URL/resource name whose content describes the type of the + // serialized protocol buffer message. + // + // For URLs which use the scheme `http`, `https`, or no scheme, the + // following restrictions and interpretations apply: + // + // * If no scheme is provided, `https` is assumed. + // * The last segment of the URL's path must represent the fully + // qualified name of the type (as in `path/google.protobuf.Duration`). + // The name should be in a canonical form (e.g., leading "." is + // not accepted). + // * An HTTP GET on the URL must yield a [google.protobuf.Type][] + // value in binary format, or produce an error. + // * Applications are allowed to cache lookup results based on the + // URL, or have them precompiled into a binary to avoid any + // lookup. Therefore, binary compatibility needs to be preserved + // on changes to types. (Use versioned type names to manage + // breaking changes.) + // + // Schemes other than `http`, `https` (or the empty scheme) might be + // used with implementation specific semantics. + // + string type_url = 1; + + // Must be a valid serialized protocol buffer of the above specified type. + bytes value = 2; +} diff --git a/tools/grpcz/census.proto b/tools/grpcz/census.proto new file mode 100644 index 00000000000..d1ff69400b0 --- /dev/null +++ b/tools/grpcz/census.proto @@ -0,0 +1,318 @@ +// Copyright 2017, Google Inc. +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// http://www.apache.org/licenses/LICENSE-2.0 +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//TODO(ericgribkoff) Depend on this directly from the instrumentation-proto +//repository. + +syntax = "proto3"; + +package google.instrumentation; + +option java_package = "com.google.instrumentation.stats.proto"; +option java_outer_classname = "CensusProto"; + +// All the census protos. +// +// Nomenclature notes: +// * Capitalized names below (like View) are protos. +// * Protos which describe types are named with a Descriptor suffix (e.g. +// MesurementDescriptor). +// +// Census lets you define the type and description of the data being measured +// (e.g. the latency of an RPC or the number of CPU cycles spent on an +// operation using MeasurementDescriptor. As individual measurements (a double +// value) for are recorded, they are aggregated together into an +// Aggregation. There are two Aggregation types available: Distribution +// (describes the distribution of all measurements, possibly with a histogram) +// and IntervalStats (the count and mean of measurements across specified time +// periods). An Aggregation is described by an AggregationDescriptor. +// +// You can define how your measurements (described by a MeasurementDescriptor) +// are broken down by Tag values and which Aggregations to use through a +// ViewDescriptor. The output (all measurements broken down by tag values into +// specific Aggregations) is called a View. + + +// The following two types are copied from +// google/protobuf/{duration,timestamp}.proto. Ideally, we would be able to +// import them, but this causes compilation issues on C-based systems +// (e.g. https://koti.kapsi.fi/jpa/nanopb/), which cannot process the C++ +// headers generated from the standard protobuf distribution. See the relevant +// proto files for full documentation of these types. + +message Duration { + // Signed seconds of the span of time. Must be from -315,576,000,000 + // to +315,576,000,000 inclusive. + int64 seconds = 1; + + // Signed fractions of a second at nanosecond resolution of the span + // of time. Durations less than one second are represented with a 0 + // `seconds` field and a positive or negative `nanos` field. For durations + // of one second or more, a non-zero value for the `nanos` field must be + // of the same sign as the `seconds` field. Must be from -999,999,999 + // to +999,999,999 inclusive. + int32 nanos = 2; +} + +message Timestamp { + // Represents seconds of UTC time since Unix epoch + // 1970-01-01T00:00:00Z. Must be from from 0001-01-01T00:00:00Z to + // 9999-12-31T23:59:59Z inclusive. + int64 seconds = 1; + + // Non-negative fractions of a second at nanosecond resolution. Negative + // second values with fractions must still have non-negative nanos values + // that count forward in time. Must be from 0 to 999,999,999 + // inclusive. + int32 nanos = 2; +} + +// MeasurementDescriptor describes a data point (measurement) type. +message MeasurementDescriptor { + // A descriptive name, e.g. rpc_latency, cpu. Must be unique. + string name = 1; + + // More detailed description of the resource, used in documentation. + string description = 2; + + // Fundamental units of measurement supported by Census + // TODO(aveitch): expand this to include other S.I. units? + enum BasicUnit { + UNKNOWN = 0; // Implementations should not use this + SCALAR = 1; // Dimensionless + BITS = 2; // A single bit + BYTES = 3; // An 8-bit byte + SECONDS = 4; // S.I. unit + CORES = 5; // CPU core usage + MAX_UNITS = 6; // Last defined value; implementations should only use + // this for validation. + } + + // MeasurementUnit lets you build compound units of the form + // 10^n * (A * B * ...) / (X * Y * ...), + // where the elements in the numerator and denominator are all BasicUnits. A + // MeasurementUnit must have at least one BasicUnit in its numerator. + // + // To specify multiplication in the numerator or denominator, simply specify + // multiple numerator or denominator fields. For example: + // + // - byte-seconds (i.e. bytes * seconds): + // numerator: BYTES + // numerator: SECS + // + // - events/sec^2 (i.e. rate of change of events/sec): + // numerator: SCALAR + // denominator: SECS + // denominator: SECS + // + // To specify multiples (in power of 10) of units, specify a non-zero + // 'power10' value, for example: + // + // - MB/s (i.e. megabytes / s): + // power10: 6 + // numerator: BYTES + // denominator: SECS + // + // - nanoseconds + // power10: -9 + // numerator: SECS + message MeasurementUnit { + int32 power10 = 1; + repeated BasicUnit numerators = 2; + repeated BasicUnit denominators = 3; + } + + // The units used by this type of measurement. + MeasurementUnit unit = 3; +} + +// An aggregation summarizes a series of individual measurements. There are +// two types of aggregation (IntervalAggregation and DistributionAggregation), +// unique types of each can be set using descriptors for each. + +// DistributionAggregation contains summary statistics for a population of +// values and, optionally, a histogram representing the distribution of those +// values across a specified set of histogram buckets, as defined in +// DistributionAggregationDescriptor.bucket_bounds. +// +// The summary statistics are the count, mean, minimum, and the maximum of the +// set of population of values. +// +// Although it is not forbidden, it is generally a bad idea to include +// non-finite values (infinities or NaNs) in the population of values, as this +// will render the `mean` field meaningless. +message DistributionAggregation { + // The number of values in the population. Must be non-negative. + int64 count = 1; + + // The arithmetic mean of the values in the population. If `count` is zero + // then this field must be zero. + double mean = 2; + + // The sum of the values in the population. If `count` is zero then this + // field must be zero. + double sum = 3; + + // Describes a range of population values. + message Range { + // The minimum of the population values. + double min = 1; + // The maximum of the population values. + double max = 2; + } + + // The range of the population values. If `count` is zero, this field will not + // be defined. + Range range = 4; + + // A Distribution may optionally contain a histogram of the values in the + // population. The histogram is given in `bucket_count` as counts of values + // that fall into one of a sequence of non-overlapping buckets, as described + // by `DistributionAggregationDescriptor.bucket_boundaries`. The sum of the + // values in `bucket_counts` must equal the value in `count`. + // + // Bucket counts are given in order under the numbering scheme described + // above (the underflow bucket has number 0; the finite buckets, if any, + // have numbers 1 through N-2; the overflow bucket has number N-1). + // + // The size of `bucket_count` must be no greater than N as defined in + // `bucket_boundaries`. + // + // Any suffix of trailing zero bucket_count fields may be omitted. + repeated int64 bucket_counts = 5; + + // Tags associated with this DistributionAggregation. These will be filled + // in based on the View specification. + repeated Tag tags = 6; +} + +message DistributionAggregationDescriptor { + // A Distribution may optionally contain a histogram of the values in the + // population. The bucket boundaries for that histogram are described by + // `bucket_bounds`. This defines `size(bucket_bounds) + 1` (= N) + // buckets. The boundaries for bucket index i are: + // + // [-infinity, bucket_bounds[i]) for i == 0 + // [bucket_bounds[i-1], bucket_bounds[i]) for 0 < i < N-2 + // [bucket_bounds[i-1], +infinity) for i == N-1 + // + // i.e. an underflow bucket (number 0), zero or more finite buckets (1 + // through N - 2, and an overflow bucket (N - 1), with inclusive lower + // bounds and exclusive upper bounds. + // + // If `bucket_bounds` has no elements (zero size), then there is no + // histogram associated with the Distribution. If `bucket_bounds` has only + // one element, there are no finite buckets, and that single element is the + // common boundary of the overflow and underflow buckets. The values must + // be monotonically increasing. + repeated double bucket_bounds = 1; +} + +// An IntervalAggreation records summary stats over various time +// windows. These stats are approximate, with the degree of accuracy +// controlled by setting the n_sub_intervals parameter in the +// IntervalAggregationDescriptor. +message IntervalAggregation { + // Summary statistic over a single time interval. + message Interval { + // The interval duration. Must be positive. + Duration interval_size = 1; + // Approximate number of measurements recorded in this interval. + double count = 2; + // The cumulative sum of measurements in this interval. + double sum = 3; + } + + // Full set of intervals for this aggregation. + repeated Interval intervals = 1; + + // Tags associated with this IntervalAggregation. These will be filled in + // based on the View specification. + repeated Tag tags = 2; +} + +// An IntervalAggreationDescriptor specifies time intervals for an +// IntervalAggregation. +message IntervalAggregationDescriptor { + // Number of internal sub-intervals to use when collecting stats for each + // interval. The max error in interval measurements will be approximately + // 1/n_sub_intervals (although in practice, this will only be approached in + // the presence of very large and bursty workload changes), and underlying + // memory usage will be roughly proportional to the value of this + // field. Must be in the range [2, 20]. A value of 5 will be used if this is + // unspecified. + int32 n_sub_intervals = 1; + + // The size of each interval, as a time duration. Must have at least one + // element. + repeated Duration interval_sizes = 2; +} + +// A Tag: key-value pair. +message Tag { + string key = 1; + string value = 2; +} + +// A ViewDescriptor specifies an AggregationDescriptor and a set of tag +// keys. Views instantiated from this descriptor will contain Aggregations +// broken down by the unique set of matching tag values for each measurement. +message ViewDescriptor { + // Name of view. Must be unique. + string name = 1; + + // More detailed description, for documentation purposes. + string description = 2; + + // Name of a MeasurementDescriptor to be used for this view. + string measurement_descriptor_name = 3; + + // Aggregation type to associate with View. + oneof aggregation { + IntervalAggregationDescriptor interval_aggregation = 4; + DistributionAggregationDescriptor distribution_aggregation = 5; + } + + // Tag keys to match with a given measurement. If no keys are specified, + // then all stats are recorded. Keys must be unique. + repeated string tag_keys = 6; +} + +// DistributionView contains all aggregations for a view specified using a +// DistributionAggregationDescriptor. +message DistributionView { + // Aggregations - each will have a unique set of tag values for the tag_keys + // associated with the corresponding View. + repeated DistributionAggregation aggregations = 1; + + // Start and end timestamps over which aggregations was accumulated. + Timestamp start = 2; + Timestamp end = 3; +} + +// IntervalView contains all aggregations for a view specified using a +// IntervalAggregationDescriptor. +message IntervalView { + // Aggregations - each will have a unique set of tag values for the tag_keys + // associated with the corresponding View. + repeated IntervalAggregation aggregations = 1; +} + +// A View contains the aggregations based on a ViewDescriptor. +message View { + // ViewDescriptor name associated with this set of View. + string view_name = 1; + + oneof view { + DistributionView distribution_view = 2; + IntervalView interval_view = 3; + } +} diff --git a/tools/grpcz/empty.proto b/tools/grpcz/empty.proto new file mode 100644 index 00000000000..03cacd23308 --- /dev/null +++ b/tools/grpcz/empty.proto @@ -0,0 +1,52 @@ +// Protocol Buffers - Google's data interchange format +// Copyright 2008 Google Inc. All rights reserved. +// https://developers.google.com/protocol-buffers/ +// +// Redistribution and use in source and binary forms, with or without +// modification, are permitted provided that the following conditions are +// met: +// +// * Redistributions of source code must retain the above copyright +// notice, this list of conditions and the following disclaimer. +// * Redistributions in binary form must reproduce the above +// copyright notice, this list of conditions and the following disclaimer +// in the documentation and/or other materials provided with the +// distribution. +// * Neither the name of Google Inc. nor the names of its +// contributors may be used to endorse or promote products derived from +// this software without specific prior written permission. +// +// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +syntax = "proto3"; + +package google.protobuf; + +option csharp_namespace = "Google.Protobuf.WellKnownTypes"; +option go_package = "github.com/golang/protobuf/ptypes/empty"; +option java_package = "com.google.protobuf"; +option java_outer_classname = "EmptyProto"; +option java_multiple_files = true; +option objc_class_prefix = "GPB"; +option cc_enable_arenas = true; + +// A generic empty message that you can re-use to avoid defining duplicated +// empty messages in your APIs. A typical example is to use it as the request +// or the response type of an API method. For instance: +// +// service Foo { +// rpc Bar(google.protobuf.Empty) returns (google.protobuf.Empty); +// } +// +// The JSON representation for `Empty` is empty JSON object `{}`. +message Empty {} diff --git a/tools/grpcz/grpcz_client.cc b/tools/grpcz/grpcz_client.cc new file mode 100644 index 00000000000..afca3b0532f --- /dev/null +++ b/tools/grpcz/grpcz_client.cc @@ -0,0 +1,174 @@ +/* + * + * Copyright 2017, Google Inc. + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are + * met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following disclaimer + * in the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Google Inc. nor the names of its + * contributors may be used to endorse or promote products derived from + * this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + * + */ + +#include +#include +#include + +#include +#include +#include + +#include "gflags/gflags.h" +#include "mongoose.h" + +// TODO (makdharma): remove local copies of these protos +#include "tools/grpcz/census.grpc.pb.h" +#include "tools/grpcz/monitoring.grpc.pb.h" + +DEFINE_string(server, "127.0.0.1:50052", + "file path (or host:port) where grpcz server is running"); +DEFINE_string(http_port, "8000", + "Port id for accessing the HTTP server that renders /grpcz page"); +DEFINE_bool(print, false, "only print the output and quit"); + +using grpc::Channel; +using grpc::ClientContext; +using grpc::Status; + +using ::grpc::instrumentation::v1alpha::CanonicalRpcStats; +using ::grpc::instrumentation::v1alpha::Monitoring; + +static const std::string static_html_header = + " \ +\ +

GRPCZ FTW

\ + "; + +class GrpczClient { + public: + GrpczClient(std::shared_ptr channel) + : stub_(Monitoring::NewStub(channel)) {} + + std::string GetStatsAsJson() { + const ::google::protobuf::Empty request; + CanonicalRpcStats reply; + ClientContext context; + Status status = stub_->GetCanonicalRpcStats(&context, request, &reply); + + if (status.ok()) { + std::string json_str; + ::google::protobuf::util::MessageToJsonString(reply, &json_str); + return json_str; + } else { + static const std::string error_message_json = + "{\"grpcz Access Error\"\ + :{\"view\":{\"viewName\":\"grpcz Access Error\",\ + \"intervalView\":\"Server not running?\"}}}"; + std::cout << status.error_code() << ":= " << status.error_message() + << std::endl; + return error_message_json; + } + } + + private: + std::unique_ptr stub_; +}; + +static struct mg_serve_http_opts s_http_server_opts; +std::unique_ptr g_grpcz_client; + +static void ev_handler(struct mg_connection *nc, int ev, void *p) { + if (ev == MG_EV_HTTP_REQUEST) { + mg_serve_http(nc, (struct http_message *)p, s_http_server_opts); + } +} + +static void grpcz_handler(struct mg_connection *nc, int ev, void *ev_data) { + (void)ev; + (void)ev_data; + gpr_log(GPR_INFO, "fetching grpcz stats from %s", FLAGS_server.c_str()); + std::string json_str = g_grpcz_client->GetStatsAsJson(); + std::string rendered_html = + static_html_header + json_str + static_html_footer; + mg_printf(nc, "HTTP/1.0 200 OK\r\n\r\n%s", rendered_html.c_str()); + nc->flags |= MG_F_SEND_AND_CLOSE; +} + +int main(int argc, char **argv) { + gflags::ParseCommandLineFlags(&argc, &argv, true); + + // Create a client + g_grpcz_client.reset(new GrpczClient( + grpc::CreateChannel(FLAGS_server, grpc::InsecureChannelCredentials()))); + if (FLAGS_print) { + g_grpcz_client->GetStatsAsJson(); + return 0; + } + + // Set up a mongoose webserver handler + struct mg_mgr mgr; + mg_mgr_init(&mgr, NULL); + gpr_log(GPR_INFO, "Starting grpcz web server on port %s\n", + FLAGS_http_port.c_str()); + + struct mg_connection *nc = mg_bind(&mgr, FLAGS_http_port.c_str(), ev_handler); + if (nc == NULL) { + gpr_log(GPR_ERROR, "Failed to create listener on port %s\n", + FLAGS_http_port.c_str()); + return -11; + } + mg_register_http_endpoint(nc, "/grpcz", grpcz_handler); + mg_set_protocol_http_websocket(nc); + + // Poll in a loop and serve /grpcz pages + for (;;) { + mg_mgr_poll(&mgr, 100); + } + mg_mgr_free(&mgr); + return 0; +} diff --git a/tools/grpcz/monitoring.proto b/tools/grpcz/monitoring.proto new file mode 100644 index 00000000000..ec3dfe9f74c --- /dev/null +++ b/tools/grpcz/monitoring.proto @@ -0,0 +1,131 @@ +// Copyright 2017, Google Inc. +// All rights reserved. +// +// Redistribution and use in source and binary forms, with or without +// modification, are permitted provided that the following conditions are +// met: +// +// * Redistributions of source code must retain the above copyright +// notice, this list of conditions and the following disclaimer. +// * Redistributions in binary form must reproduce the above +// copyright notice, this list of conditions and the following disclaimer +// in the documentation and/or other materials provided with the +// distribution. +// * Neither the name of Google Inc. nor the names of its +// contributors may be used to endorse or promote products derived from +// this software without specific prior written permission. +// +// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +// This file defines an interface for exporting monitoring information +// out of gRPC servers. +syntax = "proto3"; + +// TODO(ericgribkoff) Figure out how to manage the external Census proto +// dependency. +import "tools/grpcz/census.proto"; +import "tools/grpcz/empty.proto"; +import "tools/grpcz/any.proto"; + +package grpc.instrumentation.v1alpha; + +option java_multiple_files = true; +option java_package = "io.grpc.instrumentation.v1alpha"; +option java_outer_classname = "MonitoringProto"; + +service Monitoring { + // Return canonical RPC stats + rpc GetCanonicalRpcStats(google.protobuf.Empty) returns (CanonicalRpcStats) { + } + + // Query the server for specific stats + rpc GetStats(StatsRequest) returns (StatsResponse) { + // TODO(aveitch, ericgribkoff): Pease define the stats response message + // StatsRequest would specifically identify the stats to be returned. + } + + // Request the server to stream back snapshots of the requested stats + rpc WatchStats(StatsRequest) returns (stream StatsResponse) { + } + + + // Return request traces. + rpc GetRequestTraces(TraceRequest) returns(TraceResponse) { + // TODO(aveitch): Please define the messages here + } + + // Return application-defined groups of monitoring data. + // This is a low level facility to allow extension of the monitoring API to + // application-specific monitoring data. Frameworks may use this to define + // additional groups of monitoring data made available by servers. + rpc GetCustomMonitoringData(MonitoringDataGroup) + returns (CustomMonitoringData) { + } + +} + +// Canonical RPC stats exported by gRPC. +message CanonicalRpcStats { + // Wrapper combining View and ViewDescriptor. + message View { + google.instrumentation.MeasurementDescriptor measurement_descriptor = 1; + google.instrumentation.ViewDescriptor view_descriptor = 2; + google.instrumentation.View view = 3; + } + + View rpc_client_errors = 1; + View rpc_client_completed_rpcs = 2; + View rpc_client_started_rpcs = 3; + View rpc_client_elapsed_time = 4; + View rpc_client_server_elapsed_time = 5; + View rpc_client_request_bytes = 6; + View rpc_client_response_bytes = 7; + View rpc_client_request_count = 8; + View rpc_client_response_count = 9; + View rpc_server_errors = 10; + View rpc_server_completed_rpcs = 11; + View rpc_server_server_elapsed_time = 12; + View rpc_server_request_bytes = 13; + View rpc_server_response_bytes = 14; + View rpc_server_request_count = 15; + View rpc_server_response_count = 16; + View rpc_server_elapsed_time = 17; + //TODO(ericgribkoff) Add minute-hour interval stats. +} + +message StatsRequest { + // TODO(aveitch): Complete definition of this type +} + +message StatsResponse { + // TODO(aveitch): Complete definition of this type +} + +message TraceRequest { + // TODO(aveitch): Complete definition of this type +} + +message TraceResponse { + // TODO(aveitch): Complete definition of this type +} + +message MonitoringDataGroup { + string name = 1; // name of a group of monitoring data +} + +// The wrapper for custom monitoring data. +message CustomMonitoringData { + // can be any application specific monitoring data + Any contents = 1; +} + diff --git a/tools/run_tests/sanity/check_submodules.sh b/tools/run_tests/sanity/check_submodules.sh index 0b68319d290..cfe4e2731c0 100755 --- a/tools/run_tests/sanity/check_submodules.sh +++ b/tools/run_tests/sanity/check_submodules.sh @@ -44,7 +44,7 @@ cat << EOF | awk '{ print $1 }' | sort > $want_submodules 44c25c892a6229b20db7cd9dc05584ea865896de third_party/benchmark (v0.1.0-343-g44c25c8) 78684e5b222645828ca302e56b40b9daff2b2d27 third_party/boringssl (78684e5) 886e7d75368e3f4fab3f4d0d3584e4abfc557755 third_party/boringssl-with-bazel (version_for_cocoapods_7.0-857-g886e7d7) - f8a0efe03aa69b3336d8e228b37d4ccb17324b88 third_party/gflags (v2.2.0) + 30dbc81fb5ffdc98ea9b14b1918bfe4e8779b26e third_party/gflags (v2.2.0) c99458533a9b4c743ed51537e25989ea55944908 third_party/googletest (release-1.7.0) a428e42072765993ff674fda72863c9f1aa2d268 third_party/protobuf (v3.1.0-alpha-1) bcad91771b7f0bff28a1cac1981d7ef2b9bcef3c third_party/thrift (bcad917) From d1f3082ff608a82040d122ad3634942ac1515cf2 Mon Sep 17 00:00:00 2001 From: Nicolas 'Pixel' Noble Date: Tue, 21 Feb 2017 15:04:26 -0800 Subject: [PATCH 08/44] Adding shim for debugging node tests using vscode. --- .vscode/launch.json | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) create mode 100644 .vscode/launch.json diff --git a/.vscode/launch.json b/.vscode/launch.json new file mode 100644 index 00000000000..700c61cceae --- /dev/null +++ b/.vscode/launch.json @@ -0,0 +1,30 @@ +{ + "version": "0.2.0", + "configurations": [ + { + "type": "node", + "request": "launch", + "name": "Mocha Tests", + "cwd": "${workspaceRoot}", + "runtimeExecutable": "${workspaceRoot}/node_modules/.bin/mocha", + "windows": { + "runtimeExecutable": "${workspaceRoot}/node_modules/.bin/mocha.cmd" + }, + "runtimeArgs": [ + "-u", + "tdd", + "--timeout", + "999999", + "--colors", + "${workspaceRoot}/src/node/test" + ], + "internalConsoleOptions": "openOnSessionStart" + }, + { + "type": "node", + "request": "attach", + "name": "Attach to Process", + "port": 5858 + } + ] +} From 86db6873db5b4f09a2a3d6b430db27ac0fd40445 Mon Sep 17 00:00:00 2001 From: Makarand Dharmapurikar Date: Wed, 22 Feb 2017 10:28:26 -0800 Subject: [PATCH 09/44] s/erik/eric --- tools/grpcz/BUILD | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/grpcz/BUILD b/tools/grpcz/BUILD index 34bd072d004..4ae12d50064 100644 --- a/tools/grpcz/BUILD +++ b/tools/grpcz/BUILD @@ -36,7 +36,7 @@ load("//:bazel/generate_cc.bzl", "generate_cc") proto_library ( name = "monitoring_proto_local_copy", srcs = [ - # TODO (erikgribkoff) : remove the local copies of these protos + # TODO (ericgribkoff) : remove the local copies of these protos "monitoring.proto", "empty.proto", "any.proto", From 4be62589004152e7575ee1808fd4f52d9134296c Mon Sep 17 00:00:00 2001 From: Makarand Dharmapurikar Date: Wed, 22 Feb 2017 14:32:41 -0800 Subject: [PATCH 10/44] updated to latest mongoose BUILD --- WORKSPACE | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/WORKSPACE b/WORKSPACE index 13786ce468c..f27159d4afd 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -70,8 +70,6 @@ local_repository( # used for tools/grpcz/grpcz_client git_repository( name = "mongoose_repo", - commit = "21b9ddd490783e3afaa0fa9b45d6c1133eb922dc", + commit = "4120a97945b41195a6223a600dae8e3b19bed19e", remote = "https://github.com/makdharma/mongoose.git" ) - - From 38c10bd6d94a77a34b045ddb6511969c175248f7 Mon Sep 17 00:00:00 2001 From: Yuchen Zeng Date: Wed, 1 Mar 2017 23:37:07 -0800 Subject: [PATCH 11/44] Avoid variable length arrays --- test/core/iomgr/ev_epoll_linux_test.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/test/core/iomgr/ev_epoll_linux_test.c b/test/core/iomgr/ev_epoll_linux_test.c index 4ec959995b2..867835aa1c6 100644 --- a/test/core/iomgr/ev_epoll_linux_test.c +++ b/test/core/iomgr/ev_epoll_linux_test.c @@ -144,10 +144,10 @@ static void test_pollset_queue_merge_items() { const int num_fds = 2; const int num_pollsets = 2; const int num_closures = 4; - test_fd tfds[num_fds]; - int fds[num_fds]; - test_pollset pollsets[num_pollsets]; - grpc_closure closures[num_closures]; + test_fd tfds[2 /* num_fds */]; + int fds[2 /* num_fds */]; + test_pollset pollsets[2 /* num_pollsets */]; + grpc_closure closures[4 /* num_closures */]; int i; int result = 0; @@ -217,9 +217,9 @@ static void test_add_fd_to_pollset() { grpc_exec_ctx exec_ctx = GRPC_EXEC_CTX_INIT; const int num_fds = 8; const int num_pollsets = 4; - test_fd tfds[num_fds]; - int fds[num_fds]; - test_pollset pollsets[num_pollsets]; + test_fd tfds[8 /* num_fds */]; + int fds[8 /* num_fds */]; + test_pollset pollsets[4 /* num_pollsets */]; void *expected_pi = NULL; int i; From 5ede0e07a792627fb129522254937bfc4b3a73db Mon Sep 17 00:00:00 2001 From: Yuchen Zeng Date: Thu, 2 Mar 2017 15:49:09 -0800 Subject: [PATCH 12/44] Use macros instead of const variables --- test/core/iomgr/ev_epoll_linux_test.c | 63 ++++++++++++++++----------- 1 file changed, 37 insertions(+), 26 deletions(-) diff --git a/test/core/iomgr/ev_epoll_linux_test.c b/test/core/iomgr/ev_epoll_linux_test.c index 867835aa1c6..ba6362e3954 100644 --- a/test/core/iomgr/ev_epoll_linux_test.c +++ b/test/core/iomgr/ev_epoll_linux_test.c @@ -139,23 +139,25 @@ static void increment(grpc_exec_ctx *exec_ctx, void *arg, grpc_error *error) { * polling_island_merge()[ev_epoll_linux.c], where the parent relationship was * inverted. */ + +#define NUM_FDS 2 +#define NUM_POLLSETS 2 +#define NUM_CLOSURES 4 + static void test_pollset_queue_merge_items() { grpc_exec_ctx exec_ctx = GRPC_EXEC_CTX_INIT; - const int num_fds = 2; - const int num_pollsets = 2; - const int num_closures = 4; - test_fd tfds[2 /* num_fds */]; - int fds[2 /* num_fds */]; - test_pollset pollsets[2 /* num_pollsets */]; - grpc_closure closures[4 /* num_closures */]; + test_fd tfds[NUM_FDS]; + int fds[NUM_FDS]; + test_pollset pollsets[NUM_POLLSETS]; + grpc_closure closures[NUM_CLOSURES]; int i; int result = 0; - test_fd_init(tfds, fds, num_fds); - test_pollset_init(pollsets, num_pollsets); + test_fd_init(tfds, fds, NUM_FDS); + test_pollset_init(pollsets, NUM_POLLSETS); /* Two distinct polling islands, each with their own FD and pollset. */ - for (i = 0; i < num_fds; i++) { + for (i = 0; i < NUM_FDS; i++) { grpc_pollset_add_fd(&exec_ctx, pollsets[i].pollset, tfds[i].fd); grpc_exec_ctx_flush(&exec_ctx); } @@ -173,7 +175,7 @@ static void test_pollset_queue_merge_items() { grpc_closure_init( closures + 3, increment, &result, grpc_workqueue_scheduler(grpc_fd_get_polling_island(tfds[1].fd))); - for (i = 0; i < num_closures; ++i) { + for (i = 0; i < NUM_CLOSURES; ++i) { grpc_closure_sched(&exec_ctx, closures + i, GRPC_ERROR_NONE); } @@ -186,7 +188,7 @@ static void test_pollset_queue_merge_items() { * the merged polling island. */ grpc_pollset_worker *worker = NULL; - for (i = 0; i < num_closures; ++i) { + for (i = 0; i < NUM_CLOSURES; ++i) { const gpr_timespec deadline = gpr_time_add( gpr_now(GPR_CLOCK_MONOTONIC), gpr_time_from_seconds(2, GPR_TIMESPAN)); gpr_mu_lock(pollsets[1].mu); @@ -196,13 +198,17 @@ static void test_pollset_queue_merge_items() { gpr_now(GPR_CLOCK_MONOTONIC), deadline)); gpr_mu_unlock(pollsets[1].mu); } - GPR_ASSERT(result == num_closures); + GPR_ASSERT(result == NUM_CLOSURES); - test_fd_cleanup(&exec_ctx, tfds, num_fds); - test_pollset_cleanup(&exec_ctx, pollsets, num_pollsets); + test_fd_cleanup(&exec_ctx, tfds, NUM_FDS); + test_pollset_cleanup(&exec_ctx, pollsets, NUM_POLLSETS); grpc_exec_ctx_finish(&exec_ctx); } +#undef NUM_FDS +#undef NUM_POLLSETS +#undef NUM_CLOSURES + /* * Cases to test: * case 1) Polling islands of both fd and pollset are NULL @@ -213,18 +219,20 @@ static void test_pollset_queue_merge_items() { * case 4.2) Polling islands of fd and pollset are NOT-equal (This results * in a merge) * */ + +#define NUM_FDS 8 +#define NUM_POLLSETS 4 + static void test_add_fd_to_pollset() { grpc_exec_ctx exec_ctx = GRPC_EXEC_CTX_INIT; - const int num_fds = 8; - const int num_pollsets = 4; - test_fd tfds[8 /* num_fds */]; - int fds[8 /* num_fds */]; - test_pollset pollsets[4 /* num_pollsets */]; + test_fd tfds[NUM_FDS]; + int fds[NUM_FDS]; + test_pollset pollsets[NUM_POLLSETS]; void *expected_pi = NULL; int i; - test_fd_init(tfds, fds, num_fds); - test_pollset_init(pollsets, num_pollsets); + test_fd_init(tfds, fds, NUM_FDS); + test_pollset_init(pollsets, NUM_POLLSETS); /*Step 1. * Create three polling islands (This will exercise test case 1 and 2) with @@ -285,22 +293,25 @@ static void test_add_fd_to_pollset() { /* Compare Fd:0's polling island with that of all other Fds */ expected_pi = grpc_fd_get_polling_island(tfds[0].fd); - for (i = 1; i < num_fds; i++) { + for (i = 1; i < NUM_FDS; i++) { GPR_ASSERT(grpc_are_polling_islands_equal( expected_pi, grpc_fd_get_polling_island(tfds[i].fd))); } /* Compare Fd:0's polling island with that of all other pollsets */ - for (i = 0; i < num_pollsets; i++) { + for (i = 0; i < NUM_POLLSETS; i++) { GPR_ASSERT(grpc_are_polling_islands_equal( expected_pi, grpc_pollset_get_polling_island(pollsets[i].pollset))); } - test_fd_cleanup(&exec_ctx, tfds, num_fds); - test_pollset_cleanup(&exec_ctx, pollsets, num_pollsets); + test_fd_cleanup(&exec_ctx, tfds, NUM_FDS); + test_pollset_cleanup(&exec_ctx, pollsets, NUM_POLLSETS); grpc_exec_ctx_finish(&exec_ctx); } +#undef NUM_FDS +#undef NUM_POLLSETS + int main(int argc, char **argv) { const char *poll_strategy = NULL; grpc_test_init(argc, argv); From 1fb35b5348fa737cca189ef773a7909ef62d6469 Mon Sep 17 00:00:00 2001 From: Muxi Yan Date: Tue, 7 Mar 2017 17:42:25 -0800 Subject: [PATCH 13/44] Add comments to packet coalescing test --- src/objective-c/tests/InteropTests.m | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/objective-c/tests/InteropTests.m b/src/objective-c/tests/InteropTests.m index 4e5a5287c2f..c9941fd37de 100644 --- a/src/objective-c/tests/InteropTests.m +++ b/src/objective-c/tests/InteropTests.m @@ -164,11 +164,15 @@ expectedResponse.payload.body = [NSMutableData dataWithLength:10]; XCTAssertEqualObjects(response, expectedResponse); + // The test is a success if there is a batch of exactly 3 ops (SEND_INITIAL_METADATA, + // SEND_MESSAGE, SEND_CLOSE_FROM_CLIENT). Without packet coalescing each batch of ops contains + // only one op. NSArray *opBatches = [GRPCCall obtainAndCleanOpBatchLog]; + const NSInteger kExpectedOpBatchSize = 3; for (NSObject *o in opBatches) { if ([o isKindOfClass:[NSArray class]]) { NSArray *batch = (NSArray *)o; - if ([batch count] == 3) { + if ([batch count] == kExpectedOpBatchSize) { [expectation fulfill]; break; } From 7e49365f920a874b5133cb05884076b0f556e7ac Mon Sep 17 00:00:00 2001 From: Muxi Yan Date: Tue, 7 Mar 2017 17:47:02 -0800 Subject: [PATCH 14/44] Remove Cronet test cherry-pick --- .../InteropTestsRemoteWithCronet.m | 8 -------- 1 file changed, 8 deletions(-) diff --git a/src/objective-c/tests/InteropTestsRemoteWithCronet/InteropTestsRemoteWithCronet.m b/src/objective-c/tests/InteropTestsRemoteWithCronet/InteropTestsRemoteWithCronet.m index 660865abb44..a6720e41dce 100644 --- a/src/objective-c/tests/InteropTestsRemoteWithCronet/InteropTestsRemoteWithCronet.m +++ b/src/objective-c/tests/InteropTestsRemoteWithCronet/InteropTestsRemoteWithCronet.m @@ -47,14 +47,6 @@ static NSString * const kRemoteSSLHost = @"grpc-test.sandbox.googleapis.com"; @implementation InteropTestsRemoteWithCronet -+ (void)setUp { - // Cronet setup - [Cronet setHttp2Enabled:YES]; - [Cronet start]; - [GRPCCall useCronetWithEngine:[Cronet getGlobalEngine]]; - [Cronet startNetLogToFile:@"Documents/cronet_netlog.json" logBytes:YES]; -} - + (NSString *)host { return kRemoteSSLHost; } From 43730581800577cf0e02b66677fe73841bdbecfe Mon Sep 17 00:00:00 2001 From: Muxi Yan Date: Tue, 7 Mar 2017 17:49:13 -0800 Subject: [PATCH 15/44] Add comment to logging operations batch --- src/objective-c/GRPCClient/private/GRPCWrappedCall.m | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/objective-c/GRPCClient/private/GRPCWrappedCall.m b/src/objective-c/GRPCClient/private/GRPCWrappedCall.m index 4476a41bd97..46e9fee7e1f 100644 --- a/src/objective-c/GRPCClient/private/GRPCWrappedCall.m +++ b/src/objective-c/GRPCClient/private/GRPCWrappedCall.m @@ -276,6 +276,8 @@ } - (void)startBatchWithOperations:(NSArray *)operations errorHandler:(void (^)())errorHandler { + // Keep logs of op batches when we are running tests. Disabled when in production for improved + // performance. #ifdef GRPC_TEST_OBJC [GRPCOpBatchLog addOpBatchToLog:operations]; #endif From 016d1085fc950023f232c4a8f93723fd591c6baa Mon Sep 17 00:00:00 2001 From: Muxi Yan Date: Tue, 7 Mar 2017 18:26:42 -0800 Subject: [PATCH 16/44] Eliminate magic number 6 --- src/objective-c/GRPCClient/GRPCCall.m | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/objective-c/GRPCClient/GRPCCall.m b/src/objective-c/GRPCClient/GRPCCall.m index 1a8fc2e2ef6..1cda827e782 100644 --- a/src/objective-c/GRPCClient/GRPCCall.m +++ b/src/objective-c/GRPCClient/GRPCCall.m @@ -46,6 +46,11 @@ #import "private/NSDictionary+GRPC.h" #import "private/NSError+GRPC.h" +// At most 6 ops can be in an op batch for a client: SEND_INITIAL_METADATA, +// SEND_MESSAGE, SEND_CLOSE_FROM_CLIENT, RECV_INITIAL_METADATA, RECV_MESSAGE, +// and RECV_STATUS_ON_CLIENT. +NSInteger kMaxClientBatch = 6; + NSString * const kGRPCHeadersKey = @"io.grpc.HeadersKey"; NSString * const kGRPCTrailersKey = @"io.grpc.TrailersKey"; static NSMutableDictionary *callFlags; @@ -165,7 +170,7 @@ static NSMutableDictionary *callFlags; if ([requestWriter isKindOfClass:[GRXImmediateSingleWriter class]]) { _unaryCall = YES; - _unaryOpBatch = [NSMutableArray arrayWithCapacity:6]; + _unaryOpBatch = [NSMutableArray arrayWithCapacity:kMaxClientBatch]; } } return self; From d5bac0d38dfba3d9efec45ecbdf9c45678be13f9 Mon Sep 17 00:00:00 2001 From: Muxi Yan Date: Tue, 7 Mar 2017 18:44:19 -0800 Subject: [PATCH 17/44] Add comment and fix a trivial --- src/objective-c/GRPCClient/GRPCCall.m | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/objective-c/GRPCClient/GRPCCall.m b/src/objective-c/GRPCClient/GRPCCall.m index 1cda827e782..da861f6fca4 100644 --- a/src/objective-c/GRPCClient/GRPCCall.m +++ b/src/objective-c/GRPCClient/GRPCCall.m @@ -274,7 +274,7 @@ static NSMutableDictionary *callFlags; // TODO(jcanizales): Add error handlers for async failures GRPCOpSendMetadata *op = [[GRPCOpSendMetadata alloc] initWithMetadata:headers flags:[GRPCCall callFlagsForHost:_host path:_path] - handler:nil]; + handler:nil]; // No clean-up needed after SEND_INITIAL_METADATA if (!_unaryCall) { [_wrappedCall startBatchWithOperations:@[op]]; } else { @@ -331,7 +331,7 @@ static NSMutableDictionary *callFlags; // Only called from the call queue. The error handler will be called from the // network queue if the requests stream couldn't be closed successfully. - (void)finishRequestWithErrorHandler:(void (^)())errorHandler { - if (!_unaryOpBatch) { + if (!_unaryCall) { [_wrappedCall startBatchWithOperations:@[[[GRPCOpSendClose alloc] init]] errorHandler:errorHandler]; } else { From 35653011a32eace36c82eb7224faee2be15a09de Mon Sep 17 00:00:00 2001 From: Muxi Yan Date: Tue, 7 Mar 2017 18:54:24 -0800 Subject: [PATCH 18/44] Add comments --- src/objective-c/GRPCClient/GRPCCall.m | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/objective-c/GRPCClient/GRPCCall.m b/src/objective-c/GRPCClient/GRPCCall.m index da861f6fca4..ff474650122 100644 --- a/src/objective-c/GRPCClient/GRPCCall.m +++ b/src/objective-c/GRPCClient/GRPCCall.m @@ -286,6 +286,8 @@ static NSMutableDictionary *callFlags; // Only called from the call queue. The error handler will be called from the // network queue if the write didn't succeed. +// If the call is a unary call, parameter \a errorHandler will be ignored and +// the error handler of GRPCOpSendClose will be executed in case of error. - (void)writeMessage:(NSData *)message withErrorHandler:(void (^)())errorHandler { __weak GRPCCall *weakSelf = self; From 33c7e8b87899f7e0241e79981b406458a114e841 Mon Sep 17 00:00:00 2001 From: Muxi Yan Date: Tue, 7 Mar 2017 19:00:13 -0800 Subject: [PATCH 19/44] remove redundant if statement --- src/objective-c/GRPCClient/private/GRPCOpBatchLog.m | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/objective-c/GRPCClient/private/GRPCOpBatchLog.m b/src/objective-c/GRPCClient/private/GRPCOpBatchLog.m index e5e7100ddf0..4699b8e6b3e 100644 --- a/src/objective-c/GRPCClient/private/GRPCOpBatchLog.m +++ b/src/objective-c/GRPCClient/private/GRPCOpBatchLog.m @@ -53,9 +53,7 @@ NSMutableArray *opBatchLog = nil; + (void)addOpBatchToLog:(NSArray *)batch { @synchronized (opBatchLog) { - if (opBatchLog) { - [opBatchLog addObject:batch]; - } + [opBatchLog addObject:batch]; } } From 8ce266449705c13fc6645b4e55e4f75411fb622c Mon Sep 17 00:00:00 2001 From: Muxi Yan Date: Tue, 7 Mar 2017 19:03:25 -0800 Subject: [PATCH 20/44] remove another redundant if statement --- src/objective-c/GRPCClient/private/GRPCOpBatchLog.m | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/objective-c/GRPCClient/private/GRPCOpBatchLog.m b/src/objective-c/GRPCClient/private/GRPCOpBatchLog.m index 4699b8e6b3e..c172c3c988c 100644 --- a/src/objective-c/GRPCClient/private/GRPCOpBatchLog.m +++ b/src/objective-c/GRPCClient/private/GRPCOpBatchLog.m @@ -59,13 +59,9 @@ NSMutableArray *opBatchLog = nil; + (NSArray *)obtainAndCleanOpBatchLog { @synchronized (opBatchLog) { - if (opBatchLog) { - NSArray *out = opBatchLog; - opBatchLog = [NSMutableArray array]; - return out; - } else { - return nil; - } + NSArray *out = opBatchLog; + opBatchLog = [NSMutableArray array]; + return out; } } From 34894e4b1c46697a5b484d5f6b7734ea8795d149 Mon Sep 17 00:00:00 2001 From: Yuxuan Li Date: Wed, 8 Feb 2017 17:57:24 -0800 Subject: [PATCH 21/44] Implemented stream coalescing design (RFC:https://github.com/grpc/proposal/pull/3). Add necessary microbenchmarks to show reduced writes/iter for short streams. Add necessary end2end test to test out newly added apis. Filter those tests using *WithCoalescingApi*. --- include/grpc++/impl/codegen/async_stream.h | 205 ++++++++++- include/grpc++/impl/codegen/call.h | 46 ++- include/grpc++/impl/codegen/client_context.h | 13 +- include/grpc++/impl/codegen/sync_stream.h | 106 ++++-- include/grpc/impl/codegen/grpc_types.h | 19 +- .../chttp2/transport/chttp2_transport.c | 37 +- src/cpp/client/client_context.cc | 7 +- test/cpp/end2end/async_end2end_test.cc | 319 ++++++++++++++++++ test/cpp/end2end/end2end_test.cc | 85 +++++ test/cpp/end2end/test_service_impl.cc | 22 +- test/cpp/end2end/test_service_impl.h | 2 + test/cpp/microbenchmarks/bm_fullstack.cc | 203 ++++++++++- 12 files changed, 989 insertions(+), 75 deletions(-) diff --git a/include/grpc++/impl/codegen/async_stream.h b/include/grpc++/impl/codegen/async_stream.h index 1a5cbbd45da..f21139618f5 100644 --- a/include/grpc++/impl/codegen/async_stream.h +++ b/include/grpc++/impl/codegen/async_stream.h @@ -101,6 +101,39 @@ class AsyncWriterInterface { /// \param[in] msg The message to be written. /// \param[in] tag The tag identifying the operation. virtual void Write(const W& msg, void* tag) = 0; + + /// Request the writing of \a msg using WriteOptions \a options with + /// identifying tag \a tag. + /// + /// Only one write may be outstanding at any given time. This means that + /// after calling Write, one must wait to receive \a tag from the completion + /// queue BEFORE calling Write again. + /// WriteOptions \a options is used to set the write options of this message. + /// This is thread-safe with respect to \a Read + /// + /// \param[in] msg The message to be written. + /// \param[in] options The WriteOptions to be used to write this message. + /// \param[in] tag The tag identifying the operation. + virtual void Write(const W& msg, WriteOptions options, void* tag) = 0; + + /// Request the writing of \a msg and coalesce it with the writing + /// of trailing metadata, using WriteOptions \a options with identifying tag + /// \a tag. + /// + /// For client, WriteLast is equivalent of performing Write and WritesDone in + /// a single step. + /// For server, WriteLast buffers the \a msg. The writing of \a msg is held + /// until Finish is called, where \a msg and trailing metadata are coalesced + /// and write is initiated. Note that WriteLast can only buffer \a msg up to + /// the flow control window size. If \a msg size is larger than the window + /// size, it will be sent on wire without buffering. + /// + /// \param[in] msg The message to be written. + /// \param[in] options The WriteOptions to be used to write this message. + /// \param[in] tag The tag identifying the operation. + void WriteLast(const W& msg, WriteOptions options, void* tag) { + Write(msg, options.set_last_message(), tag); + } }; template @@ -183,11 +216,17 @@ class ClientAsyncWriter final : public ClientAsyncWriterInterface { : context_(context), call_(channel->CreateCall(method, context, cq)) { finish_ops_.RecvMessage(response); finish_ops_.AllowNoMessage(); - - init_ops_.set_output_tag(tag); - init_ops_.SendInitialMetadata(context->send_initial_metadata_, - context->initial_metadata_flags()); - call_.PerformOps(&init_ops_); + // if corked bit is set in context, we buffer up the initial metadata to + // coalesce with later message to be sent. No op is performed. + if (context_->initial_metadata_corked_) { + write_ops_.SendInitialMetadata(context->send_initial_metadata_, + context->initial_metadata_flags()); + } else { + init_ops_.set_output_tag(tag); + init_ops_.SendInitialMetadata(context->send_initial_metadata_, + context->initial_metadata_flags()); + call_.PerformOps(&init_ops_); + } } void ReadInitialMetadata(void* tag) override { @@ -205,6 +244,17 @@ class ClientAsyncWriter final : public ClientAsyncWriterInterface { call_.PerformOps(&write_ops_); } + void Write(const W& msg, WriteOptions options, void* tag) { + write_ops_.set_output_tag(tag); + if (options.is_last_message()) { + options.set_buffer_hint(); + write_ops_.ClientSendClose(); + } + // TODO(ctiller): don't assert + GPR_CODEGEN_ASSERT(write_ops_.SendMessage(msg, options).ok()); + call_.PerformOps(&write_ops_); + } + void WritesDone(void* tag) override { writes_done_ops_.set_output_tag(tag); writes_done_ops_.ClientSendClose(); @@ -225,7 +275,8 @@ class ClientAsyncWriter final : public ClientAsyncWriterInterface { Call call_; CallOpSet init_ops_; CallOpSet meta_ops_; - CallOpSet write_ops_; + CallOpSet + write_ops_; CallOpSet writes_done_ops_; CallOpSet @@ -253,10 +304,17 @@ class ClientAsyncReaderWriter final const RpcMethod& method, ClientContext* context, void* tag) : context_(context), call_(channel->CreateCall(method, context, cq)) { - init_ops_.set_output_tag(tag); - init_ops_.SendInitialMetadata(context->send_initial_metadata_, - context->initial_metadata_flags()); - call_.PerformOps(&init_ops_); + if (context_->initial_metadata_corked_) { + // if corked bit is set in context, we buffer up the initial metadata to + // coalesce with later message to be sent. No op is performed. + write_ops_.SendInitialMetadata(context->send_initial_metadata_, + context->initial_metadata_flags()); + } else { + init_ops_.set_output_tag(tag); + init_ops_.SendInitialMetadata(context->send_initial_metadata_, + context->initial_metadata_flags()); + call_.PerformOps(&init_ops_); + } } void ReadInitialMetadata(void* tag) override { @@ -283,10 +341,21 @@ class ClientAsyncReaderWriter final call_.PerformOps(&write_ops_); } + void Write(const W& msg, WriteOptions options, void* tag) { + write_ops_.set_output_tag(tag); + if (options.is_last_message()) { + options.set_buffer_hint(); + write_ops_.ClientSendClose(); + } + // TODO(ctiller): don't assert + GPR_CODEGEN_ASSERT(write_ops_.SendMessage(msg, options).ok()); + call_.PerformOps(&write_ops_); + } + void WritesDone(void* tag) override { - writes_done_ops_.set_output_tag(tag); - writes_done_ops_.ClientSendClose(); - call_.PerformOps(&writes_done_ops_); + write_ops_.set_output_tag(tag); + write_ops_.ClientSendClose(); + call_.PerformOps(&write_ops_); } void Finish(Status* status, void* tag) override { @@ -304,8 +373,8 @@ class ClientAsyncReaderWriter final CallOpSet init_ops_; CallOpSet meta_ops_; CallOpSet> read_ops_; - CallOpSet write_ops_; - CallOpSet writes_done_ops_; + CallOpSet + write_ops_; CallOpSet finish_ops_; }; @@ -395,6 +464,20 @@ class ServerAsyncWriterInterface : public ServerAsyncStreamingInterface, public AsyncWriterInterface { public: virtual void Finish(const Status& status, void* tag) = 0; + + /// Request the writing of \a msg and coalesce it with trailing metadata which + /// contains \a status, using WriteOptions options with identifying tag \a + /// tag. + /// + /// WriteAndFinish is equivalent of performing WriteLast and Finish in a + /// single step. + /// + /// \param[in] msg The message to be written. + /// \param[in] options The WriteOptions to be used to write this message. + /// \param[in] status The Status that server returns to client. + /// \param[in] tag The tag identifying the operation. + virtual void WriteAndFinish(const W& msg, WriteOptions options, + const Status& status, void* tag) = 0; }; template @@ -431,6 +514,42 @@ class ServerAsyncWriter final : public ServerAsyncWriterInterface { call_.PerformOps(&write_ops_); } + void Write(const W& msg, WriteOptions options, void* tag) override { + write_ops_.set_output_tag(tag); + if (options.is_last_message()) { + options.set_buffer_hint(); + } + + if (!ctx_->sent_initial_metadata_) { + write_ops_.SendInitialMetadata(ctx_->initial_metadata_, + ctx_->initial_metadata_flags()); + if (ctx_->compression_level_set()) { + write_ops_.set_compression_level(ctx_->compression_level()); + } + ctx_->sent_initial_metadata_ = true; + } + // TODO(ctiller): don't assert + GPR_CODEGEN_ASSERT(write_ops_.SendMessage(msg, options).ok()); + call_.PerformOps(&write_ops_); + } + + void WriteAndFinish(const W& msg, WriteOptions options, const Status& status, + void* tag) override { + write_ops_.set_output_tag(tag); + if (!ctx_->sent_initial_metadata_) { + write_ops_.SendInitialMetadata(ctx_->initial_metadata_, + ctx_->initial_metadata_flags()); + if (ctx_->compression_level_set()) { + write_ops_.set_compression_level(ctx_->compression_level()); + } + ctx_->sent_initial_metadata_ = true; + } + options.set_buffer_hint(); + GPR_CODEGEN_ASSERT(write_ops_.SendMessage(msg, options).ok()); + write_ops_.ServerSendStatus(ctx_->trailing_metadata_, status); + call_.PerformOps(&write_ops_); + } + void Finish(const Status& status, void* tag) override { finish_ops_.set_output_tag(tag); if (!ctx_->sent_initial_metadata_) { @@ -451,7 +570,9 @@ class ServerAsyncWriter final : public ServerAsyncWriterInterface { Call call_; ServerContext* ctx_; CallOpSet meta_ops_; - CallOpSet write_ops_; + CallOpSet + write_ops_; CallOpSet finish_ops_; }; @@ -462,6 +583,20 @@ class ServerAsyncReaderWriterInterface : public ServerAsyncStreamingInterface, public AsyncReaderInterface { public: virtual void Finish(const Status& status, void* tag) = 0; + + /// Request the writing of \a msg and coalesce it with trailing metadata which + /// contains \a status, using WriteOptions options with identifying tag \a + /// tag. + /// + /// WriteAndFinish is equivalent of performing WriteLast and Finish in a + /// single step. + /// + /// \param[in] msg The message to be written. + /// \param[in] options The WriteOptions to be used to write this message. + /// \param[in] status The Status that server returns to client. + /// \param[in] tag The tag identifying the operation. + virtual void WriteAndFinish(const W& msg, WriteOptions options, + const Status& status, void* tag) = 0; }; template @@ -505,6 +640,40 @@ class ServerAsyncReaderWriter final call_.PerformOps(&write_ops_); } + void Write(const W& msg, WriteOptions options, void* tag) override { + write_ops_.set_output_tag(tag); + if (options.is_last_message()) { + options.set_buffer_hint(); + } + if (!ctx_->sent_initial_metadata_) { + write_ops_.SendInitialMetadata(ctx_->initial_metadata_, + ctx_->initial_metadata_flags()); + if (ctx_->compression_level_set()) { + write_ops_.set_compression_level(ctx_->compression_level()); + } + ctx_->sent_initial_metadata_ = true; + } + GPR_CODEGEN_ASSERT(write_ops_.SendMessage(msg, options).ok()); + call_.PerformOps(&write_ops_); + } + + void WriteAndFinish(const W& msg, WriteOptions options, const Status& status, + void* tag) override { + write_ops_.set_output_tag(tag); + if (!ctx_->sent_initial_metadata_) { + write_ops_.SendInitialMetadata(ctx_->initial_metadata_, + ctx_->initial_metadata_flags()); + if (ctx_->compression_level_set()) { + write_ops_.set_compression_level(ctx_->compression_level()); + } + ctx_->sent_initial_metadata_ = true; + } + options.set_buffer_hint(); + GPR_CODEGEN_ASSERT(write_ops_.SendMessage(msg, options).ok()); + write_ops_.ServerSendStatus(ctx_->trailing_metadata_, status); + call_.PerformOps(&write_ops_); + } + void Finish(const Status& status, void* tag) override { finish_ops_.set_output_tag(tag); if (!ctx_->sent_initial_metadata_) { @@ -528,7 +697,9 @@ class ServerAsyncReaderWriter final ServerContext* ctx_; CallOpSet meta_ops_; CallOpSet> read_ops_; - CallOpSet write_ops_; + CallOpSet + write_ops_; CallOpSet finish_ops_; }; diff --git a/include/grpc++/impl/codegen/call.h b/include/grpc++/impl/codegen/call.h index 19a5ca2b2ee..f6638471d24 100644 --- a/include/grpc++/impl/codegen/call.h +++ b/include/grpc++/impl/codegen/call.h @@ -141,6 +141,47 @@ class WriteOptions { /// \sa GRPC_WRITE_BUFFER_HINT inline bool get_buffer_hint() const { return GetBit(GRPC_WRITE_BUFFER_HINT); } + /// corked bit: aliases set_buffer_hint currently, with the intent that + /// set_buffer_hint will be removed in the future + inline WriteOptions& set_corked() { + SetBit(GRPC_WRITE_BUFFER_HINT); + return *this; + } + + inline WriteOptions& clear_corked() { + ClearBit(GRPC_WRITE_BUFFER_HINT); + return *this; + } + + inline bool is_corked() const { return GetBit(GRPC_WRITE_BUFFER_HINT); } + + /// last-message bit: indicates this is the last message in a stream + /// client-side: makes Write the equivalent of performing Write, WritesDone + /// in a single step + /// server-side: hold the Write until the service handler returns (sync api) + /// or until Finish is called (async api) + /// + /// \sa GRPC_WRITE_LAST_MESSAGE + inline WriteOptions& set_last_message() { + SetBit(GRPC_WRITE_LAST_MESSAGE); + return *this; + } + + /// Clears flag indicating that this is the last message in a stream, + /// disabling coalescing. + /// + /// \sa GRPC_WRITE_LAST_MESSAGE + inline WriteOptions& clear_last_messsage() { + ClearBit(GRPC_WRITE_LAST_MESSAGE); + return *this; + } + + /// Get value for the flag indicating that this is the last message, and + /// should be coalesced with trailing metadata. + /// + /// \sa GRPC_WRITE_LAST_MESSAGE + bool is_last_message() const { return GetBit(GRPC_WRITE_LAST_MESSAGE); } + WriteOptions& operator=(const WriteOptions& rhs) { flags_ = rhs.flags_; return *this; @@ -224,7 +265,7 @@ class CallOpSendMessage { /// after use. template Status SendMessage(const M& message, - const WriteOptions& options) GRPC_MUST_USE_RESULT; + WriteOptions options) GRPC_MUST_USE_RESULT; template Status SendMessage(const M& message) GRPC_MUST_USE_RESULT; @@ -252,8 +293,7 @@ class CallOpSendMessage { }; template -Status CallOpSendMessage::SendMessage(const M& message, - const WriteOptions& options) { +Status CallOpSendMessage::SendMessage(const M& message, WriteOptions options) { write_options_ = options; return SerializationTraits::Serialize(message, &send_buf_, &own_buf_); } diff --git a/include/grpc++/impl/codegen/client_context.h b/include/grpc++/impl/codegen/client_context.h index b91c7f65d43..cc48b9c3f57 100644 --- a/include/grpc++/impl/codegen/client_context.h +++ b/include/grpc++/impl/codegen/client_context.h @@ -281,6 +281,15 @@ class ClientContext { /// \param algorithm The compression algorithm used for the client call. void set_compression_algorithm(grpc_compression_algorithm algorithm); + /// Flag whether the initial metadata should be \a corked + /// + /// If \a corked is true, then the initial metadata will be colasced with the + /// write of first message in the stream. + /// + /// \param corked The flag indicating whether the initial metadata is to be + /// corked or not. + void sent_initial_metadata_corked(bool corked); + /// Return the peer uri in a string. /// /// \warning This value is never authenticated or subject to any security @@ -357,7 +366,8 @@ class ClientContext { (cacheable_ ? GRPC_INITIAL_METADATA_CACHEABLE_REQUEST : 0) | (wait_for_ready_explicitly_set_ ? GRPC_INITIAL_METADATA_WAIT_FOR_READY_EXPLICITLY_SET - : 0); + : 0) | + (initial_metadata_corked_ ? GRPC_INITIAL_METADATA_CORKED : 0); } grpc::string authority() { return authority_; } @@ -384,6 +394,7 @@ class ClientContext { PropagationOptions propagation_options_; grpc_compression_algorithm compression_algorithm_; + bool initial_metadata_corked_; }; } // namespace grpc diff --git a/include/grpc++/impl/codegen/sync_stream.h b/include/grpc++/impl/codegen/sync_stream.h index 4d9b074e95f..859c3d0b905 100644 --- a/include/grpc++/impl/codegen/sync_stream.h +++ b/include/grpc++/impl/codegen/sync_stream.h @@ -100,22 +100,40 @@ class WriterInterface { public: virtual ~WriterInterface() {} - /// Blocking write \a msg to the stream with options. + /// Blocking write \a msg to the stream with WriteOptions \a options. /// This is thread-safe with respect to \a Read /// /// \param msg The message to be written to the stream. - /// \param options Options affecting the write operation. + /// \param options The WriteOptions affecting the write operation. /// /// \return \a true on success, \a false when the stream has been closed. - virtual bool Write(const W& msg, const WriteOptions& options) = 0; + virtual bool Write(const W& msg, WriteOptions options) = 0; - /// Blocking write \a msg to the stream with default options. + /// Blocking write \a msg to the stream with default write options. /// This is thread-safe with respect to \a Read /// /// \param msg The message to be written to the stream. /// /// \return \a true on success, \a false when the stream has been closed. inline bool Write(const W& msg) { return Write(msg, WriteOptions()); } + + /// Write \a msg and coalesce it with the writing of trailing metadata, using + /// WriteOptions \a options. + /// + /// For client, WriteLast is equivalent of performing Write and WritesDone in + /// a single step. \a msg and trailing metadata are coalesced and sent on wire + /// by calling this function. + /// For server, WriteLast buffers the \a msg. The writing of \a msg is held + /// until the service handler returns, where \a msg and trailing metadata are + /// coalesced and sent on wire. Note that WriteLast can only buffer \a msg up + /// to the flow control window size. If \a msg size is larger than the window + /// size, it will be sent on wire without buffering. + /// + /// \param[in] msg The message to be written to the stream. + /// \param[in] options The WriteOptions to be used to write this message. + void WriteLast(const W& msg, WriteOptions options) { + Write(msg, options.set_last_message()); + } }; /// Client-side interface for streaming reads of message of type \a R. @@ -213,11 +231,13 @@ class ClientWriter : public ClientWriterInterface { finish_ops_.RecvMessage(response); finish_ops_.AllowNoMessage(); - CallOpSet ops; - ops.SendInitialMetadata(context->send_initial_metadata_, - context->initial_metadata_flags()); - call_.PerformOps(&ops); - cq_.Pluck(&ops); + if (!context_->initial_metadata_corked_) { + CallOpSet ops; + ops.SendInitialMetadata(context->send_initial_metadata_, + context->initial_metadata_flags()); + call_.PerformOps(&ops); + cq_.Pluck(&ops); + } } void WaitForInitialMetadata() { @@ -230,11 +250,24 @@ class ClientWriter : public ClientWriterInterface { } using WriterInterface::Write; - bool Write(const W& msg, const WriteOptions& options) override { - CallOpSet ops; + bool Write(const W& msg, WriteOptions options) override { + CallOpSet + ops; + + if (options.is_last_message()) { + options.set_buffer_hint(); + ops.ClientSendClose(); + } + if (context_->initial_metadata_corked_) { + ops.SendInitialMetadata(context_->send_initial_metadata_, + context_->initial_metadata_flags()); + context_->sent_initial_metadata_corked(false); + } if (!ops.SendMessage(msg, options).ok()) { return false; } + call_.PerformOps(&ops); return cq_.Pluck(&ops); } @@ -293,11 +326,13 @@ class ClientReaderWriter final : public ClientReaderWriterInterface { ClientReaderWriter(ChannelInterface* channel, const RpcMethod& method, ClientContext* context) : context_(context), call_(channel->CreateCall(method, context, &cq_)) { - CallOpSet ops; - ops.SendInitialMetadata(context->send_initial_metadata_, - context->initial_metadata_flags()); - call_.PerformOps(&ops); - cq_.Pluck(&ops); + if (!context_->initial_metadata_corked_) { + CallOpSet ops; + ops.SendInitialMetadata(context->send_initial_metadata_, + context->initial_metadata_flags()); + call_.PerformOps(&ops); + cq_.Pluck(&ops); + } } void WaitForInitialMetadata() override { @@ -325,9 +360,24 @@ class ClientReaderWriter final : public ClientReaderWriterInterface { } using WriterInterface::Write; - bool Write(const W& msg, const WriteOptions& options) override { - CallOpSet ops; - if (!ops.SendMessage(msg, options).ok()) return false; + bool Write(const W& msg, WriteOptions options) override { + CallOpSet + ops; + + if (options.is_last_message()) { + options.set_buffer_hint(); + ops.ClientSendClose(); + } + if (context_->initial_metadata_corked_) { + ops.SendInitialMetadata(context_->send_initial_metadata_, + context_->initial_metadata_flags()); + context_->sent_initial_metadata_corked(false); + } + if (!ops.SendMessage(msg, options).ok()) { + return false; + } + call_.PerformOps(&ops); return cq_.Pluck(&ops); } @@ -423,7 +473,10 @@ class ServerWriter final : public ServerWriterInterface { } using WriterInterface::Write; - bool Write(const W& msg, const WriteOptions& options) override { + bool Write(const W& msg, WriteOptions options) override { + if (options.is_last_message()) { + options.set_buffer_hint(); + } CallOpSet ops; if (!ops.SendMessage(msg, options).ok()) { return false; @@ -485,7 +538,10 @@ class ServerReaderWriterBody final { return call_->cq()->Pluck(&ops) && ops.got_message; } - bool Write(const W& msg, const WriteOptions& options) { + bool Write(const W& msg, WriteOptions options) { + if (options.is_last_message()) { + options.set_buffer_hint(); + } CallOpSet ops; if (!ops.SendMessage(msg, options).ok()) { return false; @@ -523,7 +579,7 @@ class ServerReaderWriter final : public ServerReaderWriterInterface { bool Read(R* msg) override { return body_.Read(msg); } using WriterInterface::Write; - bool Write(const W& msg, const WriteOptions& options) override { + bool Write(const W& msg, WriteOptions options) override { return body_.Write(msg, options); } @@ -562,8 +618,7 @@ class ServerUnaryStreamer final } using WriterInterface::Write; - bool Write(const ResponseType& response, - const WriteOptions& options) override { + bool Write(const ResponseType& response, WriteOptions options) override { if (write_done_ || !read_done_) { return false; } @@ -604,8 +659,7 @@ class ServerSplitStreamer final } using WriterInterface::Write; - bool Write(const ResponseType& response, - const WriteOptions& options) override { + bool Write(const ResponseType& response, WriteOptions options) override { return read_done_ && body_.Write(response, options); } diff --git a/include/grpc/impl/codegen/grpc_types.h b/include/grpc/impl/codegen/grpc_types.h index e5c731304cd..8e6f62a6fe5 100644 --- a/include/grpc/impl/codegen/grpc_types.h +++ b/include/grpc/impl/codegen/grpc_types.h @@ -298,8 +298,12 @@ typedef enum grpc_call_error { /** Force compression to be disabled for a particular write (start_write/add_metadata). Illegal on invoke/accept. */ #define GRPC_WRITE_NO_COMPRESS (0x00000002u) +/** Force coalescing of last streaming message and trailing metadata into the + same core batch */ +#define GRPC_WRITE_LAST_MESSAGE (0x00000004u) /** Mask of all valid flags. */ -#define GRPC_WRITE_USED_MASK (GRPC_WRITE_BUFFER_HINT | GRPC_WRITE_NO_COMPRESS) +#define GRPC_WRITE_USED_MASK \ + (GRPC_WRITE_BUFFER_HINT | GRPC_WRITE_NO_COMPRESS | GRPC_WRITE_LAST_MESSAGE) /* Initial metadata flags */ /** Signal that the call is idempotent */ @@ -311,13 +315,16 @@ typedef enum grpc_call_error { /** Signal that GRPC_INITIAL_METADATA_WAIT_FOR_READY was explicitly set by the calling application. */ #define GRPC_INITIAL_METADATA_WAIT_FOR_READY_EXPLICITLY_SET (0x00000080u) +/** Signal that the initial metadata should be corked */ +#define GRPC_INITIAL_METADATA_CORKED (0x00000100u) /** Mask of all valid flags */ -#define GRPC_INITIAL_METADATA_USED_MASK \ - (GRPC_INITIAL_METADATA_IDEMPOTENT_REQUEST | \ - GRPC_INITIAL_METADATA_WAIT_FOR_READY | \ - GRPC_INITIAL_METADATA_CACHEABLE_REQUEST | \ - GRPC_INITIAL_METADATA_WAIT_FOR_READY_EXPLICITLY_SET) +#define GRPC_INITIAL_METADATA_USED_MASK \ + (GRPC_INITIAL_METADATA_IDEMPOTENT_REQUEST | \ + GRPC_INITIAL_METADATA_WAIT_FOR_READY | \ + GRPC_INITIAL_METADATA_CACHEABLE_REQUEST | \ + GRPC_INITIAL_METADATA_WAIT_FOR_READY_EXPLICITLY_SET | \ + GRPC_INITIAL_METADATA_CORKED) /** A single metadata element */ typedef struct grpc_metadata { diff --git a/src/core/ext/transport/chttp2/transport/chttp2_transport.c b/src/core/ext/transport/chttp2/transport/chttp2_transport.c index da4c7dc7b23..2917fc5caa1 100644 --- a/src/core/ext/transport/chttp2/transport/chttp2_transport.c +++ b/src/core/ext/transport/chttp2/transport/chttp2_transport.c @@ -501,10 +501,11 @@ static void destroy_transport_locked(grpc_exec_ctx *exec_ctx, void *tp, static void destroy_transport(grpc_exec_ctx *exec_ctx, grpc_transport *gt) { grpc_chttp2_transport *t = (grpc_chttp2_transport *)gt; - grpc_closure_sched(exec_ctx, grpc_closure_create( - destroy_transport_locked, t, - grpc_combiner_scheduler(t->combiner, false)), - GRPC_ERROR_NONE); + grpc_closure_sched( + exec_ctx, + grpc_closure_create(destroy_transport_locked, t, + grpc_combiner_scheduler(t->combiner, false)), + GRPC_ERROR_NONE); } static void close_transport_locked(grpc_exec_ctx *exec_ctx, @@ -676,8 +677,9 @@ static void destroy_stream(grpc_exec_ctx *exec_ctx, grpc_transport *gt, s->destroy_stream_arg = and_free_memory; grpc_closure_sched( - exec_ctx, grpc_closure_init(&s->destroy_stream, destroy_stream_locked, s, - grpc_combiner_scheduler(t->combiner, false)), + exec_ctx, + grpc_closure_init(&s->destroy_stream, destroy_stream_locked, s, + grpc_combiner_scheduler(t->combiner, false)), GRPC_ERROR_NONE); GPR_TIMER_END("destroy_stream", 0); } @@ -1206,8 +1208,13 @@ static void perform_stream_op_locked(grpc_exec_ctx *exec_ctx, void *stream_op, } } else { GPR_ASSERT(s->id != 0); - grpc_chttp2_become_writable(exec_ctx, t, s, - GRPC_CHTTP2_STREAM_WRITE_INITIATE_COVERED, + grpc_chttp2_stream_write_type write_type = + GRPC_CHTTP2_STREAM_WRITE_INITIATE_COVERED; + if (op->send_message != NULL && + (op->send_message->flags & GRPC_WRITE_BUFFER_HINT)) { + write_type = GRPC_CHTTP2_STREAM_WRITE_PIGGYBACK; + } + grpc_chttp2_become_writable(exec_ctx, t, s, write_type, "op.send_initial_metadata"); } } else { @@ -1470,9 +1477,10 @@ static void perform_transport_op(grpc_exec_ctx *exec_ctx, grpc_transport *gt, op->transport_private.args[0] = gt; GRPC_CHTTP2_REF_TRANSPORT(t, "transport_op"); grpc_closure_sched( - exec_ctx, grpc_closure_init(&op->transport_private.closure, - perform_transport_op_locked, op, - grpc_combiner_scheduler(t->combiner, false)), + exec_ctx, + grpc_closure_init(&op->transport_private.closure, + perform_transport_op_locked, op, + grpc_combiner_scheduler(t->combiner, false)), GRPC_ERROR_NONE); } @@ -2457,9 +2465,10 @@ static void destructive_reclaimer_locked(grpc_exec_ctx *exec_ctx, void *arg, s->id); } grpc_chttp2_cancel_stream( - exec_ctx, t, s, grpc_error_set_int(GRPC_ERROR_CREATE("Buffers full"), - GRPC_ERROR_INT_HTTP2_ERROR, - GRPC_HTTP2_ENHANCE_YOUR_CALM)); + exec_ctx, t, s, + grpc_error_set_int(GRPC_ERROR_CREATE("Buffers full"), + GRPC_ERROR_INT_HTTP2_ERROR, + GRPC_HTTP2_ENHANCE_YOUR_CALM)); if (n > 1) { /* Since we cancel one stream per destructive reclamation, if there are more streams left, we can immediately post a new diff --git a/src/cpp/client/client_context.cc b/src/cpp/client/client_context.cc index c073741dac0..96f97021cb7 100644 --- a/src/cpp/client/client_context.cc +++ b/src/cpp/client/client_context.cc @@ -67,7 +67,8 @@ ClientContext::ClientContext() call_canceled_(false), deadline_(gpr_inf_future(GPR_CLOCK_REALTIME)), census_context_(nullptr), - propagate_from_call_(nullptr) { + propagate_from_call_(nullptr), + initial_metadata_corked_(false) { g_client_callbacks->DefaultConstructor(this); } @@ -118,6 +119,10 @@ void ClientContext::set_compression_algorithm( AddMetadata(GRPC_COMPRESSION_REQUEST_ALGORITHM_MD_KEY, algorithm_name); } +void ClientContext::sent_initial_metadata_corked(bool corked) { + initial_metadata_corked_ = corked; +} + void ClientContext::TryCancel() { std::unique_lock lock(mu_); if (call_) { diff --git a/test/cpp/end2end/async_end2end_test.cc b/test/cpp/end2end/async_end2end_test.cc index 32e8a417958..47e803b948e 100644 --- a/test/cpp/end2end/async_end2end_test.cc +++ b/test/cpp/end2end/async_end2end_test.cc @@ -484,6 +484,81 @@ TEST_P(AsyncEnd2endTest, SimpleClientStreaming) { EXPECT_TRUE(recv_status.ok()); } +// Two pings and a final pong. +TEST_P(AsyncEnd2endTest, SimpleClientStreamingWithCoalescingApi) { + ResetStub(); + + EchoRequest send_request; + EchoRequest recv_request; + EchoResponse send_response; + EchoResponse recv_response; + Status recv_status; + ClientContext cli_ctx; + ServerContext srv_ctx; + ServerAsyncReader srv_stream(&srv_ctx); + + send_request.set_message(GetParam().message_content); + cli_ctx.sent_initial_metadata_corked(true); + // tag:1 never comes up since no op is performed + std::unique_ptr> cli_stream( + stub_->AsyncRequestStream(&cli_ctx, &recv_response, cq_.get(), tag(1))); + + service_.RequestRequestStream(&srv_ctx, &srv_stream, cq_.get(), cq_.get(), + tag(2)); + + cli_stream->Write(send_request, tag(3)); + + // 65536(64KB) is the default flow control window size. Should change this + // number when default flow control window size changes. For the write of + // send_request larger than the flow control window size, tag:3 will not come + // up until server read is initiated. For write of send_request smaller than + // the flow control window size, the request can take the free ride with + // initial metadata due to coalescing, thus write tag:3 will come up here. + if (GetParam().message_content.length() < 65536) { + Verifier(GetParam().disable_blocking) + .Expect(2, true) + .Expect(3, true) + .Verify(cq_.get()); + } else { + Verifier(GetParam().disable_blocking).Expect(2, true).Verify(cq_.get()); + } + + srv_stream.Read(&recv_request, tag(4)); + + if (GetParam().message_content.length() < 65536) { + Verifier(GetParam().disable_blocking).Expect(4, true).Verify(cq_.get()); + } else { + Verifier(GetParam().disable_blocking) + .Expect(3, true) + .Expect(4, true) + .Verify(cq_.get()); + } + + EXPECT_EQ(send_request.message(), recv_request.message()); + + cli_stream->WriteLast(send_request, WriteOptions(), tag(5)); + srv_stream.Read(&recv_request, tag(6)); + Verifier(GetParam().disable_blocking) + .Expect(5, true) + .Expect(6, true) + .Verify(cq_.get()); + EXPECT_EQ(send_request.message(), recv_request.message()); + + srv_stream.Read(&recv_request, tag(7)); + Verifier(GetParam().disable_blocking).Expect(7, false).Verify(cq_.get()); + + send_response.set_message(recv_request.message()); + srv_stream.Finish(send_response, Status::OK, tag(8)); + cli_stream->Finish(&recv_status, tag(9)); + Verifier(GetParam().disable_blocking) + .Expect(8, true) + .Expect(9, true) + .Verify(cq_.get()); + + EXPECT_EQ(send_response.message(), recv_response.message()); + EXPECT_TRUE(recv_status.ok()); +} + // One ping, two pongs. TEST_P(AsyncEnd2endTest, SimpleServerStreaming) { ResetStub(); @@ -540,6 +615,112 @@ TEST_P(AsyncEnd2endTest, SimpleServerStreaming) { EXPECT_TRUE(recv_status.ok()); } +// One ping, two pongs. Using WriteAndFinish API +TEST_P(AsyncEnd2endTest, SimpleServerStreamingWithCoalescingApiWAF) { + ResetStub(); + + EchoRequest send_request; + EchoRequest recv_request; + EchoResponse send_response; + EchoResponse recv_response; + Status recv_status; + ClientContext cli_ctx; + ServerContext srv_ctx; + ServerAsyncWriter srv_stream(&srv_ctx); + + send_request.set_message(GetParam().message_content); + std::unique_ptr> cli_stream( + stub_->AsyncResponseStream(&cli_ctx, send_request, cq_.get(), tag(1))); + + service_.RequestResponseStream(&srv_ctx, &recv_request, &srv_stream, + cq_.get(), cq_.get(), tag(2)); + + Verifier(GetParam().disable_blocking) + .Expect(1, true) + .Expect(2, true) + .Verify(cq_.get()); + EXPECT_EQ(send_request.message(), recv_request.message()); + + send_response.set_message(recv_request.message()); + srv_stream.Write(send_response, tag(3)); + cli_stream->Read(&recv_response, tag(4)); + Verifier(GetParam().disable_blocking) + .Expect(3, true) + .Expect(4, true) + .Verify(cq_.get()); + EXPECT_EQ(send_response.message(), recv_response.message()); + + srv_stream.WriteAndFinish(send_response, WriteOptions(), Status::OK, tag(5)); + cli_stream->Read(&recv_response, tag(6)); + Verifier(GetParam().disable_blocking) + .Expect(5, true) + .Expect(6, true) + .Verify(cq_.get()); + EXPECT_EQ(send_response.message(), recv_response.message()); + + cli_stream->Read(&recv_response, tag(7)); + Verifier(GetParam().disable_blocking).Expect(7, false).Verify(cq_.get()); + + cli_stream->Finish(&recv_status, tag(8)); + Verifier(GetParam().disable_blocking).Expect(8, true).Verify(cq_.get()); + + EXPECT_TRUE(recv_status.ok()); +} + +// One ping, two pongs. Using WriteLast API +TEST_P(AsyncEnd2endTest, SimpleServerStreamingWithCoalescingApiWL) { + ResetStub(); + + EchoRequest send_request; + EchoRequest recv_request; + EchoResponse send_response; + EchoResponse recv_response; + Status recv_status; + ClientContext cli_ctx; + ServerContext srv_ctx; + ServerAsyncWriter srv_stream(&srv_ctx); + + send_request.set_message(GetParam().message_content); + std::unique_ptr> cli_stream( + stub_->AsyncResponseStream(&cli_ctx, send_request, cq_.get(), tag(1))); + + service_.RequestResponseStream(&srv_ctx, &recv_request, &srv_stream, + cq_.get(), cq_.get(), tag(2)); + + Verifier(GetParam().disable_blocking) + .Expect(1, true) + .Expect(2, true) + .Verify(cq_.get()); + EXPECT_EQ(send_request.message(), recv_request.message()); + + send_response.set_message(recv_request.message()); + srv_stream.Write(send_response, tag(3)); + cli_stream->Read(&recv_response, tag(4)); + Verifier(GetParam().disable_blocking) + .Expect(3, true) + .Expect(4, true) + .Verify(cq_.get()); + EXPECT_EQ(send_response.message(), recv_response.message()); + + srv_stream.WriteLast(send_response, WriteOptions(), tag(5)); + cli_stream->Read(&recv_response, tag(6)); + srv_stream.Finish(Status::OK, tag(7)); + Verifier(GetParam().disable_blocking) + .Expect(5, true) + .Expect(6, true) + .Expect(7, true) + .Verify(cq_.get()); + EXPECT_EQ(send_response.message(), recv_response.message()); + + cli_stream->Read(&recv_response, tag(8)); + Verifier(GetParam().disable_blocking).Expect(8, false).Verify(cq_.get()); + + cli_stream->Finish(&recv_status, tag(9)); + Verifier(GetParam().disable_blocking).Expect(9, true).Verify(cq_.get()); + + EXPECT_TRUE(recv_status.ok()); +} + // One ping, one pong. TEST_P(AsyncEnd2endTest, SimpleBidiStreaming) { ResetStub(); @@ -599,6 +780,144 @@ TEST_P(AsyncEnd2endTest, SimpleBidiStreaming) { EXPECT_TRUE(recv_status.ok()); } +// One ping, one pong. Using server:WriteAndFinish api +TEST_P(AsyncEnd2endTest, SimpleBidiStreamingWithCoalescingApiWAF) { + ResetStub(); + + EchoRequest send_request; + EchoRequest recv_request; + EchoResponse send_response; + EchoResponse recv_response; + Status recv_status; + ClientContext cli_ctx; + ServerContext srv_ctx; + ServerAsyncReaderWriter srv_stream(&srv_ctx); + + send_request.set_message(GetParam().message_content); + cli_ctx.sent_initial_metadata_corked(true); + std::unique_ptr> + cli_stream(stub_->AsyncBidiStream(&cli_ctx, cq_.get(), tag(1))); + + service_.RequestBidiStream(&srv_ctx, &srv_stream, cq_.get(), cq_.get(), + tag(2)); + + cli_stream->WriteLast(send_request, WriteOptions(), tag(3)); + + // 65536(64KB) is the default flow control window size. Should change this + // number when default flow control window size changes. For the write of + // send_request larger than the flow control window size, tag:3 will not come + // up until server read is initiated. For write of send_request smaller than + // the flow control window size, the request can take the free ride with + // initial metadata due to coalescing, thus write tag:3 will come up here. + if (GetParam().message_content.length() < 65536) { + Verifier(GetParam().disable_blocking) + .Expect(2, true) + .Expect(3, true) + .Verify(cq_.get()); + } else { + Verifier(GetParam().disable_blocking).Expect(2, true).Verify(cq_.get()); + } + + srv_stream.Read(&recv_request, tag(4)); + + if (GetParam().message_content.length() < 65536) { + Verifier(GetParam().disable_blocking).Expect(4, true).Verify(cq_.get()); + } else { + Verifier(GetParam().disable_blocking) + .Expect(3, true) + .Expect(4, true) + .Verify(cq_.get()); + } + EXPECT_EQ(send_request.message(), recv_request.message()); + + srv_stream.Read(&recv_request, tag(5)); + Verifier(GetParam().disable_blocking).Expect(5, false).Verify(cq_.get()); + + send_response.set_message(recv_request.message()); + srv_stream.WriteAndFinish(send_response, WriteOptions(), Status::OK, tag(6)); + cli_stream->Read(&recv_response, tag(7)); + Verifier(GetParam().disable_blocking) + .Expect(6, true) + .Expect(7, true) + .Verify(cq_.get()); + EXPECT_EQ(send_response.message(), recv_response.message()); + + cli_stream->Finish(&recv_status, tag(8)); + Verifier(GetParam().disable_blocking).Expect(8, true).Verify(cq_.get()); + + EXPECT_TRUE(recv_status.ok()); +} + +// One ping, one pong. Using server:WriteLast api +TEST_P(AsyncEnd2endTest, SimpleBidiStreamingWithCoalescingApiWL) { + ResetStub(); + + EchoRequest send_request; + EchoRequest recv_request; + EchoResponse send_response; + EchoResponse recv_response; + Status recv_status; + ClientContext cli_ctx; + ServerContext srv_ctx; + ServerAsyncReaderWriter srv_stream(&srv_ctx); + + send_request.set_message(GetParam().message_content); + cli_ctx.sent_initial_metadata_corked(true); + std::unique_ptr> + cli_stream(stub_->AsyncBidiStream(&cli_ctx, cq_.get(), tag(1))); + + service_.RequestBidiStream(&srv_ctx, &srv_stream, cq_.get(), cq_.get(), + tag(2)); + + cli_stream->WriteLast(send_request, WriteOptions(), tag(3)); + + // 65536(64KB) is the default flow control window size. Should change this + // number when default flow control window size changes. For the write of + // send_request larger than the flow control window size, tag:3 will not come + // up until server read is initiated. For write of send_request smaller than + // the flow control window size, the request can take the free ride with + // initial metadata due to coalescing, thus write tag:3 will come up here. + if (GetParam().message_content.length() < 65536) { + Verifier(GetParam().disable_blocking) + .Expect(2, true) + .Expect(3, true) + .Verify(cq_.get()); + } else { + Verifier(GetParam().disable_blocking).Expect(2, true).Verify(cq_.get()); + } + + srv_stream.Read(&recv_request, tag(4)); + + if (GetParam().message_content.length() < 65536) { + Verifier(GetParam().disable_blocking).Expect(4, true).Verify(cq_.get()); + } else { + Verifier(GetParam().disable_blocking) + .Expect(3, true) + .Expect(4, true) + .Verify(cq_.get()); + } + EXPECT_EQ(send_request.message(), recv_request.message()); + + srv_stream.Read(&recv_request, tag(5)); + Verifier(GetParam().disable_blocking).Expect(5, false).Verify(cq_.get()); + + send_response.set_message(recv_request.message()); + srv_stream.WriteLast(send_response, WriteOptions(), tag(6)); + srv_stream.Finish(Status::OK, tag(7)); + cli_stream->Read(&recv_response, tag(8)); + Verifier(GetParam().disable_blocking) + .Expect(6, true) + .Expect(7, true) + .Expect(8, true) + .Verify(cq_.get()); + EXPECT_EQ(send_response.message(), recv_response.message()); + + cli_stream->Finish(&recv_status, tag(9)); + Verifier(GetParam().disable_blocking).Expect(9, true).Verify(cq_.get()); + + EXPECT_TRUE(recv_status.ok()); +} + // Metadata tests TEST_P(AsyncEnd2endTest, ClientInitialMetadataRpc) { ResetStub(); diff --git a/test/cpp/end2end/end2end_test.cc b/test/cpp/end2end/end2end_test.cc index df78557c437..222ab1ae836 100644 --- a/test/cpp/end2end/end2end_test.cc +++ b/test/cpp/end2end/end2end_test.cc @@ -702,6 +702,21 @@ TEST_P(End2endTest, RequestStreamOneRequest) { EXPECT_TRUE(s.ok()); } +TEST_P(End2endTest, RequestStreamOneRequestWithCoalescingApi) { + ResetStub(); + EchoRequest request; + EchoResponse response; + ClientContext context; + + context.sent_initial_metadata_corked(true); + auto stream = stub_->RequestStream(&context, &response); + request.set_message("hello"); + stream->WriteLast(request, WriteOptions()); + Status s = stream->Finish(); + EXPECT_EQ(response.message(), request.message()); + EXPECT_TRUE(s.ok()); +} + TEST_P(End2endTest, RequestStreamTwoRequests) { ResetStub(); EchoRequest request; @@ -718,6 +733,22 @@ TEST_P(End2endTest, RequestStreamTwoRequests) { EXPECT_TRUE(s.ok()); } +TEST_P(End2endTest, RequestStreamTwoRequestsWithCoalescingApi) { + ResetStub(); + EchoRequest request; + EchoResponse response; + ClientContext context; + + context.sent_initial_metadata_corked(true); + auto stream = stub_->RequestStream(&context, &response); + request.set_message("hello"); + EXPECT_TRUE(stream->Write(request)); + stream->WriteLast(request, WriteOptions()); + Status s = stream->Finish(); + EXPECT_EQ(response.message(), "hellohello"); + EXPECT_TRUE(s.ok()); +} + TEST_P(End2endTest, ResponseStream) { ResetStub(); EchoRequest request; @@ -738,6 +769,27 @@ TEST_P(End2endTest, ResponseStream) { EXPECT_TRUE(s.ok()); } +TEST_P(End2endTest, ResponseStreamWithCoalescingApi) { + ResetStub(); + EchoRequest request; + EchoResponse response; + ClientContext context; + request.set_message("hello"); + context.AddMetadata(kServerUseCoalescingApi, "1"); + + auto stream = stub_->ResponseStream(&context, request); + EXPECT_TRUE(stream->Read(&response)); + EXPECT_EQ(response.message(), request.message() + "0"); + EXPECT_TRUE(stream->Read(&response)); + EXPECT_EQ(response.message(), request.message() + "1"); + EXPECT_TRUE(stream->Read(&response)); + EXPECT_EQ(response.message(), request.message() + "2"); + EXPECT_FALSE(stream->Read(&response)); + + Status s = stream->Finish(); + EXPECT_TRUE(s.ok()); +} + TEST_P(End2endTest, BidiStream) { ResetStub(); EchoRequest request; @@ -770,6 +822,39 @@ TEST_P(End2endTest, BidiStream) { EXPECT_TRUE(s.ok()); } +TEST_P(End2endTest, BidiStreamWithCoalescingApi) { + ResetStub(); + EchoRequest request; + EchoResponse response; + ClientContext context; + context.AddMetadata(kServerFinishAfterNReads, "3"); + context.sent_initial_metadata_corked(true); + grpc::string msg("hello"); + + auto stream = stub_->BidiStream(&context); + + request.set_message(msg + "0"); + EXPECT_TRUE(stream->Write(request)); + EXPECT_TRUE(stream->Read(&response)); + EXPECT_EQ(response.message(), request.message()); + + request.set_message(msg + "1"); + EXPECT_TRUE(stream->Write(request)); + EXPECT_TRUE(stream->Read(&response)); + EXPECT_EQ(response.message(), request.message()); + + request.set_message(msg + "2"); + stream->WriteLast(request, WriteOptions()); + EXPECT_TRUE(stream->Read(&response)); + EXPECT_EQ(response.message(), request.message()); + + EXPECT_FALSE(stream->Read(&response)); + EXPECT_FALSE(stream->Read(&response)); + + Status s = stream->Finish(); + EXPECT_TRUE(s.ok()); +} + // Talk to the two services with the same name but different package names. // The two stubs are created on the same channel. TEST_P(End2endTest, DiffPackageServices) { diff --git a/test/cpp/end2end/test_service_impl.cc b/test/cpp/end2end/test_service_impl.cc index 59d36e9cb56..11729c425cd 100644 --- a/test/cpp/end2end/test_service_impl.cc +++ b/test/cpp/end2end/test_service_impl.cc @@ -246,6 +246,9 @@ Status TestServiceImpl::ResponseStream(ServerContext* context, int server_try_cancel = GetIntValueFromMetadata( kServerTryCancelRequest, context->client_metadata(), DO_NOT_CANCEL); + int server_coalescing_api = GetIntValueFromMetadata( + kServerUseCoalescingApi, context->client_metadata(), 0); + if (server_try_cancel == CANCEL_BEFORE_PROCESSING) { ServerTryCancel(context); return Status::CANCELLED; @@ -260,7 +263,11 @@ Status TestServiceImpl::ResponseStream(ServerContext* context, for (int i = 0; i < kNumResponseStreamsMsgs; i++) { response.set_message(request->message() + grpc::to_string(i)); - writer->Write(response); + if (i == kNumResponseStreamsMsgs - 1 && server_coalescing_api != 0) { + writer->WriteLast(response, WriteOptions()); + } else { + writer->Write(response); + } } if (server_try_cancel_thd != nullptr) { @@ -305,10 +312,21 @@ Status TestServiceImpl::BidiStream( new std::thread(&TestServiceImpl::ServerTryCancel, this, context); } + // kServerFinishAfterNReads suggests after how many reads, the server should + // write the last message and send status (coalesced using WriteLast) + int server_write_last = GetIntValueFromMetadata( + kServerFinishAfterNReads, context->client_metadata(), 0); + + int read_counts = 0; while (stream->Read(&request)) { + read_counts++; gpr_log(GPR_INFO, "recv msg %s", request.message().c_str()); response.set_message(request.message()); - stream->Write(response); + if (read_counts == server_write_last) { + stream->WriteLast(response, WriteOptions()); + } else { + stream->Write(response); + } } if (server_try_cancel_thd != nullptr) { diff --git a/test/cpp/end2end/test_service_impl.h b/test/cpp/end2end/test_service_impl.h index 88e0be7bca7..b1f02f93f65 100644 --- a/test/cpp/end2end/test_service_impl.h +++ b/test/cpp/end2end/test_service_impl.h @@ -48,6 +48,8 @@ const int kNumResponseStreamsMsgs = 3; const char* const kServerCancelAfterReads = "cancel_after_reads"; const char* const kServerTryCancelRequest = "server_try_cancel"; const char* const kDebugInfoTrailerKey = "debug-info-bin"; +const char* const kServerFinishAfterNReads = "server_finish_after_n_reads"; +const char* const kServerUseCoalescingApi = "server_use_coalescing_api"; typedef enum { DO_NOT_CANCEL = 0, diff --git a/test/cpp/microbenchmarks/bm_fullstack.cc b/test/cpp/microbenchmarks/bm_fullstack.cc index 48e131f1be0..77ea443f84d 100644 --- a/test/cpp/microbenchmarks/bm_fullstack.cc +++ b/test/cpp/microbenchmarks/bm_fullstack.cc @@ -111,9 +111,10 @@ class BaseFixture { std::ostringstream out; this->AddToLabel(out, s); #ifdef GPR_LOW_LEVEL_COUNTERS - out << " locks/iter:" << ((double)(gpr_atm_no_barrier_load(&gpr_mu_locks) - - mu_locks_at_start_) / - (double)s.iterations()) + out << " locks/iter:" + << ((double)(gpr_atm_no_barrier_load(&gpr_mu_locks) - + mu_locks_at_start_) / + (double)s.iterations()) << " atm_cas/iter:" << ((double)(gpr_atm_no_barrier_load(&gpr_counter_atm_cas) - atm_cas_at_start_) / @@ -754,6 +755,173 @@ static void BM_StreamingPingPongMsgs(benchmark::State& state) { state.SetBytesProcessed(msg_size * state.iterations() * 2); } +// Repeatedly makes Streaming Bidi calls (exchanging a configurable number of +// messages in each call) in a loop on a single channel. Different from +// BM_StreamingPingPong we are using stream coalescing api, e.g. WriteLast, +// WriteAndFinish, sent_initial_metadata_corked. These apis aim at saving +// sendmsg syscalls for streaming by coalescing 1. initial metadata with first +// message; 2. final streaming message with trailing metadata. +// +// First parmeter (i.e state.range(0)): Message size (in bytes) to use +// Second parameter (i.e state.range(1)): Number of ping pong messages. +// Note: One ping-pong means two messages (one from client to server and +// the other from server to client): +// Third parameter (i.e state.range(2)): Swtich between using WriteAndFinish +// API and WriteLast API for server. +template +static void BM_StreamingPingPongWithCoalescingApi(benchmark::State& state) { + const int msg_size = state.range(0); + const int max_ping_pongs = state.range(1); + // This options is used to test out server API: WriteLast and WriteAndFinish + // respectively, since we can not use both of them on server side at the same + // time. Value 1 means we are testing out the WriteAndFinish API, and + // otherwise we are testing out the WriteLast API. + const int write_and_finish = state.range(2); + + EchoTestService::AsyncService service; + std::unique_ptr fixture(new Fixture(&service)); + { + EchoResponse send_response; + EchoResponse recv_response; + EchoRequest send_request; + EchoRequest recv_request; + + if (msg_size > 0) { + send_request.set_message(std::string(msg_size, 'a')); + send_response.set_message(std::string(msg_size, 'b')); + } + + std::unique_ptr stub( + EchoTestService::NewStub(fixture->channel())); + + while (state.KeepRunning()) { + ServerContext svr_ctx; + ServerContextMutator svr_ctx_mut(&svr_ctx); + ServerAsyncReaderWriter response_rw(&svr_ctx); + service.RequestBidiStream(&svr_ctx, &response_rw, fixture->cq(), + fixture->cq(), tag(0)); + + ClientContext cli_ctx; + ClientContextMutator cli_ctx_mut(&cli_ctx); + cli_ctx.sent_initial_metadata_corked(true); + // tag:1 here will never comes up, since we are not performing any op due + // to initial metadata coalescing. + auto request_rw = stub->AsyncBidiStream(&cli_ctx, fixture->cq(), tag(1)); + + void* t; + bool ok; + int need_tags; + + // Send 'max_ping_pongs' number of ping pong messages + int ping_pong_cnt = 0; + while (ping_pong_cnt < max_ping_pongs) { + if (ping_pong_cnt == max_ping_pongs - 1) { + request_rw->WriteLast(send_request, WriteOptions(), tag(2)); + } else { + request_rw->Write(send_request, tag(2)); // Start client send + } + + need_tags = (1 << 2) | (1 << 3) | (1 << 4) | (1 << 5); + + if (ping_pong_cnt == 0) { + // wait for the server call structure (call_hook, etc.) to be + // initialized (async stream between client side and server side + // established). It is necessary when client init metadata is + // coalesced + GPR_ASSERT(fixture->cq()->Next(&t, &ok)); + while ((int)(intptr_t)t != 0) { + // In some cases tag:2 comes before tag:0 (write tag comes out + // first), this while loop is to make sure get tag:0. + int i = (int)(intptr_t)t; + GPR_ASSERT(need_tags & (1 << i)); + need_tags &= ~(1 << i); + GPR_ASSERT(fixture->cq()->Next(&t, &ok)); + } + } + + response_rw.Read(&recv_request, tag(3)); // Start server recv + request_rw->Read(&recv_response, tag(4)); // Start client recv + + while (need_tags) { + GPR_ASSERT(fixture->cq()->Next(&t, &ok)); + GPR_ASSERT(ok); + int i = (int)(intptr_t)t; + + // If server recv is complete, start the server send operation + if (i == 3) { + if (ping_pong_cnt == max_ping_pongs - 1) { + if (write_and_finish == 1) { + response_rw.WriteAndFinish(send_response, WriteOptions(), + Status::OK, tag(5)); + } else { + response_rw.WriteLast(send_response, WriteOptions(), tag(5)); + // WriteLast buffers the write, so neither server write op nor + // client read op will finish inside the while loop. + need_tags &= ~(1 << 4); + need_tags &= ~(1 << 5); + } + } else { + response_rw.Write(send_response, tag(5)); + } + } + + GPR_ASSERT(need_tags & (1 << i)); + need_tags &= ~(1 << i); + } + + ping_pong_cnt++; + } + + if (max_ping_pongs == 0) { + need_tags = (1 << 6) | (1 << 7) | (1 << 8); + } else { + if (write_and_finish == 1) { + need_tags = (1 << 8); + } else { + // server's buffered write and the client's read of the buffered write + // tags should come up. + need_tags = (1 << 4) | (1 << 5) | (1 << 7) | (1 << 8); + } + } + + // No message write or initial metadata write happened yet. + if (max_ping_pongs == 0) { + request_rw->WritesDone(tag(6)); + // wait for server call data structure(call_hook, etc.) to be + // initialized, since initial metadata is corked. + GPR_ASSERT(fixture->cq()->Next(&t, &ok)); + while ((int)(intptr_t)t != 0) { + int i = (int)(intptr_t)t; + GPR_ASSERT(need_tags & (1 << i)); + need_tags &= ~(1 << i); + GPR_ASSERT(fixture->cq()->Next(&t, &ok)); + } + response_rw.Finish(Status::OK, tag(7)); + } else { + if (write_and_finish != 1) { + response_rw.Finish(Status::OK, tag(7)); + } + } + + Status recv_status; + request_rw->Finish(&recv_status, tag(8)); + + while (need_tags) { + GPR_ASSERT(fixture->cq()->Next(&t, &ok)); + int i = (int)(intptr_t)t; + GPR_ASSERT(need_tags & (1 << i)); + need_tags &= ~(1 << i); + } + + GPR_ASSERT(recv_status.ok()); + } + } + + fixture->Finish(state); + fixture.reset(); + state.SetBytesProcessed(msg_size * state.iterations() * max_ping_pongs * 2); +} + template static void BM_PumpStreamClientToServer(benchmark::State& state) { EchoTestService::AsyncService service; @@ -873,8 +1041,9 @@ static void BM_PumpStreamServerToClient(benchmark::State& state) { static void TrickleCQNext(TrickledCHTTP2* fixture, void** t, bool* ok) { while (true) { switch (fixture->cq()->AsyncNext( - t, ok, gpr_time_add(gpr_now(GPR_CLOCK_MONOTONIC), - gpr_time_from_micros(100, GPR_TIMESPAN)))) { + t, ok, + gpr_time_add(gpr_now(GPR_CLOCK_MONOTONIC), + gpr_time_from_micros(100, GPR_TIMESPAN)))) { case CompletionQueue::TIMEOUT: fixture->Step(); break; @@ -1067,6 +1236,30 @@ BENCHMARK_TEMPLATE(BM_StreamingPingPong, InProcessCHTTP2, NoOpMutator, BENCHMARK_TEMPLATE(BM_StreamingPingPong, TCP, NoOpMutator, NoOpMutator) ->Apply(StreamingPingPongArgs); +// Generate Args for StreamingPingPongWithCoalescingApi benchmarks. Currently +// generates args for only "small streams" (i.e streams with 0, 1 or 2 messages) +static void StreamingPingPongWithCoalescingApiArgs( + benchmark::internal::Benchmark* b) { + int msg_size = 0; + + b->Args( + {0, 0, 0}); // spl case: 0 ping-pong msgs (msg_size doesn't matter here) + b->Args( + {0, 0, 1}); // spl case: 0 ping-pong msgs (msg_size doesn't matter here) + + for (msg_size = 0; msg_size <= 128 * 1024 * 1024; + msg_size == 0 ? msg_size++ : msg_size *= 8) { + b->Args({msg_size, 1, 0}); + b->Args({msg_size, 2, 0}); + b->Args({msg_size, 1, 1}); + b->Args({msg_size, 2, 1}); + } +} + +BENCHMARK_TEMPLATE(BM_StreamingPingPongWithCoalescingApi, InProcessCHTTP2, + NoOpMutator, NoOpMutator) + ->Apply(StreamingPingPongWithCoalescingApiArgs); + BENCHMARK_TEMPLATE(BM_StreamingPingPongMsgs, InProcessCHTTP2, NoOpMutator, NoOpMutator) ->Range(0, 128 * 1024 * 1024); From bdc76ab37b4d61a849f2f9f9742a5004864ce984 Mon Sep 17 00:00:00 2001 From: Yuxuan Li Date: Fri, 3 Mar 2017 13:59:30 -0800 Subject: [PATCH 22/44] bug fix --- include/grpc++/impl/codegen/async_stream.h | 4 ++-- include/grpc++/impl/codegen/client_context.h | 4 +++- include/grpc++/impl/codegen/sync_stream.h | 4 ++-- src/cpp/client/client_context.cc | 4 ---- test/cpp/end2end/async_end2end_test.cc | 6 +++--- test/cpp/end2end/end2end_test.cc | 6 +++--- test/cpp/end2end/mock_test.cc | 2 +- test/cpp/microbenchmarks/bm_fullstack.cc | 4 ++-- third_party/protobuf | 2 +- 9 files changed, 17 insertions(+), 19 deletions(-) diff --git a/include/grpc++/impl/codegen/async_stream.h b/include/grpc++/impl/codegen/async_stream.h index f21139618f5..a2013415902 100644 --- a/include/grpc++/impl/codegen/async_stream.h +++ b/include/grpc++/impl/codegen/async_stream.h @@ -244,7 +244,7 @@ class ClientAsyncWriter final : public ClientAsyncWriterInterface { call_.PerformOps(&write_ops_); } - void Write(const W& msg, WriteOptions options, void* tag) { + void Write(const W& msg, WriteOptions options, void* tag) override { write_ops_.set_output_tag(tag); if (options.is_last_message()) { options.set_buffer_hint(); @@ -341,7 +341,7 @@ class ClientAsyncReaderWriter final call_.PerformOps(&write_ops_); } - void Write(const W& msg, WriteOptions options, void* tag) { + void Write(const W& msg, WriteOptions options, void* tag) override { write_ops_.set_output_tag(tag); if (options.is_last_message()) { options.set_buffer_hint(); diff --git a/include/grpc++/impl/codegen/client_context.h b/include/grpc++/impl/codegen/client_context.h index cc48b9c3f57..3c50e6ba9d4 100644 --- a/include/grpc++/impl/codegen/client_context.h +++ b/include/grpc++/impl/codegen/client_context.h @@ -288,7 +288,9 @@ class ClientContext { /// /// \param corked The flag indicating whether the initial metadata is to be /// corked or not. - void sent_initial_metadata_corked(bool corked); + void set_initial_metadata_corked(bool corked) { + initial_metadata_corked_ = corked; + } /// Return the peer uri in a string. /// diff --git a/include/grpc++/impl/codegen/sync_stream.h b/include/grpc++/impl/codegen/sync_stream.h index 859c3d0b905..ae3b8e441d1 100644 --- a/include/grpc++/impl/codegen/sync_stream.h +++ b/include/grpc++/impl/codegen/sync_stream.h @@ -262,7 +262,7 @@ class ClientWriter : public ClientWriterInterface { if (context_->initial_metadata_corked_) { ops.SendInitialMetadata(context_->send_initial_metadata_, context_->initial_metadata_flags()); - context_->sent_initial_metadata_corked(false); + context_->set_initial_metadata_corked(false); } if (!ops.SendMessage(msg, options).ok()) { return false; @@ -372,7 +372,7 @@ class ClientReaderWriter final : public ClientReaderWriterInterface { if (context_->initial_metadata_corked_) { ops.SendInitialMetadata(context_->send_initial_metadata_, context_->initial_metadata_flags()); - context_->sent_initial_metadata_corked(false); + context_->set_initial_metadata_corked(false); } if (!ops.SendMessage(msg, options).ok()) { return false; diff --git a/src/cpp/client/client_context.cc b/src/cpp/client/client_context.cc index 96f97021cb7..3d884cf62e4 100644 --- a/src/cpp/client/client_context.cc +++ b/src/cpp/client/client_context.cc @@ -119,10 +119,6 @@ void ClientContext::set_compression_algorithm( AddMetadata(GRPC_COMPRESSION_REQUEST_ALGORITHM_MD_KEY, algorithm_name); } -void ClientContext::sent_initial_metadata_corked(bool corked) { - initial_metadata_corked_ = corked; -} - void ClientContext::TryCancel() { std::unique_lock lock(mu_); if (call_) { diff --git a/test/cpp/end2end/async_end2end_test.cc b/test/cpp/end2end/async_end2end_test.cc index 47e803b948e..0b5215ef8e4 100644 --- a/test/cpp/end2end/async_end2end_test.cc +++ b/test/cpp/end2end/async_end2end_test.cc @@ -498,7 +498,7 @@ TEST_P(AsyncEnd2endTest, SimpleClientStreamingWithCoalescingApi) { ServerAsyncReader srv_stream(&srv_ctx); send_request.set_message(GetParam().message_content); - cli_ctx.sent_initial_metadata_corked(true); + cli_ctx.set_initial_metadata_corked(true); // tag:1 never comes up since no op is performed std::unique_ptr> cli_stream( stub_->AsyncRequestStream(&cli_ctx, &recv_response, cq_.get(), tag(1))); @@ -794,7 +794,7 @@ TEST_P(AsyncEnd2endTest, SimpleBidiStreamingWithCoalescingApiWAF) { ServerAsyncReaderWriter srv_stream(&srv_ctx); send_request.set_message(GetParam().message_content); - cli_ctx.sent_initial_metadata_corked(true); + cli_ctx.set_initial_metadata_corked(true); std::unique_ptr> cli_stream(stub_->AsyncBidiStream(&cli_ctx, cq_.get(), tag(1))); @@ -862,7 +862,7 @@ TEST_P(AsyncEnd2endTest, SimpleBidiStreamingWithCoalescingApiWL) { ServerAsyncReaderWriter srv_stream(&srv_ctx); send_request.set_message(GetParam().message_content); - cli_ctx.sent_initial_metadata_corked(true); + cli_ctx.set_initial_metadata_corked(true); std::unique_ptr> cli_stream(stub_->AsyncBidiStream(&cli_ctx, cq_.get(), tag(1))); diff --git a/test/cpp/end2end/end2end_test.cc b/test/cpp/end2end/end2end_test.cc index 222ab1ae836..d3a83b188f9 100644 --- a/test/cpp/end2end/end2end_test.cc +++ b/test/cpp/end2end/end2end_test.cc @@ -708,7 +708,7 @@ TEST_P(End2endTest, RequestStreamOneRequestWithCoalescingApi) { EchoResponse response; ClientContext context; - context.sent_initial_metadata_corked(true); + context.set_initial_metadata_corked(true); auto stream = stub_->RequestStream(&context, &response); request.set_message("hello"); stream->WriteLast(request, WriteOptions()); @@ -739,7 +739,7 @@ TEST_P(End2endTest, RequestStreamTwoRequestsWithCoalescingApi) { EchoResponse response; ClientContext context; - context.sent_initial_metadata_corked(true); + context.set_initial_metadata_corked(true); auto stream = stub_->RequestStream(&context, &response); request.set_message("hello"); EXPECT_TRUE(stream->Write(request)); @@ -828,7 +828,7 @@ TEST_P(End2endTest, BidiStreamWithCoalescingApi) { EchoResponse response; ClientContext context; context.AddMetadata(kServerFinishAfterNReads, "3"); - context.sent_initial_metadata_corked(true); + context.set_initial_metadata_corked(true); grpc::string msg("hello"); auto stream = stub_->BidiStream(&context); diff --git a/test/cpp/end2end/mock_test.cc b/test/cpp/end2end/mock_test.cc index d6664da5a0f..fdb2732e503 100644 --- a/test/cpp/end2end/mock_test.cc +++ b/test/cpp/end2end/mock_test.cc @@ -89,7 +89,7 @@ class MockClientReaderWriter final return true; } - bool Write(const EchoRequest& msg, const WriteOptions& options) override { + bool Write(const EchoRequest& msg, WriteOptions options) override { gpr_log(GPR_INFO, "mock recv msg %s", msg.message().c_str()); last_message_ = msg.message(); return true; diff --git a/test/cpp/microbenchmarks/bm_fullstack.cc b/test/cpp/microbenchmarks/bm_fullstack.cc index 77ea443f84d..b1f3ed3597c 100644 --- a/test/cpp/microbenchmarks/bm_fullstack.cc +++ b/test/cpp/microbenchmarks/bm_fullstack.cc @@ -758,7 +758,7 @@ static void BM_StreamingPingPongMsgs(benchmark::State& state) { // Repeatedly makes Streaming Bidi calls (exchanging a configurable number of // messages in each call) in a loop on a single channel. Different from // BM_StreamingPingPong we are using stream coalescing api, e.g. WriteLast, -// WriteAndFinish, sent_initial_metadata_corked. These apis aim at saving +// WriteAndFinish, set_initial_metadata_corked. These apis aim at saving // sendmsg syscalls for streaming by coalescing 1. initial metadata with first // message; 2. final streaming message with trailing metadata. // @@ -803,7 +803,7 @@ static void BM_StreamingPingPongWithCoalescingApi(benchmark::State& state) { ClientContext cli_ctx; ClientContextMutator cli_ctx_mut(&cli_ctx); - cli_ctx.sent_initial_metadata_corked(true); + cli_ctx.set_initial_metadata_corked(true); // tag:1 here will never comes up, since we are not performing any op due // to initial metadata coalescing. auto request_rw = stub->AsyncBidiStream(&cli_ctx, fixture->cq(), tag(1)); diff --git a/third_party/protobuf b/third_party/protobuf index 593e917c176..a428e420727 160000 --- a/third_party/protobuf +++ b/third_party/protobuf @@ -1 +1 @@ -Subproject commit 593e917c176b5bc5aafa57bf9f6030d749d91cd5 +Subproject commit a428e42072765993ff674fda72863c9f1aa2d268 From 8124f66993d3c2eebb46832300d133a50e735f0f Mon Sep 17 00:00:00 2001 From: Yuxuan Li Date: Wed, 8 Mar 2017 11:26:02 -0800 Subject: [PATCH 23/44] protobuf correct head --- third_party/protobuf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/third_party/protobuf b/third_party/protobuf index a428e420727..593e917c176 160000 --- a/third_party/protobuf +++ b/third_party/protobuf @@ -1 +1 @@ -Subproject commit a428e42072765993ff674fda72863c9f1aa2d268 +Subproject commit 593e917c176b5bc5aafa57bf9f6030d749d91cd5 From 0aac9d06e24358c71472eca5bce4c51b499bb2cb Mon Sep 17 00:00:00 2001 From: Yuxuan Li Date: Fri, 10 Mar 2017 14:34:59 -0800 Subject: [PATCH 24/44] change last_message bit to be in C++ layer only --- include/grpc++/impl/codegen/call.h | 16 +++++++--------- include/grpc/impl/codegen/grpc_types.h | 6 +----- test/cpp/microbenchmarks/bm_fullstack.cc | 2 +- 3 files changed, 9 insertions(+), 15 deletions(-) diff --git a/include/grpc++/impl/codegen/call.h b/include/grpc++/impl/codegen/call.h index f6638471d24..a3f2be6bb1e 100644 --- a/include/grpc++/impl/codegen/call.h +++ b/include/grpc++/impl/codegen/call.h @@ -84,8 +84,9 @@ inline grpc_metadata* FillMetadataArray( /// Per-message write options. class WriteOptions { public: - WriteOptions() : flags_(0) {} - WriteOptions(const WriteOptions& other) : flags_(other.flags_) {} + WriteOptions() : flags_(0), last_message_(false) {} + WriteOptions(const WriteOptions& other) + : flags_(other.flags_), last_message_(other.last_message_) {} /// Clear all flags. inline void Clear() { flags_ = 0; } @@ -160,19 +161,15 @@ class WriteOptions { /// in a single step /// server-side: hold the Write until the service handler returns (sync api) /// or until Finish is called (async api) - /// - /// \sa GRPC_WRITE_LAST_MESSAGE inline WriteOptions& set_last_message() { - SetBit(GRPC_WRITE_LAST_MESSAGE); + last_message_ = true; return *this; } /// Clears flag indicating that this is the last message in a stream, /// disabling coalescing. - /// - /// \sa GRPC_WRITE_LAST_MESSAGE inline WriteOptions& clear_last_messsage() { - ClearBit(GRPC_WRITE_LAST_MESSAGE); + last_message_ = false; return *this; } @@ -180,7 +177,7 @@ class WriteOptions { /// should be coalesced with trailing metadata. /// /// \sa GRPC_WRITE_LAST_MESSAGE - bool is_last_message() const { return GetBit(GRPC_WRITE_LAST_MESSAGE); } + bool is_last_message() const { return last_message_; } WriteOptions& operator=(const WriteOptions& rhs) { flags_ = rhs.flags_; @@ -195,6 +192,7 @@ class WriteOptions { bool GetBit(const uint32_t mask) const { return (flags_ & mask) != 0; } uint32_t flags_; + bool last_message_; }; /// Default argument for CallOpSet. I is unused by the class, but can be diff --git a/include/grpc/impl/codegen/grpc_types.h b/include/grpc/impl/codegen/grpc_types.h index 8e6f62a6fe5..a076b8c619b 100644 --- a/include/grpc/impl/codegen/grpc_types.h +++ b/include/grpc/impl/codegen/grpc_types.h @@ -298,12 +298,8 @@ typedef enum grpc_call_error { /** Force compression to be disabled for a particular write (start_write/add_metadata). Illegal on invoke/accept. */ #define GRPC_WRITE_NO_COMPRESS (0x00000002u) -/** Force coalescing of last streaming message and trailing metadata into the - same core batch */ -#define GRPC_WRITE_LAST_MESSAGE (0x00000004u) /** Mask of all valid flags. */ -#define GRPC_WRITE_USED_MASK \ - (GRPC_WRITE_BUFFER_HINT | GRPC_WRITE_NO_COMPRESS | GRPC_WRITE_LAST_MESSAGE) +#define GRPC_WRITE_USED_MASK (GRPC_WRITE_BUFFER_HINT | GRPC_WRITE_NO_COMPRESS) /* Initial metadata flags */ /** Signal that the call is idempotent */ diff --git a/test/cpp/microbenchmarks/bm_fullstack.cc b/test/cpp/microbenchmarks/bm_fullstack.cc index b1f3ed3597c..b5b68c9d095 100644 --- a/test/cpp/microbenchmarks/bm_fullstack.cc +++ b/test/cpp/microbenchmarks/bm_fullstack.cc @@ -766,7 +766,7 @@ static void BM_StreamingPingPongMsgs(benchmark::State& state) { // Second parameter (i.e state.range(1)): Number of ping pong messages. // Note: One ping-pong means two messages (one from client to server and // the other from server to client): -// Third parameter (i.e state.range(2)): Swtich between using WriteAndFinish +// Third parameter (i.e state.range(2)): Switch between using WriteAndFinish // API and WriteLast API for server. template static void BM_StreamingPingPongWithCoalescingApi(benchmark::State& state) { From 44e1837ef914b6ad4133a3eaf38761ca37d9ae83 Mon Sep 17 00:00:00 2001 From: Muxi Yan Date: Fri, 10 Mar 2017 14:52:51 -0800 Subject: [PATCH 25/44] Add comment for ignoring errorHandler in GRPCOpSendMessage and TODO --- src/objective-c/GRPCClient/GRPCCall.m | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/objective-c/GRPCClient/GRPCCall.m b/src/objective-c/GRPCClient/GRPCCall.m index ff474650122..7d928f81c18 100644 --- a/src/objective-c/GRPCClient/GRPCCall.m +++ b/src/objective-c/GRPCClient/GRPCCall.m @@ -307,6 +307,8 @@ static NSMutableDictionary *callFlags; [_wrappedCall startBatchWithOperations:@[op] errorHandler:errorHandler]; } else { + // Ignored errorHandler since it is the same as the one for GRPCOpSendClose. + // TODO (mxyan): unify the error handlers of all Ops into a single closure. [_unaryOpBatch addObject:op]; } } From 01bbf87bf78d40c9a8287fccf7ea0d64ff5cf07c Mon Sep 17 00:00:00 2001 From: Muxi Yan Date: Fri, 10 Mar 2017 15:02:28 -0800 Subject: [PATCH 26/44] Add comments for map function --- src/objective-c/RxLibrary/GRXImmediateSingleWriter.m | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/objective-c/RxLibrary/GRXImmediateSingleWriter.m b/src/objective-c/RxLibrary/GRXImmediateSingleWriter.m index a0d3b771e8d..a3e9cd60a35 100644 --- a/src/objective-c/RxLibrary/GRXImmediateSingleWriter.m +++ b/src/objective-c/RxLibrary/GRXImmediateSingleWriter.m @@ -75,7 +75,11 @@ return; } +// Overrides [requestWriter(Transformations):map:] for Protocol Buffers +// encoding. - (GRXWriter *)map:(id (^)(id))map { + // Since _value is available when creating the object, we can simply + // apply the map and store the output. _value = map(_value); return self; } From 7e151845ab580a7fd5fb72922d3381034045d7b4 Mon Sep 17 00:00:00 2001 From: Muxi Yan Date: Fri, 10 Mar 2017 16:22:19 -0800 Subject: [PATCH 27/44] Wrap entire GRPCCall (InternalTests) class in GRPC_TEST_OBJC --- .../internal_testing/GRPCCall+InternalTests.h | 4 +++ .../internal_testing/GRPCCall+InternalTests.m | 17 +++------- src/objective-c/tests/Podfile | 4 +-- .../tests/Tests.xcodeproj/project.pbxproj | 32 +++++++++++++++++-- 4 files changed, 39 insertions(+), 18 deletions(-) diff --git a/src/objective-c/GRPCClient/internal_testing/GRPCCall+InternalTests.h b/src/objective-c/GRPCClient/internal_testing/GRPCCall+InternalTests.h index 6c12ce6355e..29bd12f0cfb 100644 --- a/src/objective-c/GRPCClient/internal_testing/GRPCCall+InternalTests.h +++ b/src/objective-c/GRPCClient/internal_testing/GRPCCall+InternalTests.h @@ -31,6 +31,8 @@ * */ +#ifdef GRPC_TEST_OBJC + #import "../GRPCCall.h" /** @@ -55,3 +57,5 @@ + (NSArray *)obtainAndCleanOpBatchLog; @end + +#endif diff --git a/src/objective-c/GRPCClient/internal_testing/GRPCCall+InternalTests.m b/src/objective-c/GRPCClient/internal_testing/GRPCCall+InternalTests.m index 43c1c078316..6371df6739a 100644 --- a/src/objective-c/GRPCClient/internal_testing/GRPCCall+InternalTests.m +++ b/src/objective-c/GRPCClient/internal_testing/GRPCCall+InternalTests.m @@ -31,6 +31,8 @@ * */ +#ifdef GRPC_TEST_OBJC + #import "GRPCCall+InternalTests.h" #import "../private/GRPCOpBatchLog.h" @@ -38,24 +40,13 @@ @implementation GRPCCall (InternalTests) + (void)enableOpBatchLog:(BOOL)enabled { -#ifdef GRPC_TEST_OBJC [GRPCOpBatchLog enableOpBatchLog:enabled]; -#else - NSLog(@"This function is for internal testing of gRPC only. " - "It is not part of gRPC's public interface. Do not use in production. " - "To enable, set the preprocessor flag GRPC_TEST_OBJC."); -#endif } + (NSArray *)obtainAndCleanOpBatchLog { -#ifdef GRPC_TEST_OBJC return [GRPCOpBatchLog obtainAndCleanOpBatchLog]; -#else - NSLog(@"This function is for internal testing of gRPC only. " - "It is not part of gRPC's public interface. Do not use in production. " - "To enable, set the preprocessor flag GRPC_TEST_OBJC."); - return nil; -#endif } @end + +#endif diff --git a/src/objective-c/tests/Podfile b/src/objective-c/tests/Podfile index 941031ff88e..8f1cb041d86 100644 --- a/src/objective-c/tests/Podfile +++ b/src/objective-c/tests/Podfile @@ -108,9 +108,9 @@ post_install do |installer| if target.name == 'gRPC' target.build_configurations.each do |config| if config.name == 'Cronet' - config.build_settings['GCC_PREPROCESSOR_DEFINITIONS'] = '$(inherited) COCOAPODS=1 GRPC_COMPILE_WITH_CRONET=1 GRPC_TEST_OBJC' + config.build_settings['GCC_PREPROCESSOR_DEFINITIONS'] = '$(inherited) COCOAPODS=1 GRPC_COMPILE_WITH_CRONET=1 GRPC_TEST_OBJC=1' elsif config.name == 'Test' - config.build_settings['GCC_PREPROCESSOR_DEFINITIONS'] = '$(inherited) COCOAPODS=1 GRPC_TEST_OBJC' + config.build_settings['GCC_PREPROCESSOR_DEFINITIONS'] = '$(inherited) COCOAPODS=1 GRPC_TEST_OBJC=1' end end end diff --git a/src/objective-c/tests/Tests.xcodeproj/project.pbxproj b/src/objective-c/tests/Tests.xcodeproj/project.pbxproj index 57e417c2a8a..97de723a22a 100644 --- a/src/objective-c/tests/Tests.xcodeproj/project.pbxproj +++ b/src/objective-c/tests/Tests.xcodeproj/project.pbxproj @@ -1319,6 +1319,7 @@ GCC_PREPROCESSOR_DEFINITIONS = ( "DEBUG=1", "$(inherited)", + "GRPC_TEST_OBJC=1", ); INFOPLIST_FILE = Info.plist; LD_RUNPATH_SEARCH_PATHS = "$(inherited) @executable_path/Frameworks @loader_path/Frameworks"; @@ -1346,6 +1347,13 @@ buildSettings = { DEBUG_INFORMATION_FORMAT = dwarf; ENABLE_TESTABILITY = YES; + GCC_PREPROCESSOR_DEFINITIONS = ( + "$(inherited)", + "COCOAPODS=1", + "$(inherited)", + "GPB_USE_PROTOBUF_FRAMEWORK_IMPORTS=1", + "GRPC_TEST_OBJC=1", + ); INFOPLIST_FILE = Info.plist; IPHONEOS_DEPLOYMENT_TARGET = 9.0; LD_RUNPATH_SEARCH_PATHS = "$(inherited) @executable_path/Frameworks @loader_path/Frameworks"; @@ -1360,6 +1368,13 @@ buildSettings = { DEBUG_INFORMATION_FORMAT = dwarf; ENABLE_TESTABILITY = YES; + GCC_PREPROCESSOR_DEFINITIONS = ( + "$(inherited)", + "COCOAPODS=1", + "$(inherited)", + "GPB_USE_PROTOBUF_FRAMEWORK_IMPORTS=1", + "GRPC_TEST_OBJC=1", + ); INFOPLIST_FILE = Info.plist; IPHONEOS_DEPLOYMENT_TARGET = 9.0; LD_RUNPATH_SEARCH_PATHS = "$(inherited) @executable_path/Frameworks @loader_path/Frameworks"; @@ -1374,6 +1389,12 @@ buildSettings = { DEBUG_INFORMATION_FORMAT = dwarf; ENABLE_TESTABILITY = YES; + GCC_PREPROCESSOR_DEFINITIONS = ( + "$(inherited)", + "COCOAPODS=1", + "GPB_USE_PROTOBUF_FRAMEWORK_IMPORTS=1", + "GRPC_TEST_OBJC=1", + ); INFOPLIST_FILE = Info.plist; IPHONEOS_DEPLOYMENT_TARGET = 9.0; LD_RUNPATH_SEARCH_PATHS = "$(inherited) @executable_path/Frameworks @loader_path/Frameworks"; @@ -1410,8 +1431,8 @@ GCC_PREPROCESSOR_DEFINITIONS = ( "$(inherited)", "COCOAPODS=1", - "$(inherited)", "GPB_USE_PROTOBUF_FRAMEWORK_IMPORTS=1", + "GRPC_TEST_OBJC=1", ); INFOPLIST_FILE = InteropTestsRemoteWithCronet/Info.plist; IPHONEOS_DEPLOYMENT_TARGET = 9.3; @@ -1608,6 +1629,12 @@ buildSettings = { DEBUG_INFORMATION_FORMAT = dwarf; ENABLE_TESTABILITY = YES; + GCC_PREPROCESSOR_DEFINITIONS = ( + "$(inherited)", + "COCOAPODS=1", + "$(inherited)", + "GPB_USE_PROTOBUF_FRAMEWORK_IMPORTS=1", + ); INFOPLIST_FILE = Info.plist; IPHONEOS_DEPLOYMENT_TARGET = 9.0; LD_RUNPATH_SEARCH_PATHS = "$(inherited) @executable_path/Frameworks @loader_path/Frameworks"; @@ -1672,10 +1699,10 @@ GCC_PREPROCESSOR_DEFINITIONS = ( "$(inherited)", "COCOAPODS=1", - "$(inherited)", "GPB_USE_PROTOBUF_FRAMEWORK_IMPORTS=1", "GRPC_COMPILE_WITH_CRONET=1", "GRPC_CRONET_WITH_PACKET_COALESCING=1", + "GRPC_TEST_OBJC=1", ); INFOPLIST_FILE = InteropTestsRemoteWithCronet/Info.plist; IPHONEOS_DEPLOYMENT_TARGET = 9.3; @@ -1696,7 +1723,6 @@ GCC_PREPROCESSOR_DEFINITIONS = ( "$(inherited)", "COCOAPODS=1", - "$(inherited)", "GPB_USE_PROTOBUF_FRAMEWORK_IMPORTS=1", ); INFOPLIST_FILE = InteropTestsRemoteWithCronet/Info.plist; From c2a8a8f5c248516c684136b22181f2db204df7bc Mon Sep 17 00:00:00 2001 From: Makarand Dharmapurikar Date: Fri, 17 Mar 2017 11:10:08 -0700 Subject: [PATCH 28/44] addressed review feedback using new grpc_build_system, and cleaned up typos. --- BUILD | 2 + tools/grpcz/BUILD | 38 ++++------ tools/grpcz/any.proto | 139 ----------------------------------- tools/grpcz/empty.proto | 52 ------------- tools/grpcz/grpcz_client.cc | 41 +++++++---- tools/grpcz/monitoring.proto | 6 +- 6 files changed, 44 insertions(+), 234 deletions(-) delete mode 100644 tools/grpcz/any.proto delete mode 100644 tools/grpcz/empty.proto diff --git a/BUILD b/BUILD index ca0a1c56076..5a2e7a72e9e 100644 --- a/BUILD +++ b/BUILD @@ -1132,6 +1132,7 @@ grpc_cc_library( "src/cpp/common/rpc_method.cc", "src/cpp/common/version_cc.cc", "src/cpp/server/async_generic_service.cc", + "src/cpp/server/channel_argument_option.cc", "src/cpp/server/create_default_thread_pool.cc", "src/cpp/server/dynamic_thread_pool.cc", "src/cpp/server/health/default_health_check_service.cc", @@ -1173,6 +1174,7 @@ grpc_cc_library( "include/grpc++/grpc++.h", "include/grpc++/health_check_service_interface.h", "include/grpc++/impl/call.h", + "include/grpc++/impl/channel_argument_option.h", "include/grpc++/impl/client_unary_call.h", "include/grpc++/impl/codegen/core_codegen.h", "include/grpc++/impl/grpc_library.h", diff --git a/tools/grpcz/BUILD b/tools/grpcz/BUILD index 4ae12d50064..5e1faf7064f 100644 --- a/tools/grpcz/BUILD +++ b/tools/grpcz/BUILD @@ -31,43 +31,33 @@ licenses(["notice"]) # 3-clause BSD package(default_visibility = ["//visibility:public"]) -load("//:bazel/generate_cc.bzl", "generate_cc") +load("//:bazel/grpc_build_system.bzl", "grpc_proto_library") -proto_library ( - name = "monitoring_proto_local_copy", +grpc_proto_library ( + name = "monitoring_proto", srcs = [ - # TODO (ericgribkoff) : remove the local copies of these protos "monitoring.proto", - "empty.proto", - "any.proto", - "census.proto", ], + deps = [ + ":census_proto", + ], + well_known_protos = "@submodule_protobuf//:well_known_protos", ) -generate_cc( - name = "monitoring_codegen", - srcs = [":monitoring_proto_local_copy"], -) - -generate_cc( - name = "monitoring_grpc_codegen", - srcs = [":monitoring_proto_local_copy"], - plugin = "//:grpc_cpp_plugin", -) - -cc_library( - name = "proto_lib", - srcs = [":monitoring_codegen", ":monitoring_grpc_codegen"], - hdrs = [":monitoring_codegen", ":monitoring_grpc_codegen"], - deps = ["//:grpc++", "//:grpc++_codegen_proto", "//external:protobuf"], +grpc_proto_library ( + name = "census_proto", + srcs = [ + "census.proto", + ], + well_known_protos = "@submodule_protobuf//:well_known_protos", ) cc_binary( name = "grpcz_client", srcs = ["grpcz_client.cc",], deps = [ - "proto_lib", "//external:gflags", + "monitoring_proto", "@mongoose_repo//:mongoose_lib", ], ) diff --git a/tools/grpcz/any.proto b/tools/grpcz/any.proto deleted file mode 100644 index a224a2b596e..00000000000 --- a/tools/grpcz/any.proto +++ /dev/null @@ -1,139 +0,0 @@ -// Protocol Buffers - Google's data interchange format -// Copyright 2008 Google Inc. All rights reserved. -// https://developers.google.com/protocol-buffers/ -// -// Redistribution and use in source and binary forms, with or without -// modification, are permitted provided that the following conditions are -// met: -// -// * Redistributions of source code must retain the above copyright -// notice, this list of conditions and the following disclaimer. -// * Redistributions in binary form must reproduce the above -// copyright notice, this list of conditions and the following disclaimer -// in the documentation and/or other materials provided with the -// distribution. -// * Neither the name of Google Inc. nor the names of its -// contributors may be used to endorse or promote products derived from -// this software without specific prior written permission. -// -// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS -// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT -// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR -// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT -// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, -// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT -// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, -// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY -// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT -// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE -// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. - -syntax = "proto3"; - -//package google.protobuf; - -option csharp_namespace = "Google.Protobuf.WellKnownTypes"; -option go_package = "github.com/golang/protobuf/ptypes/any"; -option java_package = "com.google.protobuf"; -option java_outer_classname = "AnyProto"; -option java_multiple_files = true; -option objc_class_prefix = "GPB"; - -// `Any` contains an arbitrary serialized protocol buffer message along with a -// URL that describes the type of the serialized message. -// -// Protobuf library provides support to pack/unpack Any values in the form -// of utility functions or additional generated methods of the Any type. -// -// Example 1: Pack and unpack a message in C++. -// -// Foo foo = ...; -// Any any; -// any.PackFrom(foo); -// ... -// if (any.UnpackTo(&foo)) { -// ... -// } -// -// Example 2: Pack and unpack a message in Java. -// -// Foo foo = ...; -// Any any = Any.pack(foo); -// ... -// if (any.is(Foo.class)) { -// foo = any.unpack(Foo.class); -// } -// -// Example 3: Pack and unpack a message in Python. -// -// foo = Foo(...) -// any = Any() -// any.Pack(foo) -// ... -// if any.Is(Foo.DESCRIPTOR): -// any.Unpack(foo) -// ... -// -// The pack methods provided by protobuf library will by default use -// 'type.googleapis.com/full.type.name' as the type URL and the unpack -// methods only use the fully qualified type name after the last '/' -// in the type URL, for example "foo.bar.com/x/y.z" will yield type -// name "y.z". -// -// -// JSON -// ==== -// The JSON representation of an `Any` value uses the regular -// representation of the deserialized, embedded message, with an -// additional field `@type` which contains the type URL. Example: -// -// package google.profile; -// message Person { -// string first_name = 1; -// string last_name = 2; -// } -// -// { -// "@type": "type.googleapis.com/google.profile.Person", -// "firstName": , -// "lastName": -// } -// -// If the embedded message type is well-known and has a custom JSON -// representation, that representation will be embedded adding a field -// `value` which holds the custom JSON in addition to the `@type` -// field. Example (for message [google.protobuf.Duration][]): -// -// { -// "@type": "type.googleapis.com/google.protobuf.Duration", -// "value": "1.212s" -// } -// -message Any { - // A URL/resource name whose content describes the type of the - // serialized protocol buffer message. - // - // For URLs which use the scheme `http`, `https`, or no scheme, the - // following restrictions and interpretations apply: - // - // * If no scheme is provided, `https` is assumed. - // * The last segment of the URL's path must represent the fully - // qualified name of the type (as in `path/google.protobuf.Duration`). - // The name should be in a canonical form (e.g., leading "." is - // not accepted). - // * An HTTP GET on the URL must yield a [google.protobuf.Type][] - // value in binary format, or produce an error. - // * Applications are allowed to cache lookup results based on the - // URL, or have them precompiled into a binary to avoid any - // lookup. Therefore, binary compatibility needs to be preserved - // on changes to types. (Use versioned type names to manage - // breaking changes.) - // - // Schemes other than `http`, `https` (or the empty scheme) might be - // used with implementation specific semantics. - // - string type_url = 1; - - // Must be a valid serialized protocol buffer of the above specified type. - bytes value = 2; -} diff --git a/tools/grpcz/empty.proto b/tools/grpcz/empty.proto deleted file mode 100644 index 03cacd23308..00000000000 --- a/tools/grpcz/empty.proto +++ /dev/null @@ -1,52 +0,0 @@ -// Protocol Buffers - Google's data interchange format -// Copyright 2008 Google Inc. All rights reserved. -// https://developers.google.com/protocol-buffers/ -// -// Redistribution and use in source and binary forms, with or without -// modification, are permitted provided that the following conditions are -// met: -// -// * Redistributions of source code must retain the above copyright -// notice, this list of conditions and the following disclaimer. -// * Redistributions in binary form must reproduce the above -// copyright notice, this list of conditions and the following disclaimer -// in the documentation and/or other materials provided with the -// distribution. -// * Neither the name of Google Inc. nor the names of its -// contributors may be used to endorse or promote products derived from -// this software without specific prior written permission. -// -// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS -// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT -// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR -// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT -// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, -// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT -// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, -// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY -// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT -// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE -// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. - -syntax = "proto3"; - -package google.protobuf; - -option csharp_namespace = "Google.Protobuf.WellKnownTypes"; -option go_package = "github.com/golang/protobuf/ptypes/empty"; -option java_package = "com.google.protobuf"; -option java_outer_classname = "EmptyProto"; -option java_multiple_files = true; -option objc_class_prefix = "GPB"; -option cc_enable_arenas = true; - -// A generic empty message that you can re-use to avoid defining duplicated -// empty messages in your APIs. A typical example is to use it as the request -// or the response type of an API method. For instance: -// -// service Foo { -// rpc Bar(google.protobuf.Empty) returns (google.protobuf.Empty); -// } -// -// The JSON representation for `Empty` is empty JSON object `{}`. -message Empty {} diff --git a/tools/grpcz/grpcz_client.cc b/tools/grpcz/grpcz_client.cc index afca3b0532f..f4611466c53 100644 --- a/tools/grpcz/grpcz_client.cc +++ b/tools/grpcz/grpcz_client.cc @@ -46,11 +46,14 @@ #include "tools/grpcz/census.grpc.pb.h" #include "tools/grpcz/monitoring.grpc.pb.h" -DEFINE_string(server, "127.0.0.1:50052", - "file path (or host:port) where grpcz server is running"); +DEFINE_string( + grpcz_server, "127.0.0.1:8080", + "Unix domain socket path (e.g. unix://tmp/grpcz.sock) or IP address" + "(host:port) where grpcz server is running."); DEFINE_string(http_port, "8000", "Port id for accessing the HTTP server that renders /grpcz page"); -DEFINE_bool(print, false, "only print the output and quit"); +DEFINE_bool(print_to_console, false, + "print the JSON retreived from grpcz server and quit"); using grpc::Channel; using grpc::ClientContext; @@ -68,11 +71,14 @@ table, td, th { border: 1px solid black; } \ static const std::string static_html_footer = "' class='hidden'>\ -

GRPCZ FTW

\ +

GRPCZ Statistics

\ "; class GrpczClient { @@ -106,11 +113,9 @@ class GrpczClient { return json_str; } else { static const std::string error_message_json = - "{\"grpcz Access Error\"\ - :{\"view\":{\"viewName\":\"grpcz Access Error\",\ - \"intervalView\":\"Server not running?\"}}}"; - std::cout << status.error_code() << ":= " << status.error_message() - << std::endl; + "{\"Error Message\":\"" + status.error_message() + "\"}"; + gpr_log(GPR_DEBUG, "%d: %s", status.error_code(), + status.error_message().c_str()); return error_message_json; } } @@ -131,7 +136,7 @@ static void ev_handler(struct mg_connection *nc, int ev, void *p) { static void grpcz_handler(struct mg_connection *nc, int ev, void *ev_data) { (void)ev; (void)ev_data; - gpr_log(GPR_INFO, "fetching grpcz stats from %s", FLAGS_server.c_str()); + gpr_log(GPR_INFO, "fetching grpcz stats from %s", FLAGS_grpcz_server.c_str()); std::string json_str = g_grpcz_client->GetStatsAsJson(); std::string rendered_html = static_html_header + json_str + static_html_footer; @@ -143,10 +148,13 @@ int main(int argc, char **argv) { gflags::ParseCommandLineFlags(&argc, &argv, true); // Create a client - g_grpcz_client.reset(new GrpczClient( - grpc::CreateChannel(FLAGS_server, grpc::InsecureChannelCredentials()))); - if (FLAGS_print) { - g_grpcz_client->GetStatsAsJson(); + g_grpcz_client.reset(new GrpczClient(grpc::CreateChannel( + FLAGS_grpcz_server, grpc::InsecureChannelCredentials()))); + if (FLAGS_print_to_console) { + // using GPR_ERROR since this is the default verbosity. _DEBUG or _INFO + // won't print unless GRPC_VERBOSITY env var is set appropriately, which + // might confuse users of this utility. + gpr_log(GPR_ERROR, "%s\n", g_grpcz_client->GetStatsAsJson().c_str()); return 0; } @@ -160,14 +168,15 @@ int main(int argc, char **argv) { if (nc == NULL) { gpr_log(GPR_ERROR, "Failed to create listener on port %s\n", FLAGS_http_port.c_str()); - return -11; + return -1; } mg_register_http_endpoint(nc, "/grpcz", grpcz_handler); mg_set_protocol_http_websocket(nc); // Poll in a loop and serve /grpcz pages for (;;) { - mg_mgr_poll(&mgr, 100); + static const int k_sleep_millis = 100; + mg_mgr_poll(&mgr, k_sleep_millis); } mg_mgr_free(&mgr); return 0; diff --git a/tools/grpcz/monitoring.proto b/tools/grpcz/monitoring.proto index ec3dfe9f74c..4d09aeb6f32 100644 --- a/tools/grpcz/monitoring.proto +++ b/tools/grpcz/monitoring.proto @@ -34,8 +34,8 @@ syntax = "proto3"; // TODO(ericgribkoff) Figure out how to manage the external Census proto // dependency. import "tools/grpcz/census.proto"; -import "tools/grpcz/empty.proto"; -import "tools/grpcz/any.proto"; +import "google/protobuf/empty.proto"; +import "google/protobuf/any.proto"; package grpc.instrumentation.v1alpha; @@ -126,6 +126,6 @@ message MonitoringDataGroup { // The wrapper for custom monitoring data. message CustomMonitoringData { // can be any application specific monitoring data - Any contents = 1; + google.protobuf.Any contents = 1; } From 25e78e3213135673c368a59f159837525f118e8e Mon Sep 17 00:00:00 2001 From: Makarand Dharmapurikar Date: Fri, 17 Mar 2017 11:13:09 -0700 Subject: [PATCH 29/44] removed unnecessary includes --- tools/grpcz/grpcz_client.cc | 2 -- 1 file changed, 2 deletions(-) diff --git a/tools/grpcz/grpcz_client.cc b/tools/grpcz/grpcz_client.cc index f4611466c53..0f1432722a1 100644 --- a/tools/grpcz/grpcz_client.cc +++ b/tools/grpcz/grpcz_client.cc @@ -31,8 +31,6 @@ * */ -#include -#include #include #include From b39eeac79e8662922b38c9d72cad81a7fe47b714 Mon Sep 17 00:00:00 2001 From: Yuxuan Li Date: Mon, 20 Mar 2017 12:00:34 -0700 Subject: [PATCH 30/44] merge init_ops, writes_done_ops and write_ops --- include/grpc++/impl/codegen/async_stream.h | 118 +++++++-------------- 1 file changed, 40 insertions(+), 78 deletions(-) diff --git a/include/grpc++/impl/codegen/async_stream.h b/include/grpc++/impl/codegen/async_stream.h index a2013415902..f5fbb53c814 100644 --- a/include/grpc++/impl/codegen/async_stream.h +++ b/include/grpc++/impl/codegen/async_stream.h @@ -222,10 +222,10 @@ class ClientAsyncWriter final : public ClientAsyncWriterInterface { write_ops_.SendInitialMetadata(context->send_initial_metadata_, context->initial_metadata_flags()); } else { - init_ops_.set_output_tag(tag); - init_ops_.SendInitialMetadata(context->send_initial_metadata_, - context->initial_metadata_flags()); - call_.PerformOps(&init_ops_); + write_ops_.set_output_tag(tag); + write_ops_.SendInitialMetadata(context->send_initial_metadata_, + context->initial_metadata_flags()); + call_.PerformOps(&write_ops_); } } @@ -256,9 +256,9 @@ class ClientAsyncWriter final : public ClientAsyncWriterInterface { } void WritesDone(void* tag) override { - writes_done_ops_.set_output_tag(tag); - writes_done_ops_.ClientSendClose(); - call_.PerformOps(&writes_done_ops_); + write_ops_.set_output_tag(tag); + write_ops_.ClientSendClose(); + call_.PerformOps(&write_ops_); } void Finish(Status* status, void* tag) override { @@ -273,11 +273,9 @@ class ClientAsyncWriter final : public ClientAsyncWriterInterface { private: ClientContext* context_; Call call_; - CallOpSet init_ops_; CallOpSet meta_ops_; CallOpSet write_ops_; - CallOpSet writes_done_ops_; CallOpSet finish_ops_; @@ -310,10 +308,10 @@ class ClientAsyncReaderWriter final write_ops_.SendInitialMetadata(context->send_initial_metadata_, context->initial_metadata_flags()); } else { - init_ops_.set_output_tag(tag); - init_ops_.SendInitialMetadata(context->send_initial_metadata_, - context->initial_metadata_flags()); - call_.PerformOps(&init_ops_); + write_ops_.set_output_tag(tag); + write_ops_.SendInitialMetadata(context->send_initial_metadata_, + context->initial_metadata_flags()); + call_.PerformOps(&write_ops_); } } @@ -370,7 +368,6 @@ class ClientAsyncReaderWriter final private: ClientContext* context_; Call call_; - CallOpSet init_ops_; CallOpSet meta_ops_; CallOpSet> read_ops_; CallOpSet @@ -454,9 +451,7 @@ class ServerAsyncReader final : public ServerAsyncReaderInterface { ServerContext* ctx_; CallOpSet meta_ops_; CallOpSet> read_ops_; - CallOpSet - finish_ops_; + CallOpSet finish_ops_; }; template @@ -499,16 +494,20 @@ class ServerAsyncWriter final : public ServerAsyncWriterInterface { call_.PerformOps(&meta_ops_); } - void Write(const W& msg, void* tag) override { - write_ops_.set_output_tag(tag); + void EnsureInitialMetadataSent(CallOpSetInterface* ops) { if (!ctx_->sent_initial_metadata_) { - write_ops_.SendInitialMetadata(ctx_->initial_metadata_, - ctx_->initial_metadata_flags()); + ops.SendInitialMetadata(ctx_->initial_metadata_, + ctx_->initial_metadata_flags()); if (ctx_->compression_level_set()) { - write_ops_.set_compression_level(ctx_->compression_level()); + ops.set_compression_level(ctx_->compression_level()); } ctx_->sent_initial_metadata_ = true; } + } + + void Write(const W& msg, void* tag) override { + write_ops_.set_output_tag(tag); + EnsureInitialMetadataSent(&write_ops_); // TODO(ctiller): don't assert GPR_CODEGEN_ASSERT(write_ops_.SendMessage(msg).ok()); call_.PerformOps(&write_ops_); @@ -520,14 +519,7 @@ class ServerAsyncWriter final : public ServerAsyncWriterInterface { options.set_buffer_hint(); } - if (!ctx_->sent_initial_metadata_) { - write_ops_.SendInitialMetadata(ctx_->initial_metadata_, - ctx_->initial_metadata_flags()); - if (ctx_->compression_level_set()) { - write_ops_.set_compression_level(ctx_->compression_level()); - } - ctx_->sent_initial_metadata_ = true; - } + EnsureInitialMetadataSent(&write_ops_); // TODO(ctiller): don't assert GPR_CODEGEN_ASSERT(write_ops_.SendMessage(msg, options).ok()); call_.PerformOps(&write_ops_); @@ -536,14 +528,7 @@ class ServerAsyncWriter final : public ServerAsyncWriterInterface { void WriteAndFinish(const W& msg, WriteOptions options, const Status& status, void* tag) override { write_ops_.set_output_tag(tag); - if (!ctx_->sent_initial_metadata_) { - write_ops_.SendInitialMetadata(ctx_->initial_metadata_, - ctx_->initial_metadata_flags()); - if (ctx_->compression_level_set()) { - write_ops_.set_compression_level(ctx_->compression_level()); - } - ctx_->sent_initial_metadata_ = true; - } + EnsureInitialMetadataSent(&write_ops_); options.set_buffer_hint(); GPR_CODEGEN_ASSERT(write_ops_.SendMessage(msg, options).ok()); write_ops_.ServerSendStatus(ctx_->trailing_metadata_, status); @@ -552,14 +537,7 @@ class ServerAsyncWriter final : public ServerAsyncWriterInterface { void Finish(const Status& status, void* tag) override { finish_ops_.set_output_tag(tag); - if (!ctx_->sent_initial_metadata_) { - finish_ops_.SendInitialMetadata(ctx_->initial_metadata_, - ctx_->initial_metadata_flags()); - if (ctx_->compression_level_set()) { - finish_ops_.set_compression_level(ctx_->compression_level()); - } - ctx_->sent_initial_metadata_ = true; - } + EnsureInitialMetadataSent(&finish_ops_); finish_ops_.ServerSendStatus(ctx_->trailing_metadata_, status); call_.PerformOps(&finish_ops_); } @@ -619,6 +597,17 @@ class ServerAsyncReaderWriter final call_.PerformOps(&meta_ops_); } + void EnsureInitialMetadataSent(CallOpSetInterface* ops) { + if (!ctx_->sent_initial_metadata_) { + ops.SendInitialMetadata(ctx_->initial_metadata_, + ctx_->initial_metadata_flags()); + if (ctx_->compression_level_set()) { + ops.set_compression_level(ctx_->compression_level()); + } + ctx_->sent_initial_metadata_ = true; + } + } + void Read(R* msg, void* tag) override { read_ops_.set_output_tag(tag); read_ops_.RecvMessage(msg); @@ -627,14 +616,7 @@ class ServerAsyncReaderWriter final void Write(const W& msg, void* tag) override { write_ops_.set_output_tag(tag); - if (!ctx_->sent_initial_metadata_) { - write_ops_.SendInitialMetadata(ctx_->initial_metadata_, - ctx_->initial_metadata_flags()); - if (ctx_->compression_level_set()) { - write_ops_.set_compression_level(ctx_->compression_level()); - } - ctx_->sent_initial_metadata_ = true; - } + EnsureInitialMetadataSent(&write_ops_); // TODO(ctiller): don't assert GPR_CODEGEN_ASSERT(write_ops_.SendMessage(msg).ok()); call_.PerformOps(&write_ops_); @@ -645,14 +627,7 @@ class ServerAsyncReaderWriter final if (options.is_last_message()) { options.set_buffer_hint(); } - if (!ctx_->sent_initial_metadata_) { - write_ops_.SendInitialMetadata(ctx_->initial_metadata_, - ctx_->initial_metadata_flags()); - if (ctx_->compression_level_set()) { - write_ops_.set_compression_level(ctx_->compression_level()); - } - ctx_->sent_initial_metadata_ = true; - } + EnsureInitialMetadataSent(&write_ops_); GPR_CODEGEN_ASSERT(write_ops_.SendMessage(msg, options).ok()); call_.PerformOps(&write_ops_); } @@ -660,14 +635,7 @@ class ServerAsyncReaderWriter final void WriteAndFinish(const W& msg, WriteOptions options, const Status& status, void* tag) override { write_ops_.set_output_tag(tag); - if (!ctx_->sent_initial_metadata_) { - write_ops_.SendInitialMetadata(ctx_->initial_metadata_, - ctx_->initial_metadata_flags()); - if (ctx_->compression_level_set()) { - write_ops_.set_compression_level(ctx_->compression_level()); - } - ctx_->sent_initial_metadata_ = true; - } + EnsureInitialMetadataSent(&write_ops_); options.set_buffer_hint(); GPR_CODEGEN_ASSERT(write_ops_.SendMessage(msg, options).ok()); write_ops_.ServerSendStatus(ctx_->trailing_metadata_, status); @@ -676,14 +644,8 @@ class ServerAsyncReaderWriter final void Finish(const Status& status, void* tag) override { finish_ops_.set_output_tag(tag); - if (!ctx_->sent_initial_metadata_) { - finish_ops_.SendInitialMetadata(ctx_->initial_metadata_, - ctx_->initial_metadata_flags()); - if (ctx_->compression_level_set()) { - finish_ops_.set_compression_level(ctx_->compression_level()); - } - ctx_->sent_initial_metadata_ = true; - } + EnsureInitialMetadataSent(&finish_ops_); + finish_ops_.ServerSendStatus(ctx_->trailing_metadata_, status); call_.PerformOps(&finish_ops_); } From 8d00f52d5b090585f4c66af0d17d359e5893ee5b Mon Sep 17 00:00:00 2001 From: Yuxuan Li Date: Mon, 20 Mar 2017 12:35:38 -0700 Subject: [PATCH 31/44] fix EnsureInitialMetadataSent type casting --- include/grpc++/impl/codegen/async_stream.h | 24 ++++++++++++++-------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/include/grpc++/impl/codegen/async_stream.h b/include/grpc++/impl/codegen/async_stream.h index f5fbb53c814..c67e5e22479 100644 --- a/include/grpc++/impl/codegen/async_stream.h +++ b/include/grpc++/impl/codegen/async_stream.h @@ -451,7 +451,9 @@ class ServerAsyncReader final : public ServerAsyncReaderInterface { ServerContext* ctx_; CallOpSet meta_ops_; CallOpSet> read_ops_; - CallOpSet finish_ops_; + CallOpSet + finish_ops_; }; template @@ -494,12 +496,14 @@ class ServerAsyncWriter final : public ServerAsyncWriterInterface { call_.PerformOps(&meta_ops_); } - void EnsureInitialMetadataSent(CallOpSetInterface* ops) { + void EnsureInitialMetadataSent(CallOpSetInterface* ops_in) { + CallOpSet* ops = + static_cast*>(ops_in); if (!ctx_->sent_initial_metadata_) { - ops.SendInitialMetadata(ctx_->initial_metadata_, - ctx_->initial_metadata_flags()); + ops->SendInitialMetadata(ctx_->initial_metadata_, + ctx_->initial_metadata_flags()); if (ctx_->compression_level_set()) { - ops.set_compression_level(ctx_->compression_level()); + ops->set_compression_level(ctx_->compression_level()); } ctx_->sent_initial_metadata_ = true; } @@ -597,12 +601,14 @@ class ServerAsyncReaderWriter final call_.PerformOps(&meta_ops_); } - void EnsureInitialMetadataSent(CallOpSetInterface* ops) { + void EnsureInitialMetadataSent(CallOpSetInterface* ops_in) { + CallOpSet* ops = + static_cast*>(ops_in); if (!ctx_->sent_initial_metadata_) { - ops.SendInitialMetadata(ctx_->initial_metadata_, - ctx_->initial_metadata_flags()); + ops->SendInitialMetadata(ctx_->initial_metadata_, + ctx_->initial_metadata_flags()); if (ctx_->compression_level_set()) { - ops.set_compression_level(ctx_->compression_level()); + ops->set_compression_level(ctx_->compression_level()); } ctx_->sent_initial_metadata_ = true; } From 2c19723e66d3f704e17454da62c680c7069b94d3 Mon Sep 17 00:00:00 2001 From: Makarand Dharmapurikar Date: Mon, 20 Mar 2017 16:00:15 -0700 Subject: [PATCH 32/44] updated monitoring.proto Also changed the name of stats attr to 'data-stats' to be more html5 compliant. --- tools/grpcz/grpcz_client.cc | 4 +- tools/grpcz/monitoring.proto | 85 +++++++++++++++++++++++------------- 2 files changed, 57 insertions(+), 32 deletions(-) diff --git a/tools/grpcz/grpcz_client.cc b/tools/grpcz/grpcz_client.cc index 0f1432722a1..47eec8dfc38 100644 --- a/tools/grpcz/grpcz_client.cc +++ b/tools/grpcz/grpcz_client.cc @@ -65,14 +65,14 @@ static const std::string static_html_header = table { border-collapse: collapse; width: 100%; } \ table, td, th { border: 1px solid black; } \ \ -\

GRPCZ Statistics

\