From 740ae63a8a816369b18b8ac9602a9acc7cc0b30d Mon Sep 17 00:00:00 2001 From: Muxi Yan Date: Fri, 21 Oct 2016 13:59:14 -0700 Subject: [PATCH 01/23] Packet coalescing transport layer and end2end test changes --- .../cronet/transport/cronet_api_dummy.c | 3 +- .../cronet/transport/cronet_transport.c | 146 ++++++--- .../CoreCronetEnd2EndTests.m | 4 + src/objective-c/tests/Podfile | 1 + .../tests/Tests.xcodeproj/project.pbxproj | 1 + test/core/end2end/end2end_nosec_tests.c | 10 + test/core/end2end/end2end_tests.c | 8 + test/core/end2end/tests/packet_coalescing.c | 288 ++++++++++++++++++ .../objective_c/Cronet/cronet_c_for_grpc.h | 115 ++++--- 9 files changed, 496 insertions(+), 80 deletions(-) create mode 100644 test/core/end2end/tests/packet_coalescing.c diff --git a/src/core/ext/transport/cronet/transport/cronet_api_dummy.c b/src/core/ext/transport/cronet/transport/cronet_api_dummy.c index 687026c9fde..38755604b97 100644 --- a/src/core/ext/transport/cronet/transport/cronet_api_dummy.c +++ b/src/core/ext/transport/cronet/transport/cronet_api_dummy.c @@ -77,9 +77,8 @@ int cronet_bidirectional_stream_write(cronet_bidirectional_stream* stream, return 0; } -int cronet_bidirectional_stream_cancel(cronet_bidirectional_stream* stream) { +void cronet_bidirectional_stream_cancel(cronet_bidirectional_stream* stream) { GPR_ASSERT(0); - return 0; } #endif /* GRPC_COMPILE_WITH_CRONET */ diff --git a/src/core/ext/transport/cronet/transport/cronet_transport.c b/src/core/ext/transport/cronet/transport/cronet_transport.c index afc59f4b12a..4063dcaefcb 100644 --- a/src/core/ext/transport/cronet/transport/cronet_transport.c +++ b/src/core/ext/transport/cronet/transport/cronet_transport.c @@ -86,7 +86,7 @@ enum e_op_id { /* Cronet callbacks. See cronet_c_for_grpc.h for documentation for each. */ -static void on_request_headers_sent(cronet_bidirectional_stream *); +static void on_stream_ready(cronet_bidirectional_stream *); static void on_response_headers_received( cronet_bidirectional_stream *, const cronet_bidirectional_stream_header_array *, const char *); @@ -99,7 +99,7 @@ static void on_succeeded(cronet_bidirectional_stream *); static void on_failed(cronet_bidirectional_stream *, int); static void on_canceled(cronet_bidirectional_stream *); static cronet_bidirectional_stream_callback cronet_callbacks = { - on_request_headers_sent, + on_stream_ready, on_response_headers_received, on_read_completed, on_write_completed, @@ -151,6 +151,11 @@ struct op_state { bool state_callback_received[OP_NUM_OPS]; bool fail_state; bool flush_read; +#ifdef GRPC_CRONET_WITH_PACKET_COALESCING + bool flush_cronet_when_ready; + bool pending_write_for_trailer; +#endif + bool unprocessed_send_message; grpc_error *cancel_error; /* data structure for storing data coming from server */ struct read_state rs; @@ -273,6 +278,9 @@ static void add_to_storage(struct stream_obj *s, grpc_transport_stream_op *op) { new_op->next = storage->head; storage->head = new_op; storage->num_pending_ops++; + if (op->send_message) { + s->state.unprocessed_send_message = true; + } CRONET_LOG(GPR_DEBUG, "adding new op %p. %d in the queue.", new_op, storage->num_pending_ops); gpr_mu_unlock(&s->mu); @@ -405,8 +413,8 @@ static void on_succeeded(cronet_bidirectional_stream *stream) { /* Cronet callback */ -static void on_request_headers_sent(cronet_bidirectional_stream *stream) { - CRONET_LOG(GPR_DEBUG, "W: on_request_headers_sent(%p)", stream); +static void on_stream_ready(cronet_bidirectional_stream *stream) { + CRONET_LOG(GPR_DEBUG, "W: on_stream_ready(%p)", stream); stream_obj *s = (stream_obj *)stream->annotation; gpr_mu_lock(&s->mu); s->state.state_op_done[OP_SEND_INITIAL_METADATA] = true; @@ -416,6 +424,14 @@ static void on_request_headers_sent(cronet_bidirectional_stream *stream) { gpr_free(s->header_array.headers); s->header_array.headers = NULL; } +/* Send the initial metadata on wire if there is no SEND_MESSAGE or + * SEND_TRAILING_METADATA ops pending */ +#ifdef GRPC_CRONET_WITH_PACKET_COALESCING + if (s->state.flush_cronet_when_ready) { + CRONET_LOG(GPR_DEBUG, "cronet_bidirectional_stream_flush (%p)", s->cbs); + cronet_bidirectional_stream_flush(stream); + } +#endif gpr_mu_unlock(&s->mu); execute_from_storage(s); } @@ -551,6 +567,10 @@ static void on_response_trailers_received( CRONET_LOG(GPR_DEBUG, "cronet_bidirectional_stream_write (%p, 0)", s->cbs); s->state.state_callback_received[OP_SEND_MESSAGE] = false; cronet_bidirectional_stream_write(s->cbs, "", 0, true); +#ifdef GRPC_CRONET_WITH_PACKET_COALESCING + CRONET_LOG(GPR_DEBUG, "cronet_bidirectional_stream_flush (%p)", s->cbs); + cronet_bidirectional_stream_flush(s->cbs); +#endif s->state.state_op_done[OP_SEND_TRAILING_METADATA] = true; gpr_mu_unlock(&s->mu); @@ -598,7 +618,7 @@ static void convert_metadata_to_cronet_headers( curr = curr->next; num_headers_available++; } - /* Allocate enough memory. It is freed in the on_request_headers_sent callback + /* Allocate enough memory. It is freed in the on_stream_ready callback */ cronet_bidirectional_stream_header *headers = (cronet_bidirectional_stream_header *)gpr_malloc( @@ -740,12 +760,16 @@ static bool op_can_be_run(grpc_transport_stream_op *curr_op, else if (!stream_state->state_callback_received[OP_SEND_INITIAL_METADATA]) result = false; /* we haven't sent message yet */ - else if (curr_op->send_message && + else if (stream_state->unprocessed_send_message && !stream_state->state_op_done[OP_SEND_MESSAGE]) result = false; /* we haven't got on_write_completed for the send yet */ else if (stream_state->state_op_done[OP_SEND_MESSAGE] && - !stream_state->state_callback_received[OP_SEND_MESSAGE]) + !stream_state->state_callback_received[OP_SEND_MESSAGE] +#ifdef GRPC_CRONET_WITH_PACKET_COALESCING + && !stream_state->pending_write_for_trailer +#endif + ) result = false; } else if (op_id == OP_CANCEL_ERROR) { /* already executed */ @@ -831,6 +855,10 @@ static enum e_op_result execute_stream_op(grpc_exec_ctx *exec_ctx, s->cbs = cronet_bidirectional_stream_create(s->curr_ct.engine, s->curr_gs, &cronet_callbacks); CRONET_LOG(GPR_DEBUG, "%p = cronet_bidirectional_stream_create()", s->cbs); +#ifdef GRPC_CRONET_WITH_PACKET_COALESCING + cronet_bidirectional_stream_disable_auto_flush(s->cbs, true); + cronet_bidirectional_stream_delay_request_headers_until_flush(s->cbs, true); +#endif char *url = NULL; const char *method = "POST"; s->header_array.headers = NULL; @@ -843,30 +871,17 @@ static enum e_op_result execute_stream_op(grpc_exec_ctx *exec_ctx, cronet_bidirectional_stream_start(s->cbs, url, 0, method, &s->header_array, false); stream_state->state_op_done[OP_SEND_INITIAL_METADATA] = true; - result = ACTION_TAKEN_WITH_CALLBACK; - } else if (stream_op->recv_initial_metadata && - op_can_be_run(stream_op, stream_state, &oas->state, - OP_RECV_INITIAL_METADATA)) { - CRONET_LOG(GPR_DEBUG, "running: %p OP_RECV_INITIAL_METADATA", oas); - if (stream_state->state_op_done[OP_CANCEL_ERROR]) { - grpc_exec_ctx_sched(exec_ctx, stream_op->recv_initial_metadata_ready, - GRPC_ERROR_CANCELLED, NULL); - } else if (stream_state->state_callback_received[OP_FAILED]) { - grpc_exec_ctx_sched( - exec_ctx, stream_op->recv_initial_metadata_ready, - make_error_with_desc(GRPC_STATUS_UNAVAILABLE, "Unavailable."), NULL); - } else { - grpc_chttp2_incoming_metadata_buffer_publish( - &oas->s->state.rs.initial_metadata, stream_op->recv_initial_metadata); - grpc_exec_ctx_sched(exec_ctx, stream_op->recv_initial_metadata_ready, - GRPC_ERROR_NONE, NULL); +#ifdef GRPC_CRONET_WITH_PACKET_COALESCING + if (!stream_op->send_message && !stream_op->send_trailing_metadata) { + s->state.flush_cronet_when_ready = true; } - stream_state->state_op_done[OP_RECV_INITIAL_METADATA] = true; - result = ACTION_TAKEN_NO_CALLBACK; +#endif + result = ACTION_TAKEN_WITH_CALLBACK; } else if (stream_op->send_message && op_can_be_run(stream_op, stream_state, &oas->state, OP_SEND_MESSAGE)) { CRONET_LOG(GPR_DEBUG, "running: %p OP_SEND_MESSAGE", oas); + stream_state->unprocessed_send_message = false; if (stream_state->state_callback_received[OP_FAILED]) { result = NO_ACTION_POSSIBLE; CRONET_LOG(GPR_DEBUG, "Stream is either cancelled or failed."); @@ -897,13 +912,63 @@ static enum e_op_result execute_stream_op(grpc_exec_ctx *exec_ctx, stream_state->state_callback_received[OP_SEND_MESSAGE] = false; cronet_bidirectional_stream_write(s->cbs, stream_state->ws.write_buffer, (int)write_buffer_size, false); +#ifdef GRPC_CRONET_WITH_PACKET_COALESCING + if (!stream_op->send_trailing_metadata) { + CRONET_LOG(GPR_DEBUG, "cronet_bidirectional_stream_flush (%p)", + s->cbs); + cronet_bidirectional_stream_flush(s->cbs); + result = ACTION_TAKEN_WITH_CALLBACK; + } else { + stream_state->pending_write_for_trailer = true; + result = ACTION_TAKEN_NO_CALLBACK; + } +#else result = ACTION_TAKEN_WITH_CALLBACK; +#endif } else { result = NO_ACTION_POSSIBLE; } } stream_state->state_op_done[OP_SEND_MESSAGE] = true; oas->state.state_op_done[OP_SEND_MESSAGE] = true; + } else if (stream_op->send_trailing_metadata && + op_can_be_run(stream_op, stream_state, &oas->state, + OP_SEND_TRAILING_METADATA)) { + CRONET_LOG(GPR_DEBUG, "running: %p OP_SEND_TRAILING_METADATA", oas); + if (stream_state->state_callback_received[OP_FAILED]) { + result = NO_ACTION_POSSIBLE; + CRONET_LOG(GPR_DEBUG, "Stream is either cancelled or failed."); + } else { + CRONET_LOG(GPR_DEBUG, "cronet_bidirectional_stream_write (%p, 0)", + s->cbs); + stream_state->state_callback_received[OP_SEND_MESSAGE] = false; + cronet_bidirectional_stream_write(s->cbs, "", 0, true); +#ifdef GRPC_CRONET_WITH_PACKET_COALESCING + CRONET_LOG(GPR_DEBUG, "cronet_bidirectional_stream_flush (%p)", s->cbs); + cronet_bidirectional_stream_flush(s->cbs); +#endif + result = ACTION_TAKEN_WITH_CALLBACK; + } + stream_state->state_op_done[OP_SEND_TRAILING_METADATA] = true; + } else if (stream_op->recv_initial_metadata && + op_can_be_run(stream_op, stream_state, &oas->state, + OP_RECV_INITIAL_METADATA)) { + CRONET_LOG(GPR_DEBUG, "running: %p OP_RECV_INITIAL_METADATA", oas); + if (stream_state->state_op_done[OP_CANCEL_ERROR]) { + grpc_exec_ctx_sched(exec_ctx, stream_op->recv_initial_metadata_ready, + GRPC_ERROR_CANCELLED, NULL); + } else if (stream_state->state_callback_received[OP_FAILED]) { + grpc_exec_ctx_sched( + exec_ctx, stream_op->recv_initial_metadata_ready, + make_error_with_desc(GRPC_STATUS_UNAVAILABLE, "Unavailable."), NULL); + } else { + grpc_chttp2_incoming_metadata_buffer_publish( + &oas->s->state.rs.initial_metadata, stream_op->recv_initial_metadata); + grpc_exec_ctx_sched(exec_ctx, stream_op->recv_initial_metadata_ready, + GRPC_ERROR_NONE, NULL); + } + stream_state->state_op_done[OP_RECV_INITIAL_METADATA] = true; + result = ACTION_TAKEN_NO_CALLBACK; } else if (stream_op->recv_message && op_can_be_run(stream_op, stream_state, &oas->state, OP_RECV_MESSAGE)) { @@ -962,6 +1027,16 @@ static enum e_op_result execute_stream_op(grpc_exec_ctx *exec_ctx, GRPC_ERROR_NONE, NULL); stream_state->state_op_done[OP_RECV_MESSAGE] = true; oas->state.state_op_done[OP_RECV_MESSAGE] = true; + + /* Extra read to trigger on_succeed */ + stream_state->rs.read_buffer = stream_state->rs.grpc_header_bytes; + stream_state->rs.remaining_bytes = GRPC_HEADER_SIZE_IN_BYTES; + stream_state->rs.received_bytes = 0; + CRONET_LOG(GPR_DEBUG, "cronet_bidirectional_stream_read(%p)", s->cbs); + stream_state->state_op_done[OP_READ_REQ_MADE] = + true; /* Indicates that at least one read request has been made */ + cronet_bidirectional_stream_read(s->cbs, stream_state->rs.read_buffer, + stream_state->rs.remaining_bytes); result = ACTION_TAKEN_NO_CALLBACK; } } else if (stream_state->rs.remaining_bytes == 0) { @@ -1020,21 +1095,6 @@ static enum e_op_result execute_stream_op(grpc_exec_ctx *exec_ctx, } stream_state->state_op_done[OP_RECV_TRAILING_METADATA] = true; result = ACTION_TAKEN_NO_CALLBACK; - } else if (stream_op->send_trailing_metadata && - op_can_be_run(stream_op, stream_state, &oas->state, - OP_SEND_TRAILING_METADATA)) { - CRONET_LOG(GPR_DEBUG, "running: %p OP_SEND_TRAILING_METADATA", oas); - if (stream_state->state_callback_received[OP_FAILED]) { - result = NO_ACTION_POSSIBLE; - CRONET_LOG(GPR_DEBUG, "Stream is either cancelled or failed."); - } else { - CRONET_LOG(GPR_DEBUG, "cronet_bidirectional_stream_write (%p, 0)", - s->cbs); - stream_state->state_callback_received[OP_SEND_MESSAGE] = false; - cronet_bidirectional_stream_write(s->cbs, "", 0, true); - result = ACTION_TAKEN_WITH_CALLBACK; - } - stream_state->state_op_done[OP_SEND_TRAILING_METADATA] = true; } else if (stream_op->cancel_error && op_can_be_run(stream_op, stream_state, &oas->state, OP_CANCEL_ERROR)) { @@ -1117,6 +1177,10 @@ static int init_stream(grpc_exec_ctx *exec_ctx, grpc_transport *gt, sizeof(s->state.state_callback_received)); s->state.fail_state = s->state.flush_read = false; s->state.cancel_error = NULL; +#ifdef GRPC_CRONET_WITH_PACKET_COALESCING + s->state.flush_cronet_when_ready = s->state.pending_write_for_trailer = false; +#endif + s->state.unprocessed_send_message = false; gpr_mu_init(&s->mu); return 0; } diff --git a/src/objective-c/tests/CoreCronetEnd2EndTests/CoreCronetEnd2EndTests.m b/src/objective-c/tests/CoreCronetEnd2EndTests/CoreCronetEnd2EndTests.m index 4ba7badd866..fe9a7c2247b 100644 --- a/src/objective-c/tests/CoreCronetEnd2EndTests/CoreCronetEnd2EndTests.m +++ b/src/objective-c/tests/CoreCronetEnd2EndTests/CoreCronetEnd2EndTests.m @@ -346,6 +346,10 @@ static char *roots_filename; [self testIndividualCase:"no_op"]; } +- (void)testPacketCoalescing { + [self testIndividualCase:"packet_coalescing"]; +} + - (void)testPayload { [self testIndividualCase:"payload"]; } diff --git a/src/objective-c/tests/Podfile b/src/objective-c/tests/Podfile index 5785b976f2d..d1ef0886fe4 100644 --- a/src/objective-c/tests/Podfile +++ b/src/objective-c/tests/Podfile @@ -92,6 +92,7 @@ post_install do |installer| # GPR_UNREACHABLE_CODE causes "Control may reach end of non-void # function" warning config.build_settings['GCC_WARN_ABOUT_RETURN_TYPE'] = 'NO' + config.build_settings['GCC_PREPROCESSOR_DEFINITIONS'] = '$(inherited) COCOAPODS=1 GRPC_CRONET_WITH_PACKET_COALESCING=1' end end diff --git a/src/objective-c/tests/Tests.xcodeproj/project.pbxproj b/src/objective-c/tests/Tests.xcodeproj/project.pbxproj index c4a6567ae0e..8455e71b026 100644 --- a/src/objective-c/tests/Tests.xcodeproj/project.pbxproj +++ b/src/objective-c/tests/Tests.xcodeproj/project.pbxproj @@ -1296,6 +1296,7 @@ "$(inherited)", "GPB_USE_PROTOBUF_FRAMEWORK_IMPORTS=1", "GRPC_COMPILE_WITH_CRONET=1", + "GRPC_CRONET_WITH_PACKET_COALESCING=1", ); INFOPLIST_FILE = InteropTestsRemoteWithCronet/Info.plist; IPHONEOS_DEPLOYMENT_TARGET = 9.3; diff --git a/test/core/end2end/end2end_nosec_tests.c b/test/core/end2end/end2end_nosec_tests.c index 663489082f9..86d6ab9c9a7 100644 --- a/test/core/end2end/end2end_nosec_tests.c +++ b/test/core/end2end/end2end_nosec_tests.c @@ -103,6 +103,8 @@ extern void no_logging(grpc_end2end_test_config config); extern void no_logging_pre_init(void); extern void no_op(grpc_end2end_test_config config); extern void no_op_pre_init(void); +extern void packet_coalescing(grpc_end2end_test_config config); +extern void packet_coalescing_pre_init(void); extern void payload(grpc_end2end_test_config config); extern void payload_pre_init(void); extern void ping(grpc_end2end_test_config config); @@ -135,6 +137,8 @@ extern void streaming_error_response(grpc_end2end_test_config config); extern void streaming_error_response_pre_init(void); extern void trailing_metadata(grpc_end2end_test_config config); extern void trailing_metadata_pre_init(void); +extern void packet_coalescing(grpc_end2end_test_config config); +extern void packet_coalescing_pre_init(void); void grpc_end2end_tests_pre_init(void) { GPR_ASSERT(!g_pre_init_called); @@ -169,6 +173,7 @@ void grpc_end2end_tests_pre_init(void) { network_status_change_pre_init(); no_logging_pre_init(); no_op_pre_init(); + packet_coalescing_pre_init(); payload_pre_init(); ping_pre_init(); ping_pong_streaming_pre_init(); @@ -224,6 +229,7 @@ void grpc_end2end_tests(int argc, char **argv, network_status_change(config); no_logging(config); no_op(config); + packet_coalescing(config); payload(config); ping(config); ping_pong_streaming(config); @@ -364,6 +370,10 @@ void grpc_end2end_tests(int argc, char **argv, no_op(config); continue; } + if (0 == strcmp("packet_coalescing", argv[i])) { + packet_coalescing(config); + continue; + } if (0 == strcmp("payload", argv[i])) { payload(config); continue; diff --git a/test/core/end2end/end2end_tests.c b/test/core/end2end/end2end_tests.c index 25c7c62fde5..66908bab606 100644 --- a/test/core/end2end/end2end_tests.c +++ b/test/core/end2end/end2end_tests.c @@ -105,6 +105,8 @@ extern void no_logging(grpc_end2end_test_config config); extern void no_logging_pre_init(void); extern void no_op(grpc_end2end_test_config config); extern void no_op_pre_init(void); +extern void packet_coalescing(grpc_end2end_test_config config); +extern void packet_coalescing_pre_init(void); extern void payload(grpc_end2end_test_config config); extern void payload_pre_init(void); extern void ping(grpc_end2end_test_config config); @@ -172,6 +174,7 @@ void grpc_end2end_tests_pre_init(void) { network_status_change_pre_init(); no_logging_pre_init(); no_op_pre_init(); + packet_coalescing_pre_init(); payload_pre_init(); ping_pre_init(); ping_pong_streaming_pre_init(); @@ -228,6 +231,7 @@ void grpc_end2end_tests(int argc, char **argv, network_status_change(config); no_logging(config); no_op(config); + packet_coalescing(config); payload(config); ping(config); ping_pong_streaming(config); @@ -372,6 +376,10 @@ void grpc_end2end_tests(int argc, char **argv, no_op(config); continue; } + if (0 == strcmp("packet_coalescing", argv[i])) { + packet_coalescing(config); + continue; + } if (0 == strcmp("payload", argv[i])) { payload(config); continue; diff --git a/test/core/end2end/tests/packet_coalescing.c b/test/core/end2end/tests/packet_coalescing.c new file mode 100644 index 00000000000..d560c57da80 --- /dev/null +++ b/test/core/end2end/tests/packet_coalescing.c @@ -0,0 +1,288 @@ +/* + * + * Copyright 2015, 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 "test/core/end2end/end2end_tests.h" + +#include +#include + +#include +#include +#include +#include +#include +#include "test/core/end2end/cq_verifier.h" + +extern void gpr_default_log(gpr_log_func_args *args); + +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, + grpc_channel_args *client_args, + grpc_channel_args *server_args) { + grpc_end2end_test_fixture f; + gpr_log(GPR_INFO, "%s/%s", test_name, config.name); + f = config.create_fixture(client_args, server_args); + config.init_server(&f, server_args); + config.init_client(&f, client_args); + return f; +} + +static gpr_timespec n_seconds_time(int n) { + return GRPC_TIMEOUT_SECONDS_TO_DEADLINE(n); +} + +static gpr_timespec five_seconds_time(void) { return n_seconds_time(5); } + +static void drain_cq(grpc_completion_queue *cq) { + grpc_event ev; + do { + ev = grpc_completion_queue_next(cq, five_seconds_time(), NULL); + } while (ev.type != GRPC_QUEUE_SHUTDOWN); +} + +static void shutdown_server(grpc_end2end_test_fixture *f) { + if (!f->server) return; + grpc_server_shutdown_and_notify(f->server, f->cq, tag(1000)); + GPR_ASSERT(grpc_completion_queue_pluck( + f->cq, tag(1000), GRPC_TIMEOUT_SECONDS_TO_DEADLINE(5), NULL) + .type == GRPC_OP_COMPLETE); + grpc_server_destroy(f->server); + f->server = NULL; +} + +static void shutdown_client(grpc_end2end_test_fixture *f) { + if (!f->client) return; + grpc_channel_destroy(f->client); + f->client = NULL; +} + +static void end_test(grpc_end2end_test_fixture *f) { + shutdown_server(f); + shutdown_client(f); + + grpc_completion_queue_shutdown(f->cq); + drain_cq(f->cq); + grpc_completion_queue_destroy(f->cq); +} + +static bool coalesced_message_and_eos; + +static void log_processor(gpr_log_func_args *args) { + const int file_len = (int)strlen(args->file); + const char suffix1[] = "secure_endpoint.c"; + const int suffix1_len = sizeof(suffix1) - 1; + const char suffix2[] = "tcp_posix.c"; + const int suffix2_len = sizeof(suffix2) - 1; + const char prefix[] = "READ"; + const int prefix_len = sizeof(prefix) - 1; + if (((file_len >= suffix1_len && + 0 == strcmp(suffix1, &args->file[file_len - suffix1_len])) || + (file_len >= suffix2_len && + 0 == strcmp(suffix2, &args->file[file_len - suffix2_len]))) && + 0 == strncmp(prefix, args->message, (size_t)prefix_len) && + strstr(args->message, + "00 00 10 00 01 00 00 00 01 00 00 00 00 0b 68 65 6c 6c 6f 20 77 " + "6f 72 6c 64")) { + fprintf(stderr, "%s, %s\n", args->file, args->message); + coalesced_message_and_eos = true; + } +} + +/* Request/response with metadata and payload.*/ +static void test_request_response_with_metadata_and_payload( + grpc_end2end_test_config config) { + grpc_call *c; + grpc_call *s; + grpc_slice request_payload_slice = + grpc_slice_from_copied_string("hello world"); + grpc_byte_buffer *request_payload = + grpc_raw_byte_buffer_create(&request_payload_slice, 1); + gpr_timespec deadline = five_seconds_time(); + grpc_metadata meta_c[2] = { + {"key1", "val1", 4, 0, {{NULL, NULL, NULL, NULL}}}, + {"key2", "val2", 4, 0, {{NULL, NULL, NULL, NULL}}}}; + grpc_metadata meta_s[2] = { + {"key3", "val3", 4, 0, {{NULL, NULL, NULL, NULL}}}, + {"key4", "val4", 4, 0, {{NULL, NULL, NULL, NULL}}}}; + grpc_end2end_test_fixture f = begin_test( + config, "test_request_response_with_metadata_and_payload", NULL, NULL); + cq_verifier *cqv = cq_verifier_create(f.cq); + grpc_op ops[6]; + grpc_op *op; + grpc_metadata_array initial_metadata_recv; + grpc_metadata_array trailing_metadata_recv; + grpc_metadata_array request_metadata_recv; + grpc_byte_buffer *request_payload_recv = NULL; + grpc_call_details call_details; + grpc_status_code status; + grpc_call_error error; + char *details = NULL; + size_t details_capacity = 0; + int was_cancelled = 2; + coalesced_message_and_eos = false; + + c = grpc_channel_create_call( + f.client, NULL, GRPC_PROPAGATE_DEFAULTS, f.cq, "/foo", + get_host_override_string("foo.test.google.fr:1234", config), deadline, + NULL); + GPR_ASSERT(c); + + grpc_metadata_array_init(&initial_metadata_recv); + grpc_metadata_array_init(&trailing_metadata_recv); + grpc_metadata_array_init(&request_metadata_recv); + grpc_call_details_init(&call_details); + + memset(ops, 0, sizeof(ops)); + op = ops; + op->op = GRPC_OP_SEND_INITIAL_METADATA; + op->data.send_initial_metadata.count = 2; + op->data.send_initial_metadata.metadata = meta_c; + op->flags = 0; + op->reserved = NULL; + op++; + op->op = GRPC_OP_SEND_MESSAGE; + op->data.send_message = request_payload; + op->flags = 0; + op->reserved = NULL; + op++; + op->op = GRPC_OP_SEND_CLOSE_FROM_CLIENT; + op->flags = 0; + op->reserved = NULL; + op++; + op->op = GRPC_OP_RECV_INITIAL_METADATA; + op->data.recv_initial_metadata = &initial_metadata_recv; + op->flags = 0; + op->reserved = NULL; + op++; + op->op = GRPC_OP_RECV_STATUS_ON_CLIENT; + op->data.recv_status_on_client.trailing_metadata = &trailing_metadata_recv; + op->data.recv_status_on_client.status = &status; + op->data.recv_status_on_client.status_details = &details; + op->data.recv_status_on_client.status_details_capacity = &details_capacity; + op->flags = 0; + op->reserved = NULL; + op++; + error = grpc_call_start_batch(c, ops, (size_t)(op - ops), tag(1), NULL); + GPR_ASSERT(GRPC_CALL_OK == error); + + error = + grpc_server_request_call(f.server, &s, &call_details, + &request_metadata_recv, f.cq, f.cq, tag(101)); + GPR_ASSERT(GRPC_CALL_OK == error); + CQ_EXPECT_COMPLETION(cqv, tag(101), 1); + cq_verify(cqv); + + memset(ops, 0, sizeof(ops)); + op = ops; + op->op = GRPC_OP_SEND_INITIAL_METADATA; + op->data.send_initial_metadata.count = 2; + op->data.send_initial_metadata.metadata = meta_s; + op->flags = 0; + op->reserved = NULL; + op++; + op->op = GRPC_OP_RECV_MESSAGE; + op->data.recv_message = &request_payload_recv; + op->flags = 0; + op->reserved = NULL; + op++; + error = grpc_call_start_batch(s, ops, (size_t)(op - ops), tag(102), NULL); + GPR_ASSERT(GRPC_CALL_OK == error); + + CQ_EXPECT_COMPLETION(cqv, tag(102), 1); + cq_verify(cqv); + + memset(ops, 0, sizeof(ops)); + op = ops; + op->op = GRPC_OP_RECV_CLOSE_ON_SERVER; + op->data.recv_close_on_server.cancelled = &was_cancelled; + op->flags = 0; + op->reserved = NULL; + op++; + op->op = GRPC_OP_SEND_STATUS_FROM_SERVER; + op->data.send_status_from_server.trailing_metadata_count = 0; + op->data.send_status_from_server.status = GRPC_STATUS_OK; + op->data.send_status_from_server.status_details = "xyz"; + op->flags = 0; + op->reserved = NULL; + op++; + error = grpc_call_start_batch(s, ops, (size_t)(op - ops), tag(103), NULL); + GPR_ASSERT(GRPC_CALL_OK == error); + + CQ_EXPECT_COMPLETION(cqv, tag(103), 1); + CQ_EXPECT_COMPLETION(cqv, tag(1), 1); + cq_verify(cqv); + + GPR_ASSERT(status == GRPC_STATUS_OK); + GPR_ASSERT(0 == strcmp(details, "xyz")); + GPR_ASSERT(0 == strcmp(call_details.method, "/foo")); + validate_host_override_string("foo.test.google.fr:1234", call_details.host, + config); + GPR_ASSERT(was_cancelled == 0); + GPR_ASSERT(byte_buffer_eq_string(request_payload_recv, "hello world")); + GPR_ASSERT(contains_metadata(&request_metadata_recv, "key1", "val1")); + GPR_ASSERT(contains_metadata(&request_metadata_recv, "key2", "val2")); + GPR_ASSERT(coalesced_message_and_eos); + + gpr_free(details); + grpc_metadata_array_destroy(&initial_metadata_recv); + grpc_metadata_array_destroy(&trailing_metadata_recv); + grpc_metadata_array_destroy(&request_metadata_recv); + grpc_call_details_destroy(&call_details); + + grpc_call_destroy(c); + grpc_call_destroy(s); + + cq_verifier_destroy(cqv); + + grpc_byte_buffer_destroy(request_payload); + grpc_byte_buffer_destroy(request_payload_recv); + + end_test(&f); + config.tear_down_data(&f); +} + +void packet_coalescing(grpc_end2end_test_config config) { + /* The test case does not support the fixture socketpair_one_byte_at_a_time */ + if (0 == strcmp(config.name, "chttp2/socketpair_one_byte_at_a_time")) { + return; + } + gpr_set_log_verbosity(GPR_LOG_SEVERITY_DEBUG); + grpc_tracer_set_enabled("all", 1); + gpr_set_log_function(log_processor); + test_request_response_with_metadata_and_payload(config); + gpr_set_log_function(gpr_default_log); + grpc_tracer_set_enabled("all", 0); +} + +void packet_coalescing_pre_init(void) {} diff --git a/third_party/objective_c/Cronet/cronet_c_for_grpc.h b/third_party/objective_c/Cronet/cronet_c_for_grpc.h index 15a511aebd0..3d58a8370ef 100644 --- a/third_party/objective_c/Cronet/cronet_c_for_grpc.h +++ b/third_party/objective_c/Cronet/cronet_c_for_grpc.h @@ -5,6 +5,8 @@ #ifndef COMPONENTS_CRONET_IOS_CRONET_C_FOR_GRPC_H_ #define COMPONENTS_CRONET_IOS_CRONET_C_FOR_GRPC_H_ +#define CRONET_EXPORT __attribute__((visibility("default"))) + #ifdef __cplusplus extern "C" { #endif @@ -15,12 +17,10 @@ extern "C" { /* Opaque object representing Cronet Engine. Created and configured outside * of this API to facilitate sharing with other components */ -typedef struct cronet_engine { void* obj; } cronet_engine; - -void cronet_engine_add_quic_hint(cronet_engine* engine, - const char* host, - int port, - int alternate_port); +typedef struct cronet_engine { + void* obj; + void* annotation; +} cronet_engine; /* Cronet Bidirectional Stream API */ @@ -45,11 +45,12 @@ typedef struct cronet_bidirectional_stream_header_array { /* Set of callbacks used to receive callbacks from bidirectional stream. */ typedef struct cronet_bidirectional_stream_callback { - /* Invoked when request headers are sent. Indicates that stream has initiated - * the request. Consumer may call cronet_bidirectional_stream_write() to start - * writing data. + /* Invoked when the stream is ready for reading and writing. + * Consumer may call cronet_bidirectional_stream_read() to start reading data. + * Consumer may call cronet_bidirectional_stream_write() to start writing + * data. */ - void (*on_request_headers_sent)(cronet_bidirectional_stream* stream); + void (*on_stream_ready)(cronet_bidirectional_stream* stream); /* Invoked when initial response headers are received. * Consumer must call cronet_bidirectional_stream_read() to start reading. @@ -67,20 +68,19 @@ typedef struct cronet_bidirectional_stream_callback { * It may be invoked after on_response_trailers_received()}, if there was * pending read data before trailers were received. * - * If count is 0, it means the remote side has signaled that it will send no - * more data; future calls to cronet_bidirectional_stream_read() will result - * in the on_data_read() callback or on_succeded() callback if + * If |bytes_read| is 0, it means the remote side has signaled that it will + * send no more data; future calls to cronet_bidirectional_stream_read() + * will result in the on_data_read() callback or on_succeded() callback if * cronet_bidirectional_stream_write() was invoked with end_of_stream set to * true. */ void (*on_read_completed)(cronet_bidirectional_stream* stream, char* data, - int count); + int bytes_read); /** * Invoked when all data passed to cronet_bidirectional_stream_write() is - * sent. - * To continue writing, call cronet_bidirectional_stream_write(). + * sent. To continue writing, call cronet_bidirectional_stream_write(). */ void (*on_write_completed)(cronet_bidirectional_stream* stream, const char* data); @@ -117,7 +117,7 @@ typedef struct cronet_bidirectional_stream_callback { void (*on_canceled)(cronet_bidirectional_stream* stream); } cronet_bidirectional_stream_callback; -/* Create a new stream object that uses |engine| and |callback|. All stream +/* Creates a new stream object that uses |engine| and |callback|. All stream * tasks are performed asynchronously on the |engine| network thread. |callback| * methods are invoked synchronously on the |engine| network thread, but must * not run tasks on the current thread to prevent blocking networking operations @@ -129,6 +129,7 @@ typedef struct cronet_bidirectional_stream_callback { * * Both |calback| and |engine| must remain valid until stream is destroyed. */ +CRONET_EXPORT cronet_bidirectional_stream* cronet_bidirectional_stream_create( cronet_engine* engine, void* annotation, @@ -136,15 +137,40 @@ cronet_bidirectional_stream* cronet_bidirectional_stream_create( /* TBD: The following methods return int. Should it be a custom type? */ -/* Destroy stream object. Destroy could be called from any thread, including +/* Destroys stream object. Destroy could be called from any thread, including * network thread, but is posted, so |stream| is valid until calling task is * complete. */ +CRONET_EXPORT int cronet_bidirectional_stream_destroy(cronet_bidirectional_stream* stream); -/* Start the stream by sending request to |url| using |method| and |headers|. If - * |end_of_stream| is true, then no data is expected to be written. +/** + * Disables or enables auto flush. By default, data is flushed after + * every cronet_bidirectional_stream_write(). If the auto flush is disabled, + * the client should explicitly call cronet_bidirectional_stream_flush to flush + * the data. + */ +CRONET_EXPORT void cronet_bidirectional_stream_disable_auto_flush( + cronet_bidirectional_stream* stream, + bool disable_auto_flush); + +/** + * Delays sending request headers until cronet_bidirectional_stream_flush() + * is called. This flag is currently only respected when QUIC is negotiated. + * When true, QUIC will send request header frame along with data frame(s) + * as a single packet when possible. + */ +CRONET_EXPORT +void cronet_bidirectional_stream_delay_request_headers_until_flush( + cronet_bidirectional_stream* stream, + bool delay_headers_until_flush); + +/* Starts the stream by sending request to |url| using |method| and |headers|. + * If |end_of_stream| is true, then no data is expected to be written. The + * |method| is HTTP verb, with PUT having a special meaning to mark idempotent + * request, which could use QUIC 0-RTT. */ +CRONET_EXPORT int cronet_bidirectional_stream_start( cronet_bidirectional_stream* stream, const char* url, @@ -153,46 +179,61 @@ int cronet_bidirectional_stream_start( const cronet_bidirectional_stream_header_array* headers, bool end_of_stream); -/* Read response data into |buffer| of |capacity| length. Must only be called at - * most once in response to each invocation of the - * on_response_headers_received() and on_read_completed() methods of the - * cronet_bidirectional_stream_callback. - * Each call will result in an invocation of one of the callback's - * on_read_completed method if data is read, its on_succeeded() method if - * the stream is closed, or its on_failed() method if there's an error. +/* Reads response data into |buffer| of |capacity| length. Must only be called + * at most once in response to each invocation of the + * on_stream_ready()/on_response_headers_received() and on_read_completed() + * methods of the cronet_bidirectional_stream_callback. + * Each call will result in an invocation of the callback's + * on_read_completed() method if data is read, or its on_failed() method if + * there's an error. The callback's on_succeeded() method is also invoked if + * there is no more data to read and |end_of_stream| was previously sent. */ +CRONET_EXPORT int cronet_bidirectional_stream_read(cronet_bidirectional_stream* stream, char* buffer, int capacity); -/* Read response data into |buffer| of |capacity| length. Must only be called at - * most once in response to each invocation of the - * on_response_headers_received() and on_read_completed() methods of the - * cronet_bidirectional_stream_callback. - * Each call will result in an invocation of one of the callback's - * on_read_completed method if data is read, its on_succeeded() method if - * the stream is closed, or its on_failed() method if there's an error. +/* Writes request data from |buffer| of |buffer_length| length. If auto flush is + * disabled, data will be sent only after cronet_bidirectional_stream_flush() is + * called. + * Each call will result in an invocation the callback's on_write_completed() + * method if data is sent, or its on_failed() method if there's an error. + * The callback's on_succeeded() method is also invoked if |end_of_stream| is + * set and all response data has been read. */ +CRONET_EXPORT int cronet_bidirectional_stream_write(cronet_bidirectional_stream* stream, const char* buffer, - int count, + int buffer_length, bool end_of_stream); +/** + * Flushes pending writes. This method should not be called before invocation of + * on_stream_ready() method of the cronet_bidirectional_stream_callback. + * For each previously called cronet_bidirectional_stream_write() + * a corresponding on_write_completed() callback will be invoked when the buffer + * is sent. + */ +CRONET_EXPORT +void cronet_bidirectional_stream_flush(cronet_bidirectional_stream* stream); + /* Cancels the stream. Can be called at any time after * cronet_bidirectional_stream_start(). The on_canceled() method of * cronet_bidirectional_stream_callback will be invoked when cancelation * is complete and no further callback methods will be invoked. If the * stream has completed or has not started, calling * cronet_bidirectional_stream_cancel() has no effect and on_canceled() will not - * be invoked. At most one callback method may be invoked after + * be invoked. At most one callback method may be invoked after * cronet_bidirectional_stream_cancel() has completed. */ -int cronet_bidirectional_stream_cancel(cronet_bidirectional_stream* stream); +CRONET_EXPORT +void cronet_bidirectional_stream_cancel(cronet_bidirectional_stream* stream); /* Returns true if the |stream| was successfully started and is now done * (succeeded, canceled, or failed). * Returns false if the |stream| stream is not yet started or is in progress. */ +CRONET_EXPORT bool cronet_bidirectional_stream_is_done(cronet_bidirectional_stream* stream); #ifdef __cplusplus From 8803ae8c3a643f35d15c9269872d3c1fd2cc8409 Mon Sep 17 00:00:00 2001 From: Muxi Yan Date: Tue, 3 Jan 2017 21:16:05 -0800 Subject: [PATCH 02/23] build changes --- Makefile | 2 + test/core/end2end/end2end_nosec_tests.c | 2 - test/core/end2end/gen_build_yaml.py | 3 +- .../generated/sources_and_headers.json | 2 + tools/run_tests/generated/tests.json | 738 +++++++++++++++++- .../end2end_nosec_tests.vcxproj | 2 + .../end2end_nosec_tests.vcxproj.filters | 3 + .../tests/end2end_tests/end2end_tests.vcxproj | 2 + .../end2end_tests.vcxproj.filters | 3 + 9 files changed, 745 insertions(+), 12 deletions(-) diff --git a/Makefile b/Makefile index 8f7328ae285..66e992ef09a 100644 --- a/Makefile +++ b/Makefile @@ -7199,6 +7199,7 @@ LIBEND2END_TESTS_SRC = \ test/core/end2end/tests/network_status_change.c \ test/core/end2end/tests/no_logging.c \ test/core/end2end/tests/no_op.c \ + test/core/end2end/tests/packet_coalescing.c \ test/core/end2end/tests/payload.c \ test/core/end2end/tests/ping.c \ test/core/end2end/tests/ping_pong_streaming.c \ @@ -7285,6 +7286,7 @@ LIBEND2END_NOSEC_TESTS_SRC = \ test/core/end2end/tests/network_status_change.c \ test/core/end2end/tests/no_logging.c \ test/core/end2end/tests/no_op.c \ + test/core/end2end/tests/packet_coalescing.c \ test/core/end2end/tests/payload.c \ test/core/end2end/tests/ping.c \ test/core/end2end/tests/ping_pong_streaming.c \ diff --git a/test/core/end2end/end2end_nosec_tests.c b/test/core/end2end/end2end_nosec_tests.c index 86d6ab9c9a7..0581607e808 100644 --- a/test/core/end2end/end2end_nosec_tests.c +++ b/test/core/end2end/end2end_nosec_tests.c @@ -137,8 +137,6 @@ extern void streaming_error_response(grpc_end2end_test_config config); extern void streaming_error_response_pre_init(void); extern void trailing_metadata(grpc_end2end_test_config config); extern void trailing_metadata_pre_init(void); -extern void packet_coalescing(grpc_end2end_test_config config); -extern void packet_coalescing_pre_init(void); void grpc_end2end_tests_pre_init(void) { GPR_ASSERT(!g_pre_init_called); diff --git a/test/core/end2end/gen_build_yaml.py b/test/core/end2end/gen_build_yaml.py index 201a92a1fdc..3ae2b2cb9c9 100755 --- a/test/core/end2end/gen_build_yaml.py +++ b/test/core/end2end/gen_build_yaml.py @@ -91,6 +91,7 @@ LOWCPU = 0.1 # maps test names to options END2END_TESTS = { + 'authority_not_supported': default_test_options, 'bad_hostname': default_test_options, 'binary_metadata': default_test_options, 'resource_quota_server': default_test_options._replace(large_writes=True, @@ -125,6 +126,7 @@ END2END_TESTS = { 'network_status_change': default_test_options, 'no_logging': default_test_options._replace(traceable=False), 'no_op': default_test_options, + 'packet_coalescing': default_test_options, 'payload': default_test_options, 'load_reporting_hook': default_test_options, 'ping_pong_streaming': default_test_options, @@ -142,7 +144,6 @@ END2END_TESTS = { 'simple_request': default_test_options, 'streaming_error_response': default_test_options, 'trailing_metadata': default_test_options, - 'authority_not_supported': default_test_options, } diff --git a/tools/run_tests/generated/sources_and_headers.json b/tools/run_tests/generated/sources_and_headers.json index 6ae269cc20d..a96856c1ab9 100644 --- a/tools/run_tests/generated/sources_and_headers.json +++ b/tools/run_tests/generated/sources_and_headers.json @@ -6347,6 +6347,7 @@ "test/core/end2end/tests/network_status_change.c", "test/core/end2end/tests/no_logging.c", "test/core/end2end/tests/no_op.c", + "test/core/end2end/tests/packet_coalescing.c", "test/core/end2end/tests/payload.c", "test/core/end2end/tests/ping.c", "test/core/end2end/tests/ping_pong_streaming.c", @@ -6416,6 +6417,7 @@ "test/core/end2end/tests/network_status_change.c", "test/core/end2end/tests/no_logging.c", "test/core/end2end/tests/no_op.c", + "test/core/end2end/tests/packet_coalescing.c", "test/core/end2end/tests/payload.c", "test/core/end2end/tests/ping.c", "test/core/end2end/tests/ping_pong_streaming.c", diff --git a/tools/run_tests/generated/tests.json b/tools/run_tests/generated/tests.json index b76263b8b98..0ba2f2a6f48 100644 --- a/tools/run_tests/generated/tests.json +++ b/tools/run_tests/generated/tests.json @@ -5712,6 +5712,29 @@ "posix" ] }, + { + "args": [ + "packet_coalescing" + ], + "ci_platforms": [ + "windows", + "linux", + "mac", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [], + "flaky": false, + "language": "c", + "name": "h2_census_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "payload" @@ -6795,6 +6818,29 @@ "posix" ] }, + { + "args": [ + "packet_coalescing" + ], + "ci_platforms": [ + "windows", + "linux", + "mac", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [], + "flaky": false, + "language": "c", + "name": "h2_compress_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "payload" @@ -7847,6 +7893,28 @@ "posix" ] }, + { + "args": [ + "packet_coalescing" + ], + "ci_platforms": [ + "windows", + "linux", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [], + "flaky": false, + "language": "c", + "name": "h2_fakesec_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "payload" @@ -8843,6 +8911,29 @@ "posix" ] }, + { + "args": [ + "packet_coalescing" + ], + "ci_platforms": [ + "linux", + "mac", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [ + "uv" + ], + "flaky": false, + "language": "c", + "name": "h2_fd_test", + "platforms": [ + "linux", + "mac", + "posix" + ] + }, { "args": [ "payload" @@ -9880,6 +9971,29 @@ "posix" ] }, + { + "args": [ + "packet_coalescing" + ], + "ci_platforms": [ + "windows", + "linux", + "mac", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [], + "flaky": false, + "language": "c", + "name": "h2_full_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "payload" @@ -10837,6 +10951,25 @@ "linux" ] }, + { + "args": [ + "packet_coalescing" + ], + "ci_platforms": [ + "linux" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [ + "uv" + ], + "flaky": false, + "language": "c", + "name": "h2_full+pipe_test", + "platforms": [ + "linux" + ] + }, { "args": [ "payload" @@ -11810,6 +11943,29 @@ "posix" ] }, + { + "args": [ + "packet_coalescing" + ], + "ci_platforms": [ + "windows", + "linux", + "mac", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [], + "flaky": false, + "language": "c", + "name": "h2_full+trace_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "payload" @@ -12922,6 +13078,30 @@ "posix" ] }, + { + "args": [ + "packet_coalescing" + ], + "ci_platforms": [ + "windows", + "linux", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [ + "uv" + ], + "flaky": false, + "language": "c", + "name": "h2_http_proxy_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "payload" @@ -14021,6 +14201,29 @@ "posix" ] }, + { + "args": [ + "packet_coalescing" + ], + "ci_platforms": [ + "windows", + "linux", + "mac", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [], + "flaky": false, + "language": "c", + "name": "h2_load_reporting_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "payload" @@ -15133,6 +15336,30 @@ "posix" ] }, + { + "args": [ + "packet_coalescing" + ], + "ci_platforms": [ + "windows", + "linux", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [ + "uv" + ], + "flaky": false, + "language": "c", + "name": "h2_oauth2_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "payload" @@ -16165,6 +16392,30 @@ "posix" ] }, + { + "args": [ + "packet_coalescing" + ], + "ci_platforms": [ + "windows", + "linux", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [ + "uv" + ], + "flaky": false, + "language": "c", + "name": "h2_proxy_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "payload" @@ -17151,7 +17402,7 @@ }, { "args": [ - "payload" + "packet_coalescing" ], "ci_platforms": [ "windows", @@ -17175,7 +17426,7 @@ }, { "args": [ - "ping_pong_streaming" + "payload" ], "ci_platforms": [ "windows", @@ -17199,7 +17450,7 @@ }, { "args": [ - "registered_call" + "ping_pong_streaming" ], "ci_platforms": [ "windows", @@ -17223,14 +17474,14 @@ }, { "args": [ - "request_with_flags" + "registered_call" ], "ci_platforms": [ "windows", "linux", "posix" ], - "cpu_cost": 0.1, + "cpu_cost": 1.0, "exclude_configs": [], "exclude_iomgrs": [ "uv" @@ -17247,14 +17498,14 @@ }, { "args": [ - "request_with_payload" + "request_with_flags" ], "ci_platforms": [ "windows", "linux", "posix" ], - "cpu_cost": 1.0, + "cpu_cost": 0.1, "exclude_configs": [], "exclude_iomgrs": [ "uv" @@ -17271,7 +17522,7 @@ }, { "args": [ - "resource_quota_server" + "request_with_payload" ], "ci_platforms": [ "windows", @@ -17295,7 +17546,31 @@ }, { "args": [ - "server_finishes_request" + "resource_quota_server" + ], + "ci_platforms": [ + "windows", + "linux", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [ + "uv" + ], + "flaky": false, + "language": "c", + "name": "h2_sockpair_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, + { + "args": [ + "server_finishes_request" ], "ci_platforms": [ "windows", @@ -18109,6 +18384,30 @@ "posix" ] }, + { + "args": [ + "packet_coalescing" + ], + "ci_platforms": [ + "windows", + "linux", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [ + "uv" + ], + "flaky": false, + "language": "c", + "name": "h2_sockpair+trace_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "payload" @@ -19149,6 +19448,32 @@ "posix" ] }, + { + "args": [ + "packet_coalescing" + ], + "ci_platforms": [ + "windows", + "linux", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [ + "msan" + ], + "exclude_iomgrs": [ + "uv" + ], + "flaky": false, + "language": "c", + "name": "h2_sockpair_1byte_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "payload" @@ -20202,6 +20527,29 @@ "posix" ] }, + { + "args": [ + "packet_coalescing" + ], + "ci_platforms": [ + "windows", + "linux", + "mac", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [], + "flaky": false, + "language": "c", + "name": "h2_ssl_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "payload" @@ -21285,6 +21633,29 @@ "posix" ] }, + { + "args": [ + "packet_coalescing" + ], + "ci_platforms": [ + "windows", + "linux", + "mac", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [], + "flaky": false, + "language": "c", + "name": "h2_ssl_cert_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "payload" @@ -22301,6 +22672,30 @@ "posix" ] }, + { + "args": [ + "packet_coalescing" + ], + "ci_platforms": [ + "windows", + "linux", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [ + "uv" + ], + "flaky": false, + "language": "c", + "name": "h2_ssl_proxy_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "payload" @@ -23303,6 +23698,29 @@ "posix" ] }, + { + "args": [ + "packet_coalescing" + ], + "ci_platforms": [ + "linux", + "mac", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [ + "uv" + ], + "flaky": false, + "language": "c", + "name": "h2_uds_test", + "platforms": [ + "linux", + "mac", + "posix" + ] + }, { "args": [ "payload" @@ -24363,6 +24781,29 @@ "posix" ] }, + { + "args": [ + "packet_coalescing" + ], + "ci_platforms": [ + "windows", + "linux", + "mac", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [], + "flaky": false, + "language": "c", + "name": "h2_census_nosec_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "payload" @@ -25423,6 +25864,29 @@ "posix" ] }, + { + "args": [ + "packet_coalescing" + ], + "ci_platforms": [ + "windows", + "linux", + "mac", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [], + "flaky": false, + "language": "c", + "name": "h2_compress_nosec_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "payload" @@ -26412,6 +26876,29 @@ "posix" ] }, + { + "args": [ + "packet_coalescing" + ], + "ci_platforms": [ + "linux", + "mac", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [ + "uv" + ], + "flaky": false, + "language": "c", + "name": "h2_fd_nosec_test", + "platforms": [ + "linux", + "mac", + "posix" + ] + }, { "args": [ "payload" @@ -27426,6 +27913,29 @@ "posix" ] }, + { + "args": [ + "packet_coalescing" + ], + "ci_platforms": [ + "windows", + "linux", + "mac", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [], + "flaky": false, + "language": "c", + "name": "h2_full_nosec_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "payload" @@ -28364,6 +28874,25 @@ "linux" ] }, + { + "args": [ + "packet_coalescing" + ], + "ci_platforms": [ + "linux" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [ + "uv" + ], + "flaky": false, + "language": "c", + "name": "h2_full+pipe_nosec_test", + "platforms": [ + "linux" + ] + }, { "args": [ "payload" @@ -29314,6 +29843,29 @@ "posix" ] }, + { + "args": [ + "packet_coalescing" + ], + "ci_platforms": [ + "windows", + "linux", + "mac", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [], + "flaky": false, + "language": "c", + "name": "h2_full+trace_nosec_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "payload" @@ -30402,6 +30954,30 @@ "posix" ] }, + { + "args": [ + "packet_coalescing" + ], + "ci_platforms": [ + "windows", + "linux", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [ + "uv" + ], + "flaky": false, + "language": "c", + "name": "h2_http_proxy_nosec_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "payload" @@ -31478,6 +32054,29 @@ "posix" ] }, + { + "args": [ + "packet_coalescing" + ], + "ci_platforms": [ + "windows", + "linux", + "mac", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [], + "flaky": false, + "language": "c", + "name": "h2_load_reporting_nosec_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "payload" @@ -32470,6 +33069,30 @@ "posix" ] }, + { + "args": [ + "packet_coalescing" + ], + "ci_platforms": [ + "windows", + "linux", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [ + "uv" + ], + "flaky": false, + "language": "c", + "name": "h2_proxy_nosec_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "payload" @@ -33430,6 +34053,30 @@ "posix" ] }, + { + "args": [ + "packet_coalescing" + ], + "ci_platforms": [ + "windows", + "linux", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [ + "uv" + ], + "flaky": false, + "language": "c", + "name": "h2_sockpair_nosec_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "payload" @@ -34366,6 +35013,30 @@ "posix" ] }, + { + "args": [ + "packet_coalescing" + ], + "ci_platforms": [ + "windows", + "linux", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [ + "uv" + ], + "flaky": false, + "language": "c", + "name": "h2_sockpair+trace_nosec_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "payload" @@ -35380,6 +36051,32 @@ "posix" ] }, + { + "args": [ + "packet_coalescing" + ], + "ci_platforms": [ + "windows", + "linux", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [ + "msan" + ], + "exclude_iomgrs": [ + "uv" + ], + "flaky": false, + "language": "c", + "name": "h2_sockpair_1byte_nosec_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "payload" @@ -36385,6 +37082,29 @@ "posix" ] }, + { + "args": [ + "packet_coalescing" + ], + "ci_platforms": [ + "linux", + "mac", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [ + "uv" + ], + "flaky": false, + "language": "c", + "name": "h2_uds_nosec_test", + "platforms": [ + "linux", + "mac", + "posix" + ] + }, { "args": [ "payload" diff --git a/vsprojects/vcxproj/test/end2end/tests/end2end_nosec_tests/end2end_nosec_tests.vcxproj b/vsprojects/vcxproj/test/end2end/tests/end2end_nosec_tests/end2end_nosec_tests.vcxproj index 4fb8f8f4a15..196d924287f 100644 --- a/vsprojects/vcxproj/test/end2end/tests/end2end_nosec_tests/end2end_nosec_tests.vcxproj +++ b/vsprojects/vcxproj/test/end2end/tests/end2end_nosec_tests/end2end_nosec_tests.vcxproj @@ -215,6 +215,8 @@ + + diff --git a/vsprojects/vcxproj/test/end2end/tests/end2end_nosec_tests/end2end_nosec_tests.vcxproj.filters b/vsprojects/vcxproj/test/end2end/tests/end2end_nosec_tests/end2end_nosec_tests.vcxproj.filters index ff82a4dd43c..73801e36354 100644 --- a/vsprojects/vcxproj/test/end2end/tests/end2end_nosec_tests/end2end_nosec_tests.vcxproj.filters +++ b/vsprojects/vcxproj/test/end2end/tests/end2end_nosec_tests/end2end_nosec_tests.vcxproj.filters @@ -97,6 +97,9 @@ test\core\end2end\tests + + test\core\end2end\tests + test\core\end2end\tests diff --git a/vsprojects/vcxproj/test/end2end/tests/end2end_tests/end2end_tests.vcxproj b/vsprojects/vcxproj/test/end2end/tests/end2end_tests/end2end_tests.vcxproj index 0b7d7c2e752..7b1701005db 100644 --- a/vsprojects/vcxproj/test/end2end/tests/end2end_tests/end2end_tests.vcxproj +++ b/vsprojects/vcxproj/test/end2end/tests/end2end_tests/end2end_tests.vcxproj @@ -217,6 +217,8 @@ + + diff --git a/vsprojects/vcxproj/test/end2end/tests/end2end_tests/end2end_tests.vcxproj.filters b/vsprojects/vcxproj/test/end2end/tests/end2end_tests/end2end_tests.vcxproj.filters index e641930e64a..2b707651768 100644 --- a/vsprojects/vcxproj/test/end2end/tests/end2end_tests/end2end_tests.vcxproj.filters +++ b/vsprojects/vcxproj/test/end2end/tests/end2end_tests/end2end_tests.vcxproj.filters @@ -100,6 +100,9 @@ test\core\end2end\tests + + test\core\end2end\tests + test\core\end2end\tests From dc739bd659384252ae31234a439f1abcd7a512ff Mon Sep 17 00:00:00 2001 From: Muxi Yan Date: Thu, 5 Jan 2017 21:04:40 -0800 Subject: [PATCH 03/23] Remove packet coalescing testing on chttp2 --- Makefile | 2 - .../CoreCronetEnd2EndTests.m | 9 +- test/core/end2end/end2end_nosec_tests.c | 8 - test/core/end2end/end2end_tests.c | 8 - test/core/end2end/gen_build_yaml.py | 1 - test/core/end2end/tests/packet_coalescing.c | 4 - .../generated/sources_and_headers.json | 2 - tools/run_tests/generated/tests.json | 736 +----------------- .../end2end_nosec_tests.vcxproj | 2 - .../end2end_nosec_tests.vcxproj.filters | 3 - .../tests/end2end_tests/end2end_tests.vcxproj | 2 - .../end2end_tests.vcxproj.filters | 3 - 12 files changed, 16 insertions(+), 764 deletions(-) diff --git a/Makefile b/Makefile index 66e992ef09a..8f7328ae285 100644 --- a/Makefile +++ b/Makefile @@ -7199,7 +7199,6 @@ LIBEND2END_TESTS_SRC = \ test/core/end2end/tests/network_status_change.c \ test/core/end2end/tests/no_logging.c \ test/core/end2end/tests/no_op.c \ - test/core/end2end/tests/packet_coalescing.c \ test/core/end2end/tests/payload.c \ test/core/end2end/tests/ping.c \ test/core/end2end/tests/ping_pong_streaming.c \ @@ -7286,7 +7285,6 @@ LIBEND2END_NOSEC_TESTS_SRC = \ test/core/end2end/tests/network_status_change.c \ test/core/end2end/tests/no_logging.c \ test/core/end2end/tests/no_op.c \ - test/core/end2end/tests/packet_coalescing.c \ test/core/end2end/tests/payload.c \ test/core/end2end/tests/ping.c \ test/core/end2end/tests/ping_pong_streaming.c \ diff --git a/src/objective-c/tests/CoreCronetEnd2EndTests/CoreCronetEnd2EndTests.m b/src/objective-c/tests/CoreCronetEnd2EndTests/CoreCronetEnd2EndTests.m index fe9a7c2247b..315d9424845 100644 --- a/src/objective-c/tests/CoreCronetEnd2EndTests/CoreCronetEnd2EndTests.m +++ b/src/objective-c/tests/CoreCronetEnd2EndTests/CoreCronetEnd2EndTests.m @@ -168,6 +168,9 @@ static grpc_end2end_test_config configs[] = { chttp2_tear_down_secure_fullstack}, }; +extern void packet_coalescing(grpc_end2end_test_config config); +extern void packet_coalescing_pre_init(void); + static char *roots_filename; @interface CoreCronetEnd2EndTests : XCTestCase @@ -347,7 +350,11 @@ static char *roots_filename; } - (void)testPacketCoalescing { - [self testIndividualCase:"packet_coalescing"]; + // Directly invoke the test function since the test is for Cronet only and thus not included in + // end2end_tests.c + // TODO (mxyan): Do the same to all test cases so that this file will no longer depend on + // end2end_tests.c + packet_coalescing(configs[0]); } - (void)testPayload { diff --git a/test/core/end2end/end2end_nosec_tests.c b/test/core/end2end/end2end_nosec_tests.c index 0581607e808..663489082f9 100644 --- a/test/core/end2end/end2end_nosec_tests.c +++ b/test/core/end2end/end2end_nosec_tests.c @@ -103,8 +103,6 @@ extern void no_logging(grpc_end2end_test_config config); extern void no_logging_pre_init(void); extern void no_op(grpc_end2end_test_config config); extern void no_op_pre_init(void); -extern void packet_coalescing(grpc_end2end_test_config config); -extern void packet_coalescing_pre_init(void); extern void payload(grpc_end2end_test_config config); extern void payload_pre_init(void); extern void ping(grpc_end2end_test_config config); @@ -171,7 +169,6 @@ void grpc_end2end_tests_pre_init(void) { network_status_change_pre_init(); no_logging_pre_init(); no_op_pre_init(); - packet_coalescing_pre_init(); payload_pre_init(); ping_pre_init(); ping_pong_streaming_pre_init(); @@ -227,7 +224,6 @@ void grpc_end2end_tests(int argc, char **argv, network_status_change(config); no_logging(config); no_op(config); - packet_coalescing(config); payload(config); ping(config); ping_pong_streaming(config); @@ -368,10 +364,6 @@ void grpc_end2end_tests(int argc, char **argv, no_op(config); continue; } - if (0 == strcmp("packet_coalescing", argv[i])) { - packet_coalescing(config); - continue; - } if (0 == strcmp("payload", argv[i])) { payload(config); continue; diff --git a/test/core/end2end/end2end_tests.c b/test/core/end2end/end2end_tests.c index 66908bab606..25c7c62fde5 100644 --- a/test/core/end2end/end2end_tests.c +++ b/test/core/end2end/end2end_tests.c @@ -105,8 +105,6 @@ extern void no_logging(grpc_end2end_test_config config); extern void no_logging_pre_init(void); extern void no_op(grpc_end2end_test_config config); extern void no_op_pre_init(void); -extern void packet_coalescing(grpc_end2end_test_config config); -extern void packet_coalescing_pre_init(void); extern void payload(grpc_end2end_test_config config); extern void payload_pre_init(void); extern void ping(grpc_end2end_test_config config); @@ -174,7 +172,6 @@ void grpc_end2end_tests_pre_init(void) { network_status_change_pre_init(); no_logging_pre_init(); no_op_pre_init(); - packet_coalescing_pre_init(); payload_pre_init(); ping_pre_init(); ping_pong_streaming_pre_init(); @@ -231,7 +228,6 @@ void grpc_end2end_tests(int argc, char **argv, network_status_change(config); no_logging(config); no_op(config); - packet_coalescing(config); payload(config); ping(config); ping_pong_streaming(config); @@ -376,10 +372,6 @@ void grpc_end2end_tests(int argc, char **argv, no_op(config); continue; } - if (0 == strcmp("packet_coalescing", argv[i])) { - packet_coalescing(config); - continue; - } if (0 == strcmp("payload", argv[i])) { payload(config); continue; diff --git a/test/core/end2end/gen_build_yaml.py b/test/core/end2end/gen_build_yaml.py index 3ae2b2cb9c9..78779908a94 100755 --- a/test/core/end2end/gen_build_yaml.py +++ b/test/core/end2end/gen_build_yaml.py @@ -126,7 +126,6 @@ END2END_TESTS = { 'network_status_change': default_test_options, 'no_logging': default_test_options._replace(traceable=False), 'no_op': default_test_options, - 'packet_coalescing': default_test_options, 'payload': default_test_options, 'load_reporting_hook': default_test_options, 'ping_pong_streaming': default_test_options, diff --git a/test/core/end2end/tests/packet_coalescing.c b/test/core/end2end/tests/packet_coalescing.c index d560c57da80..336f765a13f 100644 --- a/test/core/end2end/tests/packet_coalescing.c +++ b/test/core/end2end/tests/packet_coalescing.c @@ -273,10 +273,6 @@ static void test_request_response_with_metadata_and_payload( } void packet_coalescing(grpc_end2end_test_config config) { - /* The test case does not support the fixture socketpair_one_byte_at_a_time */ - if (0 == strcmp(config.name, "chttp2/socketpair_one_byte_at_a_time")) { - return; - } gpr_set_log_verbosity(GPR_LOG_SEVERITY_DEBUG); grpc_tracer_set_enabled("all", 1); gpr_set_log_function(log_processor); diff --git a/tools/run_tests/generated/sources_and_headers.json b/tools/run_tests/generated/sources_and_headers.json index a96856c1ab9..6ae269cc20d 100644 --- a/tools/run_tests/generated/sources_and_headers.json +++ b/tools/run_tests/generated/sources_and_headers.json @@ -6347,7 +6347,6 @@ "test/core/end2end/tests/network_status_change.c", "test/core/end2end/tests/no_logging.c", "test/core/end2end/tests/no_op.c", - "test/core/end2end/tests/packet_coalescing.c", "test/core/end2end/tests/payload.c", "test/core/end2end/tests/ping.c", "test/core/end2end/tests/ping_pong_streaming.c", @@ -6417,7 +6416,6 @@ "test/core/end2end/tests/network_status_change.c", "test/core/end2end/tests/no_logging.c", "test/core/end2end/tests/no_op.c", - "test/core/end2end/tests/packet_coalescing.c", "test/core/end2end/tests/payload.c", "test/core/end2end/tests/ping.c", "test/core/end2end/tests/ping_pong_streaming.c", diff --git a/tools/run_tests/generated/tests.json b/tools/run_tests/generated/tests.json index 0ba2f2a6f48..b76263b8b98 100644 --- a/tools/run_tests/generated/tests.json +++ b/tools/run_tests/generated/tests.json @@ -5712,29 +5712,6 @@ "posix" ] }, - { - "args": [ - "packet_coalescing" - ], - "ci_platforms": [ - "windows", - "linux", - "mac", - "posix" - ], - "cpu_cost": 1.0, - "exclude_configs": [], - "exclude_iomgrs": [], - "flaky": false, - "language": "c", - "name": "h2_census_test", - "platforms": [ - "windows", - "linux", - "mac", - "posix" - ] - }, { "args": [ "payload" @@ -6818,29 +6795,6 @@ "posix" ] }, - { - "args": [ - "packet_coalescing" - ], - "ci_platforms": [ - "windows", - "linux", - "mac", - "posix" - ], - "cpu_cost": 1.0, - "exclude_configs": [], - "exclude_iomgrs": [], - "flaky": false, - "language": "c", - "name": "h2_compress_test", - "platforms": [ - "windows", - "linux", - "mac", - "posix" - ] - }, { "args": [ "payload" @@ -7893,28 +7847,6 @@ "posix" ] }, - { - "args": [ - "packet_coalescing" - ], - "ci_platforms": [ - "windows", - "linux", - "posix" - ], - "cpu_cost": 1.0, - "exclude_configs": [], - "exclude_iomgrs": [], - "flaky": false, - "language": "c", - "name": "h2_fakesec_test", - "platforms": [ - "windows", - "linux", - "mac", - "posix" - ] - }, { "args": [ "payload" @@ -8911,29 +8843,6 @@ "posix" ] }, - { - "args": [ - "packet_coalescing" - ], - "ci_platforms": [ - "linux", - "mac", - "posix" - ], - "cpu_cost": 1.0, - "exclude_configs": [], - "exclude_iomgrs": [ - "uv" - ], - "flaky": false, - "language": "c", - "name": "h2_fd_test", - "platforms": [ - "linux", - "mac", - "posix" - ] - }, { "args": [ "payload" @@ -9971,29 +9880,6 @@ "posix" ] }, - { - "args": [ - "packet_coalescing" - ], - "ci_platforms": [ - "windows", - "linux", - "mac", - "posix" - ], - "cpu_cost": 1.0, - "exclude_configs": [], - "exclude_iomgrs": [], - "flaky": false, - "language": "c", - "name": "h2_full_test", - "platforms": [ - "windows", - "linux", - "mac", - "posix" - ] - }, { "args": [ "payload" @@ -10951,25 +10837,6 @@ "linux" ] }, - { - "args": [ - "packet_coalescing" - ], - "ci_platforms": [ - "linux" - ], - "cpu_cost": 1.0, - "exclude_configs": [], - "exclude_iomgrs": [ - "uv" - ], - "flaky": false, - "language": "c", - "name": "h2_full+pipe_test", - "platforms": [ - "linux" - ] - }, { "args": [ "payload" @@ -11943,29 +11810,6 @@ "posix" ] }, - { - "args": [ - "packet_coalescing" - ], - "ci_platforms": [ - "windows", - "linux", - "mac", - "posix" - ], - "cpu_cost": 1.0, - "exclude_configs": [], - "exclude_iomgrs": [], - "flaky": false, - "language": "c", - "name": "h2_full+trace_test", - "platforms": [ - "windows", - "linux", - "mac", - "posix" - ] - }, { "args": [ "payload" @@ -13078,30 +12922,6 @@ "posix" ] }, - { - "args": [ - "packet_coalescing" - ], - "ci_platforms": [ - "windows", - "linux", - "posix" - ], - "cpu_cost": 1.0, - "exclude_configs": [], - "exclude_iomgrs": [ - "uv" - ], - "flaky": false, - "language": "c", - "name": "h2_http_proxy_test", - "platforms": [ - "windows", - "linux", - "mac", - "posix" - ] - }, { "args": [ "payload" @@ -14201,29 +14021,6 @@ "posix" ] }, - { - "args": [ - "packet_coalescing" - ], - "ci_platforms": [ - "windows", - "linux", - "mac", - "posix" - ], - "cpu_cost": 1.0, - "exclude_configs": [], - "exclude_iomgrs": [], - "flaky": false, - "language": "c", - "name": "h2_load_reporting_test", - "platforms": [ - "windows", - "linux", - "mac", - "posix" - ] - }, { "args": [ "payload" @@ -15336,30 +15133,6 @@ "posix" ] }, - { - "args": [ - "packet_coalescing" - ], - "ci_platforms": [ - "windows", - "linux", - "posix" - ], - "cpu_cost": 1.0, - "exclude_configs": [], - "exclude_iomgrs": [ - "uv" - ], - "flaky": false, - "language": "c", - "name": "h2_oauth2_test", - "platforms": [ - "windows", - "linux", - "mac", - "posix" - ] - }, { "args": [ "payload" @@ -16392,30 +16165,6 @@ "posix" ] }, - { - "args": [ - "packet_coalescing" - ], - "ci_platforms": [ - "windows", - "linux", - "posix" - ], - "cpu_cost": 1.0, - "exclude_configs": [], - "exclude_iomgrs": [ - "uv" - ], - "flaky": false, - "language": "c", - "name": "h2_proxy_test", - "platforms": [ - "windows", - "linux", - "mac", - "posix" - ] - }, { "args": [ "payload" @@ -17402,7 +17151,7 @@ }, { "args": [ - "packet_coalescing" + "payload" ], "ci_platforms": [ "windows", @@ -17426,7 +17175,7 @@ }, { "args": [ - "payload" + "ping_pong_streaming" ], "ci_platforms": [ "windows", @@ -17450,7 +17199,7 @@ }, { "args": [ - "ping_pong_streaming" + "registered_call" ], "ci_platforms": [ "windows", @@ -17474,14 +17223,14 @@ }, { "args": [ - "registered_call" + "request_with_flags" ], "ci_platforms": [ "windows", "linux", "posix" ], - "cpu_cost": 1.0, + "cpu_cost": 0.1, "exclude_configs": [], "exclude_iomgrs": [ "uv" @@ -17498,14 +17247,14 @@ }, { "args": [ - "request_with_flags" + "request_with_payload" ], "ci_platforms": [ "windows", "linux", "posix" ], - "cpu_cost": 0.1, + "cpu_cost": 1.0, "exclude_configs": [], "exclude_iomgrs": [ "uv" @@ -17522,31 +17271,7 @@ }, { "args": [ - "request_with_payload" - ], - "ci_platforms": [ - "windows", - "linux", - "posix" - ], - "cpu_cost": 1.0, - "exclude_configs": [], - "exclude_iomgrs": [ - "uv" - ], - "flaky": false, - "language": "c", - "name": "h2_sockpair_test", - "platforms": [ - "windows", - "linux", - "mac", - "posix" - ] - }, - { - "args": [ - "resource_quota_server" + "resource_quota_server" ], "ci_platforms": [ "windows", @@ -18384,30 +18109,6 @@ "posix" ] }, - { - "args": [ - "packet_coalescing" - ], - "ci_platforms": [ - "windows", - "linux", - "posix" - ], - "cpu_cost": 1.0, - "exclude_configs": [], - "exclude_iomgrs": [ - "uv" - ], - "flaky": false, - "language": "c", - "name": "h2_sockpair+trace_test", - "platforms": [ - "windows", - "linux", - "mac", - "posix" - ] - }, { "args": [ "payload" @@ -19448,32 +19149,6 @@ "posix" ] }, - { - "args": [ - "packet_coalescing" - ], - "ci_platforms": [ - "windows", - "linux", - "posix" - ], - "cpu_cost": 1.0, - "exclude_configs": [ - "msan" - ], - "exclude_iomgrs": [ - "uv" - ], - "flaky": false, - "language": "c", - "name": "h2_sockpair_1byte_test", - "platforms": [ - "windows", - "linux", - "mac", - "posix" - ] - }, { "args": [ "payload" @@ -20527,29 +20202,6 @@ "posix" ] }, - { - "args": [ - "packet_coalescing" - ], - "ci_platforms": [ - "windows", - "linux", - "mac", - "posix" - ], - "cpu_cost": 1.0, - "exclude_configs": [], - "exclude_iomgrs": [], - "flaky": false, - "language": "c", - "name": "h2_ssl_test", - "platforms": [ - "windows", - "linux", - "mac", - "posix" - ] - }, { "args": [ "payload" @@ -21633,29 +21285,6 @@ "posix" ] }, - { - "args": [ - "packet_coalescing" - ], - "ci_platforms": [ - "windows", - "linux", - "mac", - "posix" - ], - "cpu_cost": 1.0, - "exclude_configs": [], - "exclude_iomgrs": [], - "flaky": false, - "language": "c", - "name": "h2_ssl_cert_test", - "platforms": [ - "windows", - "linux", - "mac", - "posix" - ] - }, { "args": [ "payload" @@ -22672,30 +22301,6 @@ "posix" ] }, - { - "args": [ - "packet_coalescing" - ], - "ci_platforms": [ - "windows", - "linux", - "posix" - ], - "cpu_cost": 1.0, - "exclude_configs": [], - "exclude_iomgrs": [ - "uv" - ], - "flaky": false, - "language": "c", - "name": "h2_ssl_proxy_test", - "platforms": [ - "windows", - "linux", - "mac", - "posix" - ] - }, { "args": [ "payload" @@ -23698,29 +23303,6 @@ "posix" ] }, - { - "args": [ - "packet_coalescing" - ], - "ci_platforms": [ - "linux", - "mac", - "posix" - ], - "cpu_cost": 1.0, - "exclude_configs": [], - "exclude_iomgrs": [ - "uv" - ], - "flaky": false, - "language": "c", - "name": "h2_uds_test", - "platforms": [ - "linux", - "mac", - "posix" - ] - }, { "args": [ "payload" @@ -24781,29 +24363,6 @@ "posix" ] }, - { - "args": [ - "packet_coalescing" - ], - "ci_platforms": [ - "windows", - "linux", - "mac", - "posix" - ], - "cpu_cost": 1.0, - "exclude_configs": [], - "exclude_iomgrs": [], - "flaky": false, - "language": "c", - "name": "h2_census_nosec_test", - "platforms": [ - "windows", - "linux", - "mac", - "posix" - ] - }, { "args": [ "payload" @@ -25864,29 +25423,6 @@ "posix" ] }, - { - "args": [ - "packet_coalescing" - ], - "ci_platforms": [ - "windows", - "linux", - "mac", - "posix" - ], - "cpu_cost": 1.0, - "exclude_configs": [], - "exclude_iomgrs": [], - "flaky": false, - "language": "c", - "name": "h2_compress_nosec_test", - "platforms": [ - "windows", - "linux", - "mac", - "posix" - ] - }, { "args": [ "payload" @@ -26876,29 +26412,6 @@ "posix" ] }, - { - "args": [ - "packet_coalescing" - ], - "ci_platforms": [ - "linux", - "mac", - "posix" - ], - "cpu_cost": 1.0, - "exclude_configs": [], - "exclude_iomgrs": [ - "uv" - ], - "flaky": false, - "language": "c", - "name": "h2_fd_nosec_test", - "platforms": [ - "linux", - "mac", - "posix" - ] - }, { "args": [ "payload" @@ -27913,29 +27426,6 @@ "posix" ] }, - { - "args": [ - "packet_coalescing" - ], - "ci_platforms": [ - "windows", - "linux", - "mac", - "posix" - ], - "cpu_cost": 1.0, - "exclude_configs": [], - "exclude_iomgrs": [], - "flaky": false, - "language": "c", - "name": "h2_full_nosec_test", - "platforms": [ - "windows", - "linux", - "mac", - "posix" - ] - }, { "args": [ "payload" @@ -28874,25 +28364,6 @@ "linux" ] }, - { - "args": [ - "packet_coalescing" - ], - "ci_platforms": [ - "linux" - ], - "cpu_cost": 1.0, - "exclude_configs": [], - "exclude_iomgrs": [ - "uv" - ], - "flaky": false, - "language": "c", - "name": "h2_full+pipe_nosec_test", - "platforms": [ - "linux" - ] - }, { "args": [ "payload" @@ -29843,29 +29314,6 @@ "posix" ] }, - { - "args": [ - "packet_coalescing" - ], - "ci_platforms": [ - "windows", - "linux", - "mac", - "posix" - ], - "cpu_cost": 1.0, - "exclude_configs": [], - "exclude_iomgrs": [], - "flaky": false, - "language": "c", - "name": "h2_full+trace_nosec_test", - "platforms": [ - "windows", - "linux", - "mac", - "posix" - ] - }, { "args": [ "payload" @@ -30954,30 +30402,6 @@ "posix" ] }, - { - "args": [ - "packet_coalescing" - ], - "ci_platforms": [ - "windows", - "linux", - "posix" - ], - "cpu_cost": 1.0, - "exclude_configs": [], - "exclude_iomgrs": [ - "uv" - ], - "flaky": false, - "language": "c", - "name": "h2_http_proxy_nosec_test", - "platforms": [ - "windows", - "linux", - "mac", - "posix" - ] - }, { "args": [ "payload" @@ -32054,29 +31478,6 @@ "posix" ] }, - { - "args": [ - "packet_coalescing" - ], - "ci_platforms": [ - "windows", - "linux", - "mac", - "posix" - ], - "cpu_cost": 1.0, - "exclude_configs": [], - "exclude_iomgrs": [], - "flaky": false, - "language": "c", - "name": "h2_load_reporting_nosec_test", - "platforms": [ - "windows", - "linux", - "mac", - "posix" - ] - }, { "args": [ "payload" @@ -33069,30 +32470,6 @@ "posix" ] }, - { - "args": [ - "packet_coalescing" - ], - "ci_platforms": [ - "windows", - "linux", - "posix" - ], - "cpu_cost": 1.0, - "exclude_configs": [], - "exclude_iomgrs": [ - "uv" - ], - "flaky": false, - "language": "c", - "name": "h2_proxy_nosec_test", - "platforms": [ - "windows", - "linux", - "mac", - "posix" - ] - }, { "args": [ "payload" @@ -34053,30 +33430,6 @@ "posix" ] }, - { - "args": [ - "packet_coalescing" - ], - "ci_platforms": [ - "windows", - "linux", - "posix" - ], - "cpu_cost": 1.0, - "exclude_configs": [], - "exclude_iomgrs": [ - "uv" - ], - "flaky": false, - "language": "c", - "name": "h2_sockpair_nosec_test", - "platforms": [ - "windows", - "linux", - "mac", - "posix" - ] - }, { "args": [ "payload" @@ -35013,30 +34366,6 @@ "posix" ] }, - { - "args": [ - "packet_coalescing" - ], - "ci_platforms": [ - "windows", - "linux", - "posix" - ], - "cpu_cost": 1.0, - "exclude_configs": [], - "exclude_iomgrs": [ - "uv" - ], - "flaky": false, - "language": "c", - "name": "h2_sockpair+trace_nosec_test", - "platforms": [ - "windows", - "linux", - "mac", - "posix" - ] - }, { "args": [ "payload" @@ -36051,32 +35380,6 @@ "posix" ] }, - { - "args": [ - "packet_coalescing" - ], - "ci_platforms": [ - "windows", - "linux", - "posix" - ], - "cpu_cost": 1.0, - "exclude_configs": [ - "msan" - ], - "exclude_iomgrs": [ - "uv" - ], - "flaky": false, - "language": "c", - "name": "h2_sockpair_1byte_nosec_test", - "platforms": [ - "windows", - "linux", - "mac", - "posix" - ] - }, { "args": [ "payload" @@ -37082,29 +36385,6 @@ "posix" ] }, - { - "args": [ - "packet_coalescing" - ], - "ci_platforms": [ - "linux", - "mac", - "posix" - ], - "cpu_cost": 1.0, - "exclude_configs": [], - "exclude_iomgrs": [ - "uv" - ], - "flaky": false, - "language": "c", - "name": "h2_uds_nosec_test", - "platforms": [ - "linux", - "mac", - "posix" - ] - }, { "args": [ "payload" diff --git a/vsprojects/vcxproj/test/end2end/tests/end2end_nosec_tests/end2end_nosec_tests.vcxproj b/vsprojects/vcxproj/test/end2end/tests/end2end_nosec_tests/end2end_nosec_tests.vcxproj index 196d924287f..4fb8f8f4a15 100644 --- a/vsprojects/vcxproj/test/end2end/tests/end2end_nosec_tests/end2end_nosec_tests.vcxproj +++ b/vsprojects/vcxproj/test/end2end/tests/end2end_nosec_tests/end2end_nosec_tests.vcxproj @@ -215,8 +215,6 @@ - - diff --git a/vsprojects/vcxproj/test/end2end/tests/end2end_nosec_tests/end2end_nosec_tests.vcxproj.filters b/vsprojects/vcxproj/test/end2end/tests/end2end_nosec_tests/end2end_nosec_tests.vcxproj.filters index 73801e36354..ff82a4dd43c 100644 --- a/vsprojects/vcxproj/test/end2end/tests/end2end_nosec_tests/end2end_nosec_tests.vcxproj.filters +++ b/vsprojects/vcxproj/test/end2end/tests/end2end_nosec_tests/end2end_nosec_tests.vcxproj.filters @@ -97,9 +97,6 @@ test\core\end2end\tests - - test\core\end2end\tests - test\core\end2end\tests diff --git a/vsprojects/vcxproj/test/end2end/tests/end2end_tests/end2end_tests.vcxproj b/vsprojects/vcxproj/test/end2end/tests/end2end_tests/end2end_tests.vcxproj index 7b1701005db..0b7d7c2e752 100644 --- a/vsprojects/vcxproj/test/end2end/tests/end2end_tests/end2end_tests.vcxproj +++ b/vsprojects/vcxproj/test/end2end/tests/end2end_tests/end2end_tests.vcxproj @@ -217,8 +217,6 @@ - - diff --git a/vsprojects/vcxproj/test/end2end/tests/end2end_tests/end2end_tests.vcxproj.filters b/vsprojects/vcxproj/test/end2end/tests/end2end_tests/end2end_tests.vcxproj.filters index 2b707651768..e641930e64a 100644 --- a/vsprojects/vcxproj/test/end2end/tests/end2end_tests/end2end_tests.vcxproj.filters +++ b/vsprojects/vcxproj/test/end2end/tests/end2end_tests/end2end_tests.vcxproj.filters @@ -100,9 +100,6 @@ test\core\end2end\tests - - test\core\end2end\tests - test\core\end2end\tests From b0bd22dfc7116975245c6d15fcc9485a25885845 Mon Sep 17 00:00:00 2001 From: Muxi Yan Date: Fri, 20 Jan 2017 17:06:23 -0800 Subject: [PATCH 04/23] Test packet coalescing using TLS endpoint --- .../tests/CronetUnitTests/CronetUnitTests.m | 228 +++++++++++++++++- 1 file changed, 225 insertions(+), 3 deletions(-) diff --git a/src/objective-c/tests/CronetUnitTests/CronetUnitTests.m b/src/objective-c/tests/CronetUnitTests/CronetUnitTests.m index dbd28076ddb..dcd7f2fa8db 100644 --- a/src/objective-c/tests/CronetUnitTests/CronetUnitTests.m +++ b/src/objective-c/tests/CronetUnitTests/CronetUnitTests.m @@ -50,6 +50,9 @@ #import "src/core/lib/support/string.h" #import "src/core/lib/support/tmpfile.h" #import "test/core/util/test_config.h" +#import "test/core/end2end/data/ssl_test_data.h" + +#import static void drain_cq(grpc_completion_queue *cq) { grpc_event ev; @@ -77,20 +80,70 @@ static void drain_cq(grpc_completion_queue *cq) { grpc_init(); [Cronet setHttp2Enabled:YES]; + [Cronet setSslKeyLogFileName:@"Documents/key"]; + [Cronet enableTestCertVerifierForTesting]; NSURL *url = [[[NSFileManager defaultManager] URLsForDirectory:NSDocumentDirectory inDomains:NSUserDomainMask] lastObject]; NSLog(@"Documents directory: %@", url); [Cronet start]; [Cronet startNetLogToFile:@"Documents/cronet_netlog.json" logBytes:YES]; + + init_ssl(); } + (void)tearDown { grpc_shutdown(); + cleanup_ssl(); [super tearDown]; } +void init_ssl(void) { + SSL_load_error_strings(); + OpenSSL_add_ssl_algorithms(); +} + +void cleanup_ssl(void) { + EVP_cleanup(); +} + +int alpn_cb(SSL *ssl, const unsigned char **out, unsigned char *outlen, + const unsigned char *in, unsigned int inlen, void *arg) { + // Always select "h2" as the ALPN protocol to be used + *out = (const unsigned char*)"h2"; + *outlen = 2; + return SSL_TLSEXT_ERR_OK; +} + +void init_ctx(SSL_CTX *ctx) { + // Install server certificate + BIO *pem = BIO_new_mem_buf((void*)test_server1_cert, (int)strlen(test_server1_cert)); + X509 *cert = PEM_read_bio_X509_AUX(pem, NULL, NULL, ""); + SSL_CTX_use_certificate(ctx, cert); + X509_free(cert); + BIO_free(pem); + + // Install server private key + pem = BIO_new_mem_buf((void *)test_server1_key, (int)strlen(test_server1_key)); + EVP_PKEY *key = PEM_read_bio_PrivateKey(pem, NULL, NULL, ""); + SSL_CTX_use_PrivateKey(ctx, key); + EVP_PKEY_free(key); + BIO_free(pem); + + // Select cipher suite + SSL_CTX_set_cipher_list(ctx, "ECDHE-RSA-AES128-GCM-SHA256"); + + // Select ALPN protocol + SSL_CTX_set_alpn_select_cb(ctx, alpn_cb, NULL); +} + +unsigned int parse_h2_length(const char *field) { + return ((unsigned int)(unsigned char)(field[0])) * 65536 + + ((unsigned int)(unsigned char)(field[1])) * 256 + + ((unsigned int)(unsigned char)(field[2])); +} + - (void)testInternalError { grpc_call *c; grpc_slice request_payload_slice = @@ -106,7 +159,7 @@ static void drain_cq(grpc_completion_queue *cq) { char *addr; gpr_join_host_port(&addr, "127.0.0.1", port); grpc_completion_queue *cq = grpc_completion_queue_create(NULL); - cronet_engine *cronetEngine = [Cronet getGlobalEngine]; + stream_engine *cronetEngine = [Cronet getGlobalEngine]; grpc_channel *client = grpc_cronet_secure_channel_create(cronetEngine, addr, NULL, NULL); @@ -174,14 +227,19 @@ static void drain_cq(grpc_completion_queue *cq) { dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{ int sl = socket(AF_INET, SOCK_STREAM, 0); GPR_ASSERT(sl >= 0); + + // Make and TCP endpoint to accept the connection struct sockaddr_in s_addr; memset(&s_addr, 0, sizeof(s_addr)); s_addr.sin_family = AF_INET; s_addr.sin_addr.s_addr = htonl(INADDR_ANY); s_addr.sin_port = htons(port); - bind(sl, (struct sockaddr*)&s_addr, sizeof(s_addr)); - listen(sl, 5); + GPR_ASSERT(0 == bind(sl, (struct sockaddr*)&s_addr, sizeof(s_addr))); + GPR_ASSERT(0 == listen(sl, 5)); int s = accept(sl, NULL, NULL); + GPR_ASSERT(s >= 0); + + // Close the connection after 1 second to trigger Cronet's on_failed() sleep(1); close(s); close(sl); @@ -211,4 +269,168 @@ static void drain_cq(grpc_completion_queue *cq) { grpc_completion_queue_destroy(cq); } +- (void)testPacketCoalescing { + grpc_call *c; + grpc_slice request_payload_slice = + grpc_slice_from_copied_string("hello world"); + grpc_byte_buffer *request_payload = + grpc_raw_byte_buffer_create(&request_payload_slice, 1); + gpr_timespec deadline = GRPC_TIMEOUT_SECONDS_TO_DEADLINE(5); + grpc_metadata meta_c[2] = { + {"key1", "val1", 4, 0, {{NULL, NULL, NULL, NULL}}}, + {"key2", "val2", 4, 0, {{NULL, NULL, NULL, NULL}}}}; + + int port = grpc_pick_unused_port_or_die(); + char *addr; + gpr_join_host_port(&addr, "127.0.0.1", port); + grpc_completion_queue *cq = grpc_completion_queue_create(NULL); + stream_engine *cronetEngine = [Cronet getGlobalEngine]; + grpc_channel *client = grpc_cronet_secure_channel_create(cronetEngine, addr, + NULL, NULL); + + cq_verifier *cqv = cq_verifier_create(cq); + grpc_op ops[6]; + grpc_op *op; + grpc_metadata_array initial_metadata_recv; + grpc_metadata_array trailing_metadata_recv; + grpc_metadata_array request_metadata_recv; + grpc_byte_buffer *response_payload_recv = NULL; + grpc_call_details call_details; + grpc_status_code status; + grpc_call_error error; + char *details = NULL; + size_t details_capacity = 0; + + c = grpc_channel_create_call( + client, NULL, GRPC_PROPAGATE_DEFAULTS, cq, "/foo", + NULL, deadline, NULL); + GPR_ASSERT(c); + + grpc_metadata_array_init(&initial_metadata_recv); + grpc_metadata_array_init(&trailing_metadata_recv); + grpc_metadata_array_init(&request_metadata_recv); + grpc_call_details_init(&call_details); + + memset(ops, 0, sizeof(ops)); + op = ops; + op->op = GRPC_OP_SEND_INITIAL_METADATA; + op->data.send_initial_metadata.count = 2; + op->data.send_initial_metadata.metadata = meta_c; + op->flags = 0; + op->reserved = NULL; + op++; + op->op = GRPC_OP_SEND_MESSAGE; + op->data.send_message = request_payload; + op->flags = 0; + op->reserved = NULL; + op++; + op->op = GRPC_OP_SEND_CLOSE_FROM_CLIENT; + op->flags = 0; + op->reserved = NULL; + op++; + op->op = GRPC_OP_RECV_INITIAL_METADATA; + op->data.recv_initial_metadata = &initial_metadata_recv; + op->flags = 0; + op->reserved = NULL; + op++; + op->op = GRPC_OP_RECV_MESSAGE; + op->data.recv_message = &response_payload_recv; + op->flags = 0; + op->reserved = NULL; + op++; + op->op = GRPC_OP_RECV_STATUS_ON_CLIENT; + op->data.recv_status_on_client.trailing_metadata = &trailing_metadata_recv; + op->data.recv_status_on_client.status = &status; + op->data.recv_status_on_client.status_details = &details; + op->data.recv_status_on_client.status_details_capacity = &details_capacity; + op->flags = 0; + op->reserved = NULL; + op++; + error = grpc_call_start_batch(c, ops, (size_t)(op - ops), (void*)1, NULL); + GPR_ASSERT(GRPC_CALL_OK == error); + + dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{ + int sl = socket(AF_INET, SOCK_STREAM, 0); + GPR_ASSERT(sl >= 0); + struct sockaddr_in s_addr; + memset(&s_addr, 0, sizeof(s_addr)); + s_addr.sin_family = AF_INET; + s_addr.sin_addr.s_addr = htonl(INADDR_ANY); + s_addr.sin_port = htons(port); + GPR_ASSERT(0 == bind(sl, (struct sockaddr*)&s_addr, sizeof(s_addr))); + GPR_ASSERT(0 == listen(sl, 5)); + int s = accept(sl, NULL, NULL); + GPR_ASSERT(s >= 0); + struct timeval tv; + tv.tv_sec = 2; + tv.tv_usec = 0; + setsockopt(s, SOL_SOCKET, SO_RCVTIMEO, &tv, sizeof(tv)); + + // Make an TLS endpoint to receive Cronet's transmission + SSL_CTX *ctx = SSL_CTX_new(TLSv1_2_server_method()); + init_ctx(ctx); + SSL *ssl = SSL_new(ctx); + SSL_set_fd(ssl, s); + SSL_accept(ssl); + + const char magic[] = "PRI * HTTP/2.0\r\n\r\nSM\r\n\r\n"; + + char buf[4096]; + long len; + bool coalesced = false; + while ((len = SSL_read(ssl, buf, sizeof(buf))) > 0) { + NSLog(@"Read len: %ld", len); + + // Analyze the HTTP/2 frames in the same TLS PDU to identify if + // coalescing is successful + unsigned int p = 0; + while (p < len) { + if (len - p >= 24 && 0 == memcmp(&buf[p], magic, 24)) { + p += 24; + continue; + } + + if (buf[p+3] == 0 && // Type is DATA + parse_h2_length(&buf[p]) == 0x10 && // Length is correct + (buf[p+4] & 1) != 0 && // EOS bit is set + 0 == memcmp("hello world", &buf[p+14], 11)) { // Message is correct + coalesced = true; + break; + } + p += (parse_h2_length(&buf[p]) + 9); + } + if (coalesced) { + break; + } + } + + XCTAssert(coalesced); + SSL_free(ssl); + SSL_CTX_free(ctx); + close(s); + close(sl); + }); + + CQ_EXPECT_COMPLETION(cqv, (void*)1, 1); + cq_verify(cqv); + + gpr_free(details); + grpc_metadata_array_destroy(&initial_metadata_recv); + grpc_metadata_array_destroy(&trailing_metadata_recv); + grpc_metadata_array_destroy(&request_metadata_recv); + grpc_call_details_destroy(&call_details); + + grpc_call_destroy(c); + + cq_verifier_destroy(cqv); + + grpc_byte_buffer_destroy(request_payload); + grpc_byte_buffer_destroy(response_payload_recv); + + grpc_channel_destroy(client); + grpc_completion_queue_shutdown(cq); + drain_cq(cq); + grpc_completion_queue_destroy(cq); +} + @end From 60ab7ef00ac0a988ee2672c636d946c964e6fa41 Mon Sep 17 00:00:00 2001 From: Muxi Yan Date: Mon, 23 Jan 2017 23:00:35 -0800 Subject: [PATCH 05/23] Dynamically enable/disable packet coalecsing and test it --- include/grpc/grpc_cronet.h | 2 + .../client/secure/cronet_channel_create.c | 6 ++ .../cronet/transport/cronet_transport.c | 72 +++++++++---------- .../tests/CronetUnitTests/CronetUnitTests.m | 13 +++- src/objective-c/tests/Podfile | 1 - 5 files changed, 52 insertions(+), 42 deletions(-) diff --git a/include/grpc/grpc_cronet.h b/include/grpc/grpc_cronet.h index 295e0f55e80..566c34a388d 100644 --- a/include/grpc/grpc_cronet.h +++ b/include/grpc/grpc_cronet.h @@ -44,6 +44,8 @@ GRPCAPI grpc_channel *grpc_cronet_secure_channel_create( void *engine, const char *target, const grpc_channel_args *args, void *reserved); +GRPCAPI void grpc_cronet_use_packet_coalescing(bool use_coalescing); + #ifdef __cplusplus } #endif diff --git a/src/core/ext/transport/cronet/client/secure/cronet_channel_create.c b/src/core/ext/transport/cronet/client/secure/cronet_channel_create.c index 477cf07f45d..2e40020ae05 100644 --- a/src/core/ext/transport/cronet/client/secure/cronet_channel_create.c +++ b/src/core/ext/transport/cronet/client/secure/cronet_channel_create.c @@ -51,6 +51,8 @@ typedef struct cronet_transport { extern grpc_transport_vtable grpc_cronet_vtable; +bool grpc_cronet_packet_coalescing_enabled = true; + GRPCAPI grpc_channel *grpc_cronet_secure_channel_create( void *engine, const char *target, const grpc_channel_args *args, void *reserved) { @@ -67,3 +69,7 @@ GRPCAPI grpc_channel *grpc_cronet_secure_channel_create( return grpc_channel_create(&exec_ctx, target, args, GRPC_CLIENT_DIRECT_CHANNEL, (grpc_transport *)ct); } + +GRPCAPI void grpc_cronet_use_packet_coalescing(bool use_coalescing) { + grpc_cronet_packet_coalescing_enabled = use_coalescing; +} diff --git a/src/core/ext/transport/cronet/transport/cronet_transport.c b/src/core/ext/transport/cronet/transport/cronet_transport.c index 447f3f31ec1..5429eb32e3c 100644 --- a/src/core/ext/transport/cronet/transport/cronet_transport.c +++ b/src/core/ext/transport/cronet/transport/cronet_transport.c @@ -61,6 +61,8 @@ /* TODO (makdharma): Hook up into the wider tracing mechanism */ int grpc_cronet_trace = 0; +extern bool grpc_cronet_packet_coalescing_enabled; + enum e_op_result { ACTION_TAKEN_WITH_CALLBACK, ACTION_TAKEN_NO_CALLBACK, @@ -150,12 +152,13 @@ struct op_state { bool state_callback_received[OP_NUM_OPS]; bool fail_state; bool flush_read; -#ifdef GRPC_CRONET_WITH_PACKET_COALESCING bool flush_cronet_when_ready; bool pending_write_for_trailer; -#endif bool unprocessed_send_message; grpc_error *cancel_error; + + /* Whether packet coalescing is enabled */ + bool packet_coalescing_enabled; /* data structure for storing data coming from server */ struct read_state rs; /* data structure for storing data going to the server */ @@ -425,12 +428,10 @@ static void on_stream_ready(bidirectional_stream *stream) { } /* Send the initial metadata on wire if there is no SEND_MESSAGE or * SEND_TRAILING_METADATA ops pending */ -#ifdef GRPC_CRONET_WITH_PACKET_COALESCING - if (s->state.flush_cronet_when_ready) { + if (s->state.packet_coalescing_enabled && s->state.flush_cronet_when_ready) { CRONET_LOG(GPR_DEBUG, "cronet_bidirectional_stream_flush (%p)", s->cbs); bidirectional_stream_flush(stream); } -#endif gpr_mu_unlock(&s->mu); execute_from_storage(s); } @@ -568,10 +569,10 @@ static void on_response_trailers_received( CRONET_LOG(GPR_DEBUG, "bidirectional_stream_write (%p, 0)", s->cbs); s->state.state_callback_received[OP_SEND_MESSAGE] = false; bidirectional_stream_write(s->cbs, "", 0, true); -#ifdef GRPC_CRONET_WITH_PACKET_COALESCING - CRONET_LOG(GPR_DEBUG, "bidirectional_stream_flush (%p)", s->cbs); - bidirectional_stream_flush(s->cbs); -#endif + if (s->state.packet_coalescing_enabled) { + CRONET_LOG(GPR_DEBUG, "bidirectional_stream_flush (%p)", s->cbs); + bidirectional_stream_flush(s->cbs); + } s->state.state_op_done[OP_SEND_TRAILING_METADATA] = true; gpr_mu_unlock(&s->mu); @@ -768,11 +769,9 @@ static bool op_can_be_run(grpc_transport_stream_op *curr_op, result = false; /* we haven't got on_write_completed for the send yet */ else if (stream_state->state_op_done[OP_SEND_MESSAGE] && - !stream_state->state_callback_received[OP_SEND_MESSAGE] -#ifdef GRPC_CRONET_WITH_PACKET_COALESCING - && !stream_state->pending_write_for_trailer -#endif - ) + !stream_state->state_callback_received[OP_SEND_MESSAGE] && + !(stream_state->packet_coalescing_enabled && + stream_state->pending_write_for_trailer)) result = false; } else if (op_id == OP_CANCEL_ERROR) { /* already executed */ @@ -858,10 +857,10 @@ static enum e_op_result execute_stream_op(grpc_exec_ctx *exec_ctx, s->cbs = bidirectional_stream_create(s->curr_ct.engine, s->curr_gs, &cronet_callbacks); CRONET_LOG(GPR_DEBUG, "%p = bidirectional_stream_create()", s->cbs); -#ifdef GRPC_CRONET_WITH_PACKET_COALESCING - bidirectional_stream_disable_auto_flush(s->cbs, true); - bidirectional_stream_delay_request_headers_until_flush(s->cbs, true); -#endif + if (stream_state->packet_coalescing_enabled) { + bidirectional_stream_disable_auto_flush(s->cbs, true); + bidirectional_stream_delay_request_headers_until_flush(s->cbs, true); + } char *url = NULL; const char *method = "POST"; s->header_array.headers = NULL; @@ -872,11 +871,10 @@ static enum e_op_result execute_stream_op(grpc_exec_ctx *exec_ctx, CRONET_LOG(GPR_DEBUG, "bidirectional_stream_start(%p, %s)", s->cbs, url); bidirectional_stream_start(s->cbs, url, 0, method, &s->header_array, false); stream_state->state_op_done[OP_SEND_INITIAL_METADATA] = true; -#ifdef GRPC_CRONET_WITH_PACKET_COALESCING - if (!stream_op->send_message && !stream_op->send_trailing_metadata) { + if (stream_state->packet_coalescing_enabled && !stream_op->send_message && + !stream_op->send_trailing_metadata) { s->state.flush_cronet_when_ready = true; } -#endif result = ACTION_TAKEN_WITH_CALLBACK; } else if (stream_op->send_message && op_can_be_run(stream_op, stream_state, &oas->state, @@ -913,19 +911,18 @@ static enum e_op_result execute_stream_op(grpc_exec_ctx *exec_ctx, stream_state->state_callback_received[OP_SEND_MESSAGE] = false; bidirectional_stream_write(s->cbs, stream_state->ws.write_buffer, (int)write_buffer_size, false); -#ifdef GRPC_CRONET_WITH_PACKET_COALESCING - if (!stream_op->send_trailing_metadata) { - CRONET_LOG(GPR_DEBUG, "bidirectional_stream_flush (%p)", - s->cbs); - bidirectional_stream_flush(s->cbs); - result = ACTION_TAKEN_WITH_CALLBACK; + if (stream_state->packet_coalescing_enabled) { + if (!stream_op->send_trailing_metadata) { + CRONET_LOG(GPR_DEBUG, "bidirectional_stream_flush (%p)", s->cbs); + bidirectional_stream_flush(s->cbs); + result = ACTION_TAKEN_WITH_CALLBACK; + } else { + stream_state->pending_write_for_trailer = true; + result = ACTION_TAKEN_NO_CALLBACK; + } } else { - stream_state->pending_write_for_trailer = true; - result = ACTION_TAKEN_NO_CALLBACK; + result = ACTION_TAKEN_WITH_CALLBACK; } -#else - result = ACTION_TAKEN_WITH_CALLBACK; -#endif } else { result = NO_ACTION_POSSIBLE; } @@ -944,10 +941,10 @@ static enum e_op_result execute_stream_op(grpc_exec_ctx *exec_ctx, s->cbs); stream_state->state_callback_received[OP_SEND_MESSAGE] = false; bidirectional_stream_write(s->cbs, "", 0, true); -#ifdef GRPC_CRONET_WITH_PACKET_COALESCING - CRONET_LOG(GPR_DEBUG, "bidirectional_stream_flush (%p)", s->cbs); - bidirectional_stream_flush(s->cbs); -#endif + if (stream_state->packet_coalescing_enabled) { + CRONET_LOG(GPR_DEBUG, "bidirectional_stream_flush (%p)", s->cbs); + bidirectional_stream_flush(s->cbs); + } result = ACTION_TAKEN_WITH_CALLBACK; } stream_state->state_op_done[OP_SEND_TRAILING_METADATA] = true; @@ -1176,10 +1173,9 @@ static int init_stream(grpc_exec_ctx *exec_ctx, grpc_transport *gt, sizeof(s->state.state_callback_received)); s->state.fail_state = s->state.flush_read = false; s->state.cancel_error = NULL; -#ifdef GRPC_CRONET_WITH_PACKET_COALESCING s->state.flush_cronet_when_ready = s->state.pending_write_for_trailer = false; -#endif s->state.unprocessed_send_message = false; + s->state.packet_coalescing_enabled = grpc_cronet_packet_coalescing_enabled; gpr_mu_init(&s->mu); return 0; } diff --git a/src/objective-c/tests/CronetUnitTests/CronetUnitTests.m b/src/objective-c/tests/CronetUnitTests/CronetUnitTests.m index dcd7f2fa8db..9bbf3cdb113 100644 --- a/src/objective-c/tests/CronetUnitTests/CronetUnitTests.m +++ b/src/objective-c/tests/CronetUnitTests/CronetUnitTests.m @@ -269,7 +269,9 @@ unsigned int parse_h2_length(const char *field) { grpc_completion_queue_destroy(cq); } -- (void)testPacketCoalescing { +- (void)PacketCoalescing:(bool)use_coalescing { + grpc_cronet_use_packet_coalescing(use_coalescing); + grpc_call *c; grpc_slice request_payload_slice = grpc_slice_from_copied_string("hello world"); @@ -379,7 +381,7 @@ unsigned int parse_h2_length(const char *field) { long len; bool coalesced = false; while ((len = SSL_read(ssl, buf, sizeof(buf))) > 0) { - NSLog(@"Read len: %ld", len); + gpr_log(GPR_DEBUG, "Read len: %ld", len); // Analyze the HTTP/2 frames in the same TLS PDU to identify if // coalescing is successful @@ -404,7 +406,7 @@ unsigned int parse_h2_length(const char *field) { } } - XCTAssert(coalesced); + XCTAssert(coalesced == use_coalescing); SSL_free(ssl); SSL_CTX_free(ctx); close(s); @@ -433,4 +435,9 @@ unsigned int parse_h2_length(const char *field) { grpc_completion_queue_destroy(cq); } +- (void)testPacketCoalescing { + [self PacketCoalescing:false]; + [self PacketCoalescing:true]; +} + @end diff --git a/src/objective-c/tests/Podfile b/src/objective-c/tests/Podfile index 462c6a8e0eb..3760330be9a 100644 --- a/src/objective-c/tests/Podfile +++ b/src/objective-c/tests/Podfile @@ -97,7 +97,6 @@ post_install do |installer| # GPR_UNREACHABLE_CODE causes "Control may reach end of non-void # function" warning config.build_settings['GCC_WARN_ABOUT_RETURN_TYPE'] = 'NO' - config.build_settings['GCC_PREPROCESSOR_DEFINITIONS'] = '$(inherited) COCOAPODS=1 GRPC_CRONET_WITH_PACKET_COALESCING=1' end end From 8d1d95d07cdb7e8fa78ab93399fcf3742fcee63f Mon Sep 17 00:00:00 2001 From: Muxi Yan Date: Mon, 23 Jan 2017 23:01:10 -0800 Subject: [PATCH 06/23] Clean up old packet coalescing tests --- .../CoreCronetEnd2EndTests.m | 11 - test/core/end2end/tests/packet_coalescing.c | 284 ------------------ 2 files changed, 295 deletions(-) delete mode 100644 test/core/end2end/tests/packet_coalescing.c diff --git a/src/objective-c/tests/CoreCronetEnd2EndTests/CoreCronetEnd2EndTests.m b/src/objective-c/tests/CoreCronetEnd2EndTests/CoreCronetEnd2EndTests.m index 9af38fd9b22..01612a84b28 100644 --- a/src/objective-c/tests/CoreCronetEnd2EndTests/CoreCronetEnd2EndTests.m +++ b/src/objective-c/tests/CoreCronetEnd2EndTests/CoreCronetEnd2EndTests.m @@ -170,9 +170,6 @@ static grpc_end2end_test_config configs[] = { chttp2_tear_down_secure_fullstack}, }; -extern void packet_coalescing(grpc_end2end_test_config config); -extern void packet_coalescing_pre_init(void); - static char *roots_filename; @interface CoreCronetEnd2EndTests : XCTestCase @@ -351,14 +348,6 @@ static char *roots_filename; [self testIndividualCase:"no_op"]; } -- (void)testPacketCoalescing { - // Directly invoke the test function since the test is for Cronet only and thus not included in - // end2end_tests.c - // TODO (mxyan): Do the same to all test cases so that this file will no longer depend on - // end2end_tests.c - packet_coalescing(configs[0]); -} - - (void)testPayload { [self testIndividualCase:"payload"]; } diff --git a/test/core/end2end/tests/packet_coalescing.c b/test/core/end2end/tests/packet_coalescing.c deleted file mode 100644 index 336f765a13f..00000000000 --- a/test/core/end2end/tests/packet_coalescing.c +++ /dev/null @@ -1,284 +0,0 @@ -/* - * - * Copyright 2015, 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 "test/core/end2end/end2end_tests.h" - -#include -#include - -#include -#include -#include -#include -#include -#include "test/core/end2end/cq_verifier.h" - -extern void gpr_default_log(gpr_log_func_args *args); - -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, - grpc_channel_args *client_args, - grpc_channel_args *server_args) { - grpc_end2end_test_fixture f; - gpr_log(GPR_INFO, "%s/%s", test_name, config.name); - f = config.create_fixture(client_args, server_args); - config.init_server(&f, server_args); - config.init_client(&f, client_args); - return f; -} - -static gpr_timespec n_seconds_time(int n) { - return GRPC_TIMEOUT_SECONDS_TO_DEADLINE(n); -} - -static gpr_timespec five_seconds_time(void) { return n_seconds_time(5); } - -static void drain_cq(grpc_completion_queue *cq) { - grpc_event ev; - do { - ev = grpc_completion_queue_next(cq, five_seconds_time(), NULL); - } while (ev.type != GRPC_QUEUE_SHUTDOWN); -} - -static void shutdown_server(grpc_end2end_test_fixture *f) { - if (!f->server) return; - grpc_server_shutdown_and_notify(f->server, f->cq, tag(1000)); - GPR_ASSERT(grpc_completion_queue_pluck( - f->cq, tag(1000), GRPC_TIMEOUT_SECONDS_TO_DEADLINE(5), NULL) - .type == GRPC_OP_COMPLETE); - grpc_server_destroy(f->server); - f->server = NULL; -} - -static void shutdown_client(grpc_end2end_test_fixture *f) { - if (!f->client) return; - grpc_channel_destroy(f->client); - f->client = NULL; -} - -static void end_test(grpc_end2end_test_fixture *f) { - shutdown_server(f); - shutdown_client(f); - - grpc_completion_queue_shutdown(f->cq); - drain_cq(f->cq); - grpc_completion_queue_destroy(f->cq); -} - -static bool coalesced_message_and_eos; - -static void log_processor(gpr_log_func_args *args) { - const int file_len = (int)strlen(args->file); - const char suffix1[] = "secure_endpoint.c"; - const int suffix1_len = sizeof(suffix1) - 1; - const char suffix2[] = "tcp_posix.c"; - const int suffix2_len = sizeof(suffix2) - 1; - const char prefix[] = "READ"; - const int prefix_len = sizeof(prefix) - 1; - if (((file_len >= suffix1_len && - 0 == strcmp(suffix1, &args->file[file_len - suffix1_len])) || - (file_len >= suffix2_len && - 0 == strcmp(suffix2, &args->file[file_len - suffix2_len]))) && - 0 == strncmp(prefix, args->message, (size_t)prefix_len) && - strstr(args->message, - "00 00 10 00 01 00 00 00 01 00 00 00 00 0b 68 65 6c 6c 6f 20 77 " - "6f 72 6c 64")) { - fprintf(stderr, "%s, %s\n", args->file, args->message); - coalesced_message_and_eos = true; - } -} - -/* Request/response with metadata and payload.*/ -static void test_request_response_with_metadata_and_payload( - grpc_end2end_test_config config) { - grpc_call *c; - grpc_call *s; - grpc_slice request_payload_slice = - grpc_slice_from_copied_string("hello world"); - grpc_byte_buffer *request_payload = - grpc_raw_byte_buffer_create(&request_payload_slice, 1); - gpr_timespec deadline = five_seconds_time(); - grpc_metadata meta_c[2] = { - {"key1", "val1", 4, 0, {{NULL, NULL, NULL, NULL}}}, - {"key2", "val2", 4, 0, {{NULL, NULL, NULL, NULL}}}}; - grpc_metadata meta_s[2] = { - {"key3", "val3", 4, 0, {{NULL, NULL, NULL, NULL}}}, - {"key4", "val4", 4, 0, {{NULL, NULL, NULL, NULL}}}}; - grpc_end2end_test_fixture f = begin_test( - config, "test_request_response_with_metadata_and_payload", NULL, NULL); - cq_verifier *cqv = cq_verifier_create(f.cq); - grpc_op ops[6]; - grpc_op *op; - grpc_metadata_array initial_metadata_recv; - grpc_metadata_array trailing_metadata_recv; - grpc_metadata_array request_metadata_recv; - grpc_byte_buffer *request_payload_recv = NULL; - grpc_call_details call_details; - grpc_status_code status; - grpc_call_error error; - char *details = NULL; - size_t details_capacity = 0; - int was_cancelled = 2; - coalesced_message_and_eos = false; - - c = grpc_channel_create_call( - f.client, NULL, GRPC_PROPAGATE_DEFAULTS, f.cq, "/foo", - get_host_override_string("foo.test.google.fr:1234", config), deadline, - NULL); - GPR_ASSERT(c); - - grpc_metadata_array_init(&initial_metadata_recv); - grpc_metadata_array_init(&trailing_metadata_recv); - grpc_metadata_array_init(&request_metadata_recv); - grpc_call_details_init(&call_details); - - memset(ops, 0, sizeof(ops)); - op = ops; - op->op = GRPC_OP_SEND_INITIAL_METADATA; - op->data.send_initial_metadata.count = 2; - op->data.send_initial_metadata.metadata = meta_c; - op->flags = 0; - op->reserved = NULL; - op++; - op->op = GRPC_OP_SEND_MESSAGE; - op->data.send_message = request_payload; - op->flags = 0; - op->reserved = NULL; - op++; - op->op = GRPC_OP_SEND_CLOSE_FROM_CLIENT; - op->flags = 0; - op->reserved = NULL; - op++; - op->op = GRPC_OP_RECV_INITIAL_METADATA; - op->data.recv_initial_metadata = &initial_metadata_recv; - op->flags = 0; - op->reserved = NULL; - op++; - op->op = GRPC_OP_RECV_STATUS_ON_CLIENT; - op->data.recv_status_on_client.trailing_metadata = &trailing_metadata_recv; - op->data.recv_status_on_client.status = &status; - op->data.recv_status_on_client.status_details = &details; - op->data.recv_status_on_client.status_details_capacity = &details_capacity; - op->flags = 0; - op->reserved = NULL; - op++; - error = grpc_call_start_batch(c, ops, (size_t)(op - ops), tag(1), NULL); - GPR_ASSERT(GRPC_CALL_OK == error); - - error = - grpc_server_request_call(f.server, &s, &call_details, - &request_metadata_recv, f.cq, f.cq, tag(101)); - GPR_ASSERT(GRPC_CALL_OK == error); - CQ_EXPECT_COMPLETION(cqv, tag(101), 1); - cq_verify(cqv); - - memset(ops, 0, sizeof(ops)); - op = ops; - op->op = GRPC_OP_SEND_INITIAL_METADATA; - op->data.send_initial_metadata.count = 2; - op->data.send_initial_metadata.metadata = meta_s; - op->flags = 0; - op->reserved = NULL; - op++; - op->op = GRPC_OP_RECV_MESSAGE; - op->data.recv_message = &request_payload_recv; - op->flags = 0; - op->reserved = NULL; - op++; - error = grpc_call_start_batch(s, ops, (size_t)(op - ops), tag(102), NULL); - GPR_ASSERT(GRPC_CALL_OK == error); - - CQ_EXPECT_COMPLETION(cqv, tag(102), 1); - cq_verify(cqv); - - memset(ops, 0, sizeof(ops)); - op = ops; - op->op = GRPC_OP_RECV_CLOSE_ON_SERVER; - op->data.recv_close_on_server.cancelled = &was_cancelled; - op->flags = 0; - op->reserved = NULL; - op++; - op->op = GRPC_OP_SEND_STATUS_FROM_SERVER; - op->data.send_status_from_server.trailing_metadata_count = 0; - op->data.send_status_from_server.status = GRPC_STATUS_OK; - op->data.send_status_from_server.status_details = "xyz"; - op->flags = 0; - op->reserved = NULL; - op++; - error = grpc_call_start_batch(s, ops, (size_t)(op - ops), tag(103), NULL); - GPR_ASSERT(GRPC_CALL_OK == error); - - CQ_EXPECT_COMPLETION(cqv, tag(103), 1); - CQ_EXPECT_COMPLETION(cqv, tag(1), 1); - cq_verify(cqv); - - GPR_ASSERT(status == GRPC_STATUS_OK); - GPR_ASSERT(0 == strcmp(details, "xyz")); - GPR_ASSERT(0 == strcmp(call_details.method, "/foo")); - validate_host_override_string("foo.test.google.fr:1234", call_details.host, - config); - GPR_ASSERT(was_cancelled == 0); - GPR_ASSERT(byte_buffer_eq_string(request_payload_recv, "hello world")); - GPR_ASSERT(contains_metadata(&request_metadata_recv, "key1", "val1")); - GPR_ASSERT(contains_metadata(&request_metadata_recv, "key2", "val2")); - GPR_ASSERT(coalesced_message_and_eos); - - gpr_free(details); - grpc_metadata_array_destroy(&initial_metadata_recv); - grpc_metadata_array_destroy(&trailing_metadata_recv); - grpc_metadata_array_destroy(&request_metadata_recv); - grpc_call_details_destroy(&call_details); - - grpc_call_destroy(c); - grpc_call_destroy(s); - - cq_verifier_destroy(cqv); - - grpc_byte_buffer_destroy(request_payload); - grpc_byte_buffer_destroy(request_payload_recv); - - end_test(&f); - config.tear_down_data(&f); -} - -void packet_coalescing(grpc_end2end_test_config config) { - gpr_set_log_verbosity(GPR_LOG_SEVERITY_DEBUG); - grpc_tracer_set_enabled("all", 1); - gpr_set_log_function(log_processor); - test_request_response_with_metadata_and_payload(config); - gpr_set_log_function(gpr_default_log); - grpc_tracer_set_enabled("all", 0); -} - -void packet_coalescing_pre_init(void) {} From eb5ee45eec0c6f88bcf649f383060f3dc34f2084 Mon Sep 17 00:00:00 2001 From: Muxi Yan Date: Wed, 1 Feb 2017 11:57:33 -0800 Subject: [PATCH 07/23] Revert "Dynamically enable/disable packet coalecsing and test it" This reverts commit 60ab7ef00ac0a988ee2672c636d946c964e6fa41. --- include/grpc/grpc_cronet.h | 2 - .../client/secure/cronet_channel_create.c | 6 -- .../cronet/transport/cronet_transport.c | 72 ++++++++++--------- .../tests/CronetUnitTests/CronetUnitTests.m | 13 +--- src/objective-c/tests/Podfile | 1 + 5 files changed, 42 insertions(+), 52 deletions(-) diff --git a/include/grpc/grpc_cronet.h b/include/grpc/grpc_cronet.h index 566c34a388d..295e0f55e80 100644 --- a/include/grpc/grpc_cronet.h +++ b/include/grpc/grpc_cronet.h @@ -44,8 +44,6 @@ GRPCAPI grpc_channel *grpc_cronet_secure_channel_create( void *engine, const char *target, const grpc_channel_args *args, void *reserved); -GRPCAPI void grpc_cronet_use_packet_coalescing(bool use_coalescing); - #ifdef __cplusplus } #endif diff --git a/src/core/ext/transport/cronet/client/secure/cronet_channel_create.c b/src/core/ext/transport/cronet/client/secure/cronet_channel_create.c index 2e40020ae05..477cf07f45d 100644 --- a/src/core/ext/transport/cronet/client/secure/cronet_channel_create.c +++ b/src/core/ext/transport/cronet/client/secure/cronet_channel_create.c @@ -51,8 +51,6 @@ typedef struct cronet_transport { extern grpc_transport_vtable grpc_cronet_vtable; -bool grpc_cronet_packet_coalescing_enabled = true; - GRPCAPI grpc_channel *grpc_cronet_secure_channel_create( void *engine, const char *target, const grpc_channel_args *args, void *reserved) { @@ -69,7 +67,3 @@ GRPCAPI grpc_channel *grpc_cronet_secure_channel_create( return grpc_channel_create(&exec_ctx, target, args, GRPC_CLIENT_DIRECT_CHANNEL, (grpc_transport *)ct); } - -GRPCAPI void grpc_cronet_use_packet_coalescing(bool use_coalescing) { - grpc_cronet_packet_coalescing_enabled = use_coalescing; -} diff --git a/src/core/ext/transport/cronet/transport/cronet_transport.c b/src/core/ext/transport/cronet/transport/cronet_transport.c index 5429eb32e3c..447f3f31ec1 100644 --- a/src/core/ext/transport/cronet/transport/cronet_transport.c +++ b/src/core/ext/transport/cronet/transport/cronet_transport.c @@ -61,8 +61,6 @@ /* TODO (makdharma): Hook up into the wider tracing mechanism */ int grpc_cronet_trace = 0; -extern bool grpc_cronet_packet_coalescing_enabled; - enum e_op_result { ACTION_TAKEN_WITH_CALLBACK, ACTION_TAKEN_NO_CALLBACK, @@ -152,13 +150,12 @@ struct op_state { bool state_callback_received[OP_NUM_OPS]; bool fail_state; bool flush_read; +#ifdef GRPC_CRONET_WITH_PACKET_COALESCING bool flush_cronet_when_ready; bool pending_write_for_trailer; +#endif bool unprocessed_send_message; grpc_error *cancel_error; - - /* Whether packet coalescing is enabled */ - bool packet_coalescing_enabled; /* data structure for storing data coming from server */ struct read_state rs; /* data structure for storing data going to the server */ @@ -428,10 +425,12 @@ static void on_stream_ready(bidirectional_stream *stream) { } /* Send the initial metadata on wire if there is no SEND_MESSAGE or * SEND_TRAILING_METADATA ops pending */ - if (s->state.packet_coalescing_enabled && s->state.flush_cronet_when_ready) { +#ifdef GRPC_CRONET_WITH_PACKET_COALESCING + if (s->state.flush_cronet_when_ready) { CRONET_LOG(GPR_DEBUG, "cronet_bidirectional_stream_flush (%p)", s->cbs); bidirectional_stream_flush(stream); } +#endif gpr_mu_unlock(&s->mu); execute_from_storage(s); } @@ -569,10 +568,10 @@ static void on_response_trailers_received( CRONET_LOG(GPR_DEBUG, "bidirectional_stream_write (%p, 0)", s->cbs); s->state.state_callback_received[OP_SEND_MESSAGE] = false; bidirectional_stream_write(s->cbs, "", 0, true); - if (s->state.packet_coalescing_enabled) { - CRONET_LOG(GPR_DEBUG, "bidirectional_stream_flush (%p)", s->cbs); - bidirectional_stream_flush(s->cbs); - } +#ifdef GRPC_CRONET_WITH_PACKET_COALESCING + CRONET_LOG(GPR_DEBUG, "bidirectional_stream_flush (%p)", s->cbs); + bidirectional_stream_flush(s->cbs); +#endif s->state.state_op_done[OP_SEND_TRAILING_METADATA] = true; gpr_mu_unlock(&s->mu); @@ -769,9 +768,11 @@ static bool op_can_be_run(grpc_transport_stream_op *curr_op, result = false; /* we haven't got on_write_completed for the send yet */ else if (stream_state->state_op_done[OP_SEND_MESSAGE] && - !stream_state->state_callback_received[OP_SEND_MESSAGE] && - !(stream_state->packet_coalescing_enabled && - stream_state->pending_write_for_trailer)) + !stream_state->state_callback_received[OP_SEND_MESSAGE] +#ifdef GRPC_CRONET_WITH_PACKET_COALESCING + && !stream_state->pending_write_for_trailer +#endif + ) result = false; } else if (op_id == OP_CANCEL_ERROR) { /* already executed */ @@ -857,10 +858,10 @@ static enum e_op_result execute_stream_op(grpc_exec_ctx *exec_ctx, s->cbs = bidirectional_stream_create(s->curr_ct.engine, s->curr_gs, &cronet_callbacks); CRONET_LOG(GPR_DEBUG, "%p = bidirectional_stream_create()", s->cbs); - if (stream_state->packet_coalescing_enabled) { - bidirectional_stream_disable_auto_flush(s->cbs, true); - bidirectional_stream_delay_request_headers_until_flush(s->cbs, true); - } +#ifdef GRPC_CRONET_WITH_PACKET_COALESCING + bidirectional_stream_disable_auto_flush(s->cbs, true); + bidirectional_stream_delay_request_headers_until_flush(s->cbs, true); +#endif char *url = NULL; const char *method = "POST"; s->header_array.headers = NULL; @@ -871,10 +872,11 @@ static enum e_op_result execute_stream_op(grpc_exec_ctx *exec_ctx, CRONET_LOG(GPR_DEBUG, "bidirectional_stream_start(%p, %s)", s->cbs, url); bidirectional_stream_start(s->cbs, url, 0, method, &s->header_array, false); stream_state->state_op_done[OP_SEND_INITIAL_METADATA] = true; - if (stream_state->packet_coalescing_enabled && !stream_op->send_message && - !stream_op->send_trailing_metadata) { +#ifdef GRPC_CRONET_WITH_PACKET_COALESCING + if (!stream_op->send_message && !stream_op->send_trailing_metadata) { s->state.flush_cronet_when_ready = true; } +#endif result = ACTION_TAKEN_WITH_CALLBACK; } else if (stream_op->send_message && op_can_be_run(stream_op, stream_state, &oas->state, @@ -911,18 +913,19 @@ static enum e_op_result execute_stream_op(grpc_exec_ctx *exec_ctx, stream_state->state_callback_received[OP_SEND_MESSAGE] = false; bidirectional_stream_write(s->cbs, stream_state->ws.write_buffer, (int)write_buffer_size, false); - if (stream_state->packet_coalescing_enabled) { - if (!stream_op->send_trailing_metadata) { - CRONET_LOG(GPR_DEBUG, "bidirectional_stream_flush (%p)", s->cbs); - bidirectional_stream_flush(s->cbs); - result = ACTION_TAKEN_WITH_CALLBACK; - } else { - stream_state->pending_write_for_trailer = true; - result = ACTION_TAKEN_NO_CALLBACK; - } - } else { +#ifdef GRPC_CRONET_WITH_PACKET_COALESCING + if (!stream_op->send_trailing_metadata) { + CRONET_LOG(GPR_DEBUG, "bidirectional_stream_flush (%p)", + s->cbs); + bidirectional_stream_flush(s->cbs); result = ACTION_TAKEN_WITH_CALLBACK; + } else { + stream_state->pending_write_for_trailer = true; + result = ACTION_TAKEN_NO_CALLBACK; } +#else + result = ACTION_TAKEN_WITH_CALLBACK; +#endif } else { result = NO_ACTION_POSSIBLE; } @@ -941,10 +944,10 @@ static enum e_op_result execute_stream_op(grpc_exec_ctx *exec_ctx, s->cbs); stream_state->state_callback_received[OP_SEND_MESSAGE] = false; bidirectional_stream_write(s->cbs, "", 0, true); - if (stream_state->packet_coalescing_enabled) { - CRONET_LOG(GPR_DEBUG, "bidirectional_stream_flush (%p)", s->cbs); - bidirectional_stream_flush(s->cbs); - } +#ifdef GRPC_CRONET_WITH_PACKET_COALESCING + CRONET_LOG(GPR_DEBUG, "bidirectional_stream_flush (%p)", s->cbs); + bidirectional_stream_flush(s->cbs); +#endif result = ACTION_TAKEN_WITH_CALLBACK; } stream_state->state_op_done[OP_SEND_TRAILING_METADATA] = true; @@ -1173,9 +1176,10 @@ static int init_stream(grpc_exec_ctx *exec_ctx, grpc_transport *gt, sizeof(s->state.state_callback_received)); s->state.fail_state = s->state.flush_read = false; s->state.cancel_error = NULL; +#ifdef GRPC_CRONET_WITH_PACKET_COALESCING s->state.flush_cronet_when_ready = s->state.pending_write_for_trailer = false; +#endif s->state.unprocessed_send_message = false; - s->state.packet_coalescing_enabled = grpc_cronet_packet_coalescing_enabled; gpr_mu_init(&s->mu); return 0; } diff --git a/src/objective-c/tests/CronetUnitTests/CronetUnitTests.m b/src/objective-c/tests/CronetUnitTests/CronetUnitTests.m index 9bbf3cdb113..dcd7f2fa8db 100644 --- a/src/objective-c/tests/CronetUnitTests/CronetUnitTests.m +++ b/src/objective-c/tests/CronetUnitTests/CronetUnitTests.m @@ -269,9 +269,7 @@ unsigned int parse_h2_length(const char *field) { grpc_completion_queue_destroy(cq); } -- (void)PacketCoalescing:(bool)use_coalescing { - grpc_cronet_use_packet_coalescing(use_coalescing); - +- (void)testPacketCoalescing { grpc_call *c; grpc_slice request_payload_slice = grpc_slice_from_copied_string("hello world"); @@ -381,7 +379,7 @@ unsigned int parse_h2_length(const char *field) { long len; bool coalesced = false; while ((len = SSL_read(ssl, buf, sizeof(buf))) > 0) { - gpr_log(GPR_DEBUG, "Read len: %ld", len); + NSLog(@"Read len: %ld", len); // Analyze the HTTP/2 frames in the same TLS PDU to identify if // coalescing is successful @@ -406,7 +404,7 @@ unsigned int parse_h2_length(const char *field) { } } - XCTAssert(coalesced == use_coalescing); + XCTAssert(coalesced); SSL_free(ssl); SSL_CTX_free(ctx); close(s); @@ -435,9 +433,4 @@ unsigned int parse_h2_length(const char *field) { grpc_completion_queue_destroy(cq); } -- (void)testPacketCoalescing { - [self PacketCoalescing:false]; - [self PacketCoalescing:true]; -} - @end diff --git a/src/objective-c/tests/Podfile b/src/objective-c/tests/Podfile index 3760330be9a..462c6a8e0eb 100644 --- a/src/objective-c/tests/Podfile +++ b/src/objective-c/tests/Podfile @@ -97,6 +97,7 @@ post_install do |installer| # GPR_UNREACHABLE_CODE causes "Control may reach end of non-void # function" warning config.build_settings['GCC_WARN_ABOUT_RETURN_TYPE'] = 'NO' + config.build_settings['GCC_PREPROCESSOR_DEFINITIONS'] = '$(inherited) COCOAPODS=1 GRPC_CRONET_WITH_PACKET_COALESCING=1' end end From 0a2fae9aedeef98b39c5f825cb28b5ca32ecc6a2 Mon Sep 17 00:00:00 2001 From: Muxi Yan Date: Wed, 1 Feb 2017 14:49:03 -0800 Subject: [PATCH 08/23] Dynamically enable packet coalescing by channel args --- BUILD | 1 + build.yaml | 1 + gRPC-Core.podspec | 3 +- include/grpc/impl/codegen/grpc_types.h | 4 + .../client/secure/cronet_channel_create.c | 13 +- .../cronet/transport/cronet_transport.c | 181 +++++++++++------- .../cronet/transport/cronet_transport.h | 43 +++++ .../tests/CronetUnitTests/CronetUnitTests.m | 20 +- src/objective-c/tests/Podfile | 1 - templates/gRPC-Core.podspec.template | 3 +- .../generated/sources_and_headers.json | 4 +- 11 files changed, 186 insertions(+), 88 deletions(-) create mode 100644 src/core/ext/transport/cronet/transport/cronet_transport.h diff --git a/BUILD b/BUILD index 54192514cc7..e5c2d20d25f 100644 --- a/BUILD +++ b/BUILD @@ -1028,6 +1028,7 @@ grpc_cc_library( ], hdrs = [ "third_party/Cronet/bidirectional_stream_c.h", + "src/core/ext/transport/cronet/transport/cronet_transport.h", ], language = "c", public_hdrs = [ diff --git a/build.yaml b/build.yaml index 23e2659ea1e..63c05b76c6c 100644 --- a/build.yaml +++ b/build.yaml @@ -684,6 +684,7 @@ filegroups: - include/grpc/grpc_security.h - include/grpc/grpc_security_constants.h headers: + - src/core/ext/transport/cronet/transport/cronet_transport.h - third_party/Cronet/bidirectional_stream_c.h src: - src/core/ext/transport/cronet/client/secure/cronet_channel_create.c diff --git a/gRPC-Core.podspec b/gRPC-Core.podspec index 1eb178931dc..57816d066d9 100644 --- a/gRPC-Core.podspec +++ b/gRPC-Core.podspec @@ -848,7 +848,8 @@ Pod::Spec.new do |s| s.subspec 'Cronet-Interface' do |ss| ss.header_mappings_dir = 'include/grpc' - ss.source_files = 'include/grpc/grpc_cronet.h' + ss.source_files = 'include/grpc/grpc_cronet.h', + 'src/core/ext/transport/cronet/transport/cronet_transport.h' end s.subspec 'Cronet-Implementation' do |ss| diff --git a/include/grpc/impl/codegen/grpc_types.h b/include/grpc/impl/codegen/grpc_types.h index ee8101aab8d..74be7d0ccfc 100644 --- a/include/grpc/impl/codegen/grpc_types.h +++ b/include/grpc/impl/codegen/grpc_types.h @@ -217,6 +217,10 @@ typedef struct { #define GRPC_ARG_LB_POLICY_NAME "grpc.lb_policy_name" /** The grpc_socket_mutator instance that set the socket options. A pointer. */ #define GRPC_ARG_SOCKET_MUTATOR "grpc.socket_mutator" +/** If non-zero, Cronet transport will coalesce packets to fewer frames when + * possible. */ +#define GRPC_ARG_USE_CRONET_PACKET_COALESCING \ + "grpc.use_cronet_packet_coalescing" /** \} */ /** Result of a grpc call. If the caller satisfies the prerequisites of a diff --git a/src/core/ext/transport/cronet/client/secure/cronet_channel_create.c b/src/core/ext/transport/cronet/client/secure/cronet_channel_create.c index 477cf07f45d..b6e9e845df3 100644 --- a/src/core/ext/transport/cronet/client/secure/cronet_channel_create.c +++ b/src/core/ext/transport/cronet/client/secure/cronet_channel_create.c @@ -39,6 +39,7 @@ #include #include +#include "src/core/ext/transport/cronet/transport/cronet_transport.h" #include "src/core/lib/surface/channel.h" #include "src/core/lib/transport/transport_impl.h" @@ -54,16 +55,14 @@ extern grpc_transport_vtable grpc_cronet_vtable; GRPCAPI grpc_channel *grpc_cronet_secure_channel_create( void *engine, const char *target, const grpc_channel_args *args, void *reserved) { - cronet_transport *ct = gpr_malloc(sizeof(cronet_transport)); - ct->base.vtable = &grpc_cronet_vtable; - ct->engine = engine; - ct->host = gpr_malloc(strlen(target) + 1); - strcpy(ct->host, target); gpr_log(GPR_DEBUG, "grpc_create_cronet_transport: stream_engine = %p, target=%s", engine, - ct->host); + target); + + grpc_transport *ct = + grpc_create_cronet_transport(engine, target, args, reserved); grpc_exec_ctx exec_ctx = GRPC_EXEC_CTX_INIT; return grpc_channel_create(&exec_ctx, target, args, - GRPC_CLIENT_DIRECT_CHANNEL, (grpc_transport *)ct); + GRPC_CLIENT_DIRECT_CHANNEL, ct); } diff --git a/src/core/ext/transport/cronet/transport/cronet_transport.c b/src/core/ext/transport/cronet/transport/cronet_transport.c index 447f3f31ec1..cba5365badc 100644 --- a/src/core/ext/transport/cronet/transport/cronet_transport.c +++ b/src/core/ext/transport/cronet/transport/cronet_transport.c @@ -112,6 +112,7 @@ struct grpc_cronet_transport { grpc_transport base; /* must be first element in this structure */ stream_engine *engine; char *host; + bool use_packet_coalescing; }; typedef struct grpc_cronet_transport grpc_cronet_transport; @@ -150,10 +151,8 @@ struct op_state { bool state_callback_received[OP_NUM_OPS]; bool fail_state; bool flush_read; -#ifdef GRPC_CRONET_WITH_PACKET_COALESCING bool flush_cronet_when_ready; bool pending_write_for_trailer; -#endif bool unprocessed_send_message; grpc_error *cancel_error; /* data structure for storing data coming from server */ @@ -178,7 +177,7 @@ struct op_storage { struct stream_obj { struct op_and_state *oas; grpc_transport_stream_op *curr_op; - grpc_cronet_transport curr_ct; + grpc_cronet_transport *curr_ct; grpc_stream *curr_gs; bidirectional_stream *cbs; bidirectional_stream_header_array header_array; @@ -415,6 +414,7 @@ static void on_succeeded(bidirectional_stream *stream) { static void on_stream_ready(bidirectional_stream *stream) { CRONET_LOG(GPR_DEBUG, "W: on_stream_ready(%p)", stream); stream_obj *s = (stream_obj *)stream->annotation; + grpc_cronet_transport *t = (grpc_cronet_transport *)s->curr_ct; gpr_mu_lock(&s->mu); s->state.state_op_done[OP_SEND_INITIAL_METADATA] = true; s->state.state_callback_received[OP_SEND_INITIAL_METADATA] = true; @@ -425,12 +425,12 @@ static void on_stream_ready(bidirectional_stream *stream) { } /* Send the initial metadata on wire if there is no SEND_MESSAGE or * SEND_TRAILING_METADATA ops pending */ -#ifdef GRPC_CRONET_WITH_PACKET_COALESCING - if (s->state.flush_cronet_when_ready) { - CRONET_LOG(GPR_DEBUG, "cronet_bidirectional_stream_flush (%p)", s->cbs); - bidirectional_stream_flush(stream); + if (t->use_packet_coalescing) { + if (s->state.flush_cronet_when_ready) { + CRONET_LOG(GPR_DEBUG, "cronet_bidirectional_stream_flush (%p)", s->cbs); + bidirectional_stream_flush(stream); + } } -#endif gpr_mu_unlock(&s->mu); execute_from_storage(s); } @@ -540,6 +540,7 @@ static void on_response_trailers_received( CRONET_LOG(GPR_DEBUG, "R: on_response_trailers_received(%p,%p)", stream, trailers); stream_obj *s = (stream_obj *)stream->annotation; + grpc_cronet_transport *t = (grpc_cronet_transport *)s->curr_ct; gpr_mu_lock(&s->mu); memset(&s->state.rs.trailing_metadata, 0, sizeof(s->state.rs.trailing_metadata)); @@ -568,10 +569,10 @@ static void on_response_trailers_received( CRONET_LOG(GPR_DEBUG, "bidirectional_stream_write (%p, 0)", s->cbs); s->state.state_callback_received[OP_SEND_MESSAGE] = false; bidirectional_stream_write(s->cbs, "", 0, true); -#ifdef GRPC_CRONET_WITH_PACKET_COALESCING - CRONET_LOG(GPR_DEBUG, "bidirectional_stream_flush (%p)", s->cbs); - bidirectional_stream_flush(s->cbs); -#endif + if (t->use_packet_coalescing) { + CRONET_LOG(GPR_DEBUG, "bidirectional_stream_flush (%p)", s->cbs); + bidirectional_stream_flush(s->cbs); + } s->state.state_op_done[OP_SEND_TRAILING_METADATA] = true; gpr_mu_unlock(&s->mu); @@ -695,8 +696,10 @@ static bool header_has_authority(grpc_linked_mdelem *head) { executed. This is the heart of the state machine. */ static bool op_can_be_run(grpc_transport_stream_op *curr_op, - struct op_state *stream_state, - struct op_state *op_state, enum e_op_id op_id) { + struct stream_obj *s, struct op_state *op_state, + enum e_op_id op_id) { + struct op_state *stream_state = &s->state; + grpc_cronet_transport *t = s->curr_ct; bool result = true; /* When call is canceled, every op can be run, except under following conditions @@ -768,11 +771,9 @@ static bool op_can_be_run(grpc_transport_stream_op *curr_op, result = false; /* we haven't got on_write_completed for the send yet */ else if (stream_state->state_op_done[OP_SEND_MESSAGE] && - !stream_state->state_callback_received[OP_SEND_MESSAGE] -#ifdef GRPC_CRONET_WITH_PACKET_COALESCING - && !stream_state->pending_write_for_trailer -#endif - ) + !stream_state->state_callback_received[OP_SEND_MESSAGE] && + !(t->use_packet_coalescing && + stream_state->pending_write_for_trailer)) result = false; } else if (op_id == OP_CANCEL_ERROR) { /* already executed */ @@ -845,42 +846,41 @@ static enum e_op_result execute_stream_op(grpc_exec_ctx *exec_ctx, struct op_and_state *oas) { grpc_transport_stream_op *stream_op = &oas->op; struct stream_obj *s = oas->s; + grpc_cronet_transport *t = (grpc_cronet_transport *)s->curr_ct; struct op_state *stream_state = &s->state; enum e_op_result result = NO_ACTION_POSSIBLE; if (stream_op->send_initial_metadata && - op_can_be_run(stream_op, stream_state, &oas->state, - OP_SEND_INITIAL_METADATA)) { + op_can_be_run(stream_op, s, &oas->state, OP_SEND_INITIAL_METADATA)) { CRONET_LOG(GPR_DEBUG, "running: %p OP_SEND_INITIAL_METADATA", oas); /* Start new cronet stream. It is destroyed in on_succeeded, on_canceled, * on_failed */ GPR_ASSERT(s->cbs == NULL); GPR_ASSERT(!stream_state->state_op_done[OP_SEND_INITIAL_METADATA]); - s->cbs = bidirectional_stream_create(s->curr_ct.engine, s->curr_gs, + s->cbs = bidirectional_stream_create(t->engine, s->curr_gs, &cronet_callbacks); CRONET_LOG(GPR_DEBUG, "%p = bidirectional_stream_create()", s->cbs); -#ifdef GRPC_CRONET_WITH_PACKET_COALESCING - bidirectional_stream_disable_auto_flush(s->cbs, true); - bidirectional_stream_delay_request_headers_until_flush(s->cbs, true); -#endif + if (t->use_packet_coalescing) { + bidirectional_stream_disable_auto_flush(s->cbs, true); + bidirectional_stream_delay_request_headers_until_flush(s->cbs, true); + } char *url = NULL; const char *method = "POST"; s->header_array.headers = NULL; convert_metadata_to_cronet_headers( - stream_op->send_initial_metadata->list.head, s->curr_ct.host, &url, + stream_op->send_initial_metadata->list.head, t->host, &url, &s->header_array.headers, &s->header_array.count, &method); s->header_array.capacity = s->header_array.count; CRONET_LOG(GPR_DEBUG, "bidirectional_stream_start(%p, %s)", s->cbs, url); bidirectional_stream_start(s->cbs, url, 0, method, &s->header_array, false); stream_state->state_op_done[OP_SEND_INITIAL_METADATA] = true; -#ifdef GRPC_CRONET_WITH_PACKET_COALESCING - if (!stream_op->send_message && !stream_op->send_trailing_metadata) { - s->state.flush_cronet_when_ready = true; + if (t->use_packet_coalescing) { + if (!stream_op->send_message && !stream_op->send_trailing_metadata) { + s->state.flush_cronet_when_ready = true; + } } -#endif result = ACTION_TAKEN_WITH_CALLBACK; } else if (stream_op->send_message && - op_can_be_run(stream_op, stream_state, &oas->state, - OP_SEND_MESSAGE)) { + op_can_be_run(stream_op, s, &oas->state, OP_SEND_MESSAGE)) { CRONET_LOG(GPR_DEBUG, "running: %p OP_SEND_MESSAGE", oas); stream_state->unprocessed_send_message = false; if (stream_state->state_callback_received[OP_FAILED]) { @@ -913,19 +913,18 @@ static enum e_op_result execute_stream_op(grpc_exec_ctx *exec_ctx, stream_state->state_callback_received[OP_SEND_MESSAGE] = false; bidirectional_stream_write(s->cbs, stream_state->ws.write_buffer, (int)write_buffer_size, false); -#ifdef GRPC_CRONET_WITH_PACKET_COALESCING - if (!stream_op->send_trailing_metadata) { - CRONET_LOG(GPR_DEBUG, "bidirectional_stream_flush (%p)", - s->cbs); - bidirectional_stream_flush(s->cbs); - result = ACTION_TAKEN_WITH_CALLBACK; + if (t->use_packet_coalescing) { + if (!stream_op->send_trailing_metadata) { + CRONET_LOG(GPR_DEBUG, "bidirectional_stream_flush (%p)", s->cbs); + bidirectional_stream_flush(s->cbs); + result = ACTION_TAKEN_WITH_CALLBACK; + } else { + stream_state->pending_write_for_trailer = true; + result = ACTION_TAKEN_NO_CALLBACK; + } } else { - stream_state->pending_write_for_trailer = true; - result = ACTION_TAKEN_NO_CALLBACK; + result = ACTION_TAKEN_WITH_CALLBACK; } -#else - result = ACTION_TAKEN_WITH_CALLBACK; -#endif } else { result = NO_ACTION_POSSIBLE; } @@ -933,7 +932,7 @@ static enum e_op_result execute_stream_op(grpc_exec_ctx *exec_ctx, stream_state->state_op_done[OP_SEND_MESSAGE] = true; oas->state.state_op_done[OP_SEND_MESSAGE] = true; } else if (stream_op->send_trailing_metadata && - op_can_be_run(stream_op, stream_state, &oas->state, + op_can_be_run(stream_op, s, &oas->state, OP_SEND_TRAILING_METADATA)) { CRONET_LOG(GPR_DEBUG, "running: %p OP_SEND_TRAILING_METADATA", oas); if (stream_state->state_callback_received[OP_FAILED]) { @@ -944,15 +943,15 @@ static enum e_op_result execute_stream_op(grpc_exec_ctx *exec_ctx, s->cbs); stream_state->state_callback_received[OP_SEND_MESSAGE] = false; bidirectional_stream_write(s->cbs, "", 0, true); -#ifdef GRPC_CRONET_WITH_PACKET_COALESCING - CRONET_LOG(GPR_DEBUG, "bidirectional_stream_flush (%p)", s->cbs); - bidirectional_stream_flush(s->cbs); -#endif + if (t->use_packet_coalescing) { + CRONET_LOG(GPR_DEBUG, "bidirectional_stream_flush (%p)", s->cbs); + bidirectional_stream_flush(s->cbs); + } result = ACTION_TAKEN_WITH_CALLBACK; } stream_state->state_op_done[OP_SEND_TRAILING_METADATA] = true; } else if (stream_op->recv_initial_metadata && - op_can_be_run(stream_op, stream_state, &oas->state, + op_can_be_run(stream_op, s, &oas->state, OP_RECV_INITIAL_METADATA)) { CRONET_LOG(GPR_DEBUG, "running: %p OP_RECV_INITIAL_METADATA", oas); if (stream_state->state_op_done[OP_CANCEL_ERROR]) { @@ -971,8 +970,7 @@ static enum e_op_result execute_stream_op(grpc_exec_ctx *exec_ctx, stream_state->state_op_done[OP_RECV_INITIAL_METADATA] = true; result = ACTION_TAKEN_NO_CALLBACK; } else if (stream_op->recv_message && - op_can_be_run(stream_op, stream_state, &oas->state, - OP_RECV_MESSAGE)) { + op_can_be_run(stream_op, s, &oas->state, OP_RECV_MESSAGE)) { CRONET_LOG(GPR_DEBUG, "running: %p OP_RECV_MESSAGE", oas); if (stream_state->state_op_done[OP_CANCEL_ERROR]) { CRONET_LOG(GPR_DEBUG, "Stream is cancelled."); @@ -1084,7 +1082,7 @@ static enum e_op_result execute_stream_op(grpc_exec_ctx *exec_ctx, result = ACTION_TAKEN_NO_CALLBACK; } } else if (stream_op->recv_trailing_metadata && - op_can_be_run(stream_op, stream_state, &oas->state, + op_can_be_run(stream_op, s, &oas->state, OP_RECV_TRAILING_METADATA)) { CRONET_LOG(GPR_DEBUG, "running: %p OP_RECV_TRAILING_METADATA", oas); if (oas->s->state.rs.trailing_metadata_valid) { @@ -1096,8 +1094,7 @@ static enum e_op_result execute_stream_op(grpc_exec_ctx *exec_ctx, stream_state->state_op_done[OP_RECV_TRAILING_METADATA] = true; result = ACTION_TAKEN_NO_CALLBACK; } else if (stream_op->cancel_error && - op_can_be_run(stream_op, stream_state, &oas->state, - OP_CANCEL_ERROR)) { + op_can_be_run(stream_op, s, &oas->state, OP_CANCEL_ERROR)) { CRONET_LOG(GPR_DEBUG, "running: %p OP_CANCEL_ERROR", oas); CRONET_LOG(GPR_DEBUG, "W: bidirectional_stream_cancel(%p)", s->cbs); if (s->cbs) { @@ -1111,8 +1108,7 @@ static enum e_op_result execute_stream_op(grpc_exec_ctx *exec_ctx, stream_state->cancel_error = GRPC_ERROR_REF(stream_op->cancel_error); } } else if (stream_op->on_complete && - op_can_be_run(stream_op, stream_state, &oas->state, - OP_ON_COMPLETE)) { + op_can_be_run(stream_op, s, &oas->state, OP_ON_COMPLETE)) { CRONET_LOG(GPR_DEBUG, "running: %p OP_ON_COMPLETE", oas); if (stream_state->state_op_done[OP_CANCEL_ERROR]) { grpc_closure_sched(exec_ctx, stream_op->on_complete, @@ -1176,10 +1172,12 @@ static int init_stream(grpc_exec_ctx *exec_ctx, grpc_transport *gt, sizeof(s->state.state_callback_received)); s->state.fail_state = s->state.flush_read = false; s->state.cancel_error = NULL; -#ifdef GRPC_CRONET_WITH_PACKET_COALESCING s->state.flush_cronet_when_ready = s->state.pending_write_for_trailer = false; -#endif s->state.unprocessed_send_message = false; + + s->curr_gs = gs; + s->curr_ct = (grpc_cronet_transport *)gt; + gpr_mu_init(&s->mu); return 0; } @@ -1195,8 +1193,6 @@ static void perform_stream_op(grpc_exec_ctx *exec_ctx, grpc_transport *gt, grpc_stream *gs, grpc_transport_stream_op *op) { CRONET_LOG(GPR_DEBUG, "perform_stream_op"); stream_obj *s = (stream_obj *)gs; - s->curr_gs = gs; - memcpy(&s->curr_ct, gt, sizeof(grpc_cronet_transport)); add_to_storage(s, op); if (op->send_initial_metadata && header_has_authority(op->send_initial_metadata->list.head)) { @@ -1244,14 +1240,55 @@ static grpc_endpoint *get_endpoint(grpc_exec_ctx *exec_ctx, static void perform_op(grpc_exec_ctx *exec_ctx, grpc_transport *gt, grpc_transport_op *op) {} -const grpc_transport_vtable grpc_cronet_vtable = {sizeof(stream_obj), - "cronet_http", - init_stream, - set_pollset_do_nothing, - set_pollset_set_do_nothing, - perform_stream_op, - perform_op, - destroy_stream, - destroy_transport, - get_peer, - get_endpoint}; +static const grpc_transport_vtable grpc_cronet_vtable = { + sizeof(stream_obj), + "cronet_http", + init_stream, + set_pollset_do_nothing, + set_pollset_set_do_nothing, + perform_stream_op, + perform_op, + destroy_stream, + destroy_transport, + get_peer, + get_endpoint}; + +grpc_transport *grpc_create_cronet_transport(void *engine, const char *target, + const grpc_channel_args *args, + void *reserved) { + grpc_cronet_transport *ct = gpr_malloc(sizeof(grpc_cronet_transport)); + if (!ct) { + goto error; + } + ct->base.vtable = &grpc_cronet_vtable; + ct->engine = engine; + ct->host = gpr_malloc(strlen(target) + 1); + if (!ct->host) { + goto error; + } + strcpy(ct->host, target); + + ct->use_packet_coalescing = true; + for (size_t i = 0; i < args->num_args; i++) { + if (0 == strcmp(args->args[i].key, GRPC_ARG_USE_CRONET_PACKET_COALESCING)) { + if (args->args[i].type != GRPC_ARG_INTEGER) { + gpr_log(GPR_ERROR, "%s ignored: it must be an integer", + GRPC_ARG_USE_CRONET_PACKET_COALESCING); + } else { + ct->use_packet_coalescing = (args->args[i].value.integer != 0); + } + } + } + + return &ct->base; + +error: + if (ct) { + if (ct->host) { + gpr_free(ct->host); + } + gpr_free(ct); + } + + return NULL; +} \ No newline at end of file diff --git a/src/core/ext/transport/cronet/transport/cronet_transport.h b/src/core/ext/transport/cronet/transport/cronet_transport.h new file mode 100644 index 00000000000..169ce31fd7d --- /dev/null +++ b/src/core/ext/transport/cronet/transport/cronet_transport.h @@ -0,0 +1,43 @@ +/* + * + * Copyright 2016, Google Inc. + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are + * met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following disclaimer + * in the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Google Inc. nor the names of its + * contributors may be used to endorse or promote products derived from + * this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + * + */ + +#ifndef GRPC_CORE_EXT_TRANSPORT_CRONET_TRANSPORT_CRONET_TRANSPORT_H +#define GRPC_CORE_EXT_TRANSPORT_CRONET_TRANSPORT_CRONET_TRANSPORT_H + +#include "src/core/lib/transport/transport.h" + +grpc_transport *grpc_create_cronet_transport(void *engine, const char *target, + const grpc_channel_args *args, + void *reserved); + +#endif /* GRPC_CORE_EXT_TRANSPORT_CRONET_TRANSPORT_CRONET_TRANSPORT_H */ diff --git a/src/objective-c/tests/CronetUnitTests/CronetUnitTests.m b/src/objective-c/tests/CronetUnitTests/CronetUnitTests.m index dcd7f2fa8db..d06fe7767fa 100644 --- a/src/objective-c/tests/CronetUnitTests/CronetUnitTests.m +++ b/src/objective-c/tests/CronetUnitTests/CronetUnitTests.m @@ -269,7 +269,12 @@ unsigned int parse_h2_length(const char *field) { grpc_completion_queue_destroy(cq); } -- (void)testPacketCoalescing { +- (void)PacketCoalescing:(bool)use_coalescing { + grpc_arg arg; + arg.key = GRPC_ARG_USE_CRONET_PACKET_COALESCING; + arg.type = GRPC_ARG_INTEGER; + arg.value.integer = use_coalescing ? 1:0; + grpc_channel_args *args = grpc_channel_args_copy_and_add(NULL, &arg, 1); grpc_call *c; grpc_slice request_payload_slice = grpc_slice_from_copied_string("hello world"); @@ -285,8 +290,8 @@ unsigned int parse_h2_length(const char *field) { gpr_join_host_port(&addr, "127.0.0.1", port); grpc_completion_queue *cq = grpc_completion_queue_create(NULL); stream_engine *cronetEngine = [Cronet getGlobalEngine]; - grpc_channel *client = grpc_cronet_secure_channel_create(cronetEngine, addr, - NULL, NULL); + grpc_channel *client = + grpc_cronet_secure_channel_create(cronetEngine, addr, args, NULL); cq_verifier *cqv = cq_verifier_create(cq); grpc_op ops[6]; @@ -379,7 +384,7 @@ unsigned int parse_h2_length(const char *field) { long len; bool coalesced = false; while ((len = SSL_read(ssl, buf, sizeof(buf))) > 0) { - NSLog(@"Read len: %ld", len); + gpr_log(GPR_DEBUG, "Read len: %ld", len); // Analyze the HTTP/2 frames in the same TLS PDU to identify if // coalescing is successful @@ -404,7 +409,7 @@ unsigned int parse_h2_length(const char *field) { } } - XCTAssert(coalesced); + XCTAssert(coalesced == use_coalescing); SSL_free(ssl); SSL_CTX_free(ctx); close(s); @@ -433,4 +438,9 @@ unsigned int parse_h2_length(const char *field) { grpc_completion_queue_destroy(cq); } +- (void)testPacketCoalescing { + [self PacketCoalescing:true]; + [self PacketCoalescing:false]; +} + @end diff --git a/src/objective-c/tests/Podfile b/src/objective-c/tests/Podfile index 462c6a8e0eb..3760330be9a 100644 --- a/src/objective-c/tests/Podfile +++ b/src/objective-c/tests/Podfile @@ -97,7 +97,6 @@ post_install do |installer| # GPR_UNREACHABLE_CODE causes "Control may reach end of non-void # function" warning config.build_settings['GCC_WARN_ABOUT_RETURN_TYPE'] = 'NO' - config.build_settings['GCC_PREPROCESSOR_DEFINITIONS'] = '$(inherited) COCOAPODS=1 GRPC_CRONET_WITH_PACKET_COALESCING=1' end end diff --git a/templates/gRPC-Core.podspec.template b/templates/gRPC-Core.podspec.template index 1b97d18f163..0738b7221b5 100644 --- a/templates/gRPC-Core.podspec.template +++ b/templates/gRPC-Core.podspec.template @@ -161,7 +161,8 @@ s.subspec 'Cronet-Interface' do |ss| ss.header_mappings_dir = 'include/grpc' - ss.source_files = 'include/grpc/grpc_cronet.h' + ss.source_files = 'include/grpc/grpc_cronet.h', + 'src/core/ext/transport/cronet/transport/cronet_transport.h' end s.subspec 'Cronet-Implementation' do |ss| diff --git a/tools/run_tests/generated/sources_and_headers.json b/tools/run_tests/generated/sources_and_headers.json index 9bc82486d2b..7f594110197 100644 --- a/tools/run_tests/generated/sources_and_headers.json +++ b/tools/run_tests/generated/sources_and_headers.json @@ -7873,6 +7873,7 @@ "include/grpc/grpc_cronet.h", "include/grpc/grpc_security.h", "include/grpc/grpc_security_constants.h", + "src/core/ext/transport/cronet/transport/cronet_transport.h", "third_party/Cronet/bidirectional_stream_c.h" ], "is_filegroup": true, @@ -7884,7 +7885,8 @@ "include/grpc/grpc_security_constants.h", "src/core/ext/transport/cronet/client/secure/cronet_channel_create.c", "src/core/ext/transport/cronet/transport/cronet_api_dummy.c", - "src/core/ext/transport/cronet/transport/cronet_transport.c" + "src/core/ext/transport/cronet/transport/cronet_transport.c", + "src/core/ext/transport/cronet/transport/cronet_transport.h" ], "third_party": false, "type": "filegroup" From 1a5f90fa8a1a9c647e151575fa9651ae1715045f Mon Sep 17 00:00:00 2001 From: Muxi Yan Date: Wed, 1 Feb 2017 15:46:21 -0800 Subject: [PATCH 09/23] Fix and clang-format Cronet unit tests --- .../tests/CronetUnitTests/CronetUnitTests.m | 283 +++++++++--------- 1 file changed, 146 insertions(+), 137 deletions(-) diff --git a/src/objective-c/tests/CronetUnitTests/CronetUnitTests.m b/src/objective-c/tests/CronetUnitTests/CronetUnitTests.m index a50bf4728f8..9589926d8b5 100644 --- a/src/objective-c/tests/CronetUnitTests/CronetUnitTests.m +++ b/src/objective-c/tests/CronetUnitTests/CronetUnitTests.m @@ -32,13 +32,13 @@ */ #import -#import #import +#import #import -#import -#import #import +#import +#import #import "test/core/end2end/cq_verifier.h" #import "test/core/util/port.h" @@ -49,19 +49,19 @@ #import "src/core/lib/support/env.h" #import "src/core/lib/support/string.h" #import "src/core/lib/support/tmpfile.h" -#import "test/core/util/test_config.h" #import "test/core/end2end/data/ssl_test_data.h" +#import "test/core/util/test_config.h" #import static void drain_cq(grpc_completion_queue *cq) { grpc_event ev; do { - ev = grpc_completion_queue_next(cq, grpc_timeout_seconds_to_deadline(5), NULL); + ev = grpc_completion_queue_next(cq, grpc_timeout_seconds_to_deadline(5), + NULL); } while (ev.type != GRPC_QUEUE_SHUTDOWN); } - @interface CronetUnitTests : XCTestCase @end @@ -71,8 +71,8 @@ static void drain_cq(grpc_completion_queue *cq) { + (void)setUp { [super setUp]; -/*** FILE *roots_file; - size_t roots_size = strlen(test_root_cert);*/ + /*** FILE *roots_file; + size_t roots_size = strlen(test_root_cert);*/ char *argv[] = {"CoreCronetEnd2EndTests"}; grpc_test_init(1, argv); @@ -83,8 +83,8 @@ static void drain_cq(grpc_completion_queue *cq) { [Cronet setSslKeyLogFileName:@"Documents/key"]; [Cronet enableTestCertVerifierForTesting]; NSURL *url = [[[NSFileManager defaultManager] - URLsForDirectory:NSDocumentDirectory - inDomains:NSUserDomainMask] lastObject]; + URLsForDirectory:NSDocumentDirectory + inDomains:NSUserDomainMask] lastObject]; NSLog(@"Documents directory: %@", url); [Cronet start]; [Cronet startNetLogToFile:@"Documents/cronet_netlog.json" logBytes:YES]; @@ -104,28 +104,28 @@ void init_ssl(void) { OpenSSL_add_ssl_algorithms(); } -void cleanup_ssl(void) { - EVP_cleanup(); -} +void cleanup_ssl(void) { EVP_cleanup(); } int alpn_cb(SSL *ssl, const unsigned char **out, unsigned char *outlen, const unsigned char *in, unsigned int inlen, void *arg) { // Always select "h2" as the ALPN protocol to be used - *out = (const unsigned char*)"h2"; + *out = (const unsigned char *)"h2"; *outlen = 2; return SSL_TLSEXT_ERR_OK; } void init_ctx(SSL_CTX *ctx) { // Install server certificate - BIO *pem = BIO_new_mem_buf((void*)test_server1_cert, (int)strlen(test_server1_cert)); + BIO *pem = BIO_new_mem_buf((void *)test_server1_cert, + (int)strlen(test_server1_cert)); X509 *cert = PEM_read_bio_X509_AUX(pem, NULL, NULL, ""); SSL_CTX_use_certificate(ctx, cert); X509_free(cert); BIO_free(pem); // Install server private key - pem = BIO_new_mem_buf((void *)test_server1_key, (int)strlen(test_server1_key)); + pem = + BIO_new_mem_buf((void *)test_server1_key, (int)strlen(test_server1_key)); EVP_PKEY *key = PEM_read_bio_PrivateKey(pem, NULL, NULL, ""); SSL_CTX_use_PrivateKey(ctx, key); EVP_PKEY_free(key); @@ -139,29 +139,34 @@ void init_ctx(SSL_CTX *ctx) { } unsigned int parse_h2_length(const char *field) { - return ((unsigned int)(unsigned char)(field[0])) * 65536 + - ((unsigned int)(unsigned char)(field[1])) * 256 + - ((unsigned int)(unsigned char)(field[2])); + return ((unsigned int)(unsigned char)(field[0])) * 65536 + + ((unsigned int)(unsigned char)(field[1])) * 256 + + ((unsigned int)(unsigned char)(field[2])); } - (void)testInternalError { grpc_call *c; grpc_slice request_payload_slice = - grpc_slice_from_copied_string("hello world"); + grpc_slice_from_copied_string("hello world"); grpc_byte_buffer *request_payload = - grpc_raw_byte_buffer_create(&request_payload_slice, 1); + grpc_raw_byte_buffer_create(&request_payload_slice, 1); gpr_timespec deadline = grpc_timeout_seconds_to_deadline(5); - grpc_metadata meta_c[2] = { - {"key1", "val1", 4, 0, {{NULL, NULL, NULL, NULL}}}, - {"key2", "val2", 4, 0, {{NULL, NULL, NULL, NULL}}}}; + grpc_metadata meta_c[2] = {{grpc_slice_from_static_string("key1"), + grpc_slice_from_static_string("val1"), + 0, + {{NULL, NULL, NULL, NULL}}}, + {grpc_slice_from_static_string("key2"), + grpc_slice_from_static_string("val2"), + 0, + {{NULL, NULL, NULL, NULL}}}}; int port = grpc_pick_unused_port_or_die(); char *addr; gpr_join_host_port(&addr, "127.0.0.1", port); grpc_completion_queue *cq = grpc_completion_queue_create(NULL); stream_engine *cronetEngine = [Cronet getGlobalEngine]; - grpc_channel *client = grpc_cronet_secure_channel_create(cronetEngine, addr, - NULL, NULL); + grpc_channel *client = + grpc_cronet_secure_channel_create(cronetEngine, addr, NULL, NULL); cq_verifier *cqv = cq_verifier_create(cq); grpc_op ops[6]; @@ -173,12 +178,11 @@ unsigned int parse_h2_length(const char *field) { grpc_call_details call_details; grpc_status_code status; grpc_call_error error; - char *details = NULL; - size_t details_capacity = 0; + grpc_slice details; - c = grpc_channel_create_call( - client, NULL, GRPC_PROPAGATE_DEFAULTS, cq, "/foo", - NULL, deadline, NULL); + c = grpc_channel_create_call(client, NULL, GRPC_PROPAGATE_DEFAULTS, cq, + grpc_slice_from_static_string("/foo"), NULL, + deadline, NULL); GPR_ASSERT(c); grpc_metadata_array_init(&initial_metadata_recv); @@ -217,40 +221,40 @@ unsigned int parse_h2_length(const char *field) { op->data.recv_status_on_client.trailing_metadata = &trailing_metadata_recv; op->data.recv_status_on_client.status = &status; op->data.recv_status_on_client.status_details = &details; - op->data.recv_status_on_client.status_details_capacity = &details_capacity; op->flags = 0; op->reserved = NULL; op++; - error = grpc_call_start_batch(c, ops, (size_t)(op - ops), (void*)1, NULL); + error = grpc_call_start_batch(c, ops, (size_t)(op - ops), (void *)1, NULL); GPR_ASSERT(GRPC_CALL_OK == error); - dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{ - int sl = socket(AF_INET, SOCK_STREAM, 0); - GPR_ASSERT(sl >= 0); - - // Make and TCP endpoint to accept the connection - struct sockaddr_in s_addr; - memset(&s_addr, 0, sizeof(s_addr)); - s_addr.sin_family = AF_INET; - s_addr.sin_addr.s_addr = htonl(INADDR_ANY); - s_addr.sin_port = htons(port); - GPR_ASSERT(0 == bind(sl, (struct sockaddr*)&s_addr, sizeof(s_addr))); - GPR_ASSERT(0 == listen(sl, 5)); - int s = accept(sl, NULL, NULL); - GPR_ASSERT(s >= 0); - - // Close the connection after 1 second to trigger Cronet's on_failed() - sleep(1); - close(s); - close(sl); - }); - - CQ_EXPECT_COMPLETION(cqv, (void*)1, 1); + dispatch_async( + dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{ + int sl = socket(AF_INET, SOCK_STREAM, 0); + GPR_ASSERT(sl >= 0); + + // Make and TCP endpoint to accept the connection + struct sockaddr_in s_addr; + memset(&s_addr, 0, sizeof(s_addr)); + s_addr.sin_family = AF_INET; + s_addr.sin_addr.s_addr = htonl(INADDR_ANY); + s_addr.sin_port = htons(port); + GPR_ASSERT(0 == bind(sl, (struct sockaddr *)&s_addr, sizeof(s_addr))); + GPR_ASSERT(0 == listen(sl, 5)); + int s = accept(sl, NULL, NULL); + GPR_ASSERT(s >= 0); + + // Close the connection after 1 second to trigger Cronet's on_failed() + sleep(1); + close(s); + close(sl); + }); + + CQ_EXPECT_COMPLETION(cqv, (void *)1, 1); cq_verify(cqv); GPR_ASSERT(status == GRPC_STATUS_UNAVAILABLE); - gpr_free(details); + grpc_slice_unref(details); grpc_metadata_array_destroy(&initial_metadata_recv); grpc_metadata_array_destroy(&trailing_metadata_recv); grpc_metadata_array_destroy(&request_metadata_recv); @@ -262,7 +266,7 @@ unsigned int parse_h2_length(const char *field) { grpc_byte_buffer_destroy(request_payload); grpc_byte_buffer_destroy(response_payload_recv); - + grpc_channel_destroy(client); grpc_completion_queue_shutdown(cq); drain_cq(cq); @@ -273,17 +277,22 @@ unsigned int parse_h2_length(const char *field) { grpc_arg arg; arg.key = GRPC_ARG_USE_CRONET_PACKET_COALESCING; arg.type = GRPC_ARG_INTEGER; - arg.value.integer = use_coalescing ? 1:0; + arg.value.integer = use_coalescing ? 1 : 0; grpc_channel_args *args = grpc_channel_args_copy_and_add(NULL, &arg, 1); grpc_call *c; grpc_slice request_payload_slice = - grpc_slice_from_copied_string("hello world"); + grpc_slice_from_copied_string("hello world"); grpc_byte_buffer *request_payload = - grpc_raw_byte_buffer_create(&request_payload_slice, 1); - gpr_timespec deadline = GRPC_TIMEOUT_SECONDS_TO_DEADLINE(5); - grpc_metadata meta_c[2] = { - {"key1", "val1", 4, 0, {{NULL, NULL, NULL, NULL}}}, - {"key2", "val2", 4, 0, {{NULL, NULL, NULL, NULL}}}}; + grpc_raw_byte_buffer_create(&request_payload_slice, 1); + gpr_timespec deadline = grpc_timeout_seconds_to_deadline(5); + grpc_metadata meta_c[2] = {{grpc_slice_from_static_string("key1"), + grpc_slice_from_static_string("val1"), + 0, + {{NULL, NULL, NULL, NULL}}}, + {grpc_slice_from_static_string("key2"), + grpc_slice_from_static_string("val2"), + 0, + {{NULL, NULL, NULL, NULL}}}}; int port = grpc_pick_unused_port_or_die(); char *addr; @@ -303,12 +312,11 @@ unsigned int parse_h2_length(const char *field) { grpc_call_details call_details; grpc_status_code status; grpc_call_error error; - char *details = NULL; - size_t details_capacity = 0; + grpc_slice details; - c = grpc_channel_create_call( - client, NULL, GRPC_PROPAGATE_DEFAULTS, cq, "/foo", - NULL, deadline, NULL); + c = grpc_channel_create_call(client, NULL, GRPC_PROPAGATE_DEFAULTS, cq, + grpc_slice_from_static_string("/foo"), NULL, + deadline, NULL); GPR_ASSERT(c); grpc_metadata_array_init(&initial_metadata_recv); @@ -325,7 +333,7 @@ unsigned int parse_h2_length(const char *field) { op->reserved = NULL; op++; op->op = GRPC_OP_SEND_MESSAGE; - op->data.send_message = request_payload; + op->data.send_message.send_message = request_payload; op->flags = 0; op->reserved = NULL; op++; @@ -334,12 +342,12 @@ unsigned int parse_h2_length(const char *field) { op->reserved = NULL; op++; op->op = GRPC_OP_RECV_INITIAL_METADATA; - op->data.recv_initial_metadata = &initial_metadata_recv; + op->data.recv_initial_metadata.recv_initial_metadata = &initial_metadata_recv; op->flags = 0; op->reserved = NULL; op++; op->op = GRPC_OP_RECV_MESSAGE; - op->data.recv_message = &response_payload_recv; + op->data.recv_message.recv_message = &response_payload_recv; op->flags = 0; op->reserved = NULL; op++; @@ -347,79 +355,80 @@ unsigned int parse_h2_length(const char *field) { op->data.recv_status_on_client.trailing_metadata = &trailing_metadata_recv; op->data.recv_status_on_client.status = &status; op->data.recv_status_on_client.status_details = &details; - op->data.recv_status_on_client.status_details_capacity = &details_capacity; op->flags = 0; op->reserved = NULL; op++; - error = grpc_call_start_batch(c, ops, (size_t)(op - ops), (void*)1, NULL); + error = grpc_call_start_batch(c, ops, (size_t)(op - ops), (void *)1, NULL); GPR_ASSERT(GRPC_CALL_OK == error); - dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{ - int sl = socket(AF_INET, SOCK_STREAM, 0); - GPR_ASSERT(sl >= 0); - struct sockaddr_in s_addr; - memset(&s_addr, 0, sizeof(s_addr)); - s_addr.sin_family = AF_INET; - s_addr.sin_addr.s_addr = htonl(INADDR_ANY); - s_addr.sin_port = htons(port); - GPR_ASSERT(0 == bind(sl, (struct sockaddr*)&s_addr, sizeof(s_addr))); - GPR_ASSERT(0 == listen(sl, 5)); - int s = accept(sl, NULL, NULL); - GPR_ASSERT(s >= 0); - struct timeval tv; - tv.tv_sec = 2; - tv.tv_usec = 0; - setsockopt(s, SOL_SOCKET, SO_RCVTIMEO, &tv, sizeof(tv)); - - // Make an TLS endpoint to receive Cronet's transmission - SSL_CTX *ctx = SSL_CTX_new(TLSv1_2_server_method()); - init_ctx(ctx); - SSL *ssl = SSL_new(ctx); - SSL_set_fd(ssl, s); - SSL_accept(ssl); - - const char magic[] = "PRI * HTTP/2.0\r\n\r\nSM\r\n\r\n"; - - char buf[4096]; - long len; - bool coalesced = false; - while ((len = SSL_read(ssl, buf, sizeof(buf))) > 0) { - gpr_log(GPR_DEBUG, "Read len: %ld", len); - - // Analyze the HTTP/2 frames in the same TLS PDU to identify if - // coalescing is successful - unsigned int p = 0; - while (p < len) { - if (len - p >= 24 && 0 == memcmp(&buf[p], magic, 24)) { - p += 24; - continue; + dispatch_async( + dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{ + int sl = socket(AF_INET, SOCK_STREAM, 0); + GPR_ASSERT(sl >= 0); + struct sockaddr_in s_addr; + memset(&s_addr, 0, sizeof(s_addr)); + s_addr.sin_family = AF_INET; + s_addr.sin_addr.s_addr = htonl(INADDR_ANY); + s_addr.sin_port = htons(port); + GPR_ASSERT(0 == bind(sl, (struct sockaddr *)&s_addr, sizeof(s_addr))); + GPR_ASSERT(0 == listen(sl, 5)); + int s = accept(sl, NULL, NULL); + GPR_ASSERT(s >= 0); + struct timeval tv; + tv.tv_sec = 2; + tv.tv_usec = 0; + setsockopt(s, SOL_SOCKET, SO_RCVTIMEO, &tv, sizeof(tv)); + + // Make an TLS endpoint to receive Cronet's transmission + SSL_CTX *ctx = SSL_CTX_new(TLSv1_2_server_method()); + init_ctx(ctx); + SSL *ssl = SSL_new(ctx); + SSL_set_fd(ssl, s); + SSL_accept(ssl); + + const char magic[] = "PRI * HTTP/2.0\r\n\r\nSM\r\n\r\n"; + + char buf[4096]; + long len; + bool coalesced = false; + while ((len = SSL_read(ssl, buf, sizeof(buf))) > 0) { + gpr_log(GPR_DEBUG, "Read len: %ld", len); + + // Analyze the HTTP/2 frames in the same TLS PDU to identify if + // coalescing is successful + unsigned int p = 0; + while (p < len) { + if (len - p >= 24 && 0 == memcmp(&buf[p], magic, 24)) { + p += 24; + continue; + } + + if (buf[p + 3] == 0 && // Type is DATA + parse_h2_length(&buf[p]) == 0x10 && // Length is correct + (buf[p + 4] & 1) != 0 && // EOS bit is set + 0 == memcmp("hello world", &buf[p + 14], + 11)) { // Message is correct + coalesced = true; + break; + } + p += (parse_h2_length(&buf[p]) + 9); + } + if (coalesced) { + break; + } } - if (buf[p+3] == 0 && // Type is DATA - parse_h2_length(&buf[p]) == 0x10 && // Length is correct - (buf[p+4] & 1) != 0 && // EOS bit is set - 0 == memcmp("hello world", &buf[p+14], 11)) { // Message is correct - coalesced = true; - break; - } - p += (parse_h2_length(&buf[p]) + 9); - } - if (coalesced) { - break; - } - } - - XCTAssert(coalesced == use_coalescing); - SSL_free(ssl); - SSL_CTX_free(ctx); - close(s); - close(sl); - }); - - CQ_EXPECT_COMPLETION(cqv, (void*)1, 1); + XCTAssert(coalesced == use_coalescing); + SSL_free(ssl); + SSL_CTX_free(ctx); + close(s); + close(sl); + }); + + CQ_EXPECT_COMPLETION(cqv, (void *)1, 1); cq_verify(cqv); - gpr_free(details); + grpc_slice_unref(details); grpc_metadata_array_destroy(&initial_metadata_recv); grpc_metadata_array_destroy(&trailing_metadata_recv); grpc_metadata_array_destroy(&request_metadata_recv); @@ -431,7 +440,7 @@ unsigned int parse_h2_length(const char *field) { grpc_byte_buffer_destroy(request_payload); grpc_byte_buffer_destroy(response_payload_recv); - + grpc_channel_destroy(client); grpc_completion_queue_shutdown(cq); drain_cq(cq); From 4c94e52f003b9beb999b2a9b73386f4746f55ffc Mon Sep 17 00:00:00 2001 From: Muxi Yan Date: Wed, 1 Feb 2017 15:46:39 -0800 Subject: [PATCH 10/23] bug fix for creating Cronet transport context --- .../cronet/transport/cronet_transport.c | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/core/ext/transport/cronet/transport/cronet_transport.c b/src/core/ext/transport/cronet/transport/cronet_transport.c index 1be6550bd3a..fac0a7a234f 100644 --- a/src/core/ext/transport/cronet/transport/cronet_transport.c +++ b/src/core/ext/transport/cronet/transport/cronet_transport.c @@ -1287,13 +1287,15 @@ grpc_transport *grpc_create_cronet_transport(void *engine, const char *target, strcpy(ct->host, target); ct->use_packet_coalescing = true; - for (size_t i = 0; i < args->num_args; i++) { - if (0 == strcmp(args->args[i].key, GRPC_ARG_USE_CRONET_PACKET_COALESCING)) { - if (args->args[i].type != GRPC_ARG_INTEGER) { - gpr_log(GPR_ERROR, "%s ignored: it must be an integer", - GRPC_ARG_USE_CRONET_PACKET_COALESCING); - } else { - ct->use_packet_coalescing = (args->args[i].value.integer != 0); + if (args) { + for (size_t i = 0; i < args->num_args; i++) { + if (0 == strcmp(args->args[i].key, GRPC_ARG_USE_CRONET_PACKET_COALESCING)) { + if (args->args[i].type != GRPC_ARG_INTEGER) { + gpr_log(GPR_ERROR, "%s ignored: it must be an integer", + GRPC_ARG_USE_CRONET_PACKET_COALESCING); + } else { + ct->use_packet_coalescing = (args->args[i].value.integer != 0); + } } } } From 2ee9751cbbe57d2b4bfece8926d8b3112388c4eb Mon Sep 17 00:00:00 2001 From: Muxi Yan Date: Wed, 1 Feb 2017 16:00:50 -0800 Subject: [PATCH 11/23] clang-format --- .../cronet/transport/cronet_transport.c | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/src/core/ext/transport/cronet/transport/cronet_transport.c b/src/core/ext/transport/cronet/transport/cronet_transport.c index fac0a7a234f..01a03533daf 100644 --- a/src/core/ext/transport/cronet/transport/cronet_transport.c +++ b/src/core/ext/transport/cronet/transport/cronet_transport.c @@ -425,8 +425,8 @@ static void on_stream_ready(bidirectional_stream *stream) { gpr_free(s->header_array.headers); s->header_array.headers = NULL; } -/* Send the initial metadata on wire if there is no SEND_MESSAGE or - * SEND_TRAILING_METADATA ops pending */ + /* Send the initial metadata on wire if there is no SEND_MESSAGE or + * SEND_TRAILING_METADATA ops pending */ if (t->use_packet_coalescing) { if (s->state.flush_cronet_when_ready) { CRONET_LOG(GPR_DEBUG, "cronet_bidirectional_stream_flush (%p)", s->cbs); @@ -868,8 +868,8 @@ static enum e_op_result execute_stream_op(grpc_exec_ctx *exec_ctx, * on_failed */ GPR_ASSERT(s->cbs == NULL); GPR_ASSERT(!stream_state->state_op_done[OP_SEND_INITIAL_METADATA]); - s->cbs = bidirectional_stream_create(t->engine, s->curr_gs, - &cronet_callbacks); + s->cbs = + bidirectional_stream_create(t->engine, s->curr_gs, &cronet_callbacks); CRONET_LOG(GPR_DEBUG, "%p = bidirectional_stream_create()", s->cbs); if (t->use_packet_coalescing) { bidirectional_stream_disable_auto_flush(s->cbs, true); @@ -957,8 +957,7 @@ static enum e_op_result execute_stream_op(grpc_exec_ctx *exec_ctx, result = NO_ACTION_POSSIBLE; CRONET_LOG(GPR_DEBUG, "Stream is either cancelled or failed."); } else { - CRONET_LOG(GPR_DEBUG, "bidirectional_stream_write (%p, 0)", - s->cbs); + CRONET_LOG(GPR_DEBUG, "bidirectional_stream_write (%p, 0)", s->cbs); stream_state->state_callback_received[OP_SEND_MESSAGE] = false; bidirectional_stream_write(s->cbs, "", 0, true); if (t->use_packet_coalescing) { @@ -969,8 +968,8 @@ static enum e_op_result execute_stream_op(grpc_exec_ctx *exec_ctx, } stream_state->state_op_done[OP_SEND_TRAILING_METADATA] = true; } else if (stream_op->recv_initial_metadata && - op_can_be_run(stream_op, s, &oas->state, - OP_RECV_INITIAL_METADATA)) { + op_can_be_run(stream_op, s, &oas->state, + OP_RECV_INITIAL_METADATA)) { CRONET_LOG(GPR_DEBUG, "running: %p OP_RECV_INITIAL_METADATA", oas); if (stream_state->state_op_done[OP_CANCEL_ERROR]) { grpc_closure_sched(exec_ctx, stream_op->recv_initial_metadata_ready, @@ -980,10 +979,10 @@ static enum e_op_result execute_stream_op(grpc_exec_ctx *exec_ctx, GRPC_ERROR_NONE); } else { grpc_chttp2_incoming_metadata_buffer_publish( - exec_ctx, &oas->s->state.rs.initial_metadata, - stream_op->recv_initial_metadata); + exec_ctx, &oas->s->state.rs.initial_metadata, + stream_op->recv_initial_metadata); grpc_closure_sched(exec_ctx, stream_op->recv_initial_metadata_ready, - GRPC_ERROR_NONE); + GRPC_ERROR_NONE); } stream_state->state_op_done[OP_RECV_INITIAL_METADATA] = true; result = ACTION_TAKEN_NO_CALLBACK; @@ -1052,7 +1051,7 @@ static enum e_op_result execute_stream_op(grpc_exec_ctx *exec_ctx, stream_state->state_op_done[OP_READ_REQ_MADE] = true; /* Indicates that at least one read request has been made */ bidirectional_stream_read(s->cbs, stream_state->rs.read_buffer, - stream_state->rs.remaining_bytes); + stream_state->rs.remaining_bytes); result = ACTION_TAKEN_NO_CALLBACK; } } else if (stream_state->rs.remaining_bytes == 0) { @@ -1289,7 +1288,8 @@ grpc_transport *grpc_create_cronet_transport(void *engine, const char *target, ct->use_packet_coalescing = true; if (args) { for (size_t i = 0; i < args->num_args; i++) { - if (0 == strcmp(args->args[i].key, GRPC_ARG_USE_CRONET_PACKET_COALESCING)) { + if (0 == + strcmp(args->args[i].key, GRPC_ARG_USE_CRONET_PACKET_COALESCING)) { if (args->args[i].type != GRPC_ARG_INTEGER) { gpr_log(GPR_ERROR, "%s ignored: it must be an integer", GRPC_ARG_USE_CRONET_PACKET_COALESCING); From d5923578c0c43a5716dd78c6467ccdd5199feff2 Mon Sep 17 00:00:00 2001 From: Muxi Yan Date: Mon, 6 Feb 2017 11:25:59 -0800 Subject: [PATCH 12/23] nit fixes --- gRPC-Core.podspec | 1 + .../tests/CronetUnitTests/CronetUnitTests.m | 10 +++++----- templates/gRPC-Core.podspec.template | 1 + 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/gRPC-Core.podspec b/gRPC-Core.podspec index 840d8132c66..0ef61442a4b 100644 --- a/gRPC-Core.podspec +++ b/gRPC-Core.podspec @@ -883,6 +883,7 @@ Pod::Spec.new do |s| 'test/core/end2end/end2end_test_utils.c', 'test/core/end2end/tests/*.{c,h}', 'test/core/end2end/data/*.{c,h}', + 'test/core/util/debugger_macros.c', 'test/core/util/test_config.{c,h}', 'test/core/util/port.h', 'test/core/util/port_posix.c', diff --git a/src/objective-c/tests/CronetUnitTests/CronetUnitTests.m b/src/objective-c/tests/CronetUnitTests/CronetUnitTests.m index 9589926d8b5..408952a091b 100644 --- a/src/objective-c/tests/CronetUnitTests/CronetUnitTests.m +++ b/src/objective-c/tests/CronetUnitTests/CronetUnitTests.m @@ -273,7 +273,7 @@ unsigned int parse_h2_length(const char *field) { grpc_completion_queue_destroy(cq); } -- (void)PacketCoalescing:(bool)use_coalescing { +- (void)packetCoalescing:(BOOL)use_coalescing { grpc_arg arg; arg.key = GRPC_ARG_USE_CRONET_PACKET_COALESCING; arg.type = GRPC_ARG_INTEGER; @@ -390,7 +390,7 @@ unsigned int parse_h2_length(const char *field) { char buf[4096]; long len; - bool coalesced = false; + BOOL coalesced = NO; while ((len = SSL_read(ssl, buf, sizeof(buf))) > 0) { gpr_log(GPR_DEBUG, "Read len: %ld", len); @@ -408,7 +408,7 @@ unsigned int parse_h2_length(const char *field) { (buf[p + 4] & 1) != 0 && // EOS bit is set 0 == memcmp("hello world", &buf[p + 14], 11)) { // Message is correct - coalesced = true; + coalesced = YES; break; } p += (parse_h2_length(&buf[p]) + 9); @@ -448,8 +448,8 @@ unsigned int parse_h2_length(const char *field) { } - (void)testPacketCoalescing { - [self PacketCoalescing:true]; - [self PacketCoalescing:false]; + [self packetCoalescing:YES]; + [self packetCoalescing:NO]; } @end diff --git a/templates/gRPC-Core.podspec.template b/templates/gRPC-Core.podspec.template index 0738b7221b5..0c1ff64a002 100644 --- a/templates/gRPC-Core.podspec.template +++ b/templates/gRPC-Core.podspec.template @@ -179,6 +179,7 @@ 'test/core/end2end/end2end_test_utils.c', 'test/core/end2end/tests/*.{c,h}', 'test/core/end2end/data/*.{c,h}', + 'test/core/util/debugger_macros.c', 'test/core/util/test_config.{c,h}', 'test/core/util/port.h', 'test/core/util/port_posix.c', From e1cf1b691d3e82742214c8772a71e79e3209c2ce Mon Sep 17 00:00:00 2001 From: Muxi Yan Date: Mon, 6 Feb 2017 13:31:05 -0800 Subject: [PATCH 13/23] nit fix 2 --- src/objective-c/tests/CronetUnitTests/CronetUnitTests.m | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/objective-c/tests/CronetUnitTests/CronetUnitTests.m b/src/objective-c/tests/CronetUnitTests/CronetUnitTests.m index 408952a091b..b6237c950bf 100644 --- a/src/objective-c/tests/CronetUnitTests/CronetUnitTests.m +++ b/src/objective-c/tests/CronetUnitTests/CronetUnitTests.m @@ -273,11 +273,11 @@ unsigned int parse_h2_length(const char *field) { grpc_completion_queue_destroy(cq); } -- (void)packetCoalescing:(BOOL)use_coalescing { +- (void)packetCoalescing:(BOOL)useCoalescing { grpc_arg arg; arg.key = GRPC_ARG_USE_CRONET_PACKET_COALESCING; arg.type = GRPC_ARG_INTEGER; - arg.value.integer = use_coalescing ? 1 : 0; + arg.value.integer = useCoalescing ? 1 : 0; grpc_channel_args *args = grpc_channel_args_copy_and_add(NULL, &arg, 1); grpc_call *c; grpc_slice request_payload_slice = @@ -418,7 +418,7 @@ unsigned int parse_h2_length(const char *field) { } } - XCTAssert(coalesced == use_coalescing); + XCTAssert(coalesced == useCoalescing); SSL_free(ssl); SSL_CTX_free(ctx); close(s); From 1a967c31c3a7fbd2d6d5cb6c1e575fdb2d6bce9d Mon Sep 17 00:00:00 2001 From: Mehrdad Afshari Date: Mon, 13 Feb 2017 11:38:17 -0800 Subject: [PATCH 14/23] Added Pylint to sanity tests Pylint is only enabled for "grpcio/grpc" package, and various specific checks that currently fail are disabled, each with a respective TODO item in the .pylintrc file. --- .pylintrc | 38 +++++++++++++++++++ tools/distrib/pylint_code.sh | 48 ++++++++++++++++++++++++ tools/run_tests/sanity/sanity_tests.yaml | 1 + 3 files changed, 87 insertions(+) create mode 100644 .pylintrc create mode 100755 tools/distrib/pylint_code.sh diff --git a/.pylintrc b/.pylintrc new file mode 100644 index 00000000000..a4f757f9bdd --- /dev/null +++ b/.pylintrc @@ -0,0 +1,38 @@ +[MESSAGES CONTROL] + +#TODO: Enable missing-docstring +#TODO: Enable too-few-public-methods +#TODO: Enable too-many-arguments +#TODO: Enable no-init +#TODO: Enable duplicate-code +#TODO: Enable invalid-name +#TODO: Enable suppressed-message +#TODO: Enable locally-disabled +#TODO: Enable protected-access +#TODO: Enable no-name-in-module +#TODO: Enable unused-argument +#TODO: Enable fixme +#TODO: Enable wrong-import-order +#TODO: Enable no-value-for-parameter +#TODO: Enable cyclic-import +#TODO: Enable unused-variable +#TODO: Enable redefined-outer-name +#TODO: Enable unused-import +#TODO: Enable too-many-instance-attributes +#TODO: Enable broad-except +#TODO: Enable too-many-locals +#TODO: Enable too-many-lines +#TODO: Enable redefined-variable-type +#TODO: Enable next-method-called +#TODO: Enable import-error +#TODO: Enable useless-else-on-loop +#TODO: Enable too-many-return-statements +#TODO: Enable too-many-nested-blocks +#TODO: Enable super-init-not-called +#TODO: Enable simplifiable-if-statement +#TODO: Enable no-self-use +#TODO: Enable no-member +#TODO: Enable logging-format-interpolation +#TODO: Enable dangerous-default-value + +disable=missing-docstring,too-few-public-methods,too-many-arguments,no-init,duplicate-code,invalid-name,suppressed-message,locally-disabled,protected-access,no-name-in-module,unused-argument,fixme,wrong-import-order,no-value-for-parameter,cyclic-import,unused-variable,redefined-outer-name,unused-import,too-many-instance-attributes,broad-except,too-many-locals,too-many-lines,redefined-variable-type,next-method-called,import-error,useless-else-on-loop,too-many-return-statements,too-many-nested-blocks,super-init-not-called,simplifiable-if-statement,no-self-use,no-member,logging-format-interpolation,dangerous-default-value diff --git a/tools/distrib/pylint_code.sh b/tools/distrib/pylint_code.sh new file mode 100755 index 00000000000..6369e605d53 --- /dev/null +++ b/tools/distrib/pylint_code.sh @@ -0,0 +1,48 @@ +#!/bin/bash +# 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. + +set -ex + +# change to root directory +cd $(dirname $0)/../.. + +DIRS=src/python/grpcio/grpc + +VIRTUALENV=python_pylint_venv + +virtualenv $VIRTUALENV +PYTHON=`realpath $VIRTUALENV/bin/python` +$PYTHON -m pip install pylint==1.6.5 + +for dir in $DIRS; do + $PYTHON -m pylint --rcfile=.pylintrc -rn $dir || exit $? +done + +exit 0 diff --git a/tools/run_tests/sanity/sanity_tests.yaml b/tools/run_tests/sanity/sanity_tests.yaml index ce41da802d0..445f53e5558 100644 --- a/tools/run_tests/sanity/sanity_tests.yaml +++ b/tools/run_tests/sanity/sanity_tests.yaml @@ -12,6 +12,7 @@ - script: tools/distrib/check_trailing_newlines.sh - script: tools/distrib/check_nanopb_output.sh - script: tools/distrib/check_include_guards.py +- script: tools/distrib/pylint_code.sh - script: tools/distrib/yapf_code.sh - script: tools/distrib/python/check_grpcio_tools.py From a78871e4b33929b206188291bb989c034f3c1eab Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Mon, 13 Feb 2017 14:48:30 -0800 Subject: [PATCH 15/23] Fix command line --- tools/run_tests/run_microbenchmark.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/run_tests/run_microbenchmark.py b/tools/run_tests/run_microbenchmark.py index 2a1b0246958..0d56e4c3553 100755 --- a/tools/run_tests/run_microbenchmark.py +++ b/tools/run_tests/run_microbenchmark.py @@ -134,7 +134,7 @@ def collect_perf(bm_name, args): '--benchmark_filter=^%s$' % line, '--benchmark_min_time=20']) with open('bm.perf', 'w') as f: - f.write(subprocess.check_output(['sudo', 'perf', 'script'])) + f.write(subprocess.check_output(['sudo', 'perf', 'script', '-i', 'perf.data'])) with open('bm.folded', 'w') as f: f.write(subprocess.check_output([ '%s/stackcollapse-perf.pl' % flamegraph_dir, 'bm.perf'])) From a48e6904985754c4364d95fa51675a11ce670102 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Mon, 13 Feb 2017 19:31:49 -0800 Subject: [PATCH 16/23] Use shell to redirect a few big files --- tools/run_tests/run_microbenchmark.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/tools/run_tests/run_microbenchmark.py b/tools/run_tests/run_microbenchmark.py index 0d56e4c3553..a9a563c1bec 100755 --- a/tools/run_tests/run_microbenchmark.py +++ b/tools/run_tests/run_microbenchmark.py @@ -133,11 +133,9 @@ def collect_perf(bm_name, args): 'bins/mutrace/%s' % bm_name, '--benchmark_filter=^%s$' % line, '--benchmark_min_time=20']) - with open('bm.perf', 'w') as f: - f.write(subprocess.check_output(['sudo', 'perf', 'script', '-i', 'perf.data'])) - with open('bm.folded', 'w') as f: - f.write(subprocess.check_output([ - '%s/stackcollapse-perf.pl' % flamegraph_dir, 'bm.perf'])) + subprocess.check_call(['sudo', 'perf', 'script', '-i', 'perf.data', '>', 'bm.perf'], shell=True) + subprocess.check_call([ + '%s/stackcollapse-perf.pl' % flamegraph_dir, 'bm.perf', '>', 'bm.folded'], shell=True) link(line, '%s.svg' % fnize(line)) with open('reports/%s.svg' % fnize(line), 'w') as f: f.write(subprocess.check_output([ From 9f3ba09d3cb4dd8c122a34615d6e3779047eea6f Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Tue, 14 Feb 2017 08:16:09 -0800 Subject: [PATCH 17/23] Use pre-existing script for flamegraph generation --- tools/run_tests/run_microbenchmark.py | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/tools/run_tests/run_microbenchmark.py b/tools/run_tests/run_microbenchmark.py index a9a563c1bec..ea87b6cd986 100755 --- a/tools/run_tests/run_microbenchmark.py +++ b/tools/run_tests/run_microbenchmark.py @@ -128,18 +128,19 @@ def collect_perf(bm_name, args): 'CONFIG=mutrace', '-j', '%d' % multiprocessing.cpu_count()]) for line in subprocess.check_output(['bins/mutrace/%s' % bm_name, '--benchmark_list_tests']).splitlines(): - subprocess.check_call(['sudo', 'perf', 'record', '-o', 'perf.data', + subprocess.check_call(['perf', 'record', '-o', '%s-perf.data' % fnize(line), '-g', '-c', '1000', 'bins/mutrace/%s' % bm_name, '--benchmark_filter=^%s$' % line, '--benchmark_min_time=20']) - subprocess.check_call(['sudo', 'perf', 'script', '-i', 'perf.data', '>', 'bm.perf'], shell=True) - subprocess.check_call([ - '%s/stackcollapse-perf.pl' % flamegraph_dir, 'bm.perf', '>', 'bm.folded'], shell=True) - link(line, '%s.svg' % fnize(line)) - with open('reports/%s.svg' % fnize(line), 'w') as f: - f.write(subprocess.check_output([ - '%s/flamegraph.pl' % flamegraph_dir, 'bm.folded'])) + env = os.environ.copy() + env.update({ + 'PERF_BASE_NAME': fnize(line), + 'OUTPUT_DIR': 'reports', + 'OUTPUT_FILENAME': fnize(line), + }) + subprocess.check_call(['tools/run_tests/performance/process_local_perf_flamegraphs.sh'], + env=env) def collect_summary(bm_name, args): heading('Summary: %s' % bm_name) From a74ccafc93022d40210d70dbec392ceddfa3e612 Mon Sep 17 00:00:00 2001 From: Muxi Yan Date: Tue, 14 Feb 2017 10:24:38 -0800 Subject: [PATCH 18/23] Add new dummy Cronet API --- .../transport/cronet/transport/cronet_api_dummy.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/core/ext/transport/cronet/transport/cronet_api_dummy.c b/src/core/ext/transport/cronet/transport/cronet_api_dummy.c index da6c0b4fbcc..0dc6a5152fd 100644 --- a/src/core/ext/transport/cronet/transport/cronet_api_dummy.c +++ b/src/core/ext/transport/cronet/transport/cronet_api_dummy.c @@ -80,4 +80,16 @@ void bidirectional_stream_cancel(bidirectional_stream* stream) { GPR_ASSERT(0); } +void bidirectional_stream_disable_auto_flush(bidirectional_stream* stream, + bool disable_auto_flush) { + GPR_ASSERT(0); +} + +void bidirectional_stream_delay_request_headers_until_flush( + bidirectional_stream* stream, bool delay_headers_until_flush) { + GPR_ASSERT(0); +} + +void bidirectional_stream_flush(bidirectional_stream* stream) { GPR_ASSERT(0); } + #endif /* GRPC_COMPILE_WITH_CRONET */ From 716f953532a92f3a159d26a78fdb397d0838b20e Mon Sep 17 00:00:00 2001 From: David Garcia Quintas Date: Tue, 14 Feb 2017 10:54:20 -0800 Subject: [PATCH 19/23] Update protobuf version used for nanopb compiler --- tools/codegen/core/gen_nano_proto.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/codegen/core/gen_nano_proto.sh b/tools/codegen/core/gen_nano_proto.sh index 99e49814b83..8600573e1cb 100755 --- a/tools/codegen/core/gen_nano_proto.sh +++ b/tools/codegen/core/gen_nano_proto.sh @@ -83,7 +83,7 @@ popd # this should be the same version as the submodule we compile against # ideally we'd update this as a template to ensure that -pip install protobuf==3.0.0 +pip install protobuf==3.2.0 pushd "$(dirname $INPUT_PROTO)" > /dev/null From 4d352bec3dfaee160268de94e27553f94cfa7ae7 Mon Sep 17 00:00:00 2001 From: Muxi Yan Date: Tue, 14 Feb 2017 13:04:08 -0800 Subject: [PATCH 20/23] remove some temporary comments --- src/objective-c/tests/CronetUnitTests/CronetUnitTests.m | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/objective-c/tests/CronetUnitTests/CronetUnitTests.m b/src/objective-c/tests/CronetUnitTests/CronetUnitTests.m index b6237c950bf..1833a5be495 100644 --- a/src/objective-c/tests/CronetUnitTests/CronetUnitTests.m +++ b/src/objective-c/tests/CronetUnitTests/CronetUnitTests.m @@ -71,9 +71,6 @@ static void drain_cq(grpc_completion_queue *cq) { + (void)setUp { [super setUp]; - /*** FILE *roots_file; - size_t roots_size = strlen(test_root_cert);*/ - char *argv[] = {"CoreCronetEnd2EndTests"}; grpc_test_init(1, argv); From b67243fd3dffadaabb661143bacf34b80560e066 Mon Sep 17 00:00:00 2001 From: Muxi Yan Date: Tue, 14 Feb 2017 14:12:33 -0800 Subject: [PATCH 21/23] Polish test --- src/objective-c/tests/CronetUnitTests/CronetUnitTests.m | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/objective-c/tests/CronetUnitTests/CronetUnitTests.m b/src/objective-c/tests/CronetUnitTests/CronetUnitTests.m index 1833a5be495..5e3c59f8b34 100644 --- a/src/objective-c/tests/CronetUnitTests/CronetUnitTests.m +++ b/src/objective-c/tests/CronetUnitTests/CronetUnitTests.m @@ -358,6 +358,8 @@ unsigned int parse_h2_length(const char *field) { error = grpc_call_start_batch(c, ops, (size_t)(op - ops), (void *)1, NULL); GPR_ASSERT(GRPC_CALL_OK == error); + __weak XCTestExpectation *expectation = [self expectationWithDescription:@"Coalescing"]; + dispatch_async( dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{ int sl = socket(AF_INET, SOCK_STREAM, 0); @@ -420,6 +422,7 @@ unsigned int parse_h2_length(const char *field) { SSL_CTX_free(ctx); close(s); close(sl); + [expectation fulfill]; }); CQ_EXPECT_COMPLETION(cqv, (void *)1, 1); @@ -442,6 +445,8 @@ unsigned int parse_h2_length(const char *field) { grpc_completion_queue_shutdown(cq); drain_cq(cq); grpc_completion_queue_destroy(cq); + + [self waitForExpectationsWithTimeout:4 handler:nil]; } - (void)testPacketCoalescing { From 8da82bb9cf47442774c5618b856398eddf02f713 Mon Sep 17 00:00:00 2001 From: Muxi Yan Date: Tue, 14 Feb 2017 14:20:13 -0800 Subject: [PATCH 22/23] Add CronetUnitTests into Jenkins tests --- .../xcshareddata/xcschemes/AllTests.xcscheme | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/src/objective-c/tests/Tests.xcodeproj/xcshareddata/xcschemes/AllTests.xcscheme b/src/objective-c/tests/Tests.xcodeproj/xcshareddata/xcschemes/AllTests.xcscheme index 5524a27ffde..49dc3faa3d0 100644 --- a/src/objective-c/tests/Tests.xcodeproj/xcshareddata/xcschemes/AllTests.xcscheme +++ b/src/objective-c/tests/Tests.xcodeproj/xcshareddata/xcschemes/AllTests.xcscheme @@ -59,6 +59,16 @@ ReferencedContainer = "container:Tests.xcodeproj"> + + + + + + + + From bd5a4d263d841c8bed87730173d31c5febcc6906 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Tue, 14 Feb 2017 16:45:47 -0800 Subject: [PATCH 23/23] Cleanup files during processing --- tools/run_tests/run_microbenchmark.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tools/run_tests/run_microbenchmark.py b/tools/run_tests/run_microbenchmark.py index ea87b6cd986..6bba29d74ba 100755 --- a/tools/run_tests/run_microbenchmark.py +++ b/tools/run_tests/run_microbenchmark.py @@ -132,7 +132,7 @@ def collect_perf(bm_name, args): '-g', '-c', '1000', 'bins/mutrace/%s' % bm_name, '--benchmark_filter=^%s$' % line, - '--benchmark_min_time=20']) + '--benchmark_min_time=10']) env = os.environ.copy() env.update({ 'PERF_BASE_NAME': fnize(line), @@ -141,6 +141,8 @@ def collect_perf(bm_name, args): }) subprocess.check_call(['tools/run_tests/performance/process_local_perf_flamegraphs.sh'], env=env) + subprocess.check_call(['rm', '%s-perf.data' % fnize(line)]) + subprocess.check_call(['rm', '%s-out.perf' % fnize(line)]) def collect_summary(bm_name, args): heading('Summary: %s' % bm_name)