From e1523e95c102a3eec48fef34350bca206c0a6546 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Tue, 14 Mar 2017 21:10:42 -0700 Subject: [PATCH 01/34] Track calls to gpr_now() --- src/core/lib/support/time_posix.c | 8 ++++++++ test/cpp/microbenchmarks/bm_call_create.cc | 2 ++ test/cpp/microbenchmarks/helpers.cc | 4 ++++ test/cpp/microbenchmarks/helpers.h | 3 +++ tools/profiling/microbenchmarks/bm2bq.py | 2 +- 5 files changed, 18 insertions(+), 1 deletion(-) diff --git a/src/core/lib/support/time_posix.c b/src/core/lib/support/time_posix.c index a69c501e9fb..9bfec7782a2 100644 --- a/src/core/lib/support/time_posix.c +++ b/src/core/lib/support/time_posix.c @@ -42,6 +42,7 @@ #ifdef __linux__ #include #endif +#include #include #include #include "src/core/lib/support/block_annotate.h" @@ -144,7 +145,14 @@ static gpr_timespec now_impl(gpr_clock_type clock) { gpr_timespec (*gpr_now_impl)(gpr_clock_type clock_type) = now_impl; +#ifdef GPR_LOW_LEVEL_COUNTERS +gpr_atm gpr_now_call_count; +#endif + gpr_timespec gpr_now(gpr_clock_type clock_type) { +#ifdef GPR_LOW_LEVEL_COUNTERS + __atomic_fetch_add(&gpr_now_call_count, 1, __ATOMIC_RELAXED); +#endif return gpr_now_impl(clock_type); } diff --git a/test/cpp/microbenchmarks/bm_call_create.cc b/test/cpp/microbenchmarks/bm_call_create.cc index 014e2b96b55..146f8e6bad3 100644 --- a/test/cpp/microbenchmarks/bm_call_create.cc +++ b/test/cpp/microbenchmarks/bm_call_create.cc @@ -66,10 +66,12 @@ auto &force_library_initialization = Library::get(); void BM_Zalloc(benchmark::State &state) { // speed of light for call creation is zalloc, so benchmark a few interesting // sizes + TrackCounters track_counters; size_t sz = state.range(0); while (state.KeepRunning()) { gpr_free(gpr_zalloc(sz)); } + track_counters.Finish(state); } BENCHMARK(BM_Zalloc) ->Arg(64) diff --git a/test/cpp/microbenchmarks/helpers.cc b/test/cpp/microbenchmarks/helpers.cc index d277c5984cd..6550742453a 100644 --- a/test/cpp/microbenchmarks/helpers.cc +++ b/test/cpp/microbenchmarks/helpers.cc @@ -57,6 +57,10 @@ void TrackCounters::AddToLabel(std::ostream &out, benchmark::State &state) { << ((double)(gpr_atm_no_barrier_load(&gpr_counter_atm_add) - atm_add_at_start_) / (double)state.iterations()) + << " nows/iter:" + << ((double)(gpr_atm_no_barrier_load(&gpr_now_call_count) - + now_calls_at_start_) / + (double)state.iterations()) << " allocs/iter:" << ((double)(counters_at_end.total_allocs_absolute - counters_at_start_.total_allocs_absolute) / diff --git a/test/cpp/microbenchmarks/helpers.h b/test/cpp/microbenchmarks/helpers.h index f44b7cf83a7..af401dd7bbe 100644 --- a/test/cpp/microbenchmarks/helpers.h +++ b/test/cpp/microbenchmarks/helpers.h @@ -72,6 +72,7 @@ class Library { extern "C" gpr_atm gpr_mu_locks; extern "C" gpr_atm gpr_counter_atm_cas; extern "C" gpr_atm gpr_counter_atm_add; +extern "C" gpr_atm gpr_now_call_count; #endif class TrackCounters { @@ -86,6 +87,8 @@ class TrackCounters { gpr_atm_no_barrier_load(&gpr_counter_atm_cas); const size_t atm_add_at_start_ = gpr_atm_no_barrier_load(&gpr_counter_atm_add); + const size_t now_calls_at_start_ = + gpr_atm_no_barrier_load(&gpr_now_call_count); grpc_memory_counters counters_at_start_ = grpc_memory_counters_snapshot(); #endif }; diff --git a/tools/profiling/microbenchmarks/bm2bq.py b/tools/profiling/microbenchmarks/bm2bq.py index ffb11f57d8f..99cf4a558cf 100755 --- a/tools/profiling/microbenchmarks/bm2bq.py +++ b/tools/profiling/microbenchmarks/bm2bq.py @@ -71,6 +71,7 @@ columns = [ ('end_of_stream', 'boolean'), ('header_bytes_per_iteration', 'float'), ('framing_bytes_per_iteration', 'float'), + ('nows_per_iteration', 'float'), ] SANITIZE = { @@ -103,4 +104,3 @@ for row in bm_json.expand_json(js, js2): if row[name] == '': continue sane_row[name] = SANITIZE[sql_type](row[name]) writer.writerow(sane_row) - From 895f3d83da12664e59379d9ce6cccb8ab9f244b7 Mon Sep 17 00:00:00 2001 From: Muxi Yan Date: Wed, 5 Apr 2017 13:12:30 -0700 Subject: [PATCH 02/34] Support configuring dispatch queue in GRPCCall and below --- src/objective-c/GRPCClient/GRPCCall.h | 7 +++++++ src/objective-c/GRPCClient/GRPCCall.m | 16 +++++++++++++++- .../RxLibrary/GRXConcurrentWriteable.h | 4 +++- .../RxLibrary/GRXConcurrentWriteable.m | 10 ++++++++-- 4 files changed, 33 insertions(+), 4 deletions(-) diff --git a/src/objective-c/GRPCClient/GRPCCall.h b/src/objective-c/GRPCClient/GRPCCall.h index 7645bb1d34a..85c5b47c8ea 100644 --- a/src/objective-c/GRPCClient/GRPCCall.h +++ b/src/objective-c/GRPCClient/GRPCCall.h @@ -253,6 +253,13 @@ extern id const kGRPCTrailersKey; */ + (void)setCallSafety:(GRPCCallSafety)callSafety host:(NSString *)host path:(NSString *)path; +/** + * Set the dispatch queue to be used for queue responses. + * + * This configuration is only effective before the call starts. + */ +- (void)setResponseDispatchQueue:(dispatch_queue_t)queue; + // TODO(jcanizales): Let specify a deadline. As a category of GRXWriter? @end diff --git a/src/objective-c/GRPCClient/GRPCCall.m b/src/objective-c/GRPCClient/GRPCCall.m index 051138ea4da..f9d13fea578 100644 --- a/src/objective-c/GRPCClient/GRPCCall.m +++ b/src/objective-c/GRPCClient/GRPCCall.m @@ -113,6 +113,10 @@ static NSMutableDictionary *callFlags; // the SendClose op is added. BOOL _unaryCall; NSMutableArray *_unaryOpBatch; + + // The dispatch queue to be used for enqueuing responses to user. Defaulted to the main dispatch + // queue + dispatch_queue_t _responseQueue; } @synthesize state = _state; @@ -175,10 +179,19 @@ static NSMutableDictionary *callFlags; _unaryCall = YES; _unaryOpBatch = [NSMutableArray arrayWithCapacity:kMaxClientBatch]; } + + _responseQueue = dispatch_get_main_queue(); } return self; } +- (void)setResponseDispatchQueue:(dispatch_queue_t)queue { + if (_state != GRXWriterStateNotStarted) { + return; + } + _responseQueue = queue; +} + #pragma mark Finish - (void)finishWithError:(NSError *)errorOrNil { @@ -424,7 +437,8 @@ static NSMutableDictionary *callFlags; // that the life of the instance is determined by this retain cycle. _retainSelf = self; - _responseWriteable = [[GRXConcurrentWriteable alloc] initWithWriteable:writeable]; + _responseWriteable = [[GRXConcurrentWriteable alloc] initWithWriteable:writeable + dispatchQueue:_responseQueue]; _wrappedCall = [[GRPCWrappedCall alloc] initWithHost:_host path:_path]; NSAssert(_wrappedCall, @"Error allocating RPC objects. Low memory?"); diff --git a/src/objective-c/RxLibrary/GRXConcurrentWriteable.h b/src/objective-c/RxLibrary/GRXConcurrentWriteable.h index b2775f98b56..07004f6d4dc 100644 --- a/src/objective-c/RxLibrary/GRXConcurrentWriteable.h +++ b/src/objective-c/RxLibrary/GRXConcurrentWriteable.h @@ -53,7 +53,9 @@ * The GRXWriteable instance is retained until writesFinishedWithError: is sent to it, and released * after that. */ -- (instancetype)initWithWriteable:(id)writeable NS_DESIGNATED_INITIALIZER; +- (instancetype)initWithWriteable:(id)writeable + dispatchQueue:(dispatch_queue_t)queue NS_DESIGNATED_INITIALIZER; +- (instancetype)initWithWriteable:(id)writeable; /** * Enqueues writeValue: to be sent to the writeable in the main thread. diff --git a/src/objective-c/RxLibrary/GRXConcurrentWriteable.m b/src/objective-c/RxLibrary/GRXConcurrentWriteable.m index 08bd079aea5..88aa7a7282f 100644 --- a/src/objective-c/RxLibrary/GRXConcurrentWriteable.m +++ b/src/objective-c/RxLibrary/GRXConcurrentWriteable.m @@ -51,14 +51,20 @@ } // Designated initializer -- (instancetype)initWithWriteable:(id)writeable { +- (instancetype)initWithWriteable:(id)writeable + dispatchQueue:(dispatch_queue_t)queue { if (self = [super init]) { - _writeableQueue = dispatch_get_main_queue(); + _writeableQueue = queue; _writeable = writeable; } return self; } +- (instancetype)initWithWriteable:(id)writeable { + return [self initWithWriteable:writeable + dispatchQueue:dispatch_get_main_queue()]; +} + - (void)enqueueValue:(id)value completionHandler:(void (^)())handler { dispatch_async(_writeableQueue, ^{ // We're racing a possible cancellation performed by another thread. To turn all already- From 56369ea4a2c785747d656943f469b461085c3d8d Mon Sep 17 00:00:00 2001 From: Muxi Yan Date: Thu, 6 Apr 2017 11:52:43 -0700 Subject: [PATCH 03/34] Support in ProtoRPC --- src/compiler/objective_c_generator.cc | 27 +++++++++++++++++++-------- src/compiler/objective_c_plugin.cc | 1 + 2 files changed, 20 insertions(+), 8 deletions(-) diff --git a/src/compiler/objective_c_generator.cc b/src/compiler/objective_c_generator.cc index 1d7faf120dc..03a0762a38b 100644 --- a/src/compiler/objective_c_generator.cc +++ b/src/compiler/objective_c_generator.cc @@ -149,17 +149,19 @@ void PrintMethodDeclarations(Printer *printer, const MethodDescriptor *method) { void PrintSimpleImplementation(Printer *printer, const MethodDescriptor *method, map< ::grpc::string, ::grpc::string> vars) { printer->Print("{\n"); - printer->Print(vars, " [[self RPCTo$method_name$With"); + printer->Print(vars, " GRPCProtoCall *rpc = [self RPCTo$method_name$With"); if (method->client_streaming()) { printer->Print("RequestsWriter:requestWriter"); } else { printer->Print("Request:request"); } if (method->server_streaming()) { - printer->Print(" eventHandler:eventHandler] start];\n"); + printer->Print(" eventHandler:eventHandler];\n"); } else { - printer->Print(" handler:handler] start];\n"); + printer->Print(" handler:handler];\n"); } + printer->Print(" [rpc setResponseDispatchQueue:_defaultResponseDispatchQueue];\n"); + printer->Print(" [rpc start];\n"); printer->Print("}\n"); } @@ -167,23 +169,25 @@ void PrintAdvancedImplementation(Printer *printer, const MethodDescriptor *method, map< ::grpc::string, ::grpc::string> vars) { printer->Print("{\n"); - printer->Print(vars, " return [self RPCToMethod:@\"$method_name$\"\n"); + printer->Print(vars, " GRPCProtoCall *rpc = [self RPCToMethod:@\"$method_name$\"\n"); - printer->Print(" requestsWriter:"); + printer->Print(" requestsWriter:"); if (method->client_streaming()) { printer->Print("requestWriter\n"); } else { printer->Print("[GRXWriter writerWithValue:request]\n"); } - printer->Print(vars, " responseClass:[$response_class$ class]\n"); + printer->Print(vars, " responseClass:[$response_class$ class]\n"); - printer->Print(" responsesWriteable:[GRXWriteable "); + printer->Print(" responsesWriteable:[GRXWriteable "); if (method->server_streaming()) { printer->Print("writeableWithEventHandler:eventHandler]];\n"); } else { printer->Print("writeableWithSingleHandler:handler]];\n"); } + printer->Print(" [rpc setResponseDispatchQueue:_defaultResponseDispatchQueue];\n"); + printer->Print(" return rpc;\n"); printer->Print("}\n"); } @@ -234,6 +238,7 @@ void PrintMethodImplementations(Printer *printer, "- (instancetype)initWithHost:(NSString *)host" " NS_DESIGNATED_INITIALIZER;\n"); printer.Print("+ (instancetype)serviceWithHost:(NSString *)host;\n"); + printer.Print("- (void)setDefaultResponseDispatchQueue:(dispatch_queue_t)queue;\n"); printer.Print("@end\n"); } return output; @@ -251,12 +256,15 @@ void PrintMethodImplementations(Printer *printer, {"service_class", ServiceClassName(service)}, {"package", service->file()->package()}}; - printer.Print(vars, "@implementation $service_class$\n\n"); + printer.Print(vars, "@implementation $service_class$ {\n"); + printer.Print(vars, " dispatch_queue_t _defaultResponseDispatchQueue;\n"); + printer.Print(vars, "}\n\n"); printer.Print("// Designated initializer\n"); printer.Print("- (instancetype)initWithHost:(NSString *)host {\n"); printer.Print( vars, + " _defaultResponseDispatchQueue = dispatch_get_main_queue();\n" " return (self = [super initWithHost:host" " packageName:@\"$package$\" serviceName:@\"$service_name$\"]);\n"); printer.Print("}\n\n"); @@ -270,6 +278,9 @@ void PrintMethodImplementations(Printer *printer, printer.Print("}\n\n"); printer.Print("+ (instancetype)serviceWithHost:(NSString *)host {\n"); printer.Print(" return [[self alloc] initWithHost:host];\n"); + printer.Print("}\n\n"); + printer.Print("- (void)setDefaultResponseDispatchQueue:(dispatch_queue_t)queue {\n"); + printer.Print(" _defaultResponseDispatchQueue = queue;\n"); printer.Print("}\n\n\n"); for (int i = 0; i < service->method_count(); i++) { diff --git a/src/compiler/objective_c_plugin.cc b/src/compiler/objective_c_plugin.cc index 8de0997ebea..5178115e44c 100644 --- a/src/compiler/objective_c_plugin.cc +++ b/src/compiler/objective_c_plugin.cc @@ -68,6 +68,7 @@ class ObjectiveCGrpcGenerator : public grpc::protobuf::compiler::CodeGenerator { ::grpc::string imports = ::grpc::string("#import \"") + file_name + ".pbobjc.h\"\n\n" "#import \n" + "#import \n" "#import \n" "#import \n"; From e1ed7712102363d5e6fdad31b1016119f95f0631 Mon Sep 17 00:00:00 2001 From: Muxi Yan Date: Thu, 6 Apr 2017 11:52:55 -0700 Subject: [PATCH 04/34] Add test --- src/objective-c/tests/InteropTests.m | 95 ++++++++++++++++++++++++++++ 1 file changed, 95 insertions(+) diff --git a/src/objective-c/tests/InteropTests.m b/src/objective-c/tests/InteropTests.m index 91053568690..4466d6810cc 100644 --- a/src/objective-c/tests/InteropTests.m +++ b/src/objective-c/tests/InteropTests.m @@ -457,4 +457,99 @@ [self waitForExpectationsWithTimeout:TEST_TIMEOUT handler:nil]; } +- (void)testAlternateDispatchQueue { + XCTAssertNotNil(self.class.host); + __weak XCTestExpectation *expectation1 = [self expectationWithDescription:@"AlternateDispatchQueue1"]; + + NSArray *requests = @[@27182, @8, @1828, @45904]; + NSArray *responses = @[@31415, @9, @2653, @58979]; + + // Set the default dispatch queue + NSString *queue1_label = @"test.queue1"; + NSString *queue2_label = @"test.queue2"; + dispatch_queue_t queue1 = dispatch_queue_create([queue1_label UTF8String], DISPATCH_QUEUE_SERIAL); + dispatch_queue_t queue2 = dispatch_queue_create([queue2_label UTF8String], DISPATCH_QUEUE_SERIAL); + [_service setDefaultResponseDispatchQueue:queue1]; + GRXBufferedPipe *requestsBuffer1 = [[GRXBufferedPipe alloc] init]; + + __block int index = 0; + + id request = [RMTStreamingOutputCallRequest messageWithPayloadSize:requests[index] + requestedResponseSize:responses[index]]; + [requestsBuffer1 writeValue:request]; + + [_service fullDuplexCallWithRequestsWriter:requestsBuffer1 + eventHandler:^(BOOL done, + RMTStreamingOutputCallResponse *response, + NSError *error) { + XCTAssertNil(error, @"Finished with unexpected error: %@", error); + XCTAssertTrue(done || response, @"Event handler called without an event."); + NSString *label = [NSString stringWithUTF8String:dispatch_queue_get_label(DISPATCH_CURRENT_QUEUE_LABEL)]; + XCTAssert([label isEqualToString:queue1_label]); + + if (response) { + XCTAssertLessThan(index, 4, @"More than 4 responses received."); + id expected = [RMTStreamingOutputCallResponse messageWithPayloadSize:responses[index]]; + XCTAssertEqualObjects(response, expected); + index += 1; + if (index < 4) { + id request = [RMTStreamingOutputCallRequest messageWithPayloadSize:requests[index] + requestedResponseSize:responses[index]]; + [requestsBuffer1 writeValue:request]; + } else { + [requestsBuffer1 writesFinishedWithError:nil]; + } + } + + if (done) { + XCTAssertEqual(index, 4, @"Received %i responses instead of 4.", index); + [expectation1 fulfill]; + } + }]; + + [self waitForExpectationsWithTimeout:TEST_TIMEOUT handler:nil]; + + // Test overriding default queue with another queue + __weak XCTestExpectation *expectation2 = [self expectationWithDescription:@"AlternateDispatchQueue2"]; + GRXBufferedPipe *requestsBuffer2 = [[GRXBufferedPipe alloc] init]; + + index = 0; + + [requestsBuffer2 writeValue:request]; + + GRPCProtoCall *rpc = [_service RPCToFullDuplexCallWithRequestsWriter:requestsBuffer2 + eventHandler:^(BOOL done, + RMTStreamingOutputCallResponse *response, + NSError *error) { + XCTAssertNil(error, @"Finished with unexpected error: %@", error); + XCTAssertTrue(done || response, @"Event handler called without an event."); + NSString *label = [NSString stringWithUTF8String:dispatch_queue_get_label(DISPATCH_CURRENT_QUEUE_LABEL)]; + XCTAssert([label isEqualToString:queue2_label]); + + if (response) { + XCTAssertLessThan(index, 4, @"More than 4 responses received."); + id expected = [RMTStreamingOutputCallResponse messageWithPayloadSize:responses[index]]; + XCTAssertEqualObjects(response, expected); + index += 1; + if (index < 4) { + id request = [RMTStreamingOutputCallRequest messageWithPayloadSize:requests[index] + requestedResponseSize:responses[index]]; + [requestsBuffer2 writeValue:request]; + } else { + [requestsBuffer2 writesFinishedWithError:nil]; + } + } + + if (done) { + XCTAssertEqual(index, 4, @"Received %i responses instead of 4.", index); + [expectation2 fulfill]; + } + }]; + [rpc setResponseDispatchQueue:queue2]; + [rpc start]; + + [self waitForExpectationsWithTimeout:TEST_TIMEOUT handler:nil]; + [_service setDefaultResponseDispatchQueue:dispatch_get_main_queue()]; +} + @end From 2f23be7e8c9a1342b759e5fbb73963a51ce1eddb Mon Sep 17 00:00:00 2001 From: Muxi Yan Date: Sun, 9 Apr 2017 15:57:31 -0700 Subject: [PATCH 05/34] clang-format --- src/compiler/objective_c_generator.cc | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/src/compiler/objective_c_generator.cc b/src/compiler/objective_c_generator.cc index 03a0762a38b..ba89b28b7b5 100644 --- a/src/compiler/objective_c_generator.cc +++ b/src/compiler/objective_c_generator.cc @@ -160,7 +160,8 @@ void PrintSimpleImplementation(Printer *printer, const MethodDescriptor *method, } else { printer->Print(" handler:handler];\n"); } - printer->Print(" [rpc setResponseDispatchQueue:_defaultResponseDispatchQueue];\n"); + printer->Print( + " [rpc setResponseDispatchQueue:_defaultResponseDispatchQueue];\n"); printer->Print(" [rpc start];\n"); printer->Print("}\n"); } @@ -169,7 +170,8 @@ void PrintAdvancedImplementation(Printer *printer, const MethodDescriptor *method, map< ::grpc::string, ::grpc::string> vars) { printer->Print("{\n"); - printer->Print(vars, " GRPCProtoCall *rpc = [self RPCToMethod:@\"$method_name$\"\n"); + printer->Print( + vars, " GRPCProtoCall *rpc = [self RPCToMethod:@\"$method_name$\"\n"); printer->Print(" requestsWriter:"); if (method->client_streaming()) { @@ -178,7 +180,9 @@ void PrintAdvancedImplementation(Printer *printer, printer->Print("[GRXWriter writerWithValue:request]\n"); } - printer->Print(vars, " responseClass:[$response_class$ class]\n"); + printer->Print( + vars, + " responseClass:[$response_class$ class]\n"); printer->Print(" responsesWriteable:[GRXWriteable "); if (method->server_streaming()) { @@ -186,7 +190,8 @@ void PrintAdvancedImplementation(Printer *printer, } else { printer->Print("writeableWithSingleHandler:handler]];\n"); } - printer->Print(" [rpc setResponseDispatchQueue:_defaultResponseDispatchQueue];\n"); + printer->Print( + " [rpc setResponseDispatchQueue:_defaultResponseDispatchQueue];\n"); printer->Print(" return rpc;\n"); printer->Print("}\n"); @@ -238,7 +243,8 @@ void PrintMethodImplementations(Printer *printer, "- (instancetype)initWithHost:(NSString *)host" " NS_DESIGNATED_INITIALIZER;\n"); printer.Print("+ (instancetype)serviceWithHost:(NSString *)host;\n"); - printer.Print("- (void)setDefaultResponseDispatchQueue:(dispatch_queue_t)queue;\n"); + printer.Print( + "- (void)setDefaultResponseDispatchQueue:(dispatch_queue_t)queue;\n"); printer.Print("@end\n"); } return output; @@ -279,7 +285,8 @@ void PrintMethodImplementations(Printer *printer, printer.Print("+ (instancetype)serviceWithHost:(NSString *)host {\n"); printer.Print(" return [[self alloc] initWithHost:host];\n"); printer.Print("}\n\n"); - printer.Print("- (void)setDefaultResponseDispatchQueue:(dispatch_queue_t)queue {\n"); + printer.Print( + "- (void)setDefaultResponseDispatchQueue:(dispatch_queue_t)queue {\n"); printer.Print(" _defaultResponseDispatchQueue = queue;\n"); printer.Print("}\n\n\n"); From e300670153d3c7cf62660c49c90cbfc6a63f3c0c Mon Sep 17 00:00:00 2001 From: "Mark D. Roth" Date: Wed, 19 Apr 2017 07:43:56 -0700 Subject: [PATCH 06/34] Simplify hash table API: remove per-entry vtable and take ownership of keys and values. --- .../filters/client_channel/client_channel.c | 9 +------- .../client_channel/lb_policy/grpclb/grpclb.c | 16 +++----------- .../message_size/message_size_filter.c | 11 +--------- src/core/lib/slice/slice_hash_table.c | 21 ++++++++++--------- src/core/lib/slice/slice_hash_table.h | 15 +++++-------- src/core/lib/transport/service_config.c | 21 +++++++------------ src/core/lib/transport/service_config.h | 4 ++-- 7 files changed, 31 insertions(+), 66 deletions(-) diff --git a/src/core/ext/filters/client_channel/client_channel.c b/src/core/ext/filters/client_channel/client_channel.c index ce9abdad610..8d28e829d81 100644 --- a/src/core/ext/filters/client_channel/client_channel.c +++ b/src/core/ext/filters/client_channel/client_channel.c @@ -96,17 +96,10 @@ static void method_parameters_unref(method_parameters *method_params) { } } -static void *method_parameters_copy(void *value) { - return method_parameters_ref(value); -} - static void method_parameters_free(grpc_exec_ctx *exec_ctx, void *value) { method_parameters_unref(value); } -static const grpc_slice_hash_table_vtable method_parameters_vtable = { - method_parameters_free, method_parameters_copy}; - static bool parse_wait_for_ready(grpc_json *field, wait_for_ready_value *wait_for_ready) { if (field->type != GRPC_JSON_TRUE && field->type != GRPC_JSON_FALSE) { @@ -472,7 +465,7 @@ static void on_resolver_result_changed_locked(grpc_exec_ctx *exec_ctx, grpc_uri_destroy(uri); method_params_table = grpc_service_config_create_method_config_table( exec_ctx, service_config, method_parameters_create_from_json, - &method_parameters_vtable); + method_parameters_free); grpc_service_config_destroy(service_config); } } diff --git a/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.c b/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.c index ff8d3193098..3fe3f056d33 100644 --- a/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.c +++ b/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.c @@ -750,18 +750,11 @@ static void destroy_balancer_name(grpc_exec_ctx *exec_ctx, gpr_free(balancer_name); } -static void *copy_balancer_name(void *balancer_name) { - return gpr_strdup(balancer_name); -} - static grpc_slice_hash_table_entry targets_info_entry_create( const char *address, const char *balancer_name) { - static const grpc_slice_hash_table_vtable vtable = {destroy_balancer_name, - copy_balancer_name}; grpc_slice_hash_table_entry entry; entry.key = grpc_slice_from_copied_string(address); - entry.value = (void *)balancer_name; - entry.vtable = &vtable; + entry.value = gpr_strdup(balancer_name); return entry; } @@ -825,11 +818,8 @@ static char *get_lb_uri_target_addresses(grpc_exec_ctx *exec_ctx, uri_path); gpr_free(uri_path); - *targets_info = - grpc_slice_hash_table_create(num_grpclb_addrs, targets_info_entries); - for (size_t i = 0; i < num_grpclb_addrs; i++) { - grpc_slice_unref_internal(exec_ctx, targets_info_entries[i].key); - } + *targets_info = grpc_slice_hash_table_create( + num_grpclb_addrs, targets_info_entries, destroy_balancer_name); gpr_free(targets_info_entries); return target_uri_str; diff --git a/src/core/ext/filters/message_size/message_size_filter.c b/src/core/ext/filters/message_size/message_size_filter.c index db0f0119053..69ebbc74149 100644 --- a/src/core/ext/filters/message_size/message_size_filter.c +++ b/src/core/ext/filters/message_size/message_size_filter.c @@ -50,19 +50,10 @@ typedef struct message_size_limits { int max_recv_size; } message_size_limits; -static void* message_size_limits_copy(void* value) { - void* new_value = gpr_malloc(sizeof(message_size_limits)); - memcpy(new_value, value, sizeof(message_size_limits)); - return new_value; -} - static void message_size_limits_free(grpc_exec_ctx* exec_ctx, void* value) { gpr_free(value); } -static const grpc_slice_hash_table_vtable message_size_limits_vtable = { - message_size_limits_free, message_size_limits_copy}; - static void* message_size_limits_create_from_json(const grpc_json* json) { int max_request_message_bytes = -1; int max_response_message_bytes = -1; @@ -255,7 +246,7 @@ static grpc_error* init_channel_elem(grpc_exec_ctx* exec_ctx, chand->method_limit_table = grpc_service_config_create_method_config_table( exec_ctx, service_config, message_size_limits_create_from_json, - &message_size_limits_vtable); + message_size_limits_free); grpc_service_config_destroy(service_config); } } diff --git a/src/core/lib/slice/slice_hash_table.c b/src/core/lib/slice/slice_hash_table.c index 219567f36f8..7a8b79cc336 100644 --- a/src/core/lib/slice/slice_hash_table.c +++ b/src/core/lib/slice/slice_hash_table.c @@ -42,12 +42,13 @@ struct grpc_slice_hash_table { gpr_refcount refs; + void (*destroy_value)(grpc_exec_ctx *exec_ctx, void *value); size_t size; grpc_slice_hash_table_entry* entries; }; static bool is_empty(grpc_slice_hash_table_entry* entry) { - return entry->vtable == NULL; + return entry->value == NULL; } // Helper function for insert and get operations that performs quadratic @@ -67,23 +68,23 @@ static size_t grpc_slice_hash_table_find_index( return table->size; // Not found. } -static void grpc_slice_hash_table_add( - grpc_slice_hash_table* table, grpc_slice key, void* value, - const grpc_slice_hash_table_vtable* vtable) { +static void grpc_slice_hash_table_add(grpc_slice_hash_table* table, + grpc_slice key, void* value) { GPR_ASSERT(value != NULL); const size_t idx = grpc_slice_hash_table_find_index(table, key, true /* find_empty */); GPR_ASSERT(idx != table->size); // Table should never be full. grpc_slice_hash_table_entry* entry = &table->entries[idx]; - entry->key = grpc_slice_ref_internal(key); - entry->value = vtable->copy_value(value); - entry->vtable = vtable; + entry->key = key; + entry->value = value; } grpc_slice_hash_table* grpc_slice_hash_table_create( - size_t num_entries, grpc_slice_hash_table_entry* entries) { + size_t num_entries, grpc_slice_hash_table_entry* entries, + void (*destroy_value)(grpc_exec_ctx *exec_ctx, void *value)) { grpc_slice_hash_table* table = gpr_zalloc(sizeof(*table)); gpr_ref_init(&table->refs, 1); + table->destroy_value = destroy_value; // Quadratic probing gets best performance when the table is no more // than half full. table->size = num_entries * 2; @@ -91,7 +92,7 @@ grpc_slice_hash_table* grpc_slice_hash_table_create( table->entries = gpr_zalloc(entry_size); for (size_t i = 0; i < num_entries; ++i) { grpc_slice_hash_table_entry* entry = &entries[i]; - grpc_slice_hash_table_add(table, entry->key, entry->value, entry->vtable); + grpc_slice_hash_table_add(table, entry->key, entry->value); } return table; } @@ -108,7 +109,7 @@ void grpc_slice_hash_table_unref(grpc_exec_ctx* exec_ctx, grpc_slice_hash_table_entry* entry = &table->entries[i]; if (!is_empty(entry)) { grpc_slice_unref_internal(exec_ctx, entry->key); - entry->vtable->destroy_value(exec_ctx, entry->value); + table->destroy_value(exec_ctx, entry->value); } } gpr_free(table->entries); diff --git a/src/core/lib/slice/slice_hash_table.h b/src/core/lib/slice/slice_hash_table.h index d0c27122d7f..81e92b6e21f 100644 --- a/src/core/lib/slice/slice_hash_table.h +++ b/src/core/lib/slice/slice_hash_table.h @@ -41,29 +41,24 @@ * probing (https://en.wikipedia.org/wiki/Quadratic_probing). * * The keys are \a grpc_slice objects. The values are arbitrary pointers - * with a common vtable. + * with a common destroy function. * * Hash tables are intentionally immutable, to avoid the need for locking. */ typedef struct grpc_slice_hash_table grpc_slice_hash_table; -typedef struct grpc_slice_hash_table_vtable { - void (*destroy_value)(grpc_exec_ctx *exec_ctx, void *value); - void *(*copy_value)(void *value); -} grpc_slice_hash_table_vtable; - typedef struct grpc_slice_hash_table_entry { grpc_slice key; void *value; /* Must not be NULL. */ - const grpc_slice_hash_table_vtable *vtable; } grpc_slice_hash_table_entry; /** Creates a new hash table of containing \a entries, which is an array - of length \a num_entries. - Creates its own copy of all keys and values from \a entries. */ + of length \a num_entries. Takes ownership of all keys and values in + \a entries. Values will be cleaned up via \a destroy_value(). */ grpc_slice_hash_table *grpc_slice_hash_table_create( - size_t num_entries, grpc_slice_hash_table_entry *entries); + size_t num_entries, grpc_slice_hash_table_entry *entries, + void (*destroy_value)(grpc_exec_ctx *exec_ctx, void *value)); grpc_slice_hash_table *grpc_slice_hash_table_ref(grpc_slice_hash_table *table); void grpc_slice_hash_table_unref(grpc_exec_ctx *exec_ctx, diff --git a/src/core/lib/transport/service_config.c b/src/core/lib/transport/service_config.c index 1195f75044a..3cda394723d 100644 --- a/src/core/lib/transport/service_config.c +++ b/src/core/lib/transport/service_config.c @@ -162,7 +162,7 @@ static char* parse_json_method_name(grpc_json* json) { static bool parse_json_method_config( grpc_exec_ctx* exec_ctx, grpc_json* json, void* (*create_value)(const grpc_json* method_config_json), - const grpc_slice_hash_table_vtable* vtable, + void (*destroy_value)(grpc_exec_ctx* exec_ctx, void* value), grpc_slice_hash_table_entry* entries, size_t* idx) { // Construct value. void* method_config = create_value(json); @@ -185,13 +185,12 @@ static bool parse_json_method_config( // Add entry for each path. for (size_t i = 0; i < paths.count; ++i) { entries[*idx].key = grpc_slice_from_copied_string(paths.strs[i]); - entries[*idx].value = vtable->copy_value(method_config); - entries[*idx].vtable = vtable; + entries[*idx].value = method_config; ++*idx; } success = true; done: - vtable->destroy_value(exec_ctx, method_config); + destroy_value(exec_ctx, method_config); gpr_strvec_destroy(&paths); return success; } @@ -199,7 +198,7 @@ done: grpc_slice_hash_table* grpc_service_config_create_method_config_table( grpc_exec_ctx* exec_ctx, const grpc_service_config* service_config, void* (*create_value)(const grpc_json* method_config_json), - const grpc_slice_hash_table_vtable* vtable) { + void (*destroy_value)(grpc_exec_ctx* exec_ctx, void* value)) { const grpc_json* json = service_config->json_tree; // Traverse parsed JSON tree. if (json->type != GRPC_JSON_OBJECT || json->key != NULL) return NULL; @@ -220,8 +219,8 @@ grpc_slice_hash_table* grpc_service_config_create_method_config_table( size_t idx = 0; for (grpc_json* method = field->child; method != NULL; method = method->next) { - if (!parse_json_method_config(exec_ctx, method, create_value, vtable, - entries, &idx)) { + if (!parse_json_method_config(exec_ctx, method, create_value, + destroy_value, entries, &idx)) { return NULL; } } @@ -231,12 +230,8 @@ grpc_slice_hash_table* grpc_service_config_create_method_config_table( // Instantiate method config table. grpc_slice_hash_table* method_config_table = NULL; if (entries != NULL) { - method_config_table = grpc_slice_hash_table_create(num_entries, entries); - // Clean up. - for (size_t i = 0; i < num_entries; ++i) { - grpc_slice_unref_internal(exec_ctx, entries[i].key); - vtable->destroy_value(exec_ctx, entries[i].value); - } + method_config_table = + grpc_slice_hash_table_create(num_entries, entries, destroy_value); gpr_free(entries); } return method_config_table; diff --git a/src/core/lib/transport/service_config.h b/src/core/lib/transport/service_config.h index ebfc59b5347..e0548b9c3fa 100644 --- a/src/core/lib/transport/service_config.h +++ b/src/core/lib/transport/service_config.h @@ -57,12 +57,12 @@ const char* grpc_service_config_get_lb_policy_name( /// Creates a method config table based on the data in \a json. /// The table's keys are request paths. The table's value type is /// returned by \a create_value(), based on data parsed from the JSON tree. -/// \a vtable provides methods used to manage the values. +/// \a destroy_value is used to clean up values. /// Returns NULL on error. grpc_slice_hash_table* grpc_service_config_create_method_config_table( grpc_exec_ctx* exec_ctx, const grpc_service_config* service_config, void* (*create_value)(const grpc_json* method_config_json), - const grpc_slice_hash_table_vtable* vtable); + void (*destroy_value)(grpc_exec_ctx* exec_ctx, void* value)); /// A helper function for looking up values in the table returned by /// \a grpc_service_config_create_method_config_table(). From 0c0b89a88ba6544e42cca43913ed65dea64bff3a Mon Sep 17 00:00:00 2001 From: "Mark D. Roth" Date: Wed, 19 Apr 2017 13:28:24 -0700 Subject: [PATCH 07/34] Change hash table to use linear probing and add unit test. Also add some missing rules in test/core/slice/BUILD. --- CMakeLists.txt | 32 +++ Makefile | 36 ++++ build.yaml | 10 + src/core/lib/slice/slice_hash_table.c | 55 +++-- src/core/lib/slice/slice_hash_table.h | 4 +- test/core/slice/BUILD | 23 +- test/core/slice/slice_hash_table_test.c | 202 ++++++++++++++++++ .../generated/sources_and_headers.json | 17 ++ tools/run_tests/generated/tests.json | 22 ++ vsprojects/buildtests_c.sln | 27 +++ .../slice_hash_table_test.vcxproj | 199 +++++++++++++++++ .../slice_hash_table_test.vcxproj.filters | 21 ++ 12 files changed, 616 insertions(+), 32 deletions(-) create mode 100644 test/core/slice/slice_hash_table_test.c create mode 100644 vsprojects/vcxproj/test/slice_hash_table_test/slice_hash_table_test.vcxproj create mode 100644 vsprojects/vcxproj/test/slice_hash_table_test/slice_hash_table_test.vcxproj.filters diff --git a/CMakeLists.txt b/CMakeLists.txt index a6a2af65d5d..230a3d1ac0a 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -493,6 +493,7 @@ add_dependencies(buildtests_c sequential_connectivity_test) add_dependencies(buildtests_c server_chttp2_test) add_dependencies(buildtests_c server_test) add_dependencies(buildtests_c slice_buffer_test) +add_dependencies(buildtests_c slice_hash_table_test) add_dependencies(buildtests_c slice_string_helpers_test) add_dependencies(buildtests_c slice_test) add_dependencies(buildtests_c sockaddr_resolver_test) @@ -8007,6 +8008,37 @@ target_link_libraries(slice_buffer_test endif (gRPC_BUILD_TESTS) if (gRPC_BUILD_TESTS) +add_executable(slice_hash_table_test + test/core/slice/slice_hash_table_test.c +) + + +target_include_directories(slice_hash_table_test + PRIVATE ${CMAKE_CURRENT_SOURCE_DIR} + PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/include + PRIVATE ${BORINGSSL_ROOT_DIR}/include + PRIVATE ${PROTOBUF_ROOT_DIR}/src + PRIVATE ${BENCHMARK_ROOT_DIR}/include + PRIVATE ${ZLIB_ROOT_DIR} + PRIVATE ${CMAKE_CURRENT_BINARY_DIR}/third_party/zlib + PRIVATE ${CARES_BUILD_INCLUDE_DIR} + PRIVATE ${CARES_INCLUDE_DIR} + PRIVATE ${CARES_PLATFORM_INCLUDE_DIR} + PRIVATE ${CMAKE_CURRENT_BINARY_DIR}/third_party/cares/cares + PRIVATE ${CMAKE_CURRENT_BINARY_DIR}/third_party/gflags/include +) + +target_link_libraries(slice_hash_table_test + ${_gRPC_ALLTARGETS_LIBRARIES} + grpc_test_util + grpc + gpr_test_util + gpr +) + +endif (gRPC_BUILD_TESTS) +if (gRPC_BUILD_TESTS) + add_executable(slice_string_helpers_test test/core/slice/slice_string_helpers_test.c ) diff --git a/Makefile b/Makefile index 8898939af9e..a233f712e48 100644 --- a/Makefile +++ b/Makefile @@ -1073,6 +1073,7 @@ server_chttp2_test: $(BINDIR)/$(CONFIG)/server_chttp2_test server_fuzzer: $(BINDIR)/$(CONFIG)/server_fuzzer server_test: $(BINDIR)/$(CONFIG)/server_test slice_buffer_test: $(BINDIR)/$(CONFIG)/slice_buffer_test +slice_hash_table_test: $(BINDIR)/$(CONFIG)/slice_hash_table_test slice_string_helpers_test: $(BINDIR)/$(CONFIG)/slice_string_helpers_test slice_test: $(BINDIR)/$(CONFIG)/slice_test sockaddr_resolver_test: $(BINDIR)/$(CONFIG)/sockaddr_resolver_test @@ -1443,6 +1444,7 @@ buildtests_c: privatelibs_c \ $(BINDIR)/$(CONFIG)/server_chttp2_test \ $(BINDIR)/$(CONFIG)/server_test \ $(BINDIR)/$(CONFIG)/slice_buffer_test \ + $(BINDIR)/$(CONFIG)/slice_hash_table_test \ $(BINDIR)/$(CONFIG)/slice_string_helpers_test \ $(BINDIR)/$(CONFIG)/slice_test \ $(BINDIR)/$(CONFIG)/sockaddr_resolver_test \ @@ -1915,6 +1917,8 @@ test_c: buildtests_c $(Q) $(BINDIR)/$(CONFIG)/server_test || ( echo test server_test failed ; exit 1 ) $(E) "[RUN] Testing slice_buffer_test" $(Q) $(BINDIR)/$(CONFIG)/slice_buffer_test || ( echo test slice_buffer_test failed ; exit 1 ) + $(E) "[RUN] Testing slice_hash_table_test" + $(Q) $(BINDIR)/$(CONFIG)/slice_hash_table_test || ( echo test slice_hash_table_test failed ; exit 1 ) $(E) "[RUN] Testing slice_string_helpers_test" $(Q) $(BINDIR)/$(CONFIG)/slice_string_helpers_test || ( echo test slice_string_helpers_test failed ; exit 1 ) $(E) "[RUN] Testing slice_test" @@ -12349,6 +12353,38 @@ endif endif +SLICE_HASH_TABLE_TEST_SRC = \ + test/core/slice/slice_hash_table_test.c \ + +SLICE_HASH_TABLE_TEST_OBJS = $(addprefix $(OBJDIR)/$(CONFIG)/, $(addsuffix .o, $(basename $(SLICE_HASH_TABLE_TEST_SRC)))) +ifeq ($(NO_SECURE),true) + +# You can't build secure targets if you don't have OpenSSL. + +$(BINDIR)/$(CONFIG)/slice_hash_table_test: openssl_dep_error + +else + + + +$(BINDIR)/$(CONFIG)/slice_hash_table_test: $(SLICE_HASH_TABLE_TEST_OBJS) $(LIBDIR)/$(CONFIG)/libgrpc_test_util.a $(LIBDIR)/$(CONFIG)/libgrpc.a $(LIBDIR)/$(CONFIG)/libgpr_test_util.a $(LIBDIR)/$(CONFIG)/libgpr.a + $(E) "[LD] Linking $@" + $(Q) mkdir -p `dirname $@` + $(Q) $(LD) $(LDFLAGS) $(SLICE_HASH_TABLE_TEST_OBJS) $(LIBDIR)/$(CONFIG)/libgrpc_test_util.a $(LIBDIR)/$(CONFIG)/libgrpc.a $(LIBDIR)/$(CONFIG)/libgpr_test_util.a $(LIBDIR)/$(CONFIG)/libgpr.a $(LDLIBS) $(LDLIBS_SECURE) -o $(BINDIR)/$(CONFIG)/slice_hash_table_test + +endif + +$(OBJDIR)/$(CONFIG)/test/core/slice/slice_hash_table_test.o: $(LIBDIR)/$(CONFIG)/libgrpc_test_util.a $(LIBDIR)/$(CONFIG)/libgrpc.a $(LIBDIR)/$(CONFIG)/libgpr_test_util.a $(LIBDIR)/$(CONFIG)/libgpr.a + +deps_slice_hash_table_test: $(SLICE_HASH_TABLE_TEST_OBJS:.o=.dep) + +ifneq ($(NO_SECURE),true) +ifneq ($(NO_DEPS),true) +-include $(SLICE_HASH_TABLE_TEST_OBJS:.o=.dep) +endif +endif + + SLICE_STRING_HELPERS_TEST_SRC = \ test/core/slice/slice_string_helpers_test.c \ diff --git a/build.yaml b/build.yaml index f4461e40ef1..b40834e16d4 100644 --- a/build.yaml +++ b/build.yaml @@ -2807,6 +2807,16 @@ targets: - grpc - gpr_test_util - gpr +- name: slice_hash_table_test + build: test + language: c + src: + - test/core/slice/slice_hash_table_test.c + deps: + - grpc_test_util + - grpc + - gpr_test_util + - gpr - name: slice_string_helpers_test build: test language: c diff --git a/src/core/lib/slice/slice_hash_table.c b/src/core/lib/slice/slice_hash_table.c index 7a8b79cc336..a33f6069fd5 100644 --- a/src/core/lib/slice/slice_hash_table.c +++ b/src/core/lib/slice/slice_hash_table.c @@ -44,6 +44,7 @@ struct grpc_slice_hash_table { gpr_refcount refs; void (*destroy_value)(grpc_exec_ctx *exec_ctx, void *value); size_t size; + size_t max_num_probes; grpc_slice_hash_table_entry* entries; }; @@ -51,32 +52,22 @@ static bool is_empty(grpc_slice_hash_table_entry* entry) { return entry->value == NULL; } -// Helper function for insert and get operations that performs quadratic -// probing (https://en.wikipedia.org/wiki/Quadratic_probing). -static size_t grpc_slice_hash_table_find_index( - const grpc_slice_hash_table* table, const grpc_slice key, bool find_empty) { - size_t hash = grpc_slice_hash(key); - for (size_t i = 0; i < table->size; ++i) { - const size_t idx = (hash + i * i) % table->size; - if (is_empty(&table->entries[idx])) { - return find_empty ? idx : table->size; - } - if (grpc_slice_eq(table->entries[idx].key, key)) { - return idx; - } - } - return table->size; // Not found. -} - static void grpc_slice_hash_table_add(grpc_slice_hash_table* table, grpc_slice key, void* value) { GPR_ASSERT(value != NULL); - const size_t idx = - grpc_slice_hash_table_find_index(table, key, true /* find_empty */); - GPR_ASSERT(idx != table->size); // Table should never be full. - grpc_slice_hash_table_entry* entry = &table->entries[idx]; - entry->key = key; - entry->value = value; + const size_t hash = grpc_slice_hash(key); + for (size_t offset = 0; offset < table->size; ++offset) { + const size_t idx = (hash + offset) % table->size; + if (is_empty(&table->entries[idx])) { + table->entries[idx].key = key; + table->entries[idx].value = value; + // Keep track of the maximum number of probes needed, since this + // provides an upper bound for lookups. + if (offset > table->max_num_probes) table->max_num_probes = offset; + return; + } + } + GPR_ASSERT(false); // Table should never be full. } grpc_slice_hash_table* grpc_slice_hash_table_create( @@ -85,8 +76,7 @@ grpc_slice_hash_table* grpc_slice_hash_table_create( grpc_slice_hash_table* table = gpr_zalloc(sizeof(*table)); gpr_ref_init(&table->refs, 1); table->destroy_value = destroy_value; - // Quadratic probing gets best performance when the table is no more - // than half full. + // Keep load factor low to improve performance of lookups. table->size = num_entries * 2; const size_t entry_size = sizeof(grpc_slice_hash_table_entry) * table->size; table->entries = gpr_zalloc(entry_size); @@ -119,8 +109,15 @@ void grpc_slice_hash_table_unref(grpc_exec_ctx* exec_ctx, void* grpc_slice_hash_table_get(const grpc_slice_hash_table* table, const grpc_slice key) { - const size_t idx = - grpc_slice_hash_table_find_index(table, key, false /* find_empty */); - if (idx == table->size) return NULL; // Not found. - return table->entries[idx].value; + const size_t hash = grpc_slice_hash(key); + // We cap the number of probes at the max number recorded when + // populating the table. + for (size_t offset = 0; offset <= table->max_num_probes; ++offset) { + const size_t idx = (hash + offset) % table->size; + if (is_empty(&table->entries[idx])) break; + if (grpc_slice_eq(table->entries[idx].key, key)) { + return table->entries[idx].value; + } + } + return NULL; // Not found. } diff --git a/src/core/lib/slice/slice_hash_table.h b/src/core/lib/slice/slice_hash_table.h index 81e92b6e21f..1e61c5eb116 100644 --- a/src/core/lib/slice/slice_hash_table.h +++ b/src/core/lib/slice/slice_hash_table.h @@ -37,8 +37,8 @@ /** Hash table implementation. * * This implementation uses open addressing - * (https://en.wikipedia.org/wiki/Open_addressing) with quadratic - * probing (https://en.wikipedia.org/wiki/Quadratic_probing). + * (https://en.wikipedia.org/wiki/Open_addressing) with linear + * probing (https://en.wikipedia.org/wiki/Linear_probing). * * The keys are \a grpc_slice objects. The values are arbitrary pointers * with a common destroy function. diff --git a/test/core/slice/BUILD b/test/core/slice/BUILD index 4d64d0a8183..18cf6f60af0 100644 --- a/test/core/slice/BUILD +++ b/test/core/slice/BUILD @@ -47,12 +47,33 @@ cc_test( ) cc_test( - name = "slice_buffer_test", + name = "slice_test", + srcs = ["slice_test.c"], + deps = ["//:grpc", "//test/core/util:grpc_test_util", "//:gpr", "//test/core/util:gpr_test_util"], + copts = ['-std=c99'] +) + +cc_test( + name = "slice_string_helpers_test", srcs = ["slice_string_helpers_test.c"], deps = ["//:grpc", "//test/core/util:grpc_test_util", "//:gpr", "//test/core/util:gpr_test_util"], copts = ['-std=c99'] ) +cc_test( + name = "slice_buffer_test", + srcs = ["slice_buffer_test.c"], + deps = ["//:grpc", "//test/core/util:grpc_test_util", "//:gpr", "//test/core/util:gpr_test_util"], + copts = ['-std=c99'] +) + +cc_test( + name = "slice_hash_table_test", + srcs = ["slice_hash_table_test.c"], + deps = ["//:grpc", "//test/core/util:grpc_test_util", "//:gpr", "//test/core/util:gpr_test_util"], + copts = ['-std=c99'] +) + cc_test( name = "b64_test", srcs = ["b64_test.c"], diff --git a/test/core/slice/slice_hash_table_test.c b/test/core/slice/slice_hash_table_test.c new file mode 100644 index 00000000000..24a4354b8b6 --- /dev/null +++ b/test/core/slice/slice_hash_table_test.c @@ -0,0 +1,202 @@ +/* + * + * 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 "src/core/lib/slice/slice_hash_table.h" + +#include + +#include +#include +#include + +#include "src/core/lib/slice/slice_internal.h" +#include "test/core/util/test_config.h" + +typedef struct { + char* key; + char* value; +} test_entry; + +static void populate_entries(const test_entry* input, size_t num_entries, + grpc_slice_hash_table_entry* output) { + for (size_t i = 0; i < num_entries; ++i) { + output[i].key = grpc_slice_from_copied_string(gpr_strdup(input[i].key)); + output[i].value = gpr_strdup(input[i].value); + } +} + +static void check_values(const test_entry* input, size_t num_entries, + grpc_slice_hash_table* table) { + for (size_t i = 0; i < num_entries; ++i) { + grpc_slice key = grpc_slice_from_static_string(input[i].key); + char* actual = grpc_slice_hash_table_get(table, key); + GPR_ASSERT(actual != NULL); + GPR_ASSERT(strcmp(actual, input[i].value) == 0); + grpc_slice_unref(key); + } +} + +static void check_non_existent_value(const char* key_string, + grpc_slice_hash_table* table) { + grpc_slice key = grpc_slice_from_static_string(key_string); + GPR_ASSERT(grpc_slice_hash_table_get(table, key) == NULL); + grpc_slice_unref(key); +} + +static void destroy_string(grpc_exec_ctx* exec_ctx, void* value) { + gpr_free(value); +} + +static void test_slice_hash_table() { + const test_entry test_entries[] = { + {"key_0", "value_0"}, + {"key_1", "value_1"}, + {"key_2", "value_2"}, + {"key_3", "value_3"}, + {"key_4", "value_4"}, + {"key_5", "value_5"}, + {"key_6", "value_6"}, + {"key_7", "value_7"}, + {"key_8", "value_8"}, + {"key_9", "value_9"}, + {"key_10", "value_10"}, + {"key_11", "value_11"}, + {"key_12", "value_12"}, + {"key_13", "value_13"}, + {"key_14", "value_14"}, + {"key_15", "value_15"}, + {"key_16", "value_16"}, + {"key_17", "value_17"}, + {"key_18", "value_18"}, + {"key_19", "value_19"}, + {"key_20", "value_20"}, + {"key_21", "value_21"}, + {"key_22", "value_22"}, + {"key_23", "value_23"}, + {"key_24", "value_24"}, + {"key_25", "value_25"}, + {"key_26", "value_26"}, + {"key_27", "value_27"}, + {"key_28", "value_28"}, + {"key_29", "value_29"}, + {"key_30", "value_30"}, + {"key_31", "value_31"}, + {"key_32", "value_32"}, + {"key_33", "value_33"}, + {"key_34", "value_34"}, + {"key_35", "value_35"}, + {"key_36", "value_36"}, + {"key_37", "value_37"}, + {"key_38", "value_38"}, + {"key_39", "value_39"}, + {"key_40", "value_40"}, + {"key_41", "value_41"}, + {"key_42", "value_42"}, + {"key_43", "value_43"}, + {"key_44", "value_44"}, + {"key_45", "value_45"}, + {"key_46", "value_46"}, + {"key_47", "value_47"}, + {"key_48", "value_48"}, + {"key_49", "value_49"}, + {"key_50", "value_50"}, + {"key_51", "value_51"}, + {"key_52", "value_52"}, + {"key_53", "value_53"}, + {"key_54", "value_54"}, + {"key_55", "value_55"}, + {"key_56", "value_56"}, + {"key_57", "value_57"}, + {"key_58", "value_58"}, + {"key_59", "value_59"}, + {"key_60", "value_60"}, + {"key_61", "value_61"}, + {"key_62", "value_62"}, + {"key_63", "value_63"}, + {"key_64", "value_64"}, + {"key_65", "value_65"}, + {"key_66", "value_66"}, + {"key_67", "value_67"}, + {"key_68", "value_68"}, + {"key_69", "value_69"}, + {"key_70", "value_70"}, + {"key_71", "value_71"}, + {"key_72", "value_72"}, + {"key_73", "value_73"}, + {"key_74", "value_74"}, + {"key_75", "value_75"}, + {"key_76", "value_76"}, + {"key_77", "value_77"}, + {"key_78", "value_78"}, + {"key_79", "value_79"}, + {"key_80", "value_80"}, + {"key_81", "value_81"}, + {"key_82", "value_82"}, + {"key_83", "value_83"}, + {"key_84", "value_84"}, + {"key_85", "value_85"}, + {"key_86", "value_86"}, + {"key_87", "value_87"}, + {"key_88", "value_88"}, + {"key_89", "value_89"}, + {"key_90", "value_90"}, + {"key_91", "value_91"}, + {"key_92", "value_92"}, + {"key_93", "value_93"}, + {"key_94", "value_94"}, + {"key_95", "value_95"}, + {"key_96", "value_96"}, + {"key_97", "value_97"}, + {"key_98", "value_98"}, + {"key_99", "value_99"}, + }; + const size_t num_entries = GPR_ARRAY_SIZE(test_entries); + // Construct table. + grpc_slice_hash_table_entry entries[num_entries]; + populate_entries(test_entries, num_entries, entries); + grpc_slice_hash_table* table = + grpc_slice_hash_table_create(num_entries, entries, destroy_string); + // Check contents of table. + check_values(test_entries, num_entries, table); + check_non_existent_value("XX", table); + // Clean up. + grpc_exec_ctx exec_ctx = GRPC_EXEC_CTX_INIT; + grpc_slice_hash_table_unref(&exec_ctx, table); + grpc_exec_ctx_finish(&exec_ctx); +} + +int main(int argc, char **argv) { + grpc_test_init(argc, argv); + test_slice_hash_table(); + return 0; +} diff --git a/tools/run_tests/generated/sources_and_headers.json b/tools/run_tests/generated/sources_and_headers.json index aa4869e1c8a..9635dbec839 100644 --- a/tools/run_tests/generated/sources_and_headers.json +++ b/tools/run_tests/generated/sources_and_headers.json @@ -1978,6 +1978,23 @@ "third_party": false, "type": "target" }, + { + "deps": [ + "gpr", + "gpr_test_util", + "grpc", + "grpc_test_util" + ], + "headers": [], + "is_filegroup": false, + "language": "c", + "name": "slice_hash_table_test", + "src": [ + "test/core/slice/slice_hash_table_test.c" + ], + "third_party": false, + "type": "target" + }, { "deps": [ "gpr", diff --git a/tools/run_tests/generated/tests.json b/tools/run_tests/generated/tests.json index 6338ea70127..eb8fb20d7de 100644 --- a/tools/run_tests/generated/tests.json +++ b/tools/run_tests/generated/tests.json @@ -2071,6 +2071,28 @@ "windows" ] }, + { + "args": [], + "ci_platforms": [ + "linux", + "mac", + "posix", + "windows" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [], + "flaky": false, + "gtest": false, + "language": "c", + "name": "slice_hash_table_test", + "platforms": [ + "linux", + "mac", + "posix", + "windows" + ] + }, { "args": [], "ci_platforms": [ diff --git a/vsprojects/buildtests_c.sln b/vsprojects/buildtests_c.sln index 6ae8bfa74de..539f474f9ec 100644 --- a/vsprojects/buildtests_c.sln +++ b/vsprojects/buildtests_c.sln @@ -1440,6 +1440,17 @@ Project("{8BC9CEB8-8B4A-11D0-8D11-00A0C91BC942}") = "slice_buffer_test", "vcxpro {B23D3D1A-9438-4EDA-BEB6-9A0A03D17792} = {B23D3D1A-9438-4EDA-BEB6-9A0A03D17792} EndProjectSection EndProject +Project("{8BC9CEB8-8B4A-11D0-8D11-00A0C91BC942}") = "slice_hash_table_test", "vcxproj\test\slice_hash_table_test\slice_hash_table_test.vcxproj", "{B3BC3481-FCD3-BA52-C975-E65CDB1CE9A9}" + ProjectSection(myProperties) = preProject + lib = "False" + EndProjectSection + ProjectSection(ProjectDependencies) = postProject + {17BCAFC0-5FDC-4C94-AEB9-95F3E220614B} = {17BCAFC0-5FDC-4C94-AEB9-95F3E220614B} + {29D16885-7228-4C31-81ED-5F9187C7F2A9} = {29D16885-7228-4C31-81ED-5F9187C7F2A9} + {EAB0A629-17A9-44DB-B5FF-E91A721FE037} = {EAB0A629-17A9-44DB-B5FF-E91A721FE037} + {B23D3D1A-9438-4EDA-BEB6-9A0A03D17792} = {B23D3D1A-9438-4EDA-BEB6-9A0A03D17792} + EndProjectSection +EndProject Project("{8BC9CEB8-8B4A-11D0-8D11-00A0C91BC942}") = "slice_string_helpers_test", "vcxproj\test\slice_string_helpers_test\slice_string_helpers_test.vcxproj", "{419167BB-C3F5-DDEA-403A-394D1902DE65}" ProjectSection(myProperties) = preProject lib = "False" @@ -3823,6 +3834,22 @@ Global {F0FA4A41-5695-580A-DCDA-EC719CB041B0}.Release-DLL|Win32.Build.0 = Release|Win32 {F0FA4A41-5695-580A-DCDA-EC719CB041B0}.Release-DLL|x64.ActiveCfg = Release|x64 {F0FA4A41-5695-580A-DCDA-EC719CB041B0}.Release-DLL|x64.Build.0 = Release|x64 + {B3BC3481-FCD3-BA52-C975-E65CDB1CE9A9}.Debug|Win32.ActiveCfg = Debug|Win32 + {B3BC3481-FCD3-BA52-C975-E65CDB1CE9A9}.Debug|x64.ActiveCfg = Debug|x64 + {B3BC3481-FCD3-BA52-C975-E65CDB1CE9A9}.Release|Win32.ActiveCfg = Release|Win32 + {B3BC3481-FCD3-BA52-C975-E65CDB1CE9A9}.Release|x64.ActiveCfg = Release|x64 + {B3BC3481-FCD3-BA52-C975-E65CDB1CE9A9}.Debug|Win32.Build.0 = Debug|Win32 + {B3BC3481-FCD3-BA52-C975-E65CDB1CE9A9}.Debug|x64.Build.0 = Debug|x64 + {B3BC3481-FCD3-BA52-C975-E65CDB1CE9A9}.Release|Win32.Build.0 = Release|Win32 + {B3BC3481-FCD3-BA52-C975-E65CDB1CE9A9}.Release|x64.Build.0 = Release|x64 + {B3BC3481-FCD3-BA52-C975-E65CDB1CE9A9}.Debug-DLL|Win32.ActiveCfg = Debug|Win32 + {B3BC3481-FCD3-BA52-C975-E65CDB1CE9A9}.Debug-DLL|Win32.Build.0 = Debug|Win32 + {B3BC3481-FCD3-BA52-C975-E65CDB1CE9A9}.Debug-DLL|x64.ActiveCfg = Debug|x64 + {B3BC3481-FCD3-BA52-C975-E65CDB1CE9A9}.Debug-DLL|x64.Build.0 = Debug|x64 + {B3BC3481-FCD3-BA52-C975-E65CDB1CE9A9}.Release-DLL|Win32.ActiveCfg = Release|Win32 + {B3BC3481-FCD3-BA52-C975-E65CDB1CE9A9}.Release-DLL|Win32.Build.0 = Release|Win32 + {B3BC3481-FCD3-BA52-C975-E65CDB1CE9A9}.Release-DLL|x64.ActiveCfg = Release|x64 + {B3BC3481-FCD3-BA52-C975-E65CDB1CE9A9}.Release-DLL|x64.Build.0 = Release|x64 {419167BB-C3F5-DDEA-403A-394D1902DE65}.Debug|Win32.ActiveCfg = Debug|Win32 {419167BB-C3F5-DDEA-403A-394D1902DE65}.Debug|x64.ActiveCfg = Debug|x64 {419167BB-C3F5-DDEA-403A-394D1902DE65}.Release|Win32.ActiveCfg = Release|Win32 diff --git a/vsprojects/vcxproj/test/slice_hash_table_test/slice_hash_table_test.vcxproj b/vsprojects/vcxproj/test/slice_hash_table_test/slice_hash_table_test.vcxproj new file mode 100644 index 00000000000..0fccfdcaa56 --- /dev/null +++ b/vsprojects/vcxproj/test/slice_hash_table_test/slice_hash_table_test.vcxproj @@ -0,0 +1,199 @@ + + + + + + Debug + Win32 + + + Debug + x64 + + + Release + Win32 + + + Release + x64 + + + + {B3BC3481-FCD3-BA52-C975-E65CDB1CE9A9} + true + $(SolutionDir)IntDir\$(MSBuildProjectName)\ + + + + v100 + + + v110 + + + v120 + + + v140 + + + Application + true + Unicode + + + Application + false + true + Unicode + + + + + + + + + + + + + + slice_hash_table_test + static + Debug + static + Debug + + + slice_hash_table_test + static + Release + static + Release + + + + NotUsing + Level3 + Disabled + WIN32;_DEBUG;_LIB;%(PreprocessorDefinitions) + true + MultiThreadedDebug + true + None + false + + + Console + true + false + + + + + + NotUsing + Level3 + Disabled + WIN32;_DEBUG;_LIB;%(PreprocessorDefinitions) + true + MultiThreadedDebug + true + None + false + + + Console + true + false + + + + + + NotUsing + Level3 + MaxSpeed + WIN32;NDEBUG;_LIB;%(PreprocessorDefinitions) + true + true + true + MultiThreaded + true + None + false + + + Console + true + false + true + true + + + + + + NotUsing + Level3 + MaxSpeed + WIN32;NDEBUG;_LIB;%(PreprocessorDefinitions) + true + true + true + MultiThreaded + true + None + false + + + Console + true + false + true + true + + + + + + + + + + {17BCAFC0-5FDC-4C94-AEB9-95F3E220614B} + + + {29D16885-7228-4C31-81ED-5F9187C7F2A9} + + + {EAB0A629-17A9-44DB-B5FF-E91A721FE037} + + + {B23D3D1A-9438-4EDA-BEB6-9A0A03D17792} + + + + + + + + + + + + + + + This project references NuGet package(s) that are missing on this computer. Enable NuGet Package Restore to download them. For more information, see http://go.microsoft.com/fwlink/?LinkID=322105. The missing file is {0}. + + + + + + + + + diff --git a/vsprojects/vcxproj/test/slice_hash_table_test/slice_hash_table_test.vcxproj.filters b/vsprojects/vcxproj/test/slice_hash_table_test/slice_hash_table_test.vcxproj.filters new file mode 100644 index 00000000000..19738526781 --- /dev/null +++ b/vsprojects/vcxproj/test/slice_hash_table_test/slice_hash_table_test.vcxproj.filters @@ -0,0 +1,21 @@ + + + + + test\core\slice + + + + + + {e11f5007-84da-0229-9118-2a2bb17f956c} + + + {a11f4770-5e13-eb1a-a848-8667dde98812} + + + {fb1262a8-0a70-bc85-579a-6ebfffbf25b5} + + + + From e0a873784adaff3a0dfd06d16cb621d3eddf3443 Mon Sep 17 00:00:00 2001 From: "Mark D. Roth" Date: Wed, 19 Apr 2017 13:39:29 -0700 Subject: [PATCH 08/34] Fix bug in service config code. --- src/core/lib/transport/service_config.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/core/lib/transport/service_config.c b/src/core/lib/transport/service_config.c index 3cda394723d..6aecb7fa936 100644 --- a/src/core/lib/transport/service_config.c +++ b/src/core/lib/transport/service_config.c @@ -162,7 +162,6 @@ static char* parse_json_method_name(grpc_json* json) { static bool parse_json_method_config( grpc_exec_ctx* exec_ctx, grpc_json* json, void* (*create_value)(const grpc_json* method_config_json), - void (*destroy_value)(grpc_exec_ctx* exec_ctx, void* value), grpc_slice_hash_table_entry* entries, size_t* idx) { // Construct value. void* method_config = create_value(json); @@ -190,7 +189,6 @@ static bool parse_json_method_config( } success = true; done: - destroy_value(exec_ctx, method_config); gpr_strvec_destroy(&paths); return success; } @@ -219,8 +217,8 @@ grpc_slice_hash_table* grpc_service_config_create_method_config_table( size_t idx = 0; for (grpc_json* method = field->child; method != NULL; method = method->next) { - if (!parse_json_method_config(exec_ctx, method, create_value, - destroy_value, entries, &idx)) { + if (!parse_json_method_config(exec_ctx, method, create_value, entries, + &idx)) { return NULL; } } From 880530694192f9be13b8767bfab7a06fdda9697e Mon Sep 17 00:00:00 2001 From: Muxi Yan Date: Wed, 19 Apr 2017 13:50:06 -0700 Subject: [PATCH 09/34] Revert API to change default queue for a stub --- src/compiler/objective_c_generator.cc | 34 ++++---------- src/objective-c/tests/InteropTests.m | 68 +++++---------------------- 2 files changed, 21 insertions(+), 81 deletions(-) diff --git a/src/compiler/objective_c_generator.cc b/src/compiler/objective_c_generator.cc index ba89b28b7b5..1d7faf120dc 100644 --- a/src/compiler/objective_c_generator.cc +++ b/src/compiler/objective_c_generator.cc @@ -149,20 +149,17 @@ void PrintMethodDeclarations(Printer *printer, const MethodDescriptor *method) { void PrintSimpleImplementation(Printer *printer, const MethodDescriptor *method, map< ::grpc::string, ::grpc::string> vars) { printer->Print("{\n"); - printer->Print(vars, " GRPCProtoCall *rpc = [self RPCTo$method_name$With"); + printer->Print(vars, " [[self RPCTo$method_name$With"); if (method->client_streaming()) { printer->Print("RequestsWriter:requestWriter"); } else { printer->Print("Request:request"); } if (method->server_streaming()) { - printer->Print(" eventHandler:eventHandler];\n"); + printer->Print(" eventHandler:eventHandler] start];\n"); } else { - printer->Print(" handler:handler];\n"); + printer->Print(" handler:handler] start];\n"); } - printer->Print( - " [rpc setResponseDispatchQueue:_defaultResponseDispatchQueue];\n"); - printer->Print(" [rpc start];\n"); printer->Print("}\n"); } @@ -170,29 +167,23 @@ void PrintAdvancedImplementation(Printer *printer, const MethodDescriptor *method, map< ::grpc::string, ::grpc::string> vars) { printer->Print("{\n"); - printer->Print( - vars, " GRPCProtoCall *rpc = [self RPCToMethod:@\"$method_name$\"\n"); + printer->Print(vars, " return [self RPCToMethod:@\"$method_name$\"\n"); - printer->Print(" requestsWriter:"); + printer->Print(" requestsWriter:"); if (method->client_streaming()) { printer->Print("requestWriter\n"); } else { printer->Print("[GRXWriter writerWithValue:request]\n"); } - printer->Print( - vars, - " responseClass:[$response_class$ class]\n"); + printer->Print(vars, " responseClass:[$response_class$ class]\n"); - printer->Print(" responsesWriteable:[GRXWriteable "); + printer->Print(" responsesWriteable:[GRXWriteable "); if (method->server_streaming()) { printer->Print("writeableWithEventHandler:eventHandler]];\n"); } else { printer->Print("writeableWithSingleHandler:handler]];\n"); } - printer->Print( - " [rpc setResponseDispatchQueue:_defaultResponseDispatchQueue];\n"); - printer->Print(" return rpc;\n"); printer->Print("}\n"); } @@ -243,8 +234,6 @@ void PrintMethodImplementations(Printer *printer, "- (instancetype)initWithHost:(NSString *)host" " NS_DESIGNATED_INITIALIZER;\n"); printer.Print("+ (instancetype)serviceWithHost:(NSString *)host;\n"); - printer.Print( - "- (void)setDefaultResponseDispatchQueue:(dispatch_queue_t)queue;\n"); printer.Print("@end\n"); } return output; @@ -262,15 +251,12 @@ void PrintMethodImplementations(Printer *printer, {"service_class", ServiceClassName(service)}, {"package", service->file()->package()}}; - printer.Print(vars, "@implementation $service_class$ {\n"); - printer.Print(vars, " dispatch_queue_t _defaultResponseDispatchQueue;\n"); - printer.Print(vars, "}\n\n"); + printer.Print(vars, "@implementation $service_class$\n\n"); printer.Print("// Designated initializer\n"); printer.Print("- (instancetype)initWithHost:(NSString *)host {\n"); printer.Print( vars, - " _defaultResponseDispatchQueue = dispatch_get_main_queue();\n" " return (self = [super initWithHost:host" " packageName:@\"$package$\" serviceName:@\"$service_name$\"]);\n"); printer.Print("}\n\n"); @@ -284,10 +270,6 @@ void PrintMethodImplementations(Printer *printer, printer.Print("}\n\n"); printer.Print("+ (instancetype)serviceWithHost:(NSString *)host {\n"); printer.Print(" return [[self alloc] initWithHost:host];\n"); - printer.Print("}\n\n"); - printer.Print( - "- (void)setDefaultResponseDispatchQueue:(dispatch_queue_t)queue {\n"); - printer.Print(" _defaultResponseDispatchQueue = queue;\n"); printer.Print("}\n\n\n"); for (int i = 0; i < service->method_count(); i++) { diff --git a/src/objective-c/tests/InteropTests.m b/src/objective-c/tests/InteropTests.m index 4466d6810cc..e7d28f4e459 100644 --- a/src/objective-c/tests/InteropTests.m +++ b/src/objective-c/tests/InteropTests.m @@ -459,23 +459,17 @@ - (void)testAlternateDispatchQueue { XCTAssertNotNil(self.class.host); - __weak XCTestExpectation *expectation1 = [self expectationWithDescription:@"AlternateDispatchQueue1"]; + NSNumber *kPayloadSize = @256; + id request = [RMTStreamingOutputCallRequest messageWithPayloadSize:kPayloadSize + requestedResponseSize:kPayloadSize]; - NSArray *requests = @[@27182, @8, @1828, @45904]; - NSArray *responses = @[@31415, @9, @2653, @58979]; + __weak XCTestExpectation *expectation1 = [self expectationWithDescription:@"AlternateDispatchQueue1"]; // Set the default dispatch queue - NSString *queue1_label = @"test.queue1"; - NSString *queue2_label = @"test.queue2"; - dispatch_queue_t queue1 = dispatch_queue_create([queue1_label UTF8String], DISPATCH_QUEUE_SERIAL); - dispatch_queue_t queue2 = dispatch_queue_create([queue2_label UTF8String], DISPATCH_QUEUE_SERIAL); - [_service setDefaultResponseDispatchQueue:queue1]; + NSString *queue_label = @"test.queue1"; + dispatch_queue_t queue = dispatch_queue_create([queue_label UTF8String], DISPATCH_QUEUE_SERIAL); GRXBufferedPipe *requestsBuffer1 = [[GRXBufferedPipe alloc] init]; - __block int index = 0; - - id request = [RMTStreamingOutputCallRequest messageWithPayloadSize:requests[index] - requestedResponseSize:responses[index]]; [requestsBuffer1 writeValue:request]; [_service fullDuplexCallWithRequestsWriter:requestsBuffer1 @@ -485,26 +479,11 @@ XCTAssertNil(error, @"Finished with unexpected error: %@", error); XCTAssertTrue(done || response, @"Event handler called without an event."); NSString *label = [NSString stringWithUTF8String:dispatch_queue_get_label(DISPATCH_CURRENT_QUEUE_LABEL)]; - XCTAssert([label isEqualToString:queue1_label]); - - if (response) { - XCTAssertLessThan(index, 4, @"More than 4 responses received."); - id expected = [RMTStreamingOutputCallResponse messageWithPayloadSize:responses[index]]; - XCTAssertEqualObjects(response, expected); - index += 1; - if (index < 4) { - id request = [RMTStreamingOutputCallRequest messageWithPayloadSize:requests[index] - requestedResponseSize:responses[index]]; - [requestsBuffer1 writeValue:request]; - } else { - [requestsBuffer1 writesFinishedWithError:nil]; - } - } + NSLog(@"main queue label:%@", label); + NSString *main_queue_label = [NSString stringWithUTF8String:dispatch_queue_get_label(dispatch_get_main_queue())]; + XCTAssert([label isEqualToString:main_queue_label]); - if (done) { - XCTAssertEqual(index, 4, @"Received %i responses instead of 4.", index); - [expectation1 fulfill]; - } + [expectation1 fulfill]; }]; [self waitForExpectationsWithTimeout:TEST_TIMEOUT handler:nil]; @@ -513,8 +492,6 @@ __weak XCTestExpectation *expectation2 = [self expectationWithDescription:@"AlternateDispatchQueue2"]; GRXBufferedPipe *requestsBuffer2 = [[GRXBufferedPipe alloc] init]; - index = 0; - [requestsBuffer2 writeValue:request]; GRPCProtoCall *rpc = [_service RPCToFullDuplexCallWithRequestsWriter:requestsBuffer2 @@ -524,32 +501,13 @@ XCTAssertNil(error, @"Finished with unexpected error: %@", error); XCTAssertTrue(done || response, @"Event handler called without an event."); NSString *label = [NSString stringWithUTF8String:dispatch_queue_get_label(DISPATCH_CURRENT_QUEUE_LABEL)]; - XCTAssert([label isEqualToString:queue2_label]); - - if (response) { - XCTAssertLessThan(index, 4, @"More than 4 responses received."); - id expected = [RMTStreamingOutputCallResponse messageWithPayloadSize:responses[index]]; - XCTAssertEqualObjects(response, expected); - index += 1; - if (index < 4) { - id request = [RMTStreamingOutputCallRequest messageWithPayloadSize:requests[index] - requestedResponseSize:responses[index]]; - [requestsBuffer2 writeValue:request]; - } else { - [requestsBuffer2 writesFinishedWithError:nil]; - } - } - - if (done) { - XCTAssertEqual(index, 4, @"Received %i responses instead of 4.", index); - [expectation2 fulfill]; - } + XCTAssert([label isEqualToString:queue_label]); + [expectation2 fulfill]; }]; - [rpc setResponseDispatchQueue:queue2]; + [rpc setResponseDispatchQueue:queue]; [rpc start]; [self waitForExpectationsWithTimeout:TEST_TIMEOUT handler:nil]; - [_service setDefaultResponseDispatchQueue:dispatch_get_main_queue()]; } @end From b965ff6bf764c9144862af6e4fb25bd90ecd6f55 Mon Sep 17 00:00:00 2001 From: Muxi Yan Date: Wed, 19 Apr 2017 14:27:47 -0700 Subject: [PATCH 10/34] Move test to GRPCClientTests --- src/objective-c/tests/GRPCClientTests.m | 55 +++++++++++++++++++++++++ src/objective-c/tests/InteropTests.m | 53 ------------------------ 2 files changed, 55 insertions(+), 53 deletions(-) diff --git a/src/objective-c/tests/GRPCClientTests.m b/src/objective-c/tests/GRPCClientTests.m index 76c15003f60..0a631d182ab 100644 --- a/src/objective-c/tests/GRPCClientTests.m +++ b/src/objective-c/tests/GRPCClientTests.m @@ -353,4 +353,59 @@ static GRPCProtoMethod *kUnaryCallMethod; [self waitForExpectationsWithTimeout:TEST_TIMEOUT handler:nil]; } +- (void)testAlternateDispatchQueue { + const int32_t kPayloadSize = 100; + RMTSimpleRequest *request = [RMTSimpleRequest message]; + request.responseSize = kPayloadSize; + + __weak XCTestExpectation *expectation1 = [self expectationWithDescription:@"AlternateDispatchQueue1"]; + + // Use default (main) dispatch queue + NSString *main_queue_label = [NSString stringWithUTF8String:dispatch_queue_get_label(dispatch_get_main_queue())]; + + GRXWriter *requestsWriter1 = [GRXWriter writerWithValue:[request data]]; + + GRPCCall *call1 = [[GRPCCall alloc] initWithHost:kHostAddress + path:kUnaryCallMethod.HTTPPath + requestsWriter:requestsWriter1]; + + id responsesWriteable1 = [[GRXWriteable alloc] initWithValueHandler:^(NSData *value) { + NSString *label = [NSString stringWithUTF8String:dispatch_queue_get_label(DISPATCH_CURRENT_QUEUE_LABEL)]; + XCTAssert([label isEqualToString:main_queue_label]); + + [expectation1 fulfill]; + } completionHandler:^(NSError *errorOrNil) { + }]; + + [call1 startWithWriteable:responsesWriteable1]; + + [self waitForExpectationsWithTimeout:TEST_TIMEOUT handler:nil]; + + // Use a custom queue + __weak XCTestExpectation *expectation2 = [self expectationWithDescription:@"AlternateDispatchQueue2"]; + + NSString *queue_label = @"test.queue1"; + dispatch_queue_t queue = dispatch_queue_create([queue_label UTF8String], DISPATCH_QUEUE_SERIAL); + + GRXWriter *requestsWriter2 = [GRXWriter writerWithValue:[request data]]; + + GRPCCall *call2 = [[GRPCCall alloc] initWithHost:kHostAddress + path:kUnaryCallMethod.HTTPPath + requestsWriter:requestsWriter2]; + + [call2 setResponseDispatchQueue:queue]; + + id responsesWriteable2 = [[GRXWriteable alloc] initWithValueHandler:^(NSData *value) { + NSString *label = [NSString stringWithUTF8String:dispatch_queue_get_label(DISPATCH_CURRENT_QUEUE_LABEL)]; + XCTAssert([label isEqualToString:queue_label]); + + [expectation2 fulfill]; + } completionHandler:^(NSError *errorOrNil) { + }]; + + [call2 startWithWriteable:responsesWriteable2]; + + [self waitForExpectationsWithTimeout:TEST_TIMEOUT handler:nil]; +} + @end diff --git a/src/objective-c/tests/InteropTests.m b/src/objective-c/tests/InteropTests.m index e7d28f4e459..91053568690 100644 --- a/src/objective-c/tests/InteropTests.m +++ b/src/objective-c/tests/InteropTests.m @@ -457,57 +457,4 @@ [self waitForExpectationsWithTimeout:TEST_TIMEOUT handler:nil]; } -- (void)testAlternateDispatchQueue { - XCTAssertNotNil(self.class.host); - NSNumber *kPayloadSize = @256; - id request = [RMTStreamingOutputCallRequest messageWithPayloadSize:kPayloadSize - requestedResponseSize:kPayloadSize]; - - __weak XCTestExpectation *expectation1 = [self expectationWithDescription:@"AlternateDispatchQueue1"]; - - // Set the default dispatch queue - NSString *queue_label = @"test.queue1"; - dispatch_queue_t queue = dispatch_queue_create([queue_label UTF8String], DISPATCH_QUEUE_SERIAL); - GRXBufferedPipe *requestsBuffer1 = [[GRXBufferedPipe alloc] init]; - - [requestsBuffer1 writeValue:request]; - - [_service fullDuplexCallWithRequestsWriter:requestsBuffer1 - eventHandler:^(BOOL done, - RMTStreamingOutputCallResponse *response, - NSError *error) { - XCTAssertNil(error, @"Finished with unexpected error: %@", error); - XCTAssertTrue(done || response, @"Event handler called without an event."); - NSString *label = [NSString stringWithUTF8String:dispatch_queue_get_label(DISPATCH_CURRENT_QUEUE_LABEL)]; - NSLog(@"main queue label:%@", label); - NSString *main_queue_label = [NSString stringWithUTF8String:dispatch_queue_get_label(dispatch_get_main_queue())]; - XCTAssert([label isEqualToString:main_queue_label]); - - [expectation1 fulfill]; - }]; - - [self waitForExpectationsWithTimeout:TEST_TIMEOUT handler:nil]; - - // Test overriding default queue with another queue - __weak XCTestExpectation *expectation2 = [self expectationWithDescription:@"AlternateDispatchQueue2"]; - GRXBufferedPipe *requestsBuffer2 = [[GRXBufferedPipe alloc] init]; - - [requestsBuffer2 writeValue:request]; - - GRPCProtoCall *rpc = [_service RPCToFullDuplexCallWithRequestsWriter:requestsBuffer2 - eventHandler:^(BOOL done, - RMTStreamingOutputCallResponse *response, - NSError *error) { - XCTAssertNil(error, @"Finished with unexpected error: %@", error); - XCTAssertTrue(done || response, @"Event handler called without an event."); - NSString *label = [NSString stringWithUTF8String:dispatch_queue_get_label(DISPATCH_CURRENT_QUEUE_LABEL)]; - XCTAssert([label isEqualToString:queue_label]); - [expectation2 fulfill]; - }]; - [rpc setResponseDispatchQueue:queue]; - [rpc start]; - - [self waitForExpectationsWithTimeout:TEST_TIMEOUT handler:nil]; -} - @end From a2529ce53ae729b1ebcc0fa241ec9671c891dad1 Mon Sep 17 00:00:00 2001 From: Muxi Yan Date: Wed, 19 Apr 2017 14:32:47 -0700 Subject: [PATCH 11/34] Minor comment polish --- src/objective-c/GRPCClient/GRPCCall.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/objective-c/GRPCClient/GRPCCall.h b/src/objective-c/GRPCClient/GRPCCall.h index 85c5b47c8ea..5e9324c4456 100644 --- a/src/objective-c/GRPCClient/GRPCCall.h +++ b/src/objective-c/GRPCClient/GRPCCall.h @@ -254,7 +254,7 @@ extern id const kGRPCTrailersKey; + (void)setCallSafety:(GRPCCallSafety)callSafety host:(NSString *)host path:(NSString *)path; /** - * Set the dispatch queue to be used for queue responses. + * Set the dispatch queue to be used for callbacks. * * This configuration is only effective before the call starts. */ From ba390352c9d232d5e03e5821c06aecd2fd1f98d2 Mon Sep 17 00:00:00 2001 From: "Mark D. Roth" Date: Thu, 20 Apr 2017 06:59:26 -0700 Subject: [PATCH 12/34] clang-format --- src/core/lib/slice/slice_hash_table.c | 4 +- test/core/slice/slice_hash_table_test.c | 136 ++++++------------------ 2 files changed, 37 insertions(+), 103 deletions(-) diff --git a/src/core/lib/slice/slice_hash_table.c b/src/core/lib/slice/slice_hash_table.c index a33f6069fd5..444f22aa196 100644 --- a/src/core/lib/slice/slice_hash_table.c +++ b/src/core/lib/slice/slice_hash_table.c @@ -42,7 +42,7 @@ struct grpc_slice_hash_table { gpr_refcount refs; - void (*destroy_value)(grpc_exec_ctx *exec_ctx, void *value); + void (*destroy_value)(grpc_exec_ctx* exec_ctx, void* value); size_t size; size_t max_num_probes; grpc_slice_hash_table_entry* entries; @@ -72,7 +72,7 @@ static void grpc_slice_hash_table_add(grpc_slice_hash_table* table, grpc_slice_hash_table* grpc_slice_hash_table_create( size_t num_entries, grpc_slice_hash_table_entry* entries, - void (*destroy_value)(grpc_exec_ctx *exec_ctx, void *value)) { + void (*destroy_value)(grpc_exec_ctx* exec_ctx, void* value)) { grpc_slice_hash_table* table = gpr_zalloc(sizeof(*table)); gpr_ref_init(&table->refs, 1); table->destroy_value = destroy_value; diff --git a/test/core/slice/slice_hash_table_test.c b/test/core/slice/slice_hash_table_test.c index 24a4354b8b6..9eba57e70aa 100644 --- a/test/core/slice/slice_hash_table_test.c +++ b/test/core/slice/slice_hash_table_test.c @@ -79,106 +79,40 @@ static void destroy_string(grpc_exec_ctx* exec_ctx, void* value) { static void test_slice_hash_table() { const test_entry test_entries[] = { - {"key_0", "value_0"}, - {"key_1", "value_1"}, - {"key_2", "value_2"}, - {"key_3", "value_3"}, - {"key_4", "value_4"}, - {"key_5", "value_5"}, - {"key_6", "value_6"}, - {"key_7", "value_7"}, - {"key_8", "value_8"}, - {"key_9", "value_9"}, - {"key_10", "value_10"}, - {"key_11", "value_11"}, - {"key_12", "value_12"}, - {"key_13", "value_13"}, - {"key_14", "value_14"}, - {"key_15", "value_15"}, - {"key_16", "value_16"}, - {"key_17", "value_17"}, - {"key_18", "value_18"}, - {"key_19", "value_19"}, - {"key_20", "value_20"}, - {"key_21", "value_21"}, - {"key_22", "value_22"}, - {"key_23", "value_23"}, - {"key_24", "value_24"}, - {"key_25", "value_25"}, - {"key_26", "value_26"}, - {"key_27", "value_27"}, - {"key_28", "value_28"}, - {"key_29", "value_29"}, - {"key_30", "value_30"}, - {"key_31", "value_31"}, - {"key_32", "value_32"}, - {"key_33", "value_33"}, - {"key_34", "value_34"}, - {"key_35", "value_35"}, - {"key_36", "value_36"}, - {"key_37", "value_37"}, - {"key_38", "value_38"}, - {"key_39", "value_39"}, - {"key_40", "value_40"}, - {"key_41", "value_41"}, - {"key_42", "value_42"}, - {"key_43", "value_43"}, - {"key_44", "value_44"}, - {"key_45", "value_45"}, - {"key_46", "value_46"}, - {"key_47", "value_47"}, - {"key_48", "value_48"}, - {"key_49", "value_49"}, - {"key_50", "value_50"}, - {"key_51", "value_51"}, - {"key_52", "value_52"}, - {"key_53", "value_53"}, - {"key_54", "value_54"}, - {"key_55", "value_55"}, - {"key_56", "value_56"}, - {"key_57", "value_57"}, - {"key_58", "value_58"}, - {"key_59", "value_59"}, - {"key_60", "value_60"}, - {"key_61", "value_61"}, - {"key_62", "value_62"}, - {"key_63", "value_63"}, - {"key_64", "value_64"}, - {"key_65", "value_65"}, - {"key_66", "value_66"}, - {"key_67", "value_67"}, - {"key_68", "value_68"}, - {"key_69", "value_69"}, - {"key_70", "value_70"}, - {"key_71", "value_71"}, - {"key_72", "value_72"}, - {"key_73", "value_73"}, - {"key_74", "value_74"}, - {"key_75", "value_75"}, - {"key_76", "value_76"}, - {"key_77", "value_77"}, - {"key_78", "value_78"}, - {"key_79", "value_79"}, - {"key_80", "value_80"}, - {"key_81", "value_81"}, - {"key_82", "value_82"}, - {"key_83", "value_83"}, - {"key_84", "value_84"}, - {"key_85", "value_85"}, - {"key_86", "value_86"}, - {"key_87", "value_87"}, - {"key_88", "value_88"}, - {"key_89", "value_89"}, - {"key_90", "value_90"}, - {"key_91", "value_91"}, - {"key_92", "value_92"}, - {"key_93", "value_93"}, - {"key_94", "value_94"}, - {"key_95", "value_95"}, - {"key_96", "value_96"}, - {"key_97", "value_97"}, - {"key_98", "value_98"}, - {"key_99", "value_99"}, + {"key_0", "value_0"}, {"key_1", "value_1"}, {"key_2", "value_2"}, + {"key_3", "value_3"}, {"key_4", "value_4"}, {"key_5", "value_5"}, + {"key_6", "value_6"}, {"key_7", "value_7"}, {"key_8", "value_8"}, + {"key_9", "value_9"}, {"key_10", "value_10"}, {"key_11", "value_11"}, + {"key_12", "value_12"}, {"key_13", "value_13"}, {"key_14", "value_14"}, + {"key_15", "value_15"}, {"key_16", "value_16"}, {"key_17", "value_17"}, + {"key_18", "value_18"}, {"key_19", "value_19"}, {"key_20", "value_20"}, + {"key_21", "value_21"}, {"key_22", "value_22"}, {"key_23", "value_23"}, + {"key_24", "value_24"}, {"key_25", "value_25"}, {"key_26", "value_26"}, + {"key_27", "value_27"}, {"key_28", "value_28"}, {"key_29", "value_29"}, + {"key_30", "value_30"}, {"key_31", "value_31"}, {"key_32", "value_32"}, + {"key_33", "value_33"}, {"key_34", "value_34"}, {"key_35", "value_35"}, + {"key_36", "value_36"}, {"key_37", "value_37"}, {"key_38", "value_38"}, + {"key_39", "value_39"}, {"key_40", "value_40"}, {"key_41", "value_41"}, + {"key_42", "value_42"}, {"key_43", "value_43"}, {"key_44", "value_44"}, + {"key_45", "value_45"}, {"key_46", "value_46"}, {"key_47", "value_47"}, + {"key_48", "value_48"}, {"key_49", "value_49"}, {"key_50", "value_50"}, + {"key_51", "value_51"}, {"key_52", "value_52"}, {"key_53", "value_53"}, + {"key_54", "value_54"}, {"key_55", "value_55"}, {"key_56", "value_56"}, + {"key_57", "value_57"}, {"key_58", "value_58"}, {"key_59", "value_59"}, + {"key_60", "value_60"}, {"key_61", "value_61"}, {"key_62", "value_62"}, + {"key_63", "value_63"}, {"key_64", "value_64"}, {"key_65", "value_65"}, + {"key_66", "value_66"}, {"key_67", "value_67"}, {"key_68", "value_68"}, + {"key_69", "value_69"}, {"key_70", "value_70"}, {"key_71", "value_71"}, + {"key_72", "value_72"}, {"key_73", "value_73"}, {"key_74", "value_74"}, + {"key_75", "value_75"}, {"key_76", "value_76"}, {"key_77", "value_77"}, + {"key_78", "value_78"}, {"key_79", "value_79"}, {"key_80", "value_80"}, + {"key_81", "value_81"}, {"key_82", "value_82"}, {"key_83", "value_83"}, + {"key_84", "value_84"}, {"key_85", "value_85"}, {"key_86", "value_86"}, + {"key_87", "value_87"}, {"key_88", "value_88"}, {"key_89", "value_89"}, + {"key_90", "value_90"}, {"key_91", "value_91"}, {"key_92", "value_92"}, + {"key_93", "value_93"}, {"key_94", "value_94"}, {"key_95", "value_95"}, + {"key_96", "value_96"}, {"key_97", "value_97"}, {"key_98", "value_98"}, + {"key_99", "value_99"}, }; const size_t num_entries = GPR_ARRAY_SIZE(test_entries); // Construct table. @@ -195,7 +129,7 @@ static void test_slice_hash_table() { grpc_exec_ctx_finish(&exec_ctx); } -int main(int argc, char **argv) { +int main(int argc, char** argv) { grpc_test_init(argc, argv); test_slice_hash_table(); return 0; From f186d9d35b93bed6cb47e2902eb3caf4d43be55e Mon Sep 17 00:00:00 2001 From: "Mark D. Roth" Date: Thu, 20 Apr 2017 07:05:34 -0700 Subject: [PATCH 13/34] Fix asan bug. --- test/core/slice/slice_hash_table_test.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/core/slice/slice_hash_table_test.c b/test/core/slice/slice_hash_table_test.c index 9eba57e70aa..055d7f448fd 100644 --- a/test/core/slice/slice_hash_table_test.c +++ b/test/core/slice/slice_hash_table_test.c @@ -50,7 +50,7 @@ typedef struct { static void populate_entries(const test_entry* input, size_t num_entries, grpc_slice_hash_table_entry* output) { for (size_t i = 0; i < num_entries; ++i) { - output[i].key = grpc_slice_from_copied_string(gpr_strdup(input[i].key)); + output[i].key = grpc_slice_from_copied_string(input[i].key); output[i].value = gpr_strdup(input[i].value); } } From 493be1f1b98d3c238b9b87fc478c3f4ae25444af Mon Sep 17 00:00:00 2001 From: "Mark D. Roth" Date: Thu, 20 Apr 2017 07:10:22 -0700 Subject: [PATCH 14/34] Fix windows build. --- test/core/slice/slice_hash_table_test.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/core/slice/slice_hash_table_test.c b/test/core/slice/slice_hash_table_test.c index 055d7f448fd..67041b2d5c9 100644 --- a/test/core/slice/slice_hash_table_test.c +++ b/test/core/slice/slice_hash_table_test.c @@ -116,10 +116,12 @@ static void test_slice_hash_table() { }; const size_t num_entries = GPR_ARRAY_SIZE(test_entries); // Construct table. - grpc_slice_hash_table_entry entries[num_entries]; + grpc_slice_hash_table_entry* entries = + gpr_zalloc(sizeof(*entries) * num_entries); populate_entries(test_entries, num_entries, entries); grpc_slice_hash_table* table = grpc_slice_hash_table_create(num_entries, entries, destroy_string); + gpr_free(entries); // Check contents of table. check_values(test_entries, num_entries, table); check_non_existent_value("XX", table); From 65c23aa749f1e9c8b3534aed40d9a3e282b78918 Mon Sep 17 00:00:00 2001 From: Noah Eisen Date: Thu, 20 Apr 2017 12:33:46 -0700 Subject: [PATCH 15/34] Fix error refcount bug --- src/core/ext/filters/message_size/message_size_filter.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/core/ext/filters/message_size/message_size_filter.c b/src/core/ext/filters/message_size/message_size_filter.c index db0f0119053..e3ffc41f905 100644 --- a/src/core/ext/filters/message_size/message_size_filter.c +++ b/src/core/ext/filters/message_size/message_size_filter.c @@ -130,6 +130,8 @@ static void recv_message_ready(grpc_exec_ctx* exec_ctx, void* user_data, GRPC_ERROR_UNREF(new_error); } gpr_free(message_string); + } else { + GRPC_ERROR_REF(error); } // Invoke the next callback. grpc_closure_run(exec_ctx, calld->next_recv_message_ready, error); From 5e17e4430cebe6d1a1a96988a86eb6bd8c56fac5 Mon Sep 17 00:00:00 2001 From: Noah Eisen Date: Thu, 20 Apr 2017 12:35:27 -0700 Subject: [PATCH 16/34] Add fuzz test --- .../clusterfuzz-testcase-6462055064272896 | Bin 0 -> 51 bytes tools/run_tests/generated/tests.json | 23 ++++++++++++++++++ 2 files changed, 23 insertions(+) create mode 100644 test/core/end2end/fuzzers/api_fuzzer_corpus/clusterfuzz-testcase-6462055064272896 diff --git a/test/core/end2end/fuzzers/api_fuzzer_corpus/clusterfuzz-testcase-6462055064272896 b/test/core/end2end/fuzzers/api_fuzzer_corpus/clusterfuzz-testcase-6462055064272896 new file mode 100644 index 0000000000000000000000000000000000000000..c12128324259b3f178845e2d8672bd6bca483def GIT binary patch literal 51 zcmZQ7PAw`+En;9`Vc=q@XJq*Q|8Ko8HxDBNg9GFL{|pR_KrA3201@F~Vq|1y1prnp B3akJC literal 0 HcmV?d00001 diff --git a/tools/run_tests/generated/tests.json b/tools/run_tests/generated/tests.json index 6338ea70127..be8ee76b6dd 100644 --- a/tools/run_tests/generated/tests.json +++ b/tools/run_tests/generated/tests.json @@ -85165,6 +85165,29 @@ ], "uses_polling": false }, + { + "args": [ + "test/core/end2end/fuzzers/api_fuzzer_corpus/clusterfuzz-testcase-6462055064272896" + ], + "ci_platforms": [ + "linux" + ], + "cpu_cost": 0.1, + "exclude_configs": [ + "tsan" + ], + "exclude_iomgrs": [ + "uv" + ], + "flaky": false, + "language": "c", + "name": "api_fuzzer_one_entry", + "platforms": [ + "mac", + "linux" + ], + "uses_polling": false + }, { "args": [ "test/core/end2end/fuzzers/api_fuzzer_corpus/clusterfuzz-testcase-6499902139924480" From 03876f66e17fc8a614407bb2aa317b21a2ef7319 Mon Sep 17 00:00:00 2001 From: yang-g Date: Thu, 20 Apr 2017 14:51:58 -0700 Subject: [PATCH 17/34] Fix http2_interop test for c++ and python --- src/python/grpcio_tests/tests/http2/negative_http2_client.py | 2 -- test/cpp/interop/http2_client.cc | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/src/python/grpcio_tests/tests/http2/negative_http2_client.py b/src/python/grpcio_tests/tests/http2/negative_http2_client.py index b184e62cfd4..90f54e80bbf 100644 --- a/src/python/grpcio_tests/tests/http2/negative_http2_client.py +++ b/src/python/grpcio_tests/tests/http2/negative_http2_client.py @@ -96,8 +96,6 @@ def _rst_during_data(stub): def _rst_after_data(stub): resp_future = stub.UnaryCall.future(_SIMPLE_REQUEST) - _validate_payload_type_and_length( - next(resp_future), messages_pb2.COMPRESSABLE, _RESPONSE_SIZE) _validate_status_code_and_details(resp_future, grpc.StatusCode.INTERNAL, "Received RST_STREAM with error code 0") diff --git a/test/cpp/interop/http2_client.cc b/test/cpp/interop/http2_client.cc index 38a437f39f8..2109e346164 100644 --- a/test/cpp/interop/http2_client.cc +++ b/test/cpp/interop/http2_client.cc @@ -108,7 +108,7 @@ bool Http2Client::DoRstAfterData() { SimpleResponse response; AssertStatusCode(SendUnaryCall(&response), grpc::StatusCode::INTERNAL); - GPR_ASSERT(response.has_payload()); // data should be received + // There is no guarantee that data would be received. gpr_log(GPR_DEBUG, "Done testing reset stream after data"); return true; From a935ae1505c768ca0ce6ea981af1fb03c56052d7 Mon Sep 17 00:00:00 2001 From: Vijay Pai Date: Thu, 13 Apr 2017 21:07:45 +0000 Subject: [PATCH 18/34] Fix build rules for grpc++_unsecure, test that it builds properly --- BUILD | 5 ++++- WORKSPACE | 11 +++++------ test/cpp/util/BUILD | 9 +++++++++ 3 files changed, 18 insertions(+), 7 deletions(-) diff --git a/BUILD b/BUILD index 70bd4062a49..57709e47450 100644 --- a/BUILD +++ b/BUILD @@ -1277,6 +1277,9 @@ grpc_cc_library( "src/cpp/server/thread_pool_interface.h", "src/cpp/thread_manager/thread_manager.h", ], + external_deps = [ + "nanopb", + ], language = "c++", public_hdrs = [ "include/grpc++/alarm.h", @@ -1328,7 +1331,7 @@ grpc_cc_library( "include/grpc++/support/time.h", ], deps = [ - "grpc", + "grpc_base", "grpc++_codegen_base", ], ) diff --git a/WORKSPACE b/WORKSPACE index 5ba82f3127b..a78a88979e7 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -15,17 +15,17 @@ bind( bind( name = "protobuf", - actual = "@submodule_protobuf//:protobuf", + actual = "@com_google_protobuf//:protobuf", ) bind( name = "protobuf_clib", - actual = "@submodule_protobuf//:protoc_lib", + actual = "@com_google_protobuf//:protoc_lib", ) bind( name = "protocol_compiler", - actual = "@submodule_protobuf//:protoc", + actual = "@com_google_protobuf//:protoc", ) bind( @@ -48,9 +48,8 @@ bind( actual = "@com_github_gflags_gflags//:gflags", ) -new_local_repository( +local_repository( name = "submodule_boringssl", - build_file = "third_party/boringssl-with-bazel/BUILD", path = "third_party/boringssl-with-bazel", ) @@ -61,7 +60,7 @@ new_local_repository( ) new_local_repository( - name = "submodule_protobuf", + name = "com_google_protobuf", build_file = "third_party/protobuf/BUILD", path = "third_party/protobuf", ) diff --git a/test/cpp/util/BUILD b/test/cpp/util/BUILD index 38f804ffd40..c83f89eb906 100644 --- a/test/cpp/util/BUILD +++ b/test/cpp/util/BUILD @@ -29,6 +29,15 @@ licenses(["notice"]) # 3-clause BSD +# The following builds a shared-object to confirm that grpc++_unsecure +# builds properly. Build-only is sufficient here +cc_binary( + name = "testso.so", + srcs = [], + deps = ["//:grpc++_unsecure"], + linkshared = 1, +) + cc_library( name = "test_config", srcs = [ From f74eaa69575f2570d18dbc37acafb60670f06fe5 Mon Sep 17 00:00:00 2001 From: Vijay Pai Date: Fri, 14 Apr 2017 17:19:52 -0700 Subject: [PATCH 19/34] Fix up protobuf submodule name to match bazel expectation --- bazel/BUILD | 4 ++-- bazel/cc_grpc_library.bzl | 2 +- bazel/generate_cc.bzl | 6 +++--- tools/grpcz/BUILD | 13 +++++++------ 4 files changed, 13 insertions(+), 12 deletions(-) diff --git a/bazel/BUILD b/bazel/BUILD index b86dcc2ad7e..cb2d9d66aee 100644 --- a/bazel/BUILD +++ b/bazel/BUILD @@ -35,12 +35,12 @@ load(":cc_grpc_library.bzl", "cc_grpc_library") proto_library( name = "well_known_protos_list", - srcs = ["@submodule_protobuf//:well_known_protos"], + srcs = ["@com_google_protobuf//:well_known_protos"], ) cc_grpc_library( name = "well_known_protos", srcs = "well_known_protos_list", - deps = [], proto_only = True, + deps = [], ) diff --git a/bazel/cc_grpc_library.bzl b/bazel/cc_grpc_library.bzl index ab1add476e1..a3996eca008 100644 --- a/bazel/cc_grpc_library.bzl +++ b/bazel/cc_grpc_library.bzl @@ -14,7 +14,7 @@ def cc_grpc_library(name, srcs, deps, proto_only, well_known_protos, use_externa the compiled code of any message that the services depend on. well_known_protos: The target from protobuf library that exports well known protos. Currently it will only work if the value is - "@submodule_protobuf//:well_known_protos" + "@com_google_protobuf//:well_known_protos" use_external: When True the grpc deps are prefixed with //external. This allows grpc to be used as a dependency in other bazel projects. **kwargs: rest of arguments, e.g., compatible_with and visibility. diff --git a/bazel/generate_cc.bzl b/bazel/generate_cc.bzl index f3961f0a419..35c2983b542 100644 --- a/bazel/generate_cc.bzl +++ b/bazel/generate_cc.bzl @@ -35,10 +35,10 @@ def generate_cc_impl(ctx): well_known_proto_files = [] if ctx.attr.well_known_protos: f = ctx.attr.well_known_protos.files.to_list()[0].dirname - if f != "external/submodule_protobuf/src/google/protobuf": - print("Error: Only @submodule_protobuf//:well_known_protos is supported") + if f != "external/com_google_protobuf/src/google/protobuf": + print("Error: Only @com_google_protobuf//:well_known_protos is supported") else: - # f points to "external/submodule_protobuf/src/google/protobuf" + # f points to "external/com_google_protobuf/src/google/protobuf" # add -I argument to protoc so it knows where to look for the proto files. arguments += ["-I{0}".format(f + "/../..")] well_known_proto_files = [f for f in ctx.attr.well_known_protos.files] diff --git a/tools/grpcz/BUILD b/tools/grpcz/BUILD index cac7df2a9da..cc887a53751 100644 --- a/tools/grpcz/BUILD +++ b/tools/grpcz/BUILD @@ -33,30 +33,31 @@ package(default_visibility = ["//visibility:public"]) load("//:bazel/grpc_build_system.bzl", "grpc_proto_library") -grpc_proto_library ( +grpc_proto_library( name = "monitoring_proto", srcs = [ "monitoring.proto", ], + well_known_protos = "@com_google_protobuf//:well_known_protos", deps = [ ":census_proto", ], - well_known_protos = "@submodule_protobuf//:well_known_protos", ) -grpc_proto_library ( +grpc_proto_library( name = "census_proto", srcs = [ "census.proto", ], - well_known_protos = "@submodule_protobuf//:well_known_protos", + well_known_protos = "@com_google_protobuf//:well_known_protos", ) cc_binary( name = "grpcz_client", - srcs = ["grpcz_client.cc",], + srcs = ["grpcz_client.cc"], deps = [ - "//external:gflags", "monitoring_proto", + "//external:gflags", + "@mongoose_repo//:mongoose_lib", ], ) From 922d117318f1449f17b1f1dc45ca921313efd5eb Mon Sep 17 00:00:00 2001 From: Vijay Pai Date: Sun, 16 Apr 2017 15:43:09 -0700 Subject: [PATCH 20/34] One more use of incorrect submodule name --- src/proto/grpc/status/BUILD | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/proto/grpc/status/BUILD b/src/proto/grpc/status/BUILD index c17f87eb3de..71363bd1b63 100644 --- a/src/proto/grpc/status/BUILD +++ b/src/proto/grpc/status/BUILD @@ -37,7 +37,5 @@ grpc_proto_library( name = "status_proto", srcs = ["status.proto"], has_services = False, - well_known_protos = "@submodule_protobuf//:well_known_protos", + well_known_protos = "@com_google_protobuf//:well_known_protos", ) - - From 576e546019140a05cc88f896e7ff50d70704d719 Mon Sep 17 00:00:00 2001 From: Vijay Pai Date: Thu, 20 Apr 2017 18:40:16 +0000 Subject: [PATCH 21/34] Changes required to make build possible: create separate grpc++_base{,_unsecure}, move grpc_compression_trace definition, and remove an assertion about library initialization from server --- BUILD | 23 ++++++++++--------- bazel/grpc_build_system.bzl | 13 +++++++++++ .../message_compress_filter.c | 2 -- src/core/lib/surface/call.c | 1 + src/core/lib/surface/server.c | 2 -- 5 files changed, 26 insertions(+), 15 deletions(-) diff --git a/BUILD b/BUILD index 57709e47450..09b17ad6eb2 100644 --- a/BUILD +++ b/BUILD @@ -35,7 +35,8 @@ exports_files(["LICENSE"]) package(default_visibility = ["//visibility:public"]) -load("//bazel:grpc_build_system.bzl", "grpc_cc_library", "grpc_proto_plugin") +load("//bazel:grpc_build_system.bzl", "grpc_cc_library", + "grpc_proto_plugin", "grpc_cc_libraries") # This should be updated along with build.yaml g_stands_for = "gentle" @@ -163,7 +164,7 @@ grpc_cc_library( standalone = True, deps = [ "gpr", - "grpc++_base", + "grpc++_base_unsecure", "grpc++_codegen_base", "grpc++_codegen_base_src", "grpc_unsecure", @@ -1231,8 +1232,12 @@ grpc_cc_library( ], ) -grpc_cc_library( - name = "grpc++_base", +grpc_cc_libraries( + name_list = ["grpc++_base", "grpc++_base_unsecure"], + additional_dep_list = [ + ["grpc", ], + ["grpc_unsecure", ], + ], srcs = [ "src/cpp/client/channel_cc.cc", "src/cpp/client/client_context.cc", @@ -1267,7 +1272,7 @@ grpc_cc_library( "src/cpp/util/status.cc", "src/cpp/util/string_ref.cc", "src/cpp/util/time_cc.cc", - ], + ], hdrs = [ "src/cpp/client/create_channel_internal.h", "src/cpp/common/channel_filter.h", @@ -1276,10 +1281,7 @@ grpc_cc_library( "src/cpp/server/health/health.pb.h", "src/cpp/server/thread_pool_interface.h", "src/cpp/thread_manager/thread_manager.h", - ], - external_deps = [ - "nanopb", - ], + ], language = "c++", public_hdrs = [ "include/grpc++/alarm.h", @@ -1329,9 +1331,8 @@ grpc_cc_library( "include/grpc++/support/stub_options.h", "include/grpc++/support/sync_stream.h", "include/grpc++/support/time.h", - ], + ], deps = [ - "grpc_base", "grpc++_codegen_base", ], ) diff --git a/bazel/grpc_build_system.bzl b/bazel/grpc_build_system.bzl index 8b524bd0e52..984c06de489 100644 --- a/bazel/grpc_build_system.bzl +++ b/bazel/grpc_build_system.bzl @@ -49,6 +49,19 @@ def grpc_cc_library(name, srcs = [], public_hdrs = [], hdrs = [], external_deps ] ) +def grpc_cc_libraries(name_list, additional_dep_list, srcs = [], public_hdrs = [], hdrs = [], external_deps = [], deps = [], standalone = False, language="C++"): + for i in range(len(name_list)): + grpc_cc_library( + name = name_list[i], + srcs = srcs, + hdrs = hdrs, + public_hdrs = public_hdrs, + deps = deps + additional_dep_list[i], + external_deps = external_deps, + standalone = standalone, + language = language + ) + def grpc_proto_plugin(name, srcs = [], deps = []): native.cc_binary( name = name, diff --git a/src/core/ext/filters/http/message_compress/message_compress_filter.c b/src/core/ext/filters/http/message_compress/message_compress_filter.c index f414a60eee4..4f5f41e9b04 100644 --- a/src/core/ext/filters/http/message_compress/message_compress_filter.c +++ b/src/core/ext/filters/http/message_compress/message_compress_filter.c @@ -49,8 +49,6 @@ #include "src/core/lib/support/string.h" #include "src/core/lib/transport/static_metadata.h" -int grpc_compression_trace = 0; - #define INITIAL_METADATA_UNSEEN 0 #define HAS_COMPRESSION_ALGORITHM 2 #define NO_COMPRESSION_ALGORITHM 4 diff --git a/src/core/lib/surface/call.c b/src/core/lib/surface/call.c index 9aa457d792c..75258065830 100644 --- a/src/core/lib/surface/call.c +++ b/src/core/lib/surface/call.c @@ -245,6 +245,7 @@ struct grpc_call { }; int grpc_call_error_trace = 0; +int grpc_compression_trace = 0; #define CALL_STACK_FROM_CALL(call) ((grpc_call_stack *)((call) + 1)) #define CALL_FROM_CALL_STACK(call_stack) (((grpc_call *)(call_stack)) - 1) diff --git a/src/core/lib/surface/server.c b/src/core/lib/surface/server.c index 26c81e9aca9..e133d1d2a46 100644 --- a/src/core/lib/surface/server.c +++ b/src/core/lib/surface/server.c @@ -1033,8 +1033,6 @@ grpc_server *grpc_server_create(const grpc_channel_args *args, void *reserved) { grpc_server *server = gpr_zalloc(sizeof(grpc_server)); - GPR_ASSERT(grpc_is_initialized() && "call grpc_init()"); - gpr_mu_init(&server->mu_global); gpr_mu_init(&server->mu_call); gpr_cv_init(&server->starting_cv); From 2757982c63cfbd5f4e847a06d4a94cbcef8c4e5c Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Thu, 20 Apr 2017 15:33:40 -0700 Subject: [PATCH 22/34] C++ compatibility fix for tmpfile_posix.c --- src/core/lib/support/tmpfile_posix.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/core/lib/support/tmpfile_posix.c b/src/core/lib/support/tmpfile_posix.c index 0cd4bb6fc3c..5771c158e07 100644 --- a/src/core/lib/support/tmpfile_posix.c +++ b/src/core/lib/support/tmpfile_posix.c @@ -50,34 +50,34 @@ FILE *gpr_tmpfile(const char *prefix, char **tmp_filename) { FILE *result = NULL; - char *template; + char *filename_template; int fd; if (tmp_filename != NULL) *tmp_filename = NULL; - gpr_asprintf(&template, "/tmp/%s_XXXXXX", prefix); - GPR_ASSERT(template != NULL); + gpr_asprintf(&filename_template, "/tmp/%s_XXXXXX", prefix); + GPR_ASSERT(filename_template != NULL); - fd = mkstemp(template); + fd = mkstemp(filename_template); if (fd == -1) { - gpr_log(GPR_ERROR, "mkstemp failed for template %s with error %s.", - template, strerror(errno)); + gpr_log(GPR_ERROR, "mkstemp failed for filename_template %s with error %s.", + filename_template, strerror(errno)); goto end; } result = fdopen(fd, "w+"); if (result == NULL) { gpr_log(GPR_ERROR, "Could not open file %s from fd %d (error = %s).", - template, fd, strerror(errno)); - unlink(template); + filename_template, fd, strerror(errno)); + unlink(filename_template); close(fd); goto end; } end: if (result != NULL && tmp_filename != NULL) { - *tmp_filename = template; + *tmp_filename = filename_template; } else { - gpr_free(template); + gpr_free(filename_template); } return result; } From cdd9e568641985e25adf4b0b1977e2a53295f77e Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Thu, 20 Apr 2017 15:47:23 -0700 Subject: [PATCH 23/34] C++ compatibility fix for stack_lockfree.c --- src/core/lib/support/stack_lockfree.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/core/lib/support/stack_lockfree.c b/src/core/lib/support/stack_lockfree.c index c481a3e0dc5..dfbd3fb1251 100644 --- a/src/core/lib/support/stack_lockfree.c +++ b/src/core/lib/support/stack_lockfree.c @@ -76,13 +76,13 @@ struct gpr_stack_lockfree { gpr_stack_lockfree *gpr_stack_lockfree_create(size_t entries) { gpr_stack_lockfree *stack; - stack = gpr_malloc(sizeof(*stack)); + stack = (gpr_stack_lockfree *)gpr_malloc(sizeof(*stack)); /* Since we only allocate 16 bits to represent an entry number, * make sure that we are within the desired range */ /* Reserve the highest entry number as a dummy */ GPR_ASSERT(entries < INVALID_ENTRY_INDEX); - stack->entries = gpr_malloc_aligned(entries * sizeof(stack->entries[0]), - ENTRY_ALIGNMENT_BITS); + stack->entries = (lockfree_node *)gpr_malloc_aligned( + entries * sizeof(stack->entries[0]), ENTRY_ALIGNMENT_BITS); /* Clear out all entries */ memset(stack->entries, 0, entries * sizeof(stack->entries[0])); memset(&stack->head, 0, sizeof(stack->head)); From fd00263ae91d93e7dea28c6b7f229201e46463b5 Mon Sep 17 00:00:00 2001 From: Muxi Yan Date: Thu, 20 Apr 2017 16:16:49 -0700 Subject: [PATCH 24/34] Fix nit --- src/objective-c/tests/GRPCClientTests.m | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/objective-c/tests/GRPCClientTests.m b/src/objective-c/tests/GRPCClientTests.m index 0a631d182ab..e36f5c3ee93 100644 --- a/src/objective-c/tests/GRPCClientTests.m +++ b/src/objective-c/tests/GRPCClientTests.m @@ -366,8 +366,8 @@ static GRPCProtoMethod *kUnaryCallMethod; GRXWriter *requestsWriter1 = [GRXWriter writerWithValue:[request data]]; GRPCCall *call1 = [[GRPCCall alloc] initWithHost:kHostAddress - path:kUnaryCallMethod.HTTPPath - requestsWriter:requestsWriter1]; + path:kUnaryCallMethod.HTTPPath + requestsWriter:requestsWriter1]; id responsesWriteable1 = [[GRXWriteable alloc] initWithValueHandler:^(NSData *value) { NSString *label = [NSString stringWithUTF8String:dispatch_queue_get_label(DISPATCH_CURRENT_QUEUE_LABEL)]; From a1ac2a131738e9b1708c963fa5b83826c77a1206 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Fri, 21 Apr 2017 07:20:38 -0700 Subject: [PATCH 25/34] Allow specifying a maximum run time to run_tests --- tools/run_tests/python_utils/jobset.py | 14 +++++++++++--- tools/run_tests/run_tests.py | 5 +++-- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/tools/run_tests/python_utils/jobset.py b/tools/run_tests/python_utils/jobset.py index 5d812f28ee9..460f359cf35 100755 --- a/tools/run_tests/python_utils/jobset.py +++ b/tools/run_tests/python_utils/jobset.py @@ -348,7 +348,7 @@ class Jobset(object): """Manages one run of jobs.""" def __init__(self, check_cancelled, maxjobs, newline_on_success, travis, - stop_on_failure, add_env, quiet_success): + stop_on_failure, add_env, quiet_success, max_time): self._running = set() self._check_cancelled = check_cancelled self._cancelled = False @@ -360,6 +360,7 @@ class Jobset(object): self._stop_on_failure = stop_on_failure self._add_env = add_env self._quiet_success = quiet_success + self._max_time = max_time self.resultset = {} self._remaining = None self._start_time = time.time() @@ -379,6 +380,12 @@ class Jobset(object): def start(self, spec): """Start a job. Return True on success, False on failure.""" while True: + if self._max_time > 0 and time.time() - self._start_time > self._max_time: + skipped_job_result = JobResult() + skipped_job_result.state = 'SKIPPED' + message('SKIPPED', spec.shortname, do_newline=True) + self.resultset[spec.shortname] = [skipped_job_result] + return True if self.cancelled(): return False current_cpu_cost = self.cpu_cost() if current_cpu_cost == 0: break @@ -474,7 +481,8 @@ def run(cmdlines, stop_on_failure=False, add_env={}, skip_jobs=False, - quiet_success=False): + quiet_success=False, + max_time=-1): if skip_jobs: resultset = {} skipped_job_result = JobResult() @@ -486,7 +494,7 @@ def run(cmdlines, js = Jobset(check_cancelled, maxjobs if maxjobs is not None else _DEFAULT_MAX_JOBS, newline_on_success, travis, stop_on_failure, add_env, - quiet_success) + quiet_success, max_time) for cmdline, remaining in tag_remaining(cmdlines): if not js.start(cmdline): break diff --git a/tools/run_tests/run_tests.py b/tools/run_tests/run_tests.py index 9130bc960a8..a1ec1b2f457 100755 --- a/tools/run_tests/run_tests.py +++ b/tools/run_tests/run_tests.py @@ -1210,6 +1210,7 @@ argp.add_argument('--quiet_success', 'Useful when running many iterations of each test (argument -n).') argp.add_argument('--force_default_poller', default=False, action='store_const', const=True, help='Dont try to iterate over many polling strategies when they exist') +argp.add_argument('--max_time', default=-1, type=int, help='Maximum test runtime in seconds') args = argp.parse_args() if args.force_default_poller: @@ -1465,7 +1466,7 @@ def _build_and_run( not re.search(args.regex_exclude, spec.shortname)))) # When running on travis, we want out test runs to be as similar as possible # for reproducibility purposes. - if args.travis: + if args.travis and args.max_time <= 0: massaged_one_run = sorted(one_run, key=lambda x: x.shortname) else: # whereas otherwise, we want to shuffle things up to give all tests a @@ -1493,7 +1494,7 @@ def _build_and_run( all_runs, check_cancelled, newline_on_success=newline_on_success, travis=args.travis, maxjobs=args.jobs, stop_on_failure=args.stop_on_failure, - quiet_success=args.quiet_success) + quiet_success=args.quiet_success, max_time=args.max_time) if resultset: for k, v in sorted(resultset.items()): num_runs, num_failures = _calculate_num_runs_failures(v) From ceea9691460545eb7653ff7dbcba476ab6208d6b Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Fri, 21 Apr 2017 07:49:57 -0700 Subject: [PATCH 26/34] Extend time capping to run_tests_matrix scripts --- tools/run_tests/run_tests_matrix.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tools/run_tests/run_tests_matrix.py b/tools/run_tests/run_tests_matrix.py index 1da754d9f8c..02f0ec5eff1 100755 --- a/tools/run_tests/run_tests_matrix.py +++ b/tools/run_tests/run_tests_matrix.py @@ -377,6 +377,9 @@ if __name__ == "__main__": argp.add_argument('-n', '--runs_per_test', default=1, type=_runs_per_test_type, help='How many times to run each tests. >1 runs implies ' + 'omitting passing test from the output & reports.') + argp.add_argument('--max_time', default=-1, type=int, + help='Maximum amount of time to run tests for' + + '(other tests will be skipped)') args = argp.parse_args() extra_args = [] @@ -388,6 +391,8 @@ if __name__ == "__main__": extra_args.append('-n') extra_args.append('%s' % args.runs_per_test) extra_args.append('--quiet_success') + if args.max_time > 0: + extra_args.extend(('--max_time', '%d' % args.max_time)) all_jobs = _create_test_jobs(extra_args=extra_args, inner_jobs=args.inner_jobs) + \ _create_portability_test_jobs(extra_args=extra_args, inner_jobs=args.inner_jobs) From 566fd3b36ee4077c9e02aae23bf1bfe13ea0028d Mon Sep 17 00:00:00 2001 From: Mehrdad Afshari Date: Thu, 20 Apr 2017 15:53:51 -0700 Subject: [PATCH 27/34] Fix markdown in README.md --- README.md | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index e6d8792d9c6..0edea885185 100644 --- a/README.md +++ b/README.md @@ -21,7 +21,7 @@ See [Performance dashboard](http://performance-dot-grpc-testing.appspot.com/expl # Repository Structure & Status -This repository contains source code for gRPC libraries for multiple languages written on top of shared C core library [src/core] (src/core). +This repository contains source code for gRPC libraries for multiple languages written on top of shared C core library [src/core](src/core). Libraries in different languages may be in different states of development. We are seeking contributions for all of these libraries. @@ -36,10 +36,9 @@ Libraries in different languages may be in different states of development. We a | C# | [src/csharp](src/csharp) | 1.0 | | Objective-C | [src/objective-c](src/objective-c) | 1.0 | - -Java source code is in the [grpc-java](http://github.com/grpc/grpc-java) repository. -Go source code is in the [grpc-go](http://github.com/grpc/grpc-go) repository. - +Java source code is in the [grpc-java](http://github.com/grpc/grpc-java) +repository. Go source code is in the +[grpc-go](http://github.com/grpc/grpc-go) repository. See [MANIFEST.md](MANIFEST.md) for a listing of top-level items in the repository. From 811fcef8ad385b07135141834f4315b0dd568e9a Mon Sep 17 00:00:00 2001 From: Eric Anderson Date: Thu, 20 Apr 2017 15:36:32 -0700 Subject: [PATCH 28/34] doc/interop: Fix link for CacheableUnaryCall --- doc/interop-test-descriptions.md | 1 + 1 file changed, 1 insertion(+) diff --git a/doc/interop-test-descriptions.md b/doc/interop-test-descriptions.md index 66a034d6307..050a6234c9d 100644 --- a/doc/interop-test-descriptions.md +++ b/doc/interop-test-descriptions.md @@ -986,6 +986,7 @@ for the `SimpleRequest.response_type`. If the server does not support the `response_type`, then it should fail the RPC with `INVALID_ARGUMENT`. ### CacheableUnaryCall +[CacheableUnaryCall]: #cacheableunarycall Server gets the default SimpleRequest proto as the request. The content of the request is ignored. It returns the SimpleResponse proto with the payload set From 401aa08915b427c03a5616e79127cc0f882bdd8a Mon Sep 17 00:00:00 2001 From: Eric Anderson Date: Thu, 20 Apr 2017 15:37:05 -0700 Subject: [PATCH 29/34] doc/interop: Simplify language to fix missed x-user-ip --- doc/interop-test-descriptions.md | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/doc/interop-test-descriptions.md b/doc/interop-test-descriptions.md index 050a6234c9d..b040621f883 100644 --- a/doc/interop-test-descriptions.md +++ b/doc/interop-test-descriptions.md @@ -81,9 +81,8 @@ Procedure: Client marks the request as cacheable by setting the cacheable flag in the request context. Longer term this should be driven by the method option specified in the proto file itself. - 2. Client calls CacheableUnaryCall with `SimpleRequest` request again - immediately with the same payload as the previous request. Cacheable flag is - also set for this request's context. + 2. Client calls CacheableUnaryCall again immediately with the same request and + configuration as the previous call. Client asserts: * Both calls were successful From c3fc8342b19e774ef6daf8d4736ca1af61408c75 Mon Sep 17 00:00:00 2001 From: David Garcia Quintas Date: Fri, 21 Apr 2017 15:01:18 -0700 Subject: [PATCH 30/34] Demote handshake logging msg to DEBUG --- src/core/ext/transport/chttp2/server/chttp2_server.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/ext/transport/chttp2/server/chttp2_server.c b/src/core/ext/transport/chttp2/server/chttp2_server.c index b463506a98f..b9c62c376a1 100644 --- a/src/core/ext/transport/chttp2/server/chttp2_server.c +++ b/src/core/ext/transport/chttp2/server/chttp2_server.c @@ -80,7 +80,7 @@ static void on_handshake_done(grpc_exec_ctx *exec_ctx, void *arg, gpr_mu_lock(&connection_state->server_state->mu); if (error != GRPC_ERROR_NONE || connection_state->server_state->shutdown) { const char *error_str = grpc_error_string(error); - gpr_log(GPR_ERROR, "Handshaking failed: %s", error_str); + gpr_log(GPR_DEBUG, "Handshaking failed: %s", error_str); if (error == GRPC_ERROR_NONE && args->endpoint != NULL) { // We were shut down after handshaking completed successfully, so From 20cb627339c20e86814d64ba4837d7bdd6f35195 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Fri, 21 Apr 2017 15:07:13 -0700 Subject: [PATCH 31/34] Fix HTTP proxy tests - heap allocate the pollset shutdown closure (this may be called asynchronously) - ensure a poller remains until all endpoints are closed --- .../end2end/fixtures/http_proxy_fixture.c | 59 +++++++++++++------ test/core/end2end/tests/cancel_after_invoke.c | 9 +-- 2 files changed, 47 insertions(+), 21 deletions(-) diff --git a/test/core/end2end/fixtures/http_proxy_fixture.c b/test/core/end2end/fixtures/http_proxy_fixture.c index 451ed268d30..02235d96e73 100644 --- a/test/core/end2end/fixtures/http_proxy_fixture.c +++ b/test/core/end2end/fixtures/http_proxy_fixture.c @@ -59,6 +59,7 @@ #include "src/core/lib/iomgr/sockaddr_utils.h" #include "src/core/lib/iomgr/tcp_client.h" #include "src/core/lib/iomgr/tcp_server.h" +#include "src/core/lib/iomgr/timer.h" #include "src/core/lib/slice/slice_internal.h" #include "test/core/util/port.h" @@ -69,7 +70,7 @@ struct grpc_end2end_http_proxy { grpc_channel_args* channel_args; gpr_mu* mu; grpc_pollset* pollset; - gpr_atm shutdown; + gpr_refcount users; }; // @@ -77,6 +78,8 @@ struct grpc_end2end_http_proxy { // typedef struct proxy_connection { + grpc_end2end_http_proxy* proxy; + grpc_endpoint* client_endpoint; grpc_endpoint* server_endpoint; @@ -103,13 +106,26 @@ typedef struct proxy_connection { grpc_http_request http_request; } proxy_connection; +static void proxy_connection_ref(proxy_connection* conn, const char* reason) { + gpr_log(GPR_DEBUG, "proxy_connection_ref: %p %s %" PRIdPTR " --> %" PRIdPTR, + conn, reason, gpr_atm_no_barrier_load(&conn->refcount.count), + gpr_atm_no_barrier_load(&conn->refcount.count) - 1); + gpr_ref(&conn->refcount); +} + // Helper function to destroy the proxy connection. static void proxy_connection_unref(grpc_exec_ctx* exec_ctx, - proxy_connection* conn) { + proxy_connection* conn, const char* reason) { + gpr_log(GPR_DEBUG, "proxy_connection_unref: %p %s %" PRIdPTR " --> %" PRIdPTR, + conn, reason, gpr_atm_no_barrier_load(&conn->refcount.count), + gpr_atm_no_barrier_load(&conn->refcount.count) - 1); if (gpr_unref(&conn->refcount)) { + gpr_log(GPR_DEBUG, "endpoints: %p %p", conn->client_endpoint, + conn->server_endpoint); grpc_endpoint_destroy(exec_ctx, conn->client_endpoint); - if (conn->server_endpoint != NULL) + if (conn->server_endpoint != NULL) { grpc_endpoint_destroy(exec_ctx, conn->server_endpoint); + } grpc_pollset_set_destroy(exec_ctx, conn->pollset_set); grpc_slice_buffer_destroy_internal(exec_ctx, &conn->client_read_buffer); grpc_slice_buffer_destroy_internal(exec_ctx, @@ -121,6 +137,7 @@ static void proxy_connection_unref(grpc_exec_ctx* exec_ctx, grpc_slice_buffer_destroy_internal(exec_ctx, &conn->server_write_buffer); grpc_http_parser_destroy(&conn->http_parser); grpc_http_request_destroy(&conn->http_request); + gpr_unref(&conn->proxy->users); gpr_free(conn); } } @@ -139,7 +156,7 @@ static void proxy_connection_failed(grpc_exec_ctx* exec_ctx, grpc_endpoint_shutdown(exec_ctx, conn->server_endpoint, GRPC_ERROR_REF(error)); } - proxy_connection_unref(exec_ctx, conn); + proxy_connection_unref(exec_ctx, conn, "conn_failed"); } // Callback for writing proxy data to the client. @@ -163,7 +180,7 @@ static void on_client_write_done(grpc_exec_ctx* exec_ctx, void* arg, &conn->on_client_write_done); } else { // No more writes. Unref the connection. - proxy_connection_unref(exec_ctx, conn); + proxy_connection_unref(exec_ctx, conn, "write_done"); } } @@ -188,7 +205,7 @@ static void on_server_write_done(grpc_exec_ctx* exec_ctx, void* arg, &conn->on_server_write_done); } else { // No more writes. Unref the connection. - proxy_connection_unref(exec_ctx, conn); + proxy_connection_unref(exec_ctx, conn, "server_write"); } } @@ -214,7 +231,7 @@ static void on_client_read_done(grpc_exec_ctx* exec_ctx, void* arg, } else { grpc_slice_buffer_move_into(&conn->client_read_buffer, &conn->server_write_buffer); - gpr_ref(&conn->refcount); + proxy_connection_ref(conn, "client_read"); grpc_endpoint_write(exec_ctx, conn->server_endpoint, &conn->server_write_buffer, &conn->on_server_write_done); @@ -246,7 +263,7 @@ static void on_server_read_done(grpc_exec_ctx* exec_ctx, void* arg, } else { grpc_slice_buffer_move_into(&conn->server_read_buffer, &conn->client_write_buffer); - gpr_ref(&conn->refcount); + proxy_connection_ref(conn, "server_read"); grpc_endpoint_write(exec_ctx, conn->client_endpoint, &conn->client_write_buffer, &conn->on_client_write_done); @@ -270,7 +287,9 @@ static void on_write_response_done(grpc_exec_ctx* exec_ctx, void* arg, // Start reading from both client and server. One of the read // requests inherits our ref to conn, but we need to take a new ref // for the other one. - gpr_ref(&conn->refcount); + proxy_connection_ref(conn, "client_read"); + proxy_connection_ref(conn, "server_read"); + proxy_connection_unref(exec_ctx, conn, "write_response"); grpc_endpoint_read(exec_ctx, conn->client_endpoint, &conn->client_read_buffer, &conn->on_client_read_done); grpc_endpoint_read(exec_ctx, conn->server_endpoint, &conn->server_read_buffer, @@ -312,6 +331,8 @@ static void on_server_connect_done(grpc_exec_ctx* exec_ctx, void* arg, static void on_read_request_done(grpc_exec_ctx* exec_ctx, void* arg, grpc_error* error) { proxy_connection* conn = arg; + gpr_log(GPR_DEBUG, "on_read_request_done: %p %s", conn, + grpc_error_string(error)); if (error != GRPC_ERROR_NONE) { proxy_connection_failed(exec_ctx, conn, true /* is_client */, "HTTP proxy read request", error); @@ -376,12 +397,15 @@ static void on_accept(grpc_exec_ctx* exec_ctx, void* arg, gpr_free(acceptor); grpc_end2end_http_proxy* proxy = arg; // Instantiate proxy_connection. - proxy_connection* conn = gpr_malloc(sizeof(*conn)); - memset(conn, 0, sizeof(*conn)); + proxy_connection* conn = gpr_zalloc(sizeof(*conn)); + gpr_ref(&proxy->users); conn->client_endpoint = endpoint; + conn->proxy = proxy; gpr_ref_init(&conn->refcount, 1); conn->pollset_set = grpc_pollset_set_create(); + gpr_log(GPR_DEBUG, "on_accept: %p", conn); grpc_pollset_set_add_pollset(exec_ctx, conn->pollset_set, proxy->pollset); + grpc_endpoint_add_to_pollset_set(exec_ctx, endpoint, conn->pollset_set); grpc_closure_init(&conn->on_read_request_done, on_read_request_done, conn, grpc_schedule_on_exec_ctx); grpc_closure_init(&conn->on_server_connect_done, on_server_connect_done, conn, @@ -416,6 +440,7 @@ static void thread_main(void* arg) { grpc_end2end_http_proxy* proxy = arg; grpc_exec_ctx exec_ctx = GRPC_EXEC_CTX_INIT; do { + gpr_ref(&proxy->users); const gpr_timespec now = gpr_now(GPR_CLOCK_MONOTONIC); const gpr_timespec deadline = gpr_time_add(now, gpr_time_from_seconds(1, GPR_TIMESPAN)); @@ -426,7 +451,7 @@ static void thread_main(void* arg) { grpc_pollset_work(&exec_ctx, proxy->pollset, &worker, now, deadline)); gpr_mu_unlock(proxy->mu); grpc_exec_ctx_flush(&exec_ctx); - } while (!gpr_atm_acq_load(&proxy->shutdown)); + } while (!gpr_unref(&proxy->users)); grpc_exec_ctx_finish(&exec_ctx); } @@ -434,6 +459,7 @@ grpc_end2end_http_proxy* grpc_end2end_http_proxy_create(void) { grpc_exec_ctx exec_ctx = GRPC_EXEC_CTX_INIT; grpc_end2end_http_proxy* proxy = gpr_malloc(sizeof(*proxy)); memset(proxy, 0, sizeof(*proxy)); + gpr_ref_init(&proxy->users, 1); // Construct proxy address. const int proxy_port = grpc_pick_unused_port_or_die(); gpr_join_host_port(&proxy->proxy_name, "localhost", proxy_port); @@ -474,17 +500,16 @@ static void destroy_pollset(grpc_exec_ctx* exec_ctx, void* arg, } void grpc_end2end_http_proxy_destroy(grpc_end2end_http_proxy* proxy) { - gpr_atm_rel_store(&proxy->shutdown, 1); // Signal proxy thread to shutdown. + gpr_unref(&proxy->users); // Signal proxy thread to shutdown. grpc_exec_ctx exec_ctx = GRPC_EXEC_CTX_INIT; gpr_thd_join(proxy->thd); grpc_tcp_server_shutdown_listeners(&exec_ctx, proxy->server); grpc_tcp_server_unref(&exec_ctx, proxy->server); gpr_free(proxy->proxy_name); grpc_channel_args_destroy(&exec_ctx, proxy->channel_args); - grpc_closure destroyed; - grpc_closure_init(&destroyed, destroy_pollset, proxy->pollset, - grpc_schedule_on_exec_ctx); - grpc_pollset_shutdown(&exec_ctx, proxy->pollset, &destroyed); + grpc_pollset_shutdown(&exec_ctx, proxy->pollset, + grpc_closure_create(destroy_pollset, proxy->pollset, + grpc_schedule_on_exec_ctx)); gpr_free(proxy); grpc_exec_ctx_finish(&exec_ctx); } diff --git a/test/core/end2end/tests/cancel_after_invoke.c b/test/core/end2end/tests/cancel_after_invoke.c index f2aca737ab3..5bc9ed283bb 100644 --- a/test/core/end2end/tests/cancel_after_invoke.c +++ b/test/core/end2end/tests/cancel_after_invoke.c @@ -49,11 +49,12 @@ static void *tag(intptr_t t) { return (void *)t; } static grpc_end2end_test_fixture begin_test(grpc_end2end_test_config config, const char *test_name, cancellation_mode mode, + size_t test_ops, grpc_channel_args *client_args, grpc_channel_args *server_args) { grpc_end2end_test_fixture f; - gpr_log(GPR_INFO, "Running test: %s/%s/%s", test_name, config.name, - mode.name); + gpr_log(GPR_INFO, "Running test: %s/%s/%s [%" PRIdPTR " ops]", test_name, + config.name, mode.name, test_ops); f = config.create_fixture(client_args, server_args); config.init_server(&f, server_args); config.init_client(&f, client_args); @@ -108,8 +109,8 @@ static void test_cancel_after_invoke(grpc_end2end_test_config config, grpc_op ops[6]; grpc_op *op; grpc_call *c; - grpc_end2end_test_fixture f = - begin_test(config, "test_cancel_after_invoke", mode, NULL, NULL); + grpc_end2end_test_fixture f = begin_test(config, "test_cancel_after_invoke", + mode, test_ops, NULL, NULL); cq_verifier *cqv = cq_verifier_create(f.cq); grpc_metadata_array initial_metadata_recv; grpc_metadata_array trailing_metadata_recv; From add943869a8d043ea9e07a58725d5b110650efe8 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Fri, 21 Apr 2017 15:43:55 -0700 Subject: [PATCH 32/34] Diff now too --- tools/profiling/microbenchmarks/bm_diff.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/profiling/microbenchmarks/bm_diff.py b/tools/profiling/microbenchmarks/bm_diff.py index 09b62ae1c90..6ee4cbfc7bb 100755 --- a/tools/profiling/microbenchmarks/bm_diff.py +++ b/tools/profiling/microbenchmarks/bm_diff.py @@ -56,6 +56,7 @@ _INTERESTING = ( 'writes_per_iteration', 'atm_cas_per_iteration', 'atm_add_per_iteration', + 'nows_per_iteration', ) def changed_ratio(n, o): From b2b4122c03271a16898c872b13986914ba424116 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Fri, 21 Apr 2017 19:05:59 -0700 Subject: [PATCH 33/34] Remove logging --- test/core/end2end/fixtures/http_proxy_fixture.c | 6 ------ 1 file changed, 6 deletions(-) diff --git a/test/core/end2end/fixtures/http_proxy_fixture.c b/test/core/end2end/fixtures/http_proxy_fixture.c index 02235d96e73..a9d71b5ba40 100644 --- a/test/core/end2end/fixtures/http_proxy_fixture.c +++ b/test/core/end2end/fixtures/http_proxy_fixture.c @@ -107,18 +107,12 @@ typedef struct proxy_connection { } proxy_connection; static void proxy_connection_ref(proxy_connection* conn, const char* reason) { - gpr_log(GPR_DEBUG, "proxy_connection_ref: %p %s %" PRIdPTR " --> %" PRIdPTR, - conn, reason, gpr_atm_no_barrier_load(&conn->refcount.count), - gpr_atm_no_barrier_load(&conn->refcount.count) - 1); gpr_ref(&conn->refcount); } // Helper function to destroy the proxy connection. static void proxy_connection_unref(grpc_exec_ctx* exec_ctx, proxy_connection* conn, const char* reason) { - gpr_log(GPR_DEBUG, "proxy_connection_unref: %p %s %" PRIdPTR " --> %" PRIdPTR, - conn, reason, gpr_atm_no_barrier_load(&conn->refcount.count), - gpr_atm_no_barrier_load(&conn->refcount.count) - 1); if (gpr_unref(&conn->refcount)) { gpr_log(GPR_DEBUG, "endpoints: %p %p", conn->client_endpoint, conn->server_endpoint); From 4a1925444d81ba09878c55d8576cf981d601990c Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Fri, 21 Apr 2017 19:07:00 -0700 Subject: [PATCH 34/34] Remove more spam --- test/core/end2end/fixtures/http_proxy_fixture.c | 1 - 1 file changed, 1 deletion(-) diff --git a/test/core/end2end/fixtures/http_proxy_fixture.c b/test/core/end2end/fixtures/http_proxy_fixture.c index a9d71b5ba40..f0d09487c62 100644 --- a/test/core/end2end/fixtures/http_proxy_fixture.c +++ b/test/core/end2end/fixtures/http_proxy_fixture.c @@ -397,7 +397,6 @@ static void on_accept(grpc_exec_ctx* exec_ctx, void* arg, conn->proxy = proxy; gpr_ref_init(&conn->refcount, 1); conn->pollset_set = grpc_pollset_set_create(); - gpr_log(GPR_DEBUG, "on_accept: %p", conn); grpc_pollset_set_add_pollset(exec_ctx, conn->pollset_set, proxy->pollset); grpc_endpoint_add_to_pollset_set(exec_ctx, endpoint, conn->pollset_set); grpc_closure_init(&conn->on_read_request_done, on_read_request_done, conn,