From 7c9a154803c0293a8c0b51bbec65dc00a950989e Mon Sep 17 00:00:00 2001 From: "Nicolas \"Pixel\" Noble" Date: Sat, 26 Mar 2016 01:33:34 +0100 Subject: [PATCH 01/38] Sanitize mallocs and frees. --- src/core/iomgr/tcp_server_posix.c | 2 +- src/core/iomgr/tcp_server_windows.c | 2 +- src/core/iomgr/udp_server.c | 2 +- src/core/profiling/basic_timers.c | 4 +- src/core/support/thd_posix.c | 1 + src/core/surface/byte_buffer.c | 6 +- src/core/tsi/fake_transport_security.c | 12 ++-- src/core/tsi/ssl_transport_security.c | 65 +++++++++---------- src/core/tsi/transport_security.c | 16 ++--- test/core/iomgr/endpoint_tests.c | 4 +- test/core/iomgr/tcp_posix_test.c | 4 +- .../network_benchmarks/low_level_ping_pong.c | 19 +++--- test/core/support/alloc_test.c | 4 +- 13 files changed, 65 insertions(+), 76 deletions(-) diff --git a/src/core/iomgr/tcp_server_posix.c b/src/core/iomgr/tcp_server_posix.c index 74ee68a6f11..413f7867be2 100644 --- a/src/core/iomgr/tcp_server_posix.c +++ b/src/core/iomgr/tcp_server_posix.c @@ -451,7 +451,7 @@ int grpc_tcp_server_add_port(grpc_tcp_server *s, const void *addr, &sockname_len)) { port = grpc_sockaddr_get_port((struct sockaddr *)&sockname_temp); if (port > 0) { - allocated_addr = malloc(addr_len); + allocated_addr = gpr_malloc(addr_len); memcpy(allocated_addr, addr, addr_len); grpc_sockaddr_set_port(allocated_addr, port); addr = allocated_addr; diff --git a/src/core/iomgr/tcp_server_windows.c b/src/core/iomgr/tcp_server_windows.c index a4abc5b9743..f9def9a19bb 100644 --- a/src/core/iomgr/tcp_server_windows.c +++ b/src/core/iomgr/tcp_server_windows.c @@ -467,7 +467,7 @@ int grpc_tcp_server_add_port(grpc_tcp_server *s, const void *addr, (struct sockaddr *)&sockname_temp, &sockname_len)) { port = grpc_sockaddr_get_port((struct sockaddr *)&sockname_temp); if (port > 0) { - allocated_addr = malloc(addr_len); + allocated_addr = gpr_malloc(addr_len); memcpy(allocated_addr, addr, addr_len); grpc_sockaddr_set_port(allocated_addr, port); addr = allocated_addr; diff --git a/src/core/iomgr/udp_server.c b/src/core/iomgr/udp_server.c index 174159170f3..15f3802dddc 100644 --- a/src/core/iomgr/udp_server.c +++ b/src/core/iomgr/udp_server.c @@ -337,7 +337,7 @@ int grpc_udp_server_add_port(grpc_udp_server *s, const void *addr, &sockname_len)) { port = grpc_sockaddr_get_port((struct sockaddr *)&sockname_temp); if (port > 0) { - allocated_addr = malloc(addr_len); + allocated_addr = gpr_malloc(addr_len); memcpy(allocated_addr, addr, addr_len); grpc_sockaddr_set_port(allocated_addr, port); addr = allocated_addr; diff --git a/src/core/profiling/basic_timers.c b/src/core/profiling/basic_timers.c index 3067f52c219..3c9557ab895 100644 --- a/src/core/profiling/basic_timers.c +++ b/src/core/profiling/basic_timers.c @@ -173,7 +173,7 @@ static void flush_logs(gpr_timer_log_list *list) { gpr_timer_log *log; while ((log = timer_log_pop_front(list)) != NULL) { write_log(log); - free(log); + gpr_free(log); } } @@ -208,7 +208,7 @@ static void init_output() { } static void rotate_log() { - gpr_timer_log *new = malloc(sizeof(*new)); + gpr_timer_log *new = gpr_malloc(sizeof(*new)); gpr_once_init(&g_once_init, init_output); new->num_entries = 0; pthread_mutex_lock(&g_mu); diff --git a/src/core/support/thd_posix.c b/src/core/support/thd_posix.c index 4d874d36566..1c703844b4d 100644 --- a/src/core/support/thd_posix.c +++ b/src/core/support/thd_posix.c @@ -81,6 +81,7 @@ int gpr_thd_new(gpr_thd_id *t, void (*thd_body)(void *arg), void *arg, thread_started = (pthread_create(&p, &attr, &thread_body, a) == 0); GPR_ASSERT(pthread_attr_destroy(&attr) == 0); if (!thread_started) { + /* don't use gpr_free, as this was allocated using malloc (see above) */ free(a); } *t = (gpr_thd_id)p; diff --git a/src/core/surface/byte_buffer.c b/src/core/surface/byte_buffer.c index fb39c4531d9..a093a37af32 100644 --- a/src/core/surface/byte_buffer.c +++ b/src/core/surface/byte_buffer.c @@ -44,7 +44,7 @@ grpc_byte_buffer *grpc_raw_byte_buffer_create(gpr_slice *slices, grpc_byte_buffer *grpc_raw_compressed_byte_buffer_create( gpr_slice *slices, size_t nslices, grpc_compression_algorithm compression) { size_t i; - grpc_byte_buffer *bb = malloc(sizeof(grpc_byte_buffer)); + grpc_byte_buffer *bb = gpr_malloc(sizeof(grpc_byte_buffer)); bb->type = GRPC_BB_RAW; bb->data.raw.compression = compression; gpr_slice_buffer_init(&bb->data.raw.slice_buffer); @@ -57,7 +57,7 @@ grpc_byte_buffer *grpc_raw_compressed_byte_buffer_create( grpc_byte_buffer *grpc_raw_byte_buffer_from_reader( grpc_byte_buffer_reader *reader) { - grpc_byte_buffer *bb = malloc(sizeof(grpc_byte_buffer)); + grpc_byte_buffer *bb = gpr_malloc(sizeof(grpc_byte_buffer)); gpr_slice slice; bb->type = GRPC_BB_RAW; bb->data.raw.compression = GRPC_COMPRESS_NONE; @@ -85,7 +85,7 @@ void grpc_byte_buffer_destroy(grpc_byte_buffer *bb) { gpr_slice_buffer_destroy(&bb->data.raw.slice_buffer); break; } - free(bb); + gpr_free(bb); } size_t grpc_byte_buffer_length(grpc_byte_buffer *bb) { diff --git a/src/core/tsi/fake_transport_security.c b/src/core/tsi/fake_transport_security.c index c0106f7a33c..86ffa6b83be 100644 --- a/src/core/tsi/fake_transport_security.c +++ b/src/core/tsi/fake_transport_security.c @@ -134,12 +134,12 @@ static void tsi_fake_frame_reset(tsi_fake_frame *frame, int needs_draining) { static int tsi_fake_frame_ensure_size(tsi_fake_frame *frame) { if (frame->data == NULL) { frame->allocated_size = frame->size; - frame->data = malloc(frame->allocated_size); + frame->data = gpr_malloc(frame->allocated_size); if (frame->data == NULL) return 0; } else if (frame->size > frame->allocated_size) { unsigned char *new_data = realloc(frame->data, frame->size); if (new_data == NULL) { - free(frame->data); + gpr_free(frame->data); frame->data = NULL; return 0; } @@ -160,7 +160,7 @@ static tsi_result fill_frame_from_bytes(const unsigned char *incoming_bytes, if (frame->needs_draining) return TSI_INTERNAL_ERROR; if (frame->data == NULL) { frame->allocated_size = TSI_FAKE_FRAME_INITIAL_ALLOCATED_SIZE; - frame->data = malloc(frame->allocated_size); + frame->data = gpr_malloc(frame->allocated_size); if (frame->data == NULL) return TSI_OUT_OF_RESOURCES; } @@ -226,7 +226,7 @@ static tsi_result bytes_to_frame(unsigned char *bytes, size_t bytes_size, } static void tsi_fake_frame_destruct(tsi_fake_frame *frame) { - if (frame->data != NULL) free(frame->data); + if (frame->data != NULL) gpr_free(frame->data); } /* --- tsi_frame_protector methods implementation. ---*/ @@ -366,7 +366,7 @@ static void fake_protector_destroy(tsi_frame_protector *self) { tsi_fake_frame_protector *impl = (tsi_fake_frame_protector *)self; tsi_fake_frame_destruct(&impl->protect_frame); tsi_fake_frame_destruct(&impl->unprotect_frame); - free(self); + gpr_free(self); } static const tsi_frame_protector_vtable frame_protector_vtable = { @@ -488,7 +488,7 @@ static void fake_handshaker_destroy(tsi_handshaker *self) { tsi_fake_handshaker *impl = (tsi_fake_handshaker *)self; tsi_fake_frame_destruct(&impl->incoming); tsi_fake_frame_destruct(&impl->outgoing); - free(self); + gpr_free(self); } static const tsi_handshaker_vtable handshaker_vtable = { diff --git a/src/core/tsi/ssl_transport_security.c b/src/core/tsi/ssl_transport_security.c index 8df582609b9..acf1b0e3c1d 100644 --- a/src/core/tsi/ssl_transport_security.c +++ b/src/core/tsi/ssl_transport_security.c @@ -148,8 +148,7 @@ static void init_openssl(void) { OpenSSL_add_all_algorithms(); num_locks = CRYPTO_num_locks(); GPR_ASSERT(num_locks > 0); - openssl_mutexes = malloc((size_t)num_locks * sizeof(gpr_mu)); - GPR_ASSERT(openssl_mutexes != NULL); + openssl_mutexes = gpr_malloc((size_t)num_locks * sizeof(gpr_mu)); for (i = 0; i < CRYPTO_num_locks(); i++) { gpr_mu_init(&openssl_mutexes[i]); } @@ -701,7 +700,7 @@ static tsi_result build_alpn_protocol_name_list( } *protocol_name_list_length += (size_t)alpn_protocols_lengths[i] + 1; } - *protocol_name_list = malloc(*protocol_name_list_length); + *protocol_name_list = gpr_malloc(*protocol_name_list_length); if (*protocol_name_list == NULL) return TSI_OUT_OF_RESOURCES; current = *protocol_name_list; for (i = 0; i < num_alpn_protocols; i++) { @@ -852,9 +851,9 @@ static tsi_result ssl_protector_unprotect( static void ssl_protector_destroy(tsi_frame_protector *self) { tsi_ssl_frame_protector *impl = (tsi_ssl_frame_protector *)self; - if (impl->buffer != NULL) free(impl->buffer); + if (impl->buffer != NULL) gpr_free(impl->buffer); if (impl->ssl != NULL) SSL_free(impl->ssl); - free(self); + gpr_free(self); } static const tsi_frame_protector_vtable frame_protector_vtable = { @@ -966,8 +965,8 @@ static tsi_result ssl_handshaker_extract_peer(tsi_handshaker *self, if (alpn_selected != NULL) { size_t i; tsi_peer_property *new_properties = - calloc(1, sizeof(tsi_peer_property) * (peer->property_count + 1)); - if (new_properties == NULL) return TSI_OUT_OF_RESOURCES; + gpr_malloc(sizeof(*new_properties) * (peer->property_count + 1)); + memset(new_properties, 0, sizeof(*new_properties) * (peer->property_count + 1)); for (i = 0; i < peer->property_count; i++) { new_properties[i] = peer->properties[i]; } @@ -975,10 +974,10 @@ static tsi_result ssl_handshaker_extract_peer(tsi_handshaker *self, TSI_SSL_ALPN_SELECTED_PROTOCOL, (const char *)alpn_selected, alpn_selected_len, &new_properties[peer->property_count]); if (result != TSI_OK) { - free(new_properties); + gpr_free(new_properties); return result; } - if (peer->properties != NULL) free(peer->properties); + if (peer->properties != NULL) gpr_free(peer->properties); peer->property_count++; peer->properties = new_properties; } @@ -992,10 +991,8 @@ static tsi_result ssl_handshaker_create_frame_protector( TSI_SSL_MAX_PROTECTED_FRAME_SIZE_UPPER_BOUND; tsi_ssl_handshaker *impl = (tsi_ssl_handshaker *)self; tsi_ssl_frame_protector *protector_impl = - calloc(1, sizeof(tsi_ssl_frame_protector)); - if (protector_impl == NULL) { - return TSI_OUT_OF_RESOURCES; - } + gpr_malloc(sizeof(*protector_impl)); + memset(protector_impl, 0, sizeof(*protector_impl)); if (max_output_protected_frame_size != NULL) { if (*max_output_protected_frame_size > @@ -1011,11 +1008,11 @@ static tsi_result ssl_handshaker_create_frame_protector( } protector_impl->buffer_size = actual_max_output_protected_frame_size - TSI_SSL_MAX_PROTECTION_OVERHEAD; - protector_impl->buffer = malloc(protector_impl->buffer_size); + protector_impl->buffer = gpr_malloc(protector_impl->buffer_size); if (protector_impl->buffer == NULL) { gpr_log(GPR_ERROR, "Could not allocated buffer for tsi_ssl_frame_protector."); - free(protector_impl); + gpr_free(protector_impl); return TSI_INTERNAL_ERROR; } @@ -1034,7 +1031,7 @@ static tsi_result ssl_handshaker_create_frame_protector( static void ssl_handshaker_destroy(tsi_handshaker *self) { tsi_ssl_handshaker *impl = (tsi_ssl_handshaker *)self; SSL_free(impl->ssl); /* The BIO objects are owned by ssl */ - free(impl); + gpr_free(impl); } static const tsi_handshaker_vtable handshaker_vtable = { @@ -1111,11 +1108,8 @@ static tsi_result create_tsi_ssl_handshaker(SSL_CTX *ctx, int is_client, SSL_set_accept_state(ssl); } - impl = calloc(1, sizeof(tsi_ssl_handshaker)); - if (impl == NULL) { - SSL_free(ssl); - return TSI_OUT_OF_RESOURCES; - } + impl = gpr_malloc(sizeof(*impl)); + memset(impl, 0, sizeof(*impl)); impl->ssl = ssl; impl->into_ssl = into_ssl; impl->from_ssl = from_ssl; @@ -1167,8 +1161,8 @@ static void ssl_client_handshaker_factory_destroy( tsi_ssl_client_handshaker_factory *impl = (tsi_ssl_client_handshaker_factory *)self; if (impl->ssl_context != NULL) SSL_CTX_free(impl->ssl_context); - if (impl->alpn_protocol_list != NULL) free(impl->alpn_protocol_list); - free(impl); + if (impl->alpn_protocol_list != NULL) gpr_free(impl->alpn_protocol_list); + gpr_free(impl); } static int client_handshaker_factory_npn_callback(SSL *ssl, unsigned char **out, @@ -1209,12 +1203,12 @@ static void ssl_server_handshaker_factory_destroy( tsi_peer_destruct(&impl->ssl_context_x509_subject_names[i]); } } - if (impl->ssl_contexts != NULL) free(impl->ssl_contexts); + if (impl->ssl_contexts != NULL) gpr_free(impl->ssl_contexts); if (impl->ssl_context_x509_subject_names != NULL) { - free(impl->ssl_context_x509_subject_names); + gpr_free(impl->ssl_context_x509_subject_names); } - if (impl->alpn_protocol_list != NULL) free(impl->alpn_protocol_list); - free(impl); + if (impl->alpn_protocol_list != NULL) gpr_free(impl->alpn_protocol_list); + gpr_free(impl); } static int does_entry_match_name(const char *entry, size_t entry_length, @@ -1333,11 +1327,8 @@ tsi_result tsi_create_ssl_client_handshaker_factory( return TSI_INVALID_ARGUMENT; } - impl = calloc(1, sizeof(tsi_ssl_client_handshaker_factory)); - if (impl == NULL) { - SSL_CTX_free(ssl_context); - return TSI_OUT_OF_RESOURCES; - } + impl = gpr_malloc(sizeof(*impl)); + memset(impl, 0, sizeof(*impl)); impl->ssl_context = ssl_context; do { @@ -1411,14 +1402,16 @@ tsi_result tsi_create_ssl_server_handshaker_factory( return TSI_INVALID_ARGUMENT; } - impl = calloc(1, sizeof(tsi_ssl_server_handshaker_factory)); - if (impl == NULL) return TSI_OUT_OF_RESOURCES; + impl = gpr_malloc(sizeof(*impl)); + memset(impl, 0, sizeof(*impl)); impl->base.create_handshaker = ssl_server_handshaker_factory_create_handshaker; impl->base.destroy = ssl_server_handshaker_factory_destroy; - impl->ssl_contexts = calloc(key_cert_pair_count, sizeof(SSL_CTX *)); + impl->ssl_contexts = gpr_malloc(key_cert_pair_count * sizeof(SSL_CTX *)); + memset(impl->ssl_contexts, 0, key_cert_pair_count * sizeof(SSL_CTX *)); impl->ssl_context_x509_subject_names = - calloc(key_cert_pair_count, sizeof(tsi_peer)); + gpr_malloc(key_cert_pair_count * sizeof(tsi_peer)); + memset(impl->ssl_context_x509_subject_names, 0, key_cert_pair_count * sizeof(tsi_peer)); if (impl->ssl_contexts == NULL || impl->ssl_context_x509_subject_names == NULL) { tsi_ssl_handshaker_factory_destroy(&impl->base); diff --git a/src/core/tsi/transport_security.c b/src/core/tsi/transport_security.c index db219a50a67..e17749d871a 100644 --- a/src/core/tsi/transport_security.c +++ b/src/core/tsi/transport_security.c @@ -239,16 +239,10 @@ void tsi_peer_destruct(tsi_peer *self) { tsi_result tsi_construct_allocated_string_peer_property( const char *name, size_t value_length, tsi_peer_property *property) { *property = tsi_init_peer_property(); - if (name != NULL) { - property->name = tsi_strdup(name); - if (property->name == NULL) return TSI_OUT_OF_RESOURCES; - } + if (name != NULL) property->name = gpr_strdup(name); if (value_length > 0) { - property->value.data = calloc(1, value_length); - if (property->value.data == NULL) { - tsi_peer_property_destruct(property); - return TSI_OUT_OF_RESOURCES; - } + property->value.data = gpr_malloc(value_length); + memset(value.data, 0, value_length); property->value.length = value_length; } return TSI_OK; @@ -276,8 +270,8 @@ tsi_result tsi_construct_string_peer_property(const char *name, tsi_result tsi_construct_peer(size_t property_count, tsi_peer *peer) { memset(peer, 0, sizeof(tsi_peer)); if (property_count > 0) { - peer->properties = calloc(property_count, sizeof(tsi_peer_property)); - if (peer->properties == NULL) return TSI_OUT_OF_RESOURCES; + peer->properties = gpr_malloc(property_count * sizeof(tsi_peer_property)); + memset(peer->properties, 0, property_count * sizeof(tsi_peer_property)); peer->property_count = property_count; } return TSI_OK; diff --git a/test/core/iomgr/endpoint_tests.c b/test/core/iomgr/endpoint_tests.c index f689e4ba7fb..cb23f58eca5 100644 --- a/test/core/iomgr/endpoint_tests.c +++ b/test/core/iomgr/endpoint_tests.c @@ -89,7 +89,7 @@ static void end_test(grpc_endpoint_test_config config) { config.clean_up(); } static gpr_slice *allocate_blocks(size_t num_bytes, size_t slice_size, size_t *num_blocks, uint8_t *current_data) { size_t nslices = num_bytes / slice_size + (num_bytes % slice_size ? 1 : 0); - gpr_slice *slices = malloc(sizeof(gpr_slice) * nslices); + gpr_slice *slices = gpr_malloc(sizeof(gpr_slice) * nslices); size_t num_bytes_left = num_bytes; size_t i; size_t j; @@ -164,7 +164,7 @@ static void read_and_write_test_write_handler(grpc_exec_ctx *exec_ctx, gpr_slice_buffer_addn(&state->outgoing, slices, nslices); grpc_endpoint_write(exec_ctx, state->write_ep, &state->outgoing, &state->done_write); - free(slices); + gpr_free(slices); return; } } diff --git a/test/core/iomgr/tcp_posix_test.c b/test/core/iomgr/tcp_posix_test.c index 4351642ab6e..b2cf87a0c25 100644 --- a/test/core/iomgr/tcp_posix_test.c +++ b/test/core/iomgr/tcp_posix_test.c @@ -97,7 +97,7 @@ static ssize_t fill_socket(int fd) { static size_t fill_socket_partial(int fd, size_t bytes) { ssize_t write_bytes; size_t total_bytes = 0; - unsigned char *buf = malloc(bytes); + unsigned char *buf = gpr_malloc(bytes); unsigned i; for (i = 0; i < bytes; ++i) { buf[i] = (uint8_t)(i % 256); @@ -292,7 +292,7 @@ static void write_done(grpc_exec_ctx *exec_ctx, } void drain_socket_blocking(int fd, size_t num_bytes, size_t read_size) { - unsigned char *buf = malloc(read_size); + unsigned char *buf = gpr_malloc(read_size); ssize_t bytes_read; size_t bytes_left = num_bytes; int flags; diff --git a/test/core/network_benchmarks/low_level_ping_pong.c b/test/core/network_benchmarks/low_level_ping_pong.c index 7ed3372d5d1..8179ba3b4df 100644 --- a/test/core/network_benchmarks/low_level_ping_pong.c +++ b/test/core/network_benchmarks/low_level_ping_pong.c @@ -265,19 +265,19 @@ static int epoll_setup(thread_args *args) { #endif static void server_thread(thread_args *args) { - char *buf = malloc(args->msg_size); + char *buf = gpr_malloc(args->msg_size); if (args->setup(args) < 0) { gpr_log(GPR_ERROR, "Setup failed"); } for (;;) { if (args->read_bytes(args, buf) < 0) { gpr_log(GPR_ERROR, "Server read failed"); - free(buf); + gpr_free(buf); return; } if (args->write_bytes(args, buf) < 0) { gpr_log(GPR_ERROR, "Server write failed"); - free(buf); + gpr_free(buf); return; } } @@ -304,7 +304,8 @@ static double now(void) { } static void client_thread(thread_args *args) { - char *buf = calloc(args->msg_size, sizeof(char)); + char *buf = gpr_malloc(args->msg_size * sizeof(char)); + memset(buf, 0, args->msg_size * sizeof(char)); gpr_histogram *histogram = gpr_histogram_create(0.01, 60e9); double start_time; double end_time; @@ -333,7 +334,7 @@ static void client_thread(thread_args *args) { } print_histogram(histogram); error: - free(buf); + gpr_free(buf); gpr_histogram_destroy(histogram); } @@ -596,8 +597,8 @@ static int run_all_benchmarks(size_t msg_size) { test_strategy *strategy = &test_strategies[i]; size_t j; for (j = 0; j < GPR_ARRAY_SIZE(socket_types); ++j) { - thread_args *client_args = malloc(sizeof(thread_args)); - thread_args *server_args = malloc(sizeof(thread_args)); + thread_args *client_args = gpr_malloc(sizeof(thread_args)); + thread_args *server_args = gpr_malloc(sizeof(thread_args)); char *socket_type = socket_types[j]; client_args->read_bytes = strategy->read_strategy; @@ -620,8 +621,8 @@ static int run_all_benchmarks(size_t msg_size) { } int main(int argc, char **argv) { - thread_args *client_args = malloc(sizeof(thread_args)); - thread_args *server_args = malloc(sizeof(thread_args)); + thread_args *client_args = gpr_malloc(sizeof(thread_args)); + thread_args *server_args = gpr_malloc(sizeof(thread_args)); int msg_size = -1; char *read_strategy = NULL; char *socket_type = NULL; diff --git a/test/core/support/alloc_test.c b/test/core/support/alloc_test.c index e2d0c16b41d..7eaf7a14f3e 100644 --- a/test/core/support/alloc_test.c +++ b/test/core/support/alloc_test.c @@ -46,7 +46,7 @@ static void fake_free(void *addr) { static void test_custom_allocs() { const gpr_allocation_functions default_fns = gpr_get_allocation_functions(); intptr_t addr_to_free = 0; - int *i; + char *i; gpr_allocation_functions fns = {fake_malloc, fake_realloc, fake_free}; gpr_set_allocation_functions(fns); @@ -58,7 +58,7 @@ static void test_custom_allocs() { /* Restore and check we don't get funky values and that we don't leak */ gpr_set_allocation_functions(default_fns); - GPR_ASSERT((void *)1 != (i = gpr_malloc(sizeof(*i)))); + GPR_ASSERT((void *)sizeof(*i) != (i = gpr_malloc(sizeof(*i)))); GPR_ASSERT((void *)2 != (i = gpr_realloc(i, 2))); gpr_free(i); } From 5d203f514c204083d1d81a804bd64df2d196197a Mon Sep 17 00:00:00 2001 From: "Nicolas \"Pixel\" Noble" Date: Sat, 26 Mar 2016 01:35:33 +0100 Subject: [PATCH 02/38] Sanitize sources. --- src/core/surface/byte_buffer.c | 2 +- src/core/tsi/ssl_transport_security.c | 9 +++++---- src/core/tsi/transport_security.c | 2 +- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/core/surface/byte_buffer.c b/src/core/surface/byte_buffer.c index a093a37af32..314a67dfcca 100644 --- a/src/core/surface/byte_buffer.c +++ b/src/core/surface/byte_buffer.c @@ -1,6 +1,6 @@ /* * - * Copyright 2015, Google Inc. + * Copyright 2015-2016, Google Inc. * All rights reserved. * * Redistribution and use in source and binary forms, with or without diff --git a/src/core/tsi/ssl_transport_security.c b/src/core/tsi/ssl_transport_security.c index acf1b0e3c1d..968c0b9be8f 100644 --- a/src/core/tsi/ssl_transport_security.c +++ b/src/core/tsi/ssl_transport_security.c @@ -966,7 +966,8 @@ static tsi_result ssl_handshaker_extract_peer(tsi_handshaker *self, size_t i; tsi_peer_property *new_properties = gpr_malloc(sizeof(*new_properties) * (peer->property_count + 1)); - memset(new_properties, 0, sizeof(*new_properties) * (peer->property_count + 1)); + memset(new_properties, 0, + sizeof(*new_properties) * (peer->property_count + 1)); for (i = 0; i < peer->property_count; i++) { new_properties[i] = peer->properties[i]; } @@ -990,8 +991,7 @@ static tsi_result ssl_handshaker_create_frame_protector( size_t actual_max_output_protected_frame_size = TSI_SSL_MAX_PROTECTED_FRAME_SIZE_UPPER_BOUND; tsi_ssl_handshaker *impl = (tsi_ssl_handshaker *)self; - tsi_ssl_frame_protector *protector_impl = - gpr_malloc(sizeof(*protector_impl)); + tsi_ssl_frame_protector *protector_impl = gpr_malloc(sizeof(*protector_impl)); memset(protector_impl, 0, sizeof(*protector_impl)); if (max_output_protected_frame_size != NULL) { @@ -1411,7 +1411,8 @@ tsi_result tsi_create_ssl_server_handshaker_factory( memset(impl->ssl_contexts, 0, key_cert_pair_count * sizeof(SSL_CTX *)); impl->ssl_context_x509_subject_names = gpr_malloc(key_cert_pair_count * sizeof(tsi_peer)); - memset(impl->ssl_context_x509_subject_names, 0, key_cert_pair_count * sizeof(tsi_peer)); + memset(impl->ssl_context_x509_subject_names, 0, + key_cert_pair_count * sizeof(tsi_peer)); if (impl->ssl_contexts == NULL || impl->ssl_context_x509_subject_names == NULL) { tsi_ssl_handshaker_factory_destroy(&impl->base); diff --git a/src/core/tsi/transport_security.c b/src/core/tsi/transport_security.c index e17749d871a..189f5a0228a 100644 --- a/src/core/tsi/transport_security.c +++ b/src/core/tsi/transport_security.c @@ -1,6 +1,6 @@ /* * - * Copyright 2015, Google Inc. + * Copyright 2015-2016, Google Inc. * All rights reserved. * * Redistribution and use in source and binary forms, with or without From 367e5d896b115413f278b239dd63194cb3671c17 Mon Sep 17 00:00:00 2001 From: "Nicolas \"Pixel\" Noble" Date: Sat, 26 Mar 2016 01:37:43 +0100 Subject: [PATCH 03/38] Forgot a few callocs. --- src/core/tsi/fake_transport_security.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/core/tsi/fake_transport_security.c b/src/core/tsi/fake_transport_security.c index 86ffa6b83be..85c8ff7a57c 100644 --- a/src/core/tsi/fake_transport_security.c +++ b/src/core/tsi/fake_transport_security.c @@ -501,7 +501,8 @@ static const tsi_handshaker_vtable handshaker_vtable = { }; tsi_handshaker *tsi_create_fake_handshaker(int is_client) { - tsi_fake_handshaker *impl = calloc(1, sizeof(tsi_fake_handshaker)); + tsi_fake_handshaker *impl = gpr_malloc(sizeof(*impl)); + memset(impl, 0, sizeof(*impl)); impl->base.vtable = &handshaker_vtable; impl->is_client = is_client; impl->result = TSI_HANDSHAKE_IN_PROGRESS; @@ -517,8 +518,8 @@ tsi_handshaker *tsi_create_fake_handshaker(int is_client) { tsi_frame_protector *tsi_create_fake_protector( size_t *max_protected_frame_size) { - tsi_fake_frame_protector *impl = calloc(1, sizeof(tsi_fake_frame_protector)); - if (impl == NULL) return NULL; + tsi_fake_frame_protector *impl = gpr_malloc(sizeof(*impl)); + memset(impl, 0, sizeof(*impl)); impl->max_frame_size = (max_protected_frame_size == NULL) ? TSI_FAKE_DEFAULT_FRAME_SIZE : *max_protected_frame_size; From b29d8cfeb886e951dcbacc0f42f12624a2684086 Mon Sep 17 00:00:00 2001 From: "Nicolas \"Pixel\" Noble" Date: Fri, 8 Apr 2016 01:38:29 +0200 Subject: [PATCH 04/38] Last few mallocs / frees. --- include/grpc/support/string_util.h | 2 ++ src/core/lib/support/log_linux.c | 1 + src/core/lib/tsi/fake_transport_security.c | 3 ++- src/core/lib/tsi/ssl_transport_security.c | 1 + src/core/lib/tsi/transport_security.c | 24 +++++++--------------- 5 files changed, 13 insertions(+), 18 deletions(-) diff --git a/include/grpc/support/string_util.h b/include/grpc/support/string_util.h index 0abdb4a8c86..f981bc0db0b 100644 --- a/include/grpc/support/string_util.h +++ b/include/grpc/support/string_util.h @@ -34,6 +34,8 @@ #ifndef GRPC_SUPPORT_STRING_UTIL_H #define GRPC_SUPPORT_STRING_UTIL_H +#include + #ifdef __cplusplus extern "C" { #endif diff --git a/src/core/lib/support/log_linux.c b/src/core/lib/support/log_linux.c index 6d4b63bbe0d..ff3febb38eb 100644 --- a/src/core/lib/support/log_linux.c +++ b/src/core/lib/support/log_linux.c @@ -68,6 +68,7 @@ void gpr_log(const char *file, int line, gpr_log_severity severity, } va_end(args); gpr_log_message(file, line, severity, message); + /* message has been allocated by vasprintf above, and needs free */ free(message); } diff --git a/src/core/lib/tsi/fake_transport_security.c b/src/core/lib/tsi/fake_transport_security.c index a0f69c3d64d..0e20d6fd710 100644 --- a/src/core/lib/tsi/fake_transport_security.c +++ b/src/core/lib/tsi/fake_transport_security.c @@ -36,6 +36,7 @@ #include #include +#include #include #include #include @@ -137,7 +138,7 @@ static int tsi_fake_frame_ensure_size(tsi_fake_frame *frame) { frame->data = gpr_malloc(frame->allocated_size); if (frame->data == NULL) return 0; } else if (frame->size > frame->allocated_size) { - unsigned char *new_data = realloc(frame->data, frame->size); + unsigned char *new_data = gpr_realloc(frame->data, frame->size); if (new_data == NULL) { gpr_free(frame->data); frame->data = NULL; diff --git a/src/core/lib/tsi/ssl_transport_security.c b/src/core/lib/tsi/ssl_transport_security.c index ef3aeb70901..045901cc72f 100644 --- a/src/core/lib/tsi/ssl_transport_security.c +++ b/src/core/lib/tsi/ssl_transport_security.c @@ -45,6 +45,7 @@ #include #endif +#include #include #include #include diff --git a/src/core/lib/tsi/transport_security.c b/src/core/lib/tsi/transport_security.c index 66f1e356d6a..830cf095848 100644 --- a/src/core/lib/tsi/transport_security.c +++ b/src/core/lib/tsi/transport_security.c @@ -33,6 +33,9 @@ #include "src/core/lib/tsi/transport_security.h" +#include +#include + #include #include @@ -40,19 +43,6 @@ int tsi_tracing_enabled = 0; -/* --- Utils. --- */ - -char *tsi_strdup(const char *src) { - char *dst; - size_t len; - if (!src) return NULL; - len = strlen(src) + 1; - dst = malloc(len); - if (!dst) return NULL; - memcpy(dst, src, len); - return dst; -} - /* --- tsi_result common implementation. --- */ const char *tsi_result_to_string(tsi_result result) { @@ -214,15 +204,15 @@ static void tsi_peer_destroy_list_property(tsi_peer_property *children, for (i = 0; i < child_count; i++) { tsi_peer_property_destruct(&children[i]); } - free(children); + gpr_free(children); } void tsi_peer_property_destruct(tsi_peer_property *property) { if (property->name != NULL) { - free(property->name); + gpr_free(property->name); } if (property->value.data != NULL) { - free(property->value.data); + gpr_free(property->value.data); } *property = tsi_init_peer_property(); /* Reset everything to 0. */ } @@ -242,7 +232,7 @@ tsi_result tsi_construct_allocated_string_peer_property( if (name != NULL) property->name = gpr_strdup(name); if (value_length > 0) { property->value.data = gpr_malloc(value_length); - memset(value.data, 0, value_length); + memset(property->value.data, 0, value_length); property->value.length = value_length; } return TSI_OK; From b206c154dd7605bf0dbdc7a69605b6f749d2bef4 Mon Sep 17 00:00:00 2001 From: "Nicolas \"Pixel\" Noble" Date: Fri, 8 Apr 2016 19:42:28 +0200 Subject: [PATCH 05/38] Addressing comment. --- src/core/lib/profiling/basic_timers.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/core/lib/profiling/basic_timers.c b/src/core/lib/profiling/basic_timers.c index 0a4ae3057ad..50082cd7ee4 100644 --- a/src/core/lib/profiling/basic_timers.c +++ b/src/core/lib/profiling/basic_timers.c @@ -173,7 +173,7 @@ static void flush_logs(gpr_timer_log_list *list) { gpr_timer_log *log; while ((log = timer_log_pop_front(list)) != NULL) { write_log(log); - gpr_free(log); + free(log); } } @@ -208,7 +208,8 @@ static void init_output() { } static void rotate_log() { - gpr_timer_log *new = gpr_malloc(sizeof(*new)); + /* Using malloc here, as this code could end up being called by gpr_malloc */ + gpr_timer_log *new = malloc(sizeof(*new)); gpr_once_init(&g_once_init, init_output); new->num_entries = 0; pthread_mutex_lock(&g_mu); From 85a46dd78080b4060e12330ef6d139f5e6287016 Mon Sep 17 00:00:00 2001 From: "Nicolas \"Pixel\" Noble" Date: Tue, 12 Apr 2016 01:50:51 +0200 Subject: [PATCH 06/38] Moving memory allocation tracking to its own file. --- Makefile | 2 + build.yaml | 2 + test/core/json/fuzzer.c | 41 +------ test/core/util/memory_counters.c | 104 ++++++++++++++++++ test/core/util/memory_counters.h | 48 ++++++++ tools/run_tests/sources_and_headers.json | 3 + .../grpc_test_util/grpc_test_util.vcxproj | 3 + .../grpc_test_util.vcxproj.filters | 6 + .../grpc_test_util_unsecure.vcxproj | 3 + .../grpc_test_util_unsecure.vcxproj.filters | 6 + 10 files changed, 183 insertions(+), 35 deletions(-) create mode 100644 test/core/util/memory_counters.c create mode 100644 test/core/util/memory_counters.h diff --git a/Makefile b/Makefile index 6b2b9eb2dee..ce02019f250 100644 --- a/Makefile +++ b/Makefile @@ -2695,6 +2695,7 @@ LIBGRPC_TEST_UTIL_SRC = \ test/core/end2end/fixtures/proxy.c \ test/core/iomgr/endpoint_tests.c \ test/core/util/grpc_profiler.c \ + test/core/util/memory_counters.c \ test/core/util/mock_endpoint.c \ test/core/util/parse_hexstring.c \ test/core/util/port_posix.c \ @@ -2743,6 +2744,7 @@ LIBGRPC_TEST_UTIL_UNSECURE_SRC = \ test/core/end2end/fixtures/proxy.c \ test/core/iomgr/endpoint_tests.c \ test/core/util/grpc_profiler.c \ + test/core/util/memory_counters.c \ test/core/util/mock_endpoint.c \ test/core/util/parse_hexstring.c \ test/core/util/port_posix.c \ diff --git a/build.yaml b/build.yaml index cbbc3d22464..ce9bc286a29 100644 --- a/build.yaml +++ b/build.yaml @@ -567,6 +567,7 @@ filegroups: - test/core/end2end/fixtures/proxy.h - test/core/iomgr/endpoint_tests.h - test/core/util/grpc_profiler.h + - test/core/util/memory_counters.h - test/core/util/mock_endpoint.h - test/core/util/parse_hexstring.h - test/core/util/port.h @@ -577,6 +578,7 @@ filegroups: - test/core/end2end/fixtures/proxy.c - test/core/iomgr/endpoint_tests.c - test/core/util/grpc_profiler.c + - test/core/util/memory_counters.c - test/core/util/mock_endpoint.c - test/core/util/parse_hexstring.c - test/core/util/port_posix.c diff --git a/test/core/json/fuzzer.c b/test/core/json/fuzzer.c index 044db973ab1..e94b41ca999 100644 --- a/test/core/json/fuzzer.c +++ b/test/core/json/fuzzer.c @@ -38,42 +38,12 @@ #include #include "src/core/lib/json/json.h" - -static size_t g_total_size = 0; -static gpr_allocation_functions g_old_allocs; - -void *guard_malloc(size_t size) { - size_t *ptr; - g_total_size += size; - ptr = g_old_allocs.malloc_fn(size + sizeof(size)); - *ptr++ = size; - return ptr; -} - -void *guard_realloc(void *vptr, size_t size) { - size_t *ptr = vptr; - --ptr; - g_total_size -= *ptr; - ptr = g_old_allocs.realloc_fn(ptr, size + sizeof(size)); - g_total_size += size; - *ptr++ = size; - return ptr; -} - -void guard_free(void *vptr) { - size_t *ptr = vptr; - --ptr; - g_total_size -= *ptr; - g_old_allocs.free_fn(ptr); -} - -struct gpr_allocation_functions g_guard_allocs = {guard_malloc, guard_realloc, - guard_free}; +#include "test/core/util/memory_counters.h" int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) { char *s; - g_old_allocs = gpr_get_allocation_functions(); - gpr_set_allocation_functions(g_guard_allocs); + struct grpc_memory_counters counters; + grpc_memory_counters_init(); s = gpr_malloc(size); memcpy(s, data, size); grpc_json *x; @@ -81,7 +51,8 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) { grpc_json_destroy(x); } gpr_free(s); - gpr_set_allocation_functions(g_old_allocs); - GPR_ASSERT(g_total_size == 0); + counters = grpc_memory_counters_snapshot(); + grpc_memory_counters_destroy(); + GPR_ASSERT(counters.total_size_relative == 0); return 0; } diff --git a/test/core/util/memory_counters.c b/test/core/util/memory_counters.c new file mode 100644 index 00000000000..85e85237a2f --- /dev/null +++ b/test/core/util/memory_counters.c @@ -0,0 +1,104 @@ +/* + * + * 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. + * + */ + +#include +#include + +#include +#include + +#include "test/core/util/memory_counters.h" + +static gpr_mu g_memory_mutex; +static struct grpc_memory_counters g_memory_counters; +static gpr_allocation_functions g_old_allocs; + +static void *guard_malloc(size_t size) { + size_t *ptr; + gpr_mu_lock(&g_memory_mutex); + g_memory_counters.total_size_absolute += size; + g_memory_counters.total_size_relative += size; + g_memory_counters.total_allocs_absolute++; + g_memory_counters.total_allocs_relative++; + gpr_mu_unlock(&g_memory_mutex); + ptr = g_old_allocs.malloc_fn(size + sizeof(size)); + *ptr++ = size; + return ptr; +} + +static void *guard_realloc(void *vptr, size_t size) { + size_t *ptr = vptr; + --ptr; + gpr_mu_lock(&g_memory_mutex); + g_memory_counters.total_size_absolute += size; + g_memory_counters.total_size_relative -= *ptr; + g_memory_counters.total_size_relative += size; + g_memory_counters.total_allocs_absolute++; + gpr_mu_unlock(&g_memory_mutex); + ptr = g_old_allocs.realloc_fn(ptr, size + sizeof(size)); + *ptr++ = size; + return ptr; +} + +static void guard_free(void *vptr) { + size_t *ptr = vptr; + --ptr; + gpr_mu_lock(&g_memory_mutex); + g_memory_counters.total_size_relative -= *ptr; + g_memory_counters.total_allocs_relative--; + gpr_mu_unlock(&g_memory_mutex); + g_old_allocs.free_fn(ptr); +} + +struct gpr_allocation_functions g_guard_allocs = {guard_malloc, guard_realloc, + guard_free}; + +void grpc_memory_counters_init() { + memset(&g_memory_counters, 0, sizeof(g_memory_counters)); + gpr_mu_init(&g_memory_mutex); + g_old_allocs = gpr_get_allocation_functions(); + gpr_set_allocation_functions(g_guard_allocs); +} + +void grpc_memory_counters_destroy() { + gpr_set_allocation_functions(g_old_allocs); + gpr_mu_destroy(&g_memory_mutex); +} + +struct grpc_memory_counters grpc_memory_counters_snapshot() { + struct grpc_memory_counters counters; + gpr_mu_lock(&g_memory_mutex); + counters = g_memory_counters; + gpr_mu_unlock(&g_memory_mutex); + return counters; +} diff --git a/test/core/util/memory_counters.h b/test/core/util/memory_counters.h new file mode 100644 index 00000000000..f3328165014 --- /dev/null +++ b/test/core/util/memory_counters.h @@ -0,0 +1,48 @@ +/* + * + * 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_TEST_CORE_UTIL_MEMORY_COUNTERS_H +#define GRPC_TEST_CORE_UTIL_MEMORY_COUNTERS_H + +struct grpc_memory_counters { + size_t total_size_relative; + size_t total_size_absolute; + size_t total_allocs_relative; + size_t total_allocs_absolute; +}; + +void grpc_memory_counters_init(); +void grpc_memory_counters_destroy(); +struct grpc_memory_counters grpc_memory_counters_snapshot(); + +#endif diff --git a/tools/run_tests/sources_and_headers.json b/tools/run_tests/sources_and_headers.json index b2f2e1eb524..4a7a3c8c86d 100644 --- a/tools/run_tests/sources_and_headers.json +++ b/tools/run_tests/sources_and_headers.json @@ -6164,6 +6164,7 @@ "test/core/end2end/fixtures/proxy.h", "test/core/iomgr/endpoint_tests.h", "test/core/util/grpc_profiler.h", + "test/core/util/memory_counters.h", "test/core/util/mock_endpoint.h", "test/core/util/parse_hexstring.h", "test/core/util/port.h", @@ -6181,6 +6182,8 @@ "test/core/iomgr/endpoint_tests.h", "test/core/util/grpc_profiler.c", "test/core/util/grpc_profiler.h", + "test/core/util/memory_counters.c", + "test/core/util/memory_counters.h", "test/core/util/mock_endpoint.c", "test/core/util/mock_endpoint.h", "test/core/util/parse_hexstring.c", diff --git a/vsprojects/vcxproj/grpc_test_util/grpc_test_util.vcxproj b/vsprojects/vcxproj/grpc_test_util/grpc_test_util.vcxproj index cb033a5aa4a..d1f67ee44e3 100644 --- a/vsprojects/vcxproj/grpc_test_util/grpc_test_util.vcxproj +++ b/vsprojects/vcxproj/grpc_test_util/grpc_test_util.vcxproj @@ -153,6 +153,7 @@ + @@ -176,6 +177,8 @@ + + diff --git a/vsprojects/vcxproj/grpc_test_util/grpc_test_util.vcxproj.filters b/vsprojects/vcxproj/grpc_test_util/grpc_test_util.vcxproj.filters index 81b2a810530..2fee6aab62b 100644 --- a/vsprojects/vcxproj/grpc_test_util/grpc_test_util.vcxproj.filters +++ b/vsprojects/vcxproj/grpc_test_util/grpc_test_util.vcxproj.filters @@ -25,6 +25,9 @@ test\core\util + + test\core\util + test\core\util @@ -63,6 +66,9 @@ test\core\util + + test\core\util + test\core\util diff --git a/vsprojects/vcxproj/grpc_test_util_unsecure/grpc_test_util_unsecure.vcxproj b/vsprojects/vcxproj/grpc_test_util_unsecure/grpc_test_util_unsecure.vcxproj index bb93b2c6f35..336825353c7 100644 --- a/vsprojects/vcxproj/grpc_test_util_unsecure/grpc_test_util_unsecure.vcxproj +++ b/vsprojects/vcxproj/grpc_test_util_unsecure/grpc_test_util_unsecure.vcxproj @@ -151,6 +151,7 @@ + @@ -166,6 +167,8 @@ + + diff --git a/vsprojects/vcxproj/grpc_test_util_unsecure/grpc_test_util_unsecure.vcxproj.filters b/vsprojects/vcxproj/grpc_test_util_unsecure/grpc_test_util_unsecure.vcxproj.filters index 4c4620a288d..5c2b961b676 100644 --- a/vsprojects/vcxproj/grpc_test_util_unsecure/grpc_test_util_unsecure.vcxproj.filters +++ b/vsprojects/vcxproj/grpc_test_util_unsecure/grpc_test_util_unsecure.vcxproj.filters @@ -13,6 +13,9 @@ test\core\util + + test\core\util + test\core\util @@ -45,6 +48,9 @@ test\core\util + + test\core\util + test\core\util From 839cdb51d5521b2792d04a351d864d9c35ab7511 Mon Sep 17 00:00:00 2001 From: "Nicolas \"Pixel\" Noble" Date: Tue, 12 Apr 2016 02:11:10 +0200 Subject: [PATCH 07/38] Adding memory checks for server fuzzer. --- test/core/end2end/fuzzers/server_fuzzer.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/test/core/end2end/fuzzers/server_fuzzer.c b/test/core/end2end/fuzzers/server_fuzzer.c index 393d33033ba..c82df42ebfc 100644 --- a/test/core/end2end/fuzzers/server_fuzzer.c +++ b/test/core/end2end/fuzzers/server_fuzzer.c @@ -35,6 +35,7 @@ #include "src/core/ext/transport/chttp2/transport/chttp2_transport.h" #include "src/core/lib/surface/server.h" +#include "test/core/util/memory_counters.h" #include "test/core/util/mock_endpoint.h" static const bool squelch = true; @@ -48,8 +49,10 @@ static void dont_log(gpr_log_func_args *args) {} int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) { grpc_test_only_set_metadata_hash_seed(0); + struct grpc_memory_counters counters; if (squelch) gpr_set_log_function(dont_log); grpc_init(); + grpc_memory_counters_init(); grpc_exec_ctx exec_ctx = GRPC_EXEC_CTX_INIT; grpc_endpoint *mock_endpoint = grpc_mock_endpoint_create(discard_write); @@ -116,6 +119,9 @@ done: } grpc_server_destroy(server); grpc_completion_queue_destroy(cq); + counters = grpc_memory_counters_snapshot(); + grpc_memory_counters_destroy(); grpc_shutdown(); + GPR_ASSERT(counters.total_size_relative == 0); return 0; } From 2e1a5ac748ca21e87a012eb2c0d86ec273350389 Mon Sep 17 00:00:00 2001 From: "Nicolas \"Pixel\" Noble" Date: Tue, 12 Apr 2016 08:42:34 +0200 Subject: [PATCH 08/38] Initializing counters before grpc_init, and destroying them after grpc_shutdown. --- test/core/end2end/fuzzers/server_fuzzer.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/core/end2end/fuzzers/server_fuzzer.c b/test/core/end2end/fuzzers/server_fuzzer.c index c82df42ebfc..06dc5b8bf34 100644 --- a/test/core/end2end/fuzzers/server_fuzzer.c +++ b/test/core/end2end/fuzzers/server_fuzzer.c @@ -51,8 +51,8 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) { grpc_test_only_set_metadata_hash_seed(0); struct grpc_memory_counters counters; if (squelch) gpr_set_log_function(dont_log); - grpc_init(); grpc_memory_counters_init(); + grpc_init(); grpc_exec_ctx exec_ctx = GRPC_EXEC_CTX_INIT; grpc_endpoint *mock_endpoint = grpc_mock_endpoint_create(discard_write); @@ -120,8 +120,8 @@ done: grpc_server_destroy(server); grpc_completion_queue_destroy(cq); counters = grpc_memory_counters_snapshot(); - grpc_memory_counters_destroy(); grpc_shutdown(); + grpc_memory_counters_destroy(); GPR_ASSERT(counters.total_size_relative == 0); return 0; } From 7c37a687eb884c78d119d4b7903f173cca351255 Mon Sep 17 00:00:00 2001 From: "Nicolas \"Pixel\" Noble" Date: Tue, 12 Apr 2016 19:08:00 +0200 Subject: [PATCH 09/38] Preventing crashes for known realloc behaviors. --- test/core/util/memory_counters.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/test/core/util/memory_counters.c b/test/core/util/memory_counters.c index 85e85237a2f..bebe94e5826 100644 --- a/test/core/util/memory_counters.c +++ b/test/core/util/memory_counters.c @@ -43,8 +43,13 @@ static gpr_mu g_memory_mutex; static struct grpc_memory_counters g_memory_counters; static gpr_allocation_functions g_old_allocs; +static void *guard_malloc(size_t size); +static void *guard_realloc(void *vptr, size_t size); +static void guard_free(void *vptr); + static void *guard_malloc(size_t size) { size_t *ptr; + if (!size) return NULL; gpr_mu_lock(&g_memory_mutex); g_memory_counters.total_size_absolute += size; g_memory_counters.total_size_relative += size; @@ -58,6 +63,13 @@ static void *guard_malloc(size_t size) { static void *guard_realloc(void *vptr, size_t size) { size_t *ptr = vptr; + if (vptr == NULL) { + return guard_malloc(size); + } + if (size == 0) { + guard_free(vptr); + return NULL; + } --ptr; gpr_mu_lock(&g_memory_mutex); g_memory_counters.total_size_absolute += size; @@ -72,6 +84,7 @@ static void *guard_realloc(void *vptr, size_t size) { static void guard_free(void *vptr) { size_t *ptr = vptr; + if (!vptr) return; --ptr; gpr_mu_lock(&g_memory_mutex); g_memory_counters.total_size_relative -= *ptr; From 8079afa46ec457920de3790378984aa168911ce6 Mon Sep 17 00:00:00 2001 From: vjpai Date: Tue, 12 Apr 2016 10:22:05 -0700 Subject: [PATCH 10/38] New CQ for each client call --- src/ruby/lib/grpc/generic/client_stub.rb | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/ruby/lib/grpc/generic/client_stub.rb b/src/ruby/lib/grpc/generic/client_stub.rb index 98e83a83964..a6bb92d72c8 100644 --- a/src/ruby/lib/grpc/generic/client_stub.rb +++ b/src/ruby/lib/grpc/generic/client_stub.rb @@ -85,7 +85,8 @@ module GRPC # when present, this is the default timeout used for calls # # @param host [String] the host the stub connects to - # @param q [Core::CompletionQueue] used to wait for events + # @param q [Core::CompletionQueue] used to wait for events - now deprecated + # since each new active call gets its own separately # @param creds [Core::ChannelCredentials|Symbol] the channel credentials, or # :this_channel_is_insecure # @param channel_override [Core::Channel] a pre-created channel @@ -97,7 +98,6 @@ module GRPC propagate_mask: nil, **kw) fail(TypeError, '!CompletionQueue') unless q.is_a?(Core::CompletionQueue) - @queue = q @ch = ClientStub.setup_channel(channel_override, host, creds, **kw) alt_host = kw[Core::Channel::SSL_TARGET] @host = alt_host.nil? ? host : alt_host @@ -458,14 +458,17 @@ module GRPC if deadline.nil? deadline = from_relative_time(timeout.nil? ? @timeout : timeout) end - call = @ch.create_call(@queue, + # Provide each new client call with its own completion queue + call_queue = Core::CompletionQueue.new + call = @ch.create_call(call_queue, parent, # parent call @propagate_mask, # propagation options method, nil, # host use nil, deadline) call.set_credentials! credentials unless credentials.nil? - ActiveCall.new(call, @queue, marshal, unmarshal, deadline, started: false) + ActiveCall.new(call, call_queue, marshal, unmarshal, deadline, + started: false) end end end From 5b304753965d4179f374b5cded063e79d3d17b0a Mon Sep 17 00:00:00 2001 From: "Nicolas \"Pixel\" Noble" Date: Tue, 12 Apr 2016 19:29:22 +0200 Subject: [PATCH 11/38] Fixing a couple of issues within core. --- src/core/lib/channel/channel_args.c | 3 ++- src/core/lib/surface/server.c | 4 +++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/core/lib/channel/channel_args.c b/src/core/lib/channel/channel_args.c index 28d2d78d00f..97c40e84810 100644 --- a/src/core/lib/channel/channel_args.c +++ b/src/core/lib/channel/channel_args.c @@ -74,7 +74,8 @@ grpc_channel_args *grpc_channel_args_copy_and_add(const grpc_channel_args *src, return dst; } dst->num_args = src_num_args + num_to_add; - dst->args = gpr_malloc(sizeof(grpc_arg) * dst->num_args); + dst->args = + dst->num_args ? gpr_malloc(sizeof(grpc_arg) * dst->num_args) : NULL; for (i = 0; i < src_num_args; i++) { dst->args[i] = copy_arg(&src->args[i]); } diff --git a/src/core/lib/surface/server.c b/src/core/lib/surface/server.c index ad8ee8c7a99..e3b54ac012e 100644 --- a/src/core/lib/surface/server.c +++ b/src/core/lib/surface/server.c @@ -251,7 +251,9 @@ static void channel_broadcaster_init(grpc_server *s, channel_broadcaster *cb) { count++; } cb->num_channels = count; - cb->channels = gpr_malloc(sizeof(*cb->channels) * cb->num_channels); + cb->channels = cb->num_channels + ? gpr_malloc(sizeof(*cb->channels) * cb->num_channels) + : NULL; count = 0; for (c = s->root_channel_data.next; c != &s->root_channel_data; c = c->next) { cb->channels[count++] = c->channel; From 14b5dfeafb1f5f351ea3e114131afd7c63adc4b9 Mon Sep 17 00:00:00 2001 From: "Nicolas \"Pixel\" Noble" Date: Tue, 12 Apr 2016 19:53:07 +0200 Subject: [PATCH 12/38] Properly grabbing counters at the proper time. --- test/core/end2end/fuzzers/server_fuzzer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/core/end2end/fuzzers/server_fuzzer.c b/test/core/end2end/fuzzers/server_fuzzer.c index 06dc5b8bf34..40273711ab7 100644 --- a/test/core/end2end/fuzzers/server_fuzzer.c +++ b/test/core/end2end/fuzzers/server_fuzzer.c @@ -119,8 +119,8 @@ done: } grpc_server_destroy(server); grpc_completion_queue_destroy(cq); - counters = grpc_memory_counters_snapshot(); grpc_shutdown(); + counters = grpc_memory_counters_snapshot(); grpc_memory_counters_destroy(); GPR_ASSERT(counters.total_size_relative == 0); return 0; From a592ffa7451e67d453e42e4f79bc5e7cb498e2b3 Mon Sep 17 00:00:00 2001 From: "Nicolas \"Pixel\" Noble" Date: Tue, 12 Apr 2016 19:56:57 +0200 Subject: [PATCH 13/38] Memory checking the client fuzzer. --- test/core/end2end/fuzzers/client_fuzzer.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/test/core/end2end/fuzzers/client_fuzzer.c b/test/core/end2end/fuzzers/client_fuzzer.c index 4e22e268f04..afcf7638f72 100644 --- a/test/core/end2end/fuzzers/client_fuzzer.c +++ b/test/core/end2end/fuzzers/client_fuzzer.c @@ -36,6 +36,7 @@ #include "src/core/ext/transport/chttp2/transport/chttp2_transport.h" #include "src/core/lib/surface/channel.h" +#include "test/core/util/memory_counters.h" #include "test/core/util/mock_endpoint.h" static const bool squelch = true; @@ -48,7 +49,9 @@ static void dont_log(gpr_log_func_args *args) {} int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) { grpc_test_only_set_metadata_hash_seed(0); + struct grpc_memory_counters counters; if (squelch) gpr_set_log_function(dont_log); + grpc_memory_counters_init(); grpc_init(); grpc_exec_ctx exec_ctx = GRPC_EXEC_CTX_INIT; @@ -150,5 +153,8 @@ done: grpc_byte_buffer_destroy(response_payload_recv); } grpc_shutdown(); + counters = grpc_memory_counters_snapshot(); + grpc_memory_counters_destroy(); + GPR_ASSERT(counters.total_size_relative == 0); return 0; } From 546c2763a8170adb9bf56c8d0335a4fadfa08ff3 Mon Sep 17 00:00:00 2001 From: Vijay Pai Date: Tue, 12 Apr 2016 14:38:51 -0700 Subject: [PATCH 14/38] List facter as a dependence, used by QPS test --- grpc.gemspec | 1 + 1 file changed, 1 insertion(+) diff --git a/grpc.gemspec b/grpc.gemspec index b05f238c43e..b8cfc4e6c4e 100755 --- a/grpc.gemspec +++ b/grpc.gemspec @@ -35,6 +35,7 @@ Gem::Specification.new do |s| s.add_dependency 'googleauth', '~> 0.5.1' s.add_development_dependency 'bundler', '~> 1.9' + s.add_development_dependency 'facter', '~> 2.4' s.add_development_dependency 'logging', '~> 2.0' s.add_development_dependency 'simplecov', '~> 0.9' s.add_development_dependency 'rake', '~> 10.4' From c6e24602c6c6f98c07eee874e0b39988d0766848 Mon Sep 17 00:00:00 2001 From: vjpai Date: Tue, 12 Apr 2016 15:10:44 -0700 Subject: [PATCH 15/38] Put facter development dependence in template as well. --- templates/grpc.gemspec.template | 1 + 1 file changed, 1 insertion(+) diff --git a/templates/grpc.gemspec.template b/templates/grpc.gemspec.template index 701e1c7485b..6f8d1fb9e6c 100644 --- a/templates/grpc.gemspec.template +++ b/templates/grpc.gemspec.template @@ -37,6 +37,7 @@ s.add_dependency 'googleauth', '~> 0.5.1' s.add_development_dependency 'bundler', '~> 1.9' + s.add_development_dependency 'facter', '~> 2.4' s.add_development_dependency 'logging', '~> 2.0' s.add_development_dependency 'simplecov', '~> 0.9' s.add_development_dependency 'rake', '~> 10.4' From b71bf8eb214894d11db77eae78a9abb8c4cebf44 Mon Sep 17 00:00:00 2001 From: "Nicolas \"Pixel\" Noble" Date: Wed, 13 Apr 2016 01:30:13 +0200 Subject: [PATCH 16/38] Revert "Fixing a couple of issues within core." This reverts commit 5b304753965d4179f374b5cded063e79d3d17b0a. --- src/core/lib/channel/channel_args.c | 3 +-- src/core/lib/surface/server.c | 4 +--- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/src/core/lib/channel/channel_args.c b/src/core/lib/channel/channel_args.c index 97c40e84810..28d2d78d00f 100644 --- a/src/core/lib/channel/channel_args.c +++ b/src/core/lib/channel/channel_args.c @@ -74,8 +74,7 @@ grpc_channel_args *grpc_channel_args_copy_and_add(const grpc_channel_args *src, return dst; } dst->num_args = src_num_args + num_to_add; - dst->args = - dst->num_args ? gpr_malloc(sizeof(grpc_arg) * dst->num_args) : NULL; + dst->args = gpr_malloc(sizeof(grpc_arg) * dst->num_args); for (i = 0; i < src_num_args; i++) { dst->args[i] = copy_arg(&src->args[i]); } diff --git a/src/core/lib/surface/server.c b/src/core/lib/surface/server.c index e3b54ac012e..ad8ee8c7a99 100644 --- a/src/core/lib/surface/server.c +++ b/src/core/lib/surface/server.c @@ -251,9 +251,7 @@ static void channel_broadcaster_init(grpc_server *s, channel_broadcaster *cb) { count++; } cb->num_channels = count; - cb->channels = cb->num_channels - ? gpr_malloc(sizeof(*cb->channels) * cb->num_channels) - : NULL; + cb->channels = gpr_malloc(sizeof(*cb->channels) * cb->num_channels); count = 0; for (c = s->root_channel_data.next; c != &s->root_channel_data; c = c->next) { cb->channels[count++] = c->channel; From 23e4baefc2669c271f78402f5e9523d9f60a4cfe Mon Sep 17 00:00:00 2001 From: "Nicolas \"Pixel\" Noble" Date: Wed, 13 Apr 2016 01:35:42 +0200 Subject: [PATCH 17/38] Enforcing undefined behavior of malloc. --- src/core/lib/support/alloc.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/core/lib/support/alloc.c b/src/core/lib/support/alloc.c index 020917f79c8..618c3f6acd8 100644 --- a/src/core/lib/support/alloc.c +++ b/src/core/lib/support/alloc.c @@ -53,6 +53,7 @@ void gpr_set_allocation_functions(gpr_allocation_functions functions) { void *gpr_malloc(size_t size) { void *p; + if (size == 0) return NULL; GPR_TIMER_BEGIN("gpr_malloc", 0); p = g_alloc_functions.malloc_fn(size); if (!p) { @@ -69,6 +70,7 @@ void gpr_free(void *p) { } void *gpr_realloc(void *p, size_t size) { + if ((size == 0) && (p == NULL)) return NULL; GPR_TIMER_BEGIN("gpr_realloc", 0); p = g_alloc_functions.realloc_fn(p, size); if (!p) { From 478bd4449bf39a99abc0fa09749cd71c67a36241 Mon Sep 17 00:00:00 2001 From: Sree Kuchibhotla Date: Tue, 12 Apr 2016 17:40:24 -0700 Subject: [PATCH 18/38] Misc changes to stress test scripts --- tools/gcp/stress_test/stress_test_utils.py | 22 ++++++++++++------- tools/run_tests/stress_test/configs/asan.json | 6 ++--- tools/run_tests/stress_test/configs/opt.json | 2 +- tools/run_tests/stress_test/configs/tsan.json | 6 ++--- tools/run_tests/stress_test/run_on_gke.py | 18 +++++++++++++++ 5 files changed, 39 insertions(+), 15 deletions(-) diff --git a/tools/gcp/stress_test/stress_test_utils.py b/tools/gcp/stress_test/stress_test_utils.py index 6c7fe44dc12..19d59c0df10 100755 --- a/tools/gcp/stress_test/stress_test_utils.py +++ b/tools/gcp/stress_test/stress_test_utils.py @@ -103,23 +103,29 @@ class BigQueryHelper: return bq_utils.insert_rows(self.bq, self.project_id, self.dataset_id, self.qps_table_id, [row]) - def check_if_any_tests_failed(self, num_query_retries=3): + def check_if_any_tests_failed(self, num_query_retries=3, timeout_msec=30000): query = ('SELECT event_type FROM %s.%s WHERE run_id = \'%s\' AND ' 'event_type="%s"') % (self.dataset_id, self.summary_table_id, self.run_id, EventType.FAILURE) + page = None try: query_job = bq_utils.sync_query_job(self.bq, self.project_id, query) + job_id = query_job['jobReference']['jobId'] + project_id = query_job['jobReference']['projectId'] page = self.bq.jobs().getQueryResults( - **query_job['jobReference']).execute(num_retries=num_query_retries) + projectId=project_id, + jobId=job_id, + timeoutMs=timeout_msec).execute(num_retries=num_query_retries) + + if not page['jobComplete']: + print('TIMEOUT ERROR: The query %s timed out. Current timeout value is' + ' %d msec. Returning False (i.e assuming there are no failures)' + ) % (query, timeoout_msec) + return False + num_failures = int(page['totalRows']) print 'num rows: ', num_failures return num_failures > 0 - # TODO (sreek): Cleanup the following lines once we have a better idea of - # why we sometimes get KeyError exceptions in long running test cases - except KeyError: - print 'KeyError in check_if_any_tests_failed()' - print 'Query:', query - print 'Query result page:', page except: print 'Exception in check_if_any_tests_failed(). Info: ', sys.exc_info() print 'Query: ', query diff --git a/tools/run_tests/stress_test/configs/asan.json b/tools/run_tests/stress_test/configs/asan.json index 768088d93dd..c5588553147 100644 --- a/tools/run_tests/stress_test/configs/asan.json +++ b/tools/run_tests/stress_test/configs/asan.json @@ -11,13 +11,13 @@ "baseTemplates": { "default": { "wrapperScriptPath": "/var/local/git/grpc/tools/gcp/stress_test/run_client.py", - "pollIntervalSecs": 60, + "pollIntervalSecs": 120, "clientArgs": { "num_channels_per_server":5, "num_stubs_per_channel":10, "test_cases": "empty_unary:1,large_unary:1,client_streaming:1,server_streaming:1,empty_stream:1", "metrics_port": 8081, - "metrics_collection_interval_secs":60 + "metrics_collection_interval_secs":120 }, "metricsPort": 8081, "metricsArgs": { @@ -66,7 +66,7 @@ "stress-client-asan": { "clientTemplate": "cxx_client_asan", "dockerImage": "grpc_stress_cxx_asan", - "numInstances": 20, + "numInstances": 5, "serverPodSpec": "stress-server-asan" } } diff --git a/tools/run_tests/stress_test/configs/opt.json b/tools/run_tests/stress_test/configs/opt.json index ffd4a704c34..75505186f20 100644 --- a/tools/run_tests/stress_test/configs/opt.json +++ b/tools/run_tests/stress_test/configs/opt.json @@ -66,7 +66,7 @@ "stress-client-opt": { "clientTemplate": "cxx_client_opt", "dockerImage": "grpc_stress_cxx_opt", - "numInstances": 10, + "numInstances": 15, "serverPodSpec": "stress-server-opt" } } diff --git a/tools/run_tests/stress_test/configs/tsan.json b/tools/run_tests/stress_test/configs/tsan.json index f8d3f878e16..a7ec08313d6 100644 --- a/tools/run_tests/stress_test/configs/tsan.json +++ b/tools/run_tests/stress_test/configs/tsan.json @@ -11,13 +11,13 @@ "baseTemplates": { "default": { "wrapperScriptPath": "/var/local/git/grpc/tools/gcp/stress_test/run_client.py", - "pollIntervalSecs": 60, + "pollIntervalSecs": 120, "clientArgs": { "num_channels_per_server":5, "num_stubs_per_channel":10, "test_cases": "empty_unary:1,large_unary:1,client_streaming:1,server_streaming:1,empty_stream:1", "metrics_port": 8081, - "metrics_collection_interval_secs":60 + "metrics_collection_interval_secs":120 }, "metricsPort": 8081, "metricsArgs": { @@ -66,7 +66,7 @@ "stress-client-tsan": { "clientTemplate": "cxx_client_tsan", "dockerImage": "grpc_stress_cxx_tsan", - "numInstances": 20, + "numInstances": 5, "serverPodSpec": "stress-server-tsan" } } diff --git a/tools/run_tests/stress_test/run_on_gke.py b/tools/run_tests/stress_test/run_on_gke.py index db3ba28346d..916c890cbd5 100755 --- a/tools/run_tests/stress_test/run_on_gke.py +++ b/tools/run_tests/stress_test/run_on_gke.py @@ -604,6 +604,17 @@ def run_tests(config): return is_success +def tear_down(config): + gke = Gke(config.global_settings.gcp_project_id, '', '', + config.global_settings.summary_table_id, + config.global_settings.qps_table_id, + config.global_settings.kubernetes_proxy_port) + for name, server_pod_spec in config.server_pod_specs_dict.iteritems(): + gke.delete_servers(server_pod_spec) + for name, client_pod_spec in config.client_pod_specs_dict.iteritems(): + gke.delete_clients(client_pod_spec) + + argp = argparse.ArgumentParser( description='Launch stress tests in GKE', formatter_class=argparse.ArgumentDefaultsHelpFormatter) @@ -614,6 +625,7 @@ argp.add_argument('--config_file', required=True, type=str, help='The test config file') +argp.add_argument('--tear_down', action='store_true', default=False) if __name__ == '__main__': args = argp.parse_args() @@ -636,5 +648,11 @@ if __name__ == '__main__': os.path.dirname(sys.argv[0]), '../../..')) os.chdir(grpc_root) + # Note that tear_down is only in cases where we want to manually tear down a + # test that for some reason run_tests() could not cleanup + if args.tear_down: + tear_down(config) + sys.exit(1) + if not run_tests(config): sys.exit(1) From af09a735714a48356d264c54f244a20d133c7ba6 Mon Sep 17 00:00:00 2001 From: Wouter van Oortmerssen Date: Wed, 9 Mar 2016 17:03:21 -0800 Subject: [PATCH 19/38] Refactored C++ code generator to not directly depend on protobuf. This opens the door for other serializers (such as e.g. FlatBuffers) to share this code generator when using GRPC. --- src/compiler/cpp_generator.cc | 364 +++++++++++++++------------------- src/compiler/cpp_generator.h | 93 +++++++-- src/compiler/cpp_plugin.cc | 117 ++++++++++- 3 files changed, 342 insertions(+), 232 deletions(-) diff --git a/src/compiler/cpp_generator.cc b/src/compiler/cpp_generator.cc index 08b1123a512..b1336993067 100644 --- a/src/compiler/cpp_generator.cc +++ b/src/compiler/cpp_generator.cc @@ -34,9 +34,6 @@ #include #include "src/compiler/cpp_generator.h" -#include "src/compiler/cpp_generator_helpers.h" - -#include "src/compiler/config.h" #include @@ -50,22 +47,6 @@ grpc::string as_string(T x) { return out.str(); } -bool NoStreaming(const grpc::protobuf::MethodDescriptor *method) { - return !method->client_streaming() && !method->server_streaming(); -} - -bool ClientOnlyStreaming(const grpc::protobuf::MethodDescriptor *method) { - return method->client_streaming() && !method->server_streaming(); -} - -bool ServerOnlyStreaming(const grpc::protobuf::MethodDescriptor *method) { - return !method->client_streaming() && method->server_streaming(); -} - -bool BidiStreaming(const grpc::protobuf::MethodDescriptor *method) { - return method->client_streaming() && method->server_streaming(); -} - grpc::string FilenameIdentifier(const grpc::string &filename) { grpc::string result; for (unsigned i = 0; i < filename.size(); i++) { @@ -86,7 +67,7 @@ grpc::string FilenameIdentifier(const grpc::string &filename) { template T *array_end(T (&array)[N]) { return array + N; } -void PrintIncludes(grpc::protobuf::io::Printer *printer, const std::vector& headers, const Parameters ¶ms) { +void PrintIncludes(Printer *printer, const std::vector& headers, const Parameters ¶ms) { std::map vars; vars["l"] = params.use_system_headers ? '<' : '"'; @@ -105,39 +86,36 @@ void PrintIncludes(grpc::protobuf::io::Printer *printer, const std::vectorCreatePrinter(&output); std::map vars; - vars["filename"] = file->name(); - vars["filename_identifier"] = FilenameIdentifier(file->name()); - vars["filename_base"] = grpc_generator::StripProto(file->name()); + vars["filename"] = file->filename(); + vars["filename_identifier"] = FilenameIdentifier(file->filename()); + vars["filename_base"] = file->filename_without_ext(); - printer.Print(vars, "// Generated by the gRPC protobuf plugin.\n"); - printer.Print(vars, + printer->Print(vars, "// Generated by the gRPC protobuf plugin.\n"); + printer->Print(vars, "// If you make any local change, they will be lost.\n"); - printer.Print(vars, "// source: $filename$\n"); - printer.Print(vars, "#ifndef GRPC_$filename_identifier$__INCLUDED\n"); - printer.Print(vars, "#define GRPC_$filename_identifier$__INCLUDED\n"); - printer.Print(vars, "\n"); - printer.Print(vars, "#include \"$filename_base$.pb.h\"\n"); - printer.Print(vars, "\n"); + printer->Print(vars, "// source: $filename$\n"); + printer->Print(vars, "#ifndef GRPC_$filename_identifier$__INCLUDED\n"); + printer->Print(vars, "#define GRPC_$filename_identifier$__INCLUDED\n"); + printer->Print(vars, "\n"); + printer->Print(vars, "#include \"$filename_base$.pb.h\"\n"); + printer->Print(vars, "\n"); } return output; } -grpc::string GetHeaderIncludes(const grpc::protobuf::FileDescriptor *file, +grpc::string GetHeaderIncludes(File *file, const Parameters ¶ms) { grpc::string output; { // Scope the output stream so it closes and finalizes output to the string. - grpc::protobuf::io::StringOutputStream output_stream(&output); - grpc::protobuf::io::Printer printer(&output_stream, '$'); + auto printer = file->CreatePrinter(&output); std::map vars; static const char *headers_strs[] = { @@ -151,42 +129,38 @@ grpc::string GetHeaderIncludes(const grpc::protobuf::FileDescriptor *file, "grpc++/impl/codegen/sync_stream.h" }; std::vector headers(headers_strs, array_end(headers_strs)); - PrintIncludes(&printer, headers, params); - printer.Print(vars, "\n"); - printer.Print(vars, "namespace grpc {\n"); - printer.Print(vars, "class CompletionQueue;\n"); - printer.Print(vars, "class Channel;\n"); - printer.Print(vars, "class RpcService;\n"); - printer.Print(vars, "class ServerCompletionQueue;\n"); - printer.Print(vars, "class ServerContext;\n"); - printer.Print(vars, "} // namespace grpc\n\n"); + PrintIncludes(printer.get(), headers, params); + printer->Print(vars, "\n"); + printer->Print(vars, "namespace grpc {\n"); + printer->Print(vars, "class CompletionQueue;\n"); + printer->Print(vars, "class Channel;\n"); + printer->Print(vars, "class RpcService;\n"); + printer->Print(vars, "class ServerCompletionQueue;\n"); + printer->Print(vars, "class ServerContext;\n"); + printer->Print(vars, "} // namespace grpc\n\n"); if (!file->package().empty()) { - std::vector parts = - grpc_generator::tokenize(file->package(), "."); + std::vector parts = file->package_parts(); for (auto part = parts.begin(); part != parts.end(); part++) { vars["part"] = *part; - printer.Print(vars, "namespace $part$ {\n"); + printer->Print(vars, "namespace $part$ {\n"); } - printer.Print(vars, "\n"); + printer->Print(vars, "\n"); } } return output; } void PrintHeaderClientMethodInterfaces( - grpc::protobuf::io::Printer *printer, - const grpc::protobuf::MethodDescriptor *method, + Printer *printer, const Method *method, std::map *vars, bool is_public) { (*vars)["Method"] = method->name(); - (*vars)["Request"] = - grpc_cpp_generator::ClassName(method->input_type(), true); - (*vars)["Response"] = - grpc_cpp_generator::ClassName(method->output_type(), true); + (*vars)["Request"] = method->input_type_name(); + (*vars)["Response"] = method->output_type_name(); if (is_public) { - if (NoStreaming(method)) { + if (method->NoStreaming()) { printer->Print( *vars, "virtual ::grpc::Status $Method$(::grpc::ClientContext* context, " @@ -204,7 +178,7 @@ void PrintHeaderClientMethodInterfaces( "Async$Method$Raw(context, request, cq));\n"); printer->Outdent(); printer->Print("}\n"); - } else if (ClientOnlyStreaming(method)) { + } else if (method->ClientOnlyStreaming()) { printer->Print( *vars, "std::unique_ptr< ::grpc::ClientWriterInterface< $Request$>>" @@ -230,7 +204,7 @@ void PrintHeaderClientMethodInterfaces( "Async$Method$Raw(context, response, cq, tag));\n"); printer->Outdent(); printer->Print("}\n"); - } else if (ServerOnlyStreaming(method)) { + } else if (method->ServerOnlyStreaming()) { printer->Print( *vars, "std::unique_ptr< ::grpc::ClientReaderInterface< $Response$>>" @@ -256,7 +230,7 @@ void PrintHeaderClientMethodInterfaces( "Async$Method$Raw(context, request, cq, tag));\n"); printer->Outdent(); printer->Print("}\n"); - } else if (BidiStreaming(method)) { + } else if (method->BidiStreaming()) { printer->Print(*vars, "std::unique_ptr< ::grpc::ClientReaderWriterInterface< " "$Request$, $Response$>> " @@ -285,14 +259,14 @@ void PrintHeaderClientMethodInterfaces( printer->Print("}\n"); } } else { - if (NoStreaming(method)) { + if (method->NoStreaming()) { printer->Print( *vars, "virtual ::grpc::ClientAsyncResponseReaderInterface< $Response$>* " "Async$Method$Raw(::grpc::ClientContext* context, " "const $Request$& request, " "::grpc::CompletionQueue* cq) = 0;\n"); - } else if (ClientOnlyStreaming(method)) { + } else if (method->ClientOnlyStreaming()) { printer->Print( *vars, "virtual ::grpc::ClientWriterInterface< $Request$>*" @@ -303,7 +277,7 @@ void PrintHeaderClientMethodInterfaces( " Async$Method$Raw(::grpc::ClientContext* context, " "$Response$* response, " "::grpc::CompletionQueue* cq, void* tag) = 0;\n"); - } else if (ServerOnlyStreaming(method)) { + } else if (method->ServerOnlyStreaming()) { printer->Print( *vars, "virtual ::grpc::ClientReaderInterface< $Response$>* $Method$Raw(" @@ -314,7 +288,7 @@ void PrintHeaderClientMethodInterfaces( "Async$Method$Raw(" "::grpc::ClientContext* context, const $Request$& request, " "::grpc::CompletionQueue* cq, void* tag) = 0;\n"); - } else if (BidiStreaming(method)) { + } else if (method->BidiStreaming()) { printer->Print(*vars, "virtual ::grpc::ClientReaderWriterInterface< $Request$, " "$Response$>* " @@ -328,17 +302,15 @@ void PrintHeaderClientMethodInterfaces( } } -void PrintHeaderClientMethod(grpc::protobuf::io::Printer *printer, - const grpc::protobuf::MethodDescriptor *method, +void PrintHeaderClientMethod(Printer *printer, + const Method *method, std::map *vars, bool is_public) { (*vars)["Method"] = method->name(); - (*vars)["Request"] = - grpc_cpp_generator::ClassName(method->input_type(), true); - (*vars)["Response"] = - grpc_cpp_generator::ClassName(method->output_type(), true); + (*vars)["Request"] = method->input_type_name(); + (*vars)["Response"] = method->output_type_name(); if (is_public) { - if (NoStreaming(method)) { + if (method->NoStreaming()) { printer->Print( *vars, "::grpc::Status $Method$(::grpc::ClientContext* context, " @@ -356,7 +328,7 @@ void PrintHeaderClientMethod(grpc::protobuf::io::Printer *printer, "Async$Method$Raw(context, request, cq));\n"); printer->Outdent(); printer->Print("}\n"); - } else if (ClientOnlyStreaming(method)) { + } else if (method->ClientOnlyStreaming()) { printer->Print( *vars, "std::unique_ptr< ::grpc::ClientWriter< $Request$>>" @@ -380,7 +352,7 @@ void PrintHeaderClientMethod(grpc::protobuf::io::Printer *printer, "Async$Method$Raw(context, response, cq, tag));\n"); printer->Outdent(); printer->Print("}\n"); - } else if (ServerOnlyStreaming(method)) { + } else if (method->ServerOnlyStreaming()) { printer->Print( *vars, "std::unique_ptr< ::grpc::ClientReader< $Response$>>" @@ -406,7 +378,7 @@ void PrintHeaderClientMethod(grpc::protobuf::io::Printer *printer, "Async$Method$Raw(context, request, cq, tag));\n"); printer->Outdent(); printer->Print("}\n"); - } else if (BidiStreaming(method)) { + } else if (method->BidiStreaming()) { printer->Print( *vars, "std::unique_ptr< ::grpc::ClientReaderWriter< $Request$, $Response$>>" @@ -432,13 +404,13 @@ void PrintHeaderClientMethod(grpc::protobuf::io::Printer *printer, printer->Print("}\n"); } } else { - if (NoStreaming(method)) { + if (method->NoStreaming()) { printer->Print(*vars, "::grpc::ClientAsyncResponseReader< $Response$>* " "Async$Method$Raw(::grpc::ClientContext* context, " "const $Request$& request, " "::grpc::CompletionQueue* cq) GRPC_OVERRIDE;\n"); - } else if (ClientOnlyStreaming(method)) { + } else if (method->ClientOnlyStreaming()) { printer->Print(*vars, "::grpc::ClientWriter< $Request$>* $Method$Raw(" "::grpc::ClientContext* context, $Response$* response) " @@ -448,7 +420,7 @@ void PrintHeaderClientMethod(grpc::protobuf::io::Printer *printer, "::grpc::ClientAsyncWriter< $Request$>* Async$Method$Raw(" "::grpc::ClientContext* context, $Response$* response, " "::grpc::CompletionQueue* cq, void* tag) GRPC_OVERRIDE;\n"); - } else if (ServerOnlyStreaming(method)) { + } else if (method->ServerOnlyStreaming()) { printer->Print(*vars, "::grpc::ClientReader< $Response$>* $Method$Raw(" "::grpc::ClientContext* context, const $Request$& request)" @@ -458,7 +430,7 @@ void PrintHeaderClientMethod(grpc::protobuf::io::Printer *printer, "::grpc::ClientAsyncReader< $Response$>* Async$Method$Raw(" "::grpc::ClientContext* context, const $Request$& request, " "::grpc::CompletionQueue* cq, void* tag) GRPC_OVERRIDE;\n"); - } else if (BidiStreaming(method)) { + } else if (method->BidiStreaming()) { printer->Print( *vars, "::grpc::ClientReaderWriter< $Request$, $Response$>* " @@ -472,38 +444,34 @@ void PrintHeaderClientMethod(grpc::protobuf::io::Printer *printer, } } -void PrintHeaderClientMethodData(grpc::protobuf::io::Printer *printer, - const grpc::protobuf::MethodDescriptor *method, +void PrintHeaderClientMethodData(Printer *printer, const Method *method, std::map *vars) { (*vars)["Method"] = method->name(); printer->Print(*vars, "const ::grpc::RpcMethod rpcmethod_$Method$_;\n"); } -void PrintHeaderServerMethodSync(grpc::protobuf::io::Printer *printer, - const grpc::protobuf::MethodDescriptor *method, +void PrintHeaderServerMethodSync(Printer *printer, const Method *method, std::map *vars) { (*vars)["Method"] = method->name(); - (*vars)["Request"] = - grpc_cpp_generator::ClassName(method->input_type(), true); - (*vars)["Response"] = - grpc_cpp_generator::ClassName(method->output_type(), true); - if (NoStreaming(method)) { + (*vars)["Request"] = method->input_type_name(); + (*vars)["Response"] = method->output_type_name(); + if (method->NoStreaming()) { printer->Print(*vars, "virtual ::grpc::Status $Method$(" "::grpc::ServerContext* context, const $Request$* request, " "$Response$* response);\n"); - } else if (ClientOnlyStreaming(method)) { + } else if (method->ClientOnlyStreaming()) { printer->Print(*vars, "virtual ::grpc::Status $Method$(" "::grpc::ServerContext* context, " "::grpc::ServerReader< $Request$>* reader, " "$Response$* response);\n"); - } else if (ServerOnlyStreaming(method)) { + } else if (method->ServerOnlyStreaming()) { printer->Print(*vars, "virtual ::grpc::Status $Method$(" "::grpc::ServerContext* context, const $Request$* request, " "::grpc::ServerWriter< $Response$>* writer);\n"); - } else if (BidiStreaming(method)) { + } else if (method->BidiStreaming()) { printer->Print( *vars, "virtual ::grpc::Status $Method$(" @@ -514,20 +482,18 @@ void PrintHeaderServerMethodSync(grpc::protobuf::io::Printer *printer, } void PrintHeaderServerMethodAsync( - grpc::protobuf::io::Printer *printer, - const grpc::protobuf::MethodDescriptor *method, + Printer *printer, + const Method *method, std::map *vars) { (*vars)["Method"] = method->name(); - (*vars)["Request"] = - grpc_cpp_generator::ClassName(method->input_type(), true); - (*vars)["Response"] = - grpc_cpp_generator::ClassName(method->output_type(), true); + (*vars)["Request"] = method->input_type_name(); + (*vars)["Response"] = method->output_type_name(); printer->Print(*vars, "template \n"); printer->Print(*vars, "class WithAsyncMethod_$Method$ : public BaseClass {\n"); printer->Print( " private:\n" - " void BaseClassMustBeDerivedFromService(Service *service) {}\n"); + " void BaseClassMustBeDerivedFromService(const Service *service) {}\n"); printer->Print(" public:\n"); printer->Indent(); printer->Print(*vars, @@ -538,7 +504,7 @@ void PrintHeaderServerMethodAsync( "~WithAsyncMethod_$Method$() GRPC_OVERRIDE {\n" " BaseClassMustBeDerivedFromService(this);\n" "}\n"); - if (NoStreaming(method)) { + if (method->NoStreaming()) { printer->Print( *vars, "// disable synchronous version of this method\n" @@ -559,7 +525,7 @@ void PrintHeaderServerMethodAsync( " ::grpc::Service::RequestAsyncUnary($Idx$, context, " "request, response, new_call_cq, notification_cq, tag);\n"); printer->Print("}\n"); - } else if (ClientOnlyStreaming(method)) { + } else if (method->ClientOnlyStreaming()) { printer->Print( *vars, "// disable synchronous version of this method\n" @@ -581,7 +547,7 @@ void PrintHeaderServerMethodAsync( " ::grpc::Service::RequestAsyncClientStreaming($Idx$, " "context, reader, new_call_cq, notification_cq, tag);\n"); printer->Print("}\n"); - } else if (ServerOnlyStreaming(method)) { + } else if (method->ServerOnlyStreaming()) { printer->Print( *vars, "// disable synchronous version of this method\n" @@ -604,7 +570,7 @@ void PrintHeaderServerMethodAsync( " ::grpc::Service::RequestAsyncServerStreaming($Idx$, " "context, request, writer, new_call_cq, notification_cq, tag);\n"); printer->Print("}\n"); - } else if (BidiStreaming(method)) { + } else if (method->BidiStreaming()) { printer->Print( *vars, "// disable synchronous version of this method\n" @@ -632,20 +598,18 @@ void PrintHeaderServerMethodAsync( } void PrintHeaderServerMethodGeneric( - grpc::protobuf::io::Printer *printer, - const grpc::protobuf::MethodDescriptor *method, + Printer *printer, + const Method *method, std::map *vars) { (*vars)["Method"] = method->name(); - (*vars)["Request"] = - grpc_cpp_generator::ClassName(method->input_type(), true); - (*vars)["Response"] = - grpc_cpp_generator::ClassName(method->output_type(), true); + (*vars)["Request"] = method->input_type_name(); + (*vars)["Response"] = method->output_type_name(); printer->Print(*vars, "template \n"); printer->Print(*vars, "class WithGenericMethod_$Method$ : public BaseClass {\n"); printer->Print( " private:\n" - " void BaseClassMustBeDerivedFromService(Service *service) {}\n"); + " void BaseClassMustBeDerivedFromService(const Service *service) {}\n"); printer->Print(" public:\n"); printer->Indent(); printer->Print(*vars, @@ -656,7 +620,7 @@ void PrintHeaderServerMethodGeneric( "~WithGenericMethod_$Method$() GRPC_OVERRIDE {\n" " BaseClassMustBeDerivedFromService(this);\n" "}\n"); - if (NoStreaming(method)) { + if (method->NoStreaming()) { printer->Print( *vars, "// disable synchronous version of this method\n" @@ -666,7 +630,7 @@ void PrintHeaderServerMethodGeneric( " abort();\n" " return ::grpc::Status(::grpc::StatusCode::UNIMPLEMENTED, \"\");\n" "}\n"); - } else if (ClientOnlyStreaming(method)) { + } else if (method->ClientOnlyStreaming()) { printer->Print( *vars, "// disable synchronous version of this method\n" @@ -677,7 +641,7 @@ void PrintHeaderServerMethodGeneric( " abort();\n" " return ::grpc::Status(::grpc::StatusCode::UNIMPLEMENTED, \"\");\n" "}\n"); - } else if (ServerOnlyStreaming(method)) { + } else if (method->ServerOnlyStreaming()) { printer->Print( *vars, "// disable synchronous version of this method\n" @@ -688,7 +652,7 @@ void PrintHeaderServerMethodGeneric( " abort();\n" " return ::grpc::Status(::grpc::StatusCode::UNIMPLEMENTED, \"\");\n" "}\n"); - } else if (BidiStreaming(method)) { + } else if (method->BidiStreaming()) { printer->Print( *vars, "// disable synchronous version of this method\n" @@ -704,8 +668,8 @@ void PrintHeaderServerMethodGeneric( printer->Print(*vars, "};\n"); } -void PrintHeaderService(grpc::protobuf::io::Printer *printer, - const grpc::protobuf::ServiceDescriptor *service, +void PrintHeaderService(Printer *printer, + const Service *service, std::map *vars) { (*vars)["Service"] = service->name(); @@ -721,13 +685,13 @@ void PrintHeaderService(grpc::protobuf::io::Printer *printer, printer->Indent(); printer->Print("virtual ~StubInterface() {}\n"); for (int i = 0; i < service->method_count(); ++i) { - PrintHeaderClientMethodInterfaces(printer, service->method(i), vars, true); + PrintHeaderClientMethodInterfaces(printer, service->method(i).get(), vars, true); } printer->Outdent(); printer->Print("private:\n"); printer->Indent(); for (int i = 0; i < service->method_count(); ++i) { - PrintHeaderClientMethodInterfaces(printer, service->method(i), vars, false); + PrintHeaderClientMethodInterfaces(printer, service->method(i).get(), vars, false); } printer->Outdent(); printer->Print("};\n"); @@ -737,17 +701,17 @@ void PrintHeaderService(grpc::protobuf::io::Printer *printer, printer->Indent(); printer->Print("Stub(const std::shared_ptr< ::grpc::ChannelInterface>& channel);\n"); for (int i = 0; i < service->method_count(); ++i) { - PrintHeaderClientMethod(printer, service->method(i), vars, true); + PrintHeaderClientMethod(printer, service->method(i).get(), vars, true); } printer->Outdent(); printer->Print("\n private:\n"); printer->Indent(); printer->Print("std::shared_ptr< ::grpc::ChannelInterface> channel_;\n"); for (int i = 0; i < service->method_count(); ++i) { - PrintHeaderClientMethod(printer, service->method(i), vars, false); + PrintHeaderClientMethod(printer, service->method(i).get(), vars, false); } for (int i = 0; i < service->method_count(); ++i) { - PrintHeaderClientMethodData(printer, service->method(i), vars); + PrintHeaderClientMethodData(printer, service->method(i).get(), vars); } printer->Outdent(); printer->Print("};\n"); @@ -766,7 +730,7 @@ void PrintHeaderService(grpc::protobuf::io::Printer *printer, printer->Print("Service();\n"); printer->Print("virtual ~Service();\n"); for (int i = 0; i < service->method_count(); ++i) { - PrintHeaderServerMethodSync(printer, service->method(i), vars); + PrintHeaderServerMethodSync(printer, service->method(i).get(), vars); } printer->Outdent(); printer->Print("};\n"); @@ -774,13 +738,13 @@ void PrintHeaderService(grpc::protobuf::io::Printer *printer, // Server side - Asynchronous for (int i = 0; i < service->method_count(); ++i) { (*vars)["Idx"] = as_string(i); - PrintHeaderServerMethodAsync(printer, service->method(i), vars); + PrintHeaderServerMethodAsync(printer, service->method(i).get(), vars); } printer->Print("typedef "); for (int i = 0; i < service->method_count(); ++i) { - (*vars)["method_name"] = service->method(i)->name(); + (*vars)["method_name"] = service->method(i).get()->name(); printer->Print(*vars, "WithAsyncMethod_$method_name$<"); } printer->Print("Service"); @@ -792,20 +756,19 @@ void PrintHeaderService(grpc::protobuf::io::Printer *printer, // Server side - Generic for (int i = 0; i < service->method_count(); ++i) { (*vars)["Idx"] = as_string(i); - PrintHeaderServerMethodGeneric(printer, service->method(i), vars); + PrintHeaderServerMethodGeneric(printer, service->method(i).get(), vars); } printer->Outdent(); printer->Print("};\n"); } -grpc::string GetHeaderServices(const grpc::protobuf::FileDescriptor *file, +grpc::string GetHeaderServices(File *file, const Parameters ¶ms) { grpc::string output; { // Scope the output stream so it closes and finalizes output to the string. - grpc::protobuf::io::StringOutputStream output_stream(&output); - grpc::protobuf::io::Printer printer(&output_stream, '$'); + auto printer = file->CreatePrinter(&output); std::map vars; // Package string is empty or ends with a dot. It is used to fully qualify // method names. @@ -816,80 +779,76 @@ grpc::string GetHeaderServices(const grpc::protobuf::FileDescriptor *file, if (!params.services_namespace.empty()) { vars["services_namespace"] = params.services_namespace; - printer.Print(vars, "\nnamespace $services_namespace$ {\n\n"); + printer->Print(vars, "\nnamespace $services_namespace$ {\n\n"); } for (int i = 0; i < file->service_count(); ++i) { - PrintHeaderService(&printer, file->service(i), &vars); - printer.Print("\n"); + PrintHeaderService(printer.get(), file->service(i).get(), &vars); + printer->Print("\n"); } if (!params.services_namespace.empty()) { - printer.Print(vars, "} // namespace $services_namespace$\n\n"); + printer->Print(vars, "} // namespace $services_namespace$\n\n"); } } return output; } -grpc::string GetHeaderEpilogue(const grpc::protobuf::FileDescriptor *file, +grpc::string GetHeaderEpilogue(File *file, const Parameters ¶ms) { grpc::string output; { // Scope the output stream so it closes and finalizes output to the string. - grpc::protobuf::io::StringOutputStream output_stream(&output); - grpc::protobuf::io::Printer printer(&output_stream, '$'); + auto printer = file->CreatePrinter(&output); std::map vars; - vars["filename"] = file->name(); - vars["filename_identifier"] = FilenameIdentifier(file->name()); + vars["filename"] = file->filename(); + vars["filename_identifier"] = FilenameIdentifier(file->filename()); if (!file->package().empty()) { - std::vector parts = - grpc_generator::tokenize(file->package(), "."); + std::vector parts = file->package_parts(); for (auto part = parts.rbegin(); part != parts.rend(); part++) { vars["part"] = *part; - printer.Print(vars, "} // namespace $part$\n"); + printer->Print(vars, "} // namespace $part$\n"); } - printer.Print(vars, "\n"); + printer->Print(vars, "\n"); } - printer.Print(vars, "\n"); - printer.Print(vars, "#endif // GRPC_$filename_identifier$__INCLUDED\n"); + printer->Print(vars, "\n"); + printer->Print(vars, "#endif // GRPC_$filename_identifier$__INCLUDED\n"); } return output; } -grpc::string GetSourcePrologue(const grpc::protobuf::FileDescriptor *file, +grpc::string GetSourcePrologue(File *file, const Parameters ¶ms) { grpc::string output; { // Scope the output stream so it closes and finalizes output to the string. - grpc::protobuf::io::StringOutputStream output_stream(&output); - grpc::protobuf::io::Printer printer(&output_stream, '$'); + auto printer = file->CreatePrinter(&output); std::map vars; - vars["filename"] = file->name(); - vars["filename_base"] = grpc_generator::StripProto(file->name()); + vars["filename"] = file->filename(); + vars["filename_base"] = file->filename_without_ext(); - printer.Print(vars, "// Generated by the gRPC protobuf plugin.\n"); - printer.Print(vars, + printer->Print(vars, "// Generated by the gRPC protobuf plugin.\n"); + printer->Print(vars, "// If you make any local change, they will be lost.\n"); - printer.Print(vars, "// source: $filename$\n\n"); - printer.Print(vars, "#include \"$filename_base$.pb.h\"\n"); - printer.Print(vars, "#include \"$filename_base$.grpc.pb.h\"\n"); - printer.Print(vars, "\n"); + printer->Print(vars, "// source: $filename$\n\n"); + printer->Print(vars, "#include \"$filename_base$.pb.h\"\n"); + printer->Print(vars, "#include \"$filename_base$.grpc.pb.h\"\n"); + printer->Print(vars, "\n"); } return output; } -grpc::string GetSourceIncludes(const grpc::protobuf::FileDescriptor *file, +grpc::string GetSourceIncludes(File *file, const Parameters ¶ms) { grpc::string output; { // Scope the output stream so it closes and finalizes output to the string. - grpc::protobuf::io::StringOutputStream output_stream(&output); - grpc::protobuf::io::Printer printer(&output_stream, '$'); + auto printer = file->CreatePrinter(&output); std::map vars; static const char *headers_strs[] = { @@ -903,32 +862,29 @@ grpc::string GetSourceIncludes(const grpc::protobuf::FileDescriptor *file, "grpc++/impl/codegen/sync_stream.h" }; std::vector headers(headers_strs, array_end(headers_strs)); - PrintIncludes(&printer, headers, params); + PrintIncludes(printer.get(), headers, params); if (!file->package().empty()) { - std::vector parts = - grpc_generator::tokenize(file->package(), "."); + std::vector parts = file->package_parts(); for (auto part = parts.begin(); part != parts.end(); part++) { vars["part"] = *part; - printer.Print(vars, "namespace $part$ {\n"); + printer->Print(vars, "namespace $part$ {\n"); } } - printer.Print(vars, "\n"); + printer->Print(vars, "\n"); } return output; } -void PrintSourceClientMethod(grpc::protobuf::io::Printer *printer, - const grpc::protobuf::MethodDescriptor *method, +void PrintSourceClientMethod(Printer *printer, + const Method *method, std::map *vars) { (*vars)["Method"] = method->name(); - (*vars)["Request"] = - grpc_cpp_generator::ClassName(method->input_type(), true); - (*vars)["Response"] = - grpc_cpp_generator::ClassName(method->output_type(), true); - if (NoStreaming(method)) { + (*vars)["Request"] = method->input_type_name(); + (*vars)["Response"] = method->output_type_name(); + if (method->NoStreaming()) { printer->Print(*vars, "::grpc::Status $ns$$Service$::Stub::$Method$(" "::grpc::ClientContext* context, " @@ -951,7 +907,7 @@ void PrintSourceClientMethod(grpc::protobuf::io::Printer *printer, "rpcmethod_$Method$_, " "context, request);\n" "}\n\n"); - } else if (ClientOnlyStreaming(method)) { + } else if (method->ClientOnlyStreaming()) { printer->Print(*vars, "::grpc::ClientWriter< $Request$>* " "$ns$$Service$::Stub::$Method$Raw(" @@ -973,7 +929,7 @@ void PrintSourceClientMethod(grpc::protobuf::io::Printer *printer, "rpcmethod_$Method$_, " "context, response, tag);\n" "}\n\n"); - } else if (ServerOnlyStreaming(method)) { + } else if (method->ServerOnlyStreaming()) { printer->Print( *vars, "::grpc::ClientReader< $Response$>* " @@ -996,7 +952,7 @@ void PrintSourceClientMethod(grpc::protobuf::io::Printer *printer, "rpcmethod_$Method$_, " "context, request, tag);\n" "}\n\n"); - } else if (BidiStreaming(method)) { + } else if (method->BidiStreaming()) { printer->Print( *vars, "::grpc::ClientReaderWriter< $Request$, $Response$>* " @@ -1023,15 +979,13 @@ void PrintSourceClientMethod(grpc::protobuf::io::Printer *printer, } } -void PrintSourceServerMethod(grpc::protobuf::io::Printer *printer, - const grpc::protobuf::MethodDescriptor *method, +void PrintSourceServerMethod(Printer *printer, + const Method *method, std::map *vars) { (*vars)["Method"] = method->name(); - (*vars)["Request"] = - grpc_cpp_generator::ClassName(method->input_type(), true); - (*vars)["Response"] = - grpc_cpp_generator::ClassName(method->output_type(), true); - if (NoStreaming(method)) { + (*vars)["Request"] = method->input_type_name(); + (*vars)["Response"] = method->output_type_name(); + if (method->NoStreaming()) { printer->Print(*vars, "::grpc::Status $ns$$Service$::Service::$Method$(" "::grpc::ServerContext* context, " @@ -1043,7 +997,7 @@ void PrintSourceServerMethod(grpc::protobuf::io::Printer *printer, " return ::grpc::Status(" "::grpc::StatusCode::UNIMPLEMENTED, \"\");\n"); printer->Print("}\n\n"); - } else if (ClientOnlyStreaming(method)) { + } else if (method->ClientOnlyStreaming()) { printer->Print(*vars, "::grpc::Status $ns$$Service$::Service::$Method$(" "::grpc::ServerContext* context, " @@ -1056,7 +1010,7 @@ void PrintSourceServerMethod(grpc::protobuf::io::Printer *printer, " return ::grpc::Status(" "::grpc::StatusCode::UNIMPLEMENTED, \"\");\n"); printer->Print("}\n\n"); - } else if (ServerOnlyStreaming(method)) { + } else if (method->ServerOnlyStreaming()) { printer->Print(*vars, "::grpc::Status $ns$$Service$::Service::$Method$(" "::grpc::ServerContext* context, " @@ -1069,7 +1023,7 @@ void PrintSourceServerMethod(grpc::protobuf::io::Printer *printer, " return ::grpc::Status(" "::grpc::StatusCode::UNIMPLEMENTED, \"\");\n"); printer->Print("}\n\n"); - } else if (BidiStreaming(method)) { + } else if (method->BidiStreaming()) { printer->Print(*vars, "::grpc::Status $ns$$Service$::Service::$Method$(" "::grpc::ServerContext* context, " @@ -1084,15 +1038,15 @@ void PrintSourceServerMethod(grpc::protobuf::io::Printer *printer, } } -void PrintSourceService(grpc::protobuf::io::Printer *printer, - const grpc::protobuf::ServiceDescriptor *service, +void PrintSourceService(Printer *printer, + const Service *service, std::map *vars) { (*vars)["Service"] = service->name(); printer->Print(*vars, "static const char* $prefix$$Service$_method_names[] = {\n"); for (int i = 0; i < service->method_count(); ++i) { - (*vars)["Method"] = service->method(i)->name(); + (*vars)["Method"] = service->method(i).get()->name(); printer->Print(*vars, " \"/$Package$$Service$/$Method$\",\n"); } printer->Print(*vars, "};\n\n"); @@ -1111,14 +1065,14 @@ void PrintSourceService(grpc::protobuf::io::Printer *printer, printer->Indent(); printer->Print(": channel_(channel)"); for (int i = 0; i < service->method_count(); ++i) { - const grpc::protobuf::MethodDescriptor *method = service->method(i); + auto method = service->method(i); (*vars)["Method"] = method->name(); (*vars)["Idx"] = as_string(i); - if (NoStreaming(method)) { + if (method->NoStreaming()) { (*vars)["StreamingType"] = "NORMAL_RPC"; - } else if (ClientOnlyStreaming(method)) { + } else if (method->ClientOnlyStreaming()) { (*vars)["StreamingType"] = "CLIENT_STREAMING"; - } else if (ServerOnlyStreaming(method)) { + } else if (method->ServerOnlyStreaming()) { (*vars)["StreamingType"] = "SERVER_STREAMING"; } else { (*vars)["StreamingType"] = "BIDI_STREAMING"; @@ -1135,21 +1089,19 @@ void PrintSourceService(grpc::protobuf::io::Printer *printer, for (int i = 0; i < service->method_count(); ++i) { (*vars)["Idx"] = as_string(i); - PrintSourceClientMethod(printer, service->method(i), vars); + PrintSourceClientMethod(printer, service->method(i).get(), vars); } printer->Print(*vars, "$ns$$Service$::Service::Service() {\n"); printer->Indent(); printer->Print(*vars, "(void)$prefix$$Service$_method_names;\n"); for (int i = 0; i < service->method_count(); ++i) { - const grpc::protobuf::MethodDescriptor *method = service->method(i); + auto method = service->method(i); (*vars)["Idx"] = as_string(i); (*vars)["Method"] = method->name(); - (*vars)["Request"] = - grpc_cpp_generator::ClassName(method->input_type(), true); - (*vars)["Response"] = - grpc_cpp_generator::ClassName(method->output_type(), true); - if (NoStreaming(method)) { + (*vars)["Request"] = method->input_type_name(); + (*vars)["Response"] = method->output_type_name(); + if (method->NoStreaming()) { printer->Print( *vars, "AddMethod(new ::grpc::RpcServiceMethod(\n" @@ -1159,7 +1111,7 @@ void PrintSourceService(grpc::protobuf::io::Printer *printer, "$Request$, " "$Response$>(\n" " std::mem_fn(&$ns$$Service$::Service::$Method$), this)));\n"); - } else if (ClientOnlyStreaming(method)) { + } else if (method->ClientOnlyStreaming()) { printer->Print( *vars, "AddMethod(new ::grpc::RpcServiceMethod(\n" @@ -1168,7 +1120,7 @@ void PrintSourceService(grpc::protobuf::io::Printer *printer, " new ::grpc::ClientStreamingHandler< " "$ns$$Service$::Service, $Request$, $Response$>(\n" " std::mem_fn(&$ns$$Service$::Service::$Method$), this)));\n"); - } else if (ServerOnlyStreaming(method)) { + } else if (method->ServerOnlyStreaming()) { printer->Print( *vars, "AddMethod(new ::grpc::RpcServiceMethod(\n" @@ -1177,7 +1129,7 @@ void PrintSourceService(grpc::protobuf::io::Printer *printer, " new ::grpc::ServerStreamingHandler< " "$ns$$Service$::Service, $Request$, $Response$>(\n" " std::mem_fn(&$ns$$Service$::Service::$Method$), this)));\n"); - } else if (BidiStreaming(method)) { + } else if (method->BidiStreaming()) { printer->Print( *vars, "AddMethod(new ::grpc::RpcServiceMethod(\n" @@ -1195,17 +1147,16 @@ void PrintSourceService(grpc::protobuf::io::Printer *printer, "}\n\n"); for (int i = 0; i < service->method_count(); ++i) { (*vars)["Idx"] = as_string(i); - PrintSourceServerMethod(printer, service->method(i), vars); + PrintSourceServerMethod(printer, service->method(i).get(), vars); } } -grpc::string GetSourceServices(const grpc::protobuf::FileDescriptor *file, +grpc::string GetSourceServices(File *file, const Parameters ¶ms) { grpc::string output; { // Scope the output stream so it closes and finalizes output to the string. - grpc::protobuf::io::StringOutputStream output_stream(&output); - grpc::protobuf::io::Printer printer(&output_stream, '$'); + auto printer = file->CreatePrinter(&output); std::map vars; // Package string is empty or ends with a dot. It is used to fully qualify // method names. @@ -1222,20 +1173,19 @@ grpc::string GetSourceServices(const grpc::protobuf::FileDescriptor *file, } for (int i = 0; i < file->service_count(); ++i) { - PrintSourceService(&printer, file->service(i), &vars); - printer.Print("\n"); + PrintSourceService(printer.get(), file->service(i).get(), &vars); + printer->Print("\n"); } } return output; } -grpc::string GetSourceEpilogue(const grpc::protobuf::FileDescriptor *file, +grpc::string GetSourceEpilogue(File *file, const Parameters ¶ms) { grpc::string temp; if (!file->package().empty()) { - std::vector parts = - grpc_generator::tokenize(file->package(), "."); + std::vector parts = file->package_parts(); for (auto part = parts.begin(); part != parts.end(); part++) { temp.append("} // namespace "); diff --git a/src/compiler/cpp_generator.h b/src/compiler/cpp_generator.h index 03621c8794c..99a60a2eaec 100644 --- a/src/compiler/cpp_generator.h +++ b/src/compiler/cpp_generator.h @@ -34,7 +34,23 @@ #ifndef GRPC_INTERNAL_COMPILER_CPP_GENERATOR_H #define GRPC_INTERNAL_COMPILER_CPP_GENERATOR_H -#include "src/compiler/config.h" +// cpp_generator.h/.cc do not directly depend on GRPC/ProtoBuf, such that they +// can be used to generate code for other serialization systems, such as +// FlatBuffers. + +#include +#include + +#ifndef GRPC_CUSTOM_STRING +#include +#define GRPC_CUSTOM_STRING std::string +#endif + +namespace grpc { + +typedef GRPC_CUSTOM_STRING string; + +} // namespace grpc namespace grpc_cpp_generator { @@ -48,37 +64,80 @@ struct Parameters { grpc::string grpc_search_path; }; +// An abstract interface representing a method. +struct Method { + virtual ~Method() {} + + virtual grpc::string name() const = 0; + + virtual grpc::string input_type_name() const = 0; + virtual grpc::string output_type_name() const = 0; + + virtual bool NoStreaming() const = 0; + virtual bool ClientOnlyStreaming() const = 0; + virtual bool ServerOnlyStreaming() const = 0; + virtual bool BidiStreaming() const = 0; +}; + +// An abstract interface representing a service. +struct Service { + virtual ~Service() {} + + virtual grpc::string name() const = 0; + + virtual int method_count() const = 0; + virtual std::unique_ptr method(int i) const = 0; +}; + +struct Printer { + virtual ~Printer() {} + + virtual void Print(const std::map &vars, + const char *template_string) = 0; + virtual void Print(const char *string) = 0; + virtual void Indent() = 0; + virtual void Outdent() = 0; +}; + +// An interface that allows the source generated to be output using various +// libraries/idls/serializers. +struct File { + virtual ~File() {} + + virtual grpc::string filename() const = 0; + virtual grpc::string filename_without_ext() const = 0; + virtual grpc::string package() const = 0; + virtual std::vector package_parts() const = 0; + + virtual int service_count() const = 0; + virtual std::unique_ptr service(int i) const = 0; + + virtual std::unique_ptr CreatePrinter(grpc::string *str) const = 0; +}; + // Return the prologue of the generated header file. -grpc::string GetHeaderPrologue(const grpc::protobuf::FileDescriptor *file, - const Parameters ¶ms); +grpc::string GetHeaderPrologue(File *file, const Parameters ¶ms); // Return the includes needed for generated header file. -grpc::string GetHeaderIncludes(const grpc::protobuf::FileDescriptor *file, - const Parameters ¶ms); +grpc::string GetHeaderIncludes(File *file, const Parameters ¶ms); // Return the includes needed for generated source file. -grpc::string GetSourceIncludes(const grpc::protobuf::FileDescriptor *file, - const Parameters ¶ms); +grpc::string GetSourceIncludes(File *file, const Parameters ¶ms); // Return the epilogue of the generated header file. -grpc::string GetHeaderEpilogue(const grpc::protobuf::FileDescriptor *file, - const Parameters ¶ms); +grpc::string GetHeaderEpilogue(File *file, const Parameters ¶ms); // Return the prologue of the generated source file. -grpc::string GetSourcePrologue(const grpc::protobuf::FileDescriptor *file, - const Parameters ¶ms); +grpc::string GetSourcePrologue(File *file, const Parameters ¶ms); // Return the services for generated header file. -grpc::string GetHeaderServices(const grpc::protobuf::FileDescriptor *file, - const Parameters ¶ms); +grpc::string GetHeaderServices(File *file, const Parameters ¶ms); // Return the services for generated source file. -grpc::string GetSourceServices(const grpc::protobuf::FileDescriptor *file, - const Parameters ¶ms); +grpc::string GetSourceServices(File *file, const Parameters ¶ms); // Return the epilogue of the generated source file. -grpc::string GetSourceEpilogue(const grpc::protobuf::FileDescriptor *file, - const Parameters ¶ms); +grpc::string GetSourceEpilogue(File *file, const Parameters ¶ms); } // namespace grpc_cpp_generator diff --git a/src/compiler/cpp_plugin.cc b/src/compiler/cpp_plugin.cc index 92a9ba75491..f703c6453d5 100644 --- a/src/compiler/cpp_plugin.cc +++ b/src/compiler/cpp_plugin.cc @@ -41,6 +41,105 @@ #include "src/compiler/cpp_generator.h" #include "src/compiler/cpp_generator_helpers.h" +class ProtoBufMethod : public grpc_cpp_generator::Method { + public: + ProtoBufMethod(const grpc::protobuf::MethodDescriptor *method) + : method_(method) {} + + grpc::string name() const { return method_->name(); } + + grpc::string input_type_name() const { + return grpc_cpp_generator::ClassName(method_->input_type(), true); + } + grpc::string output_type_name() const { + return grpc_cpp_generator::ClassName(method_->output_type(), true); + } + + bool NoStreaming() const { + return !method_->client_streaming() && !method_->server_streaming(); + } + + bool ClientOnlyStreaming() const { + return method_->client_streaming() && !method_->server_streaming(); + } + + bool ServerOnlyStreaming() const { + return !method_->client_streaming() && method_->server_streaming(); + } + + bool BidiStreaming() const { + return method_->client_streaming() && method_->server_streaming(); + } + + private: + const grpc::protobuf::MethodDescriptor *method_; +}; + +class ProtoBufService : public grpc_cpp_generator::Service { + public: + ProtoBufService(const grpc::protobuf::ServiceDescriptor *service) + : service_(service) {} + + grpc::string name() const { return service_->name(); } + + int method_count() const { return service_->method_count(); }; + std::unique_ptr method(int i) const { + return std::unique_ptr( + new ProtoBufMethod(service_->method(i))); + }; + + private: + const grpc::protobuf::ServiceDescriptor *service_; +}; + +class ProtoBufPrinter : public grpc_cpp_generator::Printer { + public: + ProtoBufPrinter(grpc::string *str) + : output_stream_(str), printer_(&output_stream_, '$') {} + + void Print(const std::map &vars, + const char *string_template) { + printer_.Print(vars, string_template); + } + + void Print(const char *string) { printer_.Print(string); } + void Indent() { printer_.Indent(); } + void Outdent() { printer_.Outdent(); } + + private: + grpc::protobuf::io::StringOutputStream output_stream_; + grpc::protobuf::io::Printer printer_; +}; + +class ProtoBufFile : public grpc_cpp_generator::File { + public: + ProtoBufFile(const grpc::protobuf::FileDescriptor *file) : file_(file) {} + + grpc::string filename() const { return file_->name(); } + grpc::string filename_without_ext() const { + return grpc_generator::StripProto(filename()); + } + + grpc::string package() const { return file_->package(); } + std::vector package_parts() const { + return grpc_generator::tokenize(package(), "."); + } + + int service_count() const { return file_->service_count(); }; + std::unique_ptr service(int i) const { + return std::unique_ptr ( + new ProtoBufService(file_->service(i))); + } + + std::unique_ptr CreatePrinter(grpc::string *str) const { + return std::unique_ptr( + new ProtoBufPrinter(str)); + } + + private: + const grpc::protobuf::FileDescriptor *file_; +}; + class CppGrpcGenerator : public grpc::protobuf::compiler::CodeGenerator { public: CppGrpcGenerator() {} @@ -61,6 +160,8 @@ class CppGrpcGenerator : public grpc::protobuf::compiler::CodeGenerator { grpc_cpp_generator::Parameters generator_parameters; generator_parameters.use_system_headers = true; + ProtoBufFile pbfile(file); + if (!parameter.empty()) { std::vector parameters_list = grpc_generator::tokenize(parameter, ","); @@ -92,10 +193,10 @@ class CppGrpcGenerator : public grpc::protobuf::compiler::CodeGenerator { grpc::string file_name = grpc_generator::StripProto(file->name()); grpc::string header_code = - grpc_cpp_generator::GetHeaderPrologue(file, generator_parameters) + - grpc_cpp_generator::GetHeaderIncludes(file, generator_parameters) + - grpc_cpp_generator::GetHeaderServices(file, generator_parameters) + - grpc_cpp_generator::GetHeaderEpilogue(file, generator_parameters); + grpc_cpp_generator::GetHeaderPrologue(&pbfile, generator_parameters) + + grpc_cpp_generator::GetHeaderIncludes(&pbfile, generator_parameters) + + grpc_cpp_generator::GetHeaderServices(&pbfile, generator_parameters) + + grpc_cpp_generator::GetHeaderEpilogue(&pbfile, generator_parameters); std::unique_ptr header_output( context->Open(file_name + ".grpc.pb.h")); grpc::protobuf::io::CodedOutputStream header_coded_out( @@ -103,10 +204,10 @@ class CppGrpcGenerator : public grpc::protobuf::compiler::CodeGenerator { header_coded_out.WriteRaw(header_code.data(), header_code.size()); grpc::string source_code = - grpc_cpp_generator::GetSourcePrologue(file, generator_parameters) + - grpc_cpp_generator::GetSourceIncludes(file, generator_parameters) + - grpc_cpp_generator::GetSourceServices(file, generator_parameters) + - grpc_cpp_generator::GetSourceEpilogue(file, generator_parameters); + grpc_cpp_generator::GetSourcePrologue(&pbfile, generator_parameters) + + grpc_cpp_generator::GetSourceIncludes(&pbfile, generator_parameters) + + grpc_cpp_generator::GetSourceServices(&pbfile, generator_parameters) + + grpc_cpp_generator::GetSourceEpilogue(&pbfile, generator_parameters); std::unique_ptr source_output( context->Open(file_name + ".grpc.pb.cc")); grpc::protobuf::io::CodedOutputStream source_coded_out( From 0d17ee3acfb7972ffe9ee9334836e1ddd4d0abd1 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Thu, 14 Apr 2016 09:30:32 -0700 Subject: [PATCH 20/38] Fix bug causing all fuzzers to run forever --- templates/tools/fuzzer/runners.template | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/templates/tools/fuzzer/runners.template b/templates/tools/fuzzer/runners.template index 72d0b363f22..4b8d4f3c68f 100644 --- a/templates/tools/fuzzer/runners.template +++ b/templates/tools/fuzzer/runners.template @@ -39,7 +39,7 @@ template: | if [ "$jobs" != "1" ] then - flags="-jobs=$jobs -workers=$jobs" + flags="-jobs=$jobs -workers=$jobs $flags" fi if [ "$config" == "asan-trace-cmp" ] From 69f1f4330b19404ff0939696868ecff736a72915 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Thu, 14 Apr 2016 12:50:08 -0700 Subject: [PATCH 21/38] Fix fuzzing sanity --- tools/fuzzer/runners/client_fuzzer.sh | 2 +- tools/fuzzer/runners/hpack_parser_fuzzer_test.sh | 2 +- tools/fuzzer/runners/http_fuzzer_test.sh | 2 +- tools/fuzzer/runners/json_fuzzer_test.sh | 2 +- tools/fuzzer/runners/nanopb_fuzzer_response_test.sh | 2 +- tools/fuzzer/runners/nanopb_fuzzer_serverlist_test.sh | 2 +- tools/fuzzer/runners/server_fuzzer.sh | 2 +- tools/fuzzer/runners/uri_fuzzer_test.sh | 2 +- 8 files changed, 8 insertions(+), 8 deletions(-) diff --git a/tools/fuzzer/runners/client_fuzzer.sh b/tools/fuzzer/runners/client_fuzzer.sh index 239d552c57d..97d4e60d908 100644 --- a/tools/fuzzer/runners/client_fuzzer.sh +++ b/tools/fuzzer/runners/client_fuzzer.sh @@ -33,7 +33,7 @@ flags="-max_total_time=$runtime -artifact_prefix=fuzzer_output/ -max_len=2048" if [ "$jobs" != "1" ] then - flags="-jobs=$jobs -workers=$jobs" + flags="-jobs=$jobs -workers=$jobs $flags" fi if [ "$config" == "asan-trace-cmp" ] diff --git a/tools/fuzzer/runners/hpack_parser_fuzzer_test.sh b/tools/fuzzer/runners/hpack_parser_fuzzer_test.sh index e69b4b4dfe2..c6f70a623dc 100644 --- a/tools/fuzzer/runners/hpack_parser_fuzzer_test.sh +++ b/tools/fuzzer/runners/hpack_parser_fuzzer_test.sh @@ -33,7 +33,7 @@ flags="-max_total_time=$runtime -artifact_prefix=fuzzer_output/ -max_len=512" if [ "$jobs" != "1" ] then - flags="-jobs=$jobs -workers=$jobs" + flags="-jobs=$jobs -workers=$jobs $flags" fi if [ "$config" == "asan-trace-cmp" ] diff --git a/tools/fuzzer/runners/http_fuzzer_test.sh b/tools/fuzzer/runners/http_fuzzer_test.sh index c190ba40b60..bb54a238145 100644 --- a/tools/fuzzer/runners/http_fuzzer_test.sh +++ b/tools/fuzzer/runners/http_fuzzer_test.sh @@ -33,7 +33,7 @@ flags="-max_total_time=$runtime -artifact_prefix=fuzzer_output/ -max_len=2048" if [ "$jobs" != "1" ] then - flags="-jobs=$jobs -workers=$jobs" + flags="-jobs=$jobs -workers=$jobs $flags" fi if [ "$config" == "asan-trace-cmp" ] diff --git a/tools/fuzzer/runners/json_fuzzer_test.sh b/tools/fuzzer/runners/json_fuzzer_test.sh index 9fc6271976b..e11e25dc097 100644 --- a/tools/fuzzer/runners/json_fuzzer_test.sh +++ b/tools/fuzzer/runners/json_fuzzer_test.sh @@ -33,7 +33,7 @@ flags="-max_total_time=$runtime -artifact_prefix=fuzzer_output/ -max_len=512" if [ "$jobs" != "1" ] then - flags="-jobs=$jobs -workers=$jobs" + flags="-jobs=$jobs -workers=$jobs $flags" fi if [ "$config" == "asan-trace-cmp" ] diff --git a/tools/fuzzer/runners/nanopb_fuzzer_response_test.sh b/tools/fuzzer/runners/nanopb_fuzzer_response_test.sh index bbcebf11cce..97359277ce2 100644 --- a/tools/fuzzer/runners/nanopb_fuzzer_response_test.sh +++ b/tools/fuzzer/runners/nanopb_fuzzer_response_test.sh @@ -33,7 +33,7 @@ flags="-max_total_time=$runtime -artifact_prefix=fuzzer_output/ -max_len=128" if [ "$jobs" != "1" ] then - flags="-jobs=$jobs -workers=$jobs" + flags="-jobs=$jobs -workers=$jobs $flags" fi if [ "$config" == "asan-trace-cmp" ] diff --git a/tools/fuzzer/runners/nanopb_fuzzer_serverlist_test.sh b/tools/fuzzer/runners/nanopb_fuzzer_serverlist_test.sh index e9099bac046..2dfaa2372fc 100644 --- a/tools/fuzzer/runners/nanopb_fuzzer_serverlist_test.sh +++ b/tools/fuzzer/runners/nanopb_fuzzer_serverlist_test.sh @@ -33,7 +33,7 @@ flags="-max_total_time=$runtime -artifact_prefix=fuzzer_output/ -max_len=128" if [ "$jobs" != "1" ] then - flags="-jobs=$jobs -workers=$jobs" + flags="-jobs=$jobs -workers=$jobs $flags" fi if [ "$config" == "asan-trace-cmp" ] diff --git a/tools/fuzzer/runners/server_fuzzer.sh b/tools/fuzzer/runners/server_fuzzer.sh index 28ca8b32719..fc0567f670b 100644 --- a/tools/fuzzer/runners/server_fuzzer.sh +++ b/tools/fuzzer/runners/server_fuzzer.sh @@ -33,7 +33,7 @@ flags="-max_total_time=$runtime -artifact_prefix=fuzzer_output/ -max_len=2048" if [ "$jobs" != "1" ] then - flags="-jobs=$jobs -workers=$jobs" + flags="-jobs=$jobs -workers=$jobs $flags" fi if [ "$config" == "asan-trace-cmp" ] diff --git a/tools/fuzzer/runners/uri_fuzzer_test.sh b/tools/fuzzer/runners/uri_fuzzer_test.sh index 7dac54ec518..5f33e734654 100644 --- a/tools/fuzzer/runners/uri_fuzzer_test.sh +++ b/tools/fuzzer/runners/uri_fuzzer_test.sh @@ -33,7 +33,7 @@ flags="-max_total_time=$runtime -artifact_prefix=fuzzer_output/ -max_len=128" if [ "$jobs" != "1" ] then - flags="-jobs=$jobs -workers=$jobs" + flags="-jobs=$jobs -workers=$jobs $flags" fi if [ "$config" == "asan-trace-cmp" ] From 8c839e839b3624c9953b11b9a59b04276f40a0b9 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Tue, 12 Apr 2016 15:12:08 -0700 Subject: [PATCH 22/38] check for histogramParams presence --- src/csharp/Grpc.IntegrationTesting/ClientRunners.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/csharp/Grpc.IntegrationTesting/ClientRunners.cs b/src/csharp/Grpc.IntegrationTesting/ClientRunners.cs index f954ca5f34c..b4572756f24 100644 --- a/src/csharp/Grpc.IntegrationTesting/ClientRunners.cs +++ b/src/csharp/Grpc.IntegrationTesting/ClientRunners.cs @@ -129,6 +129,7 @@ namespace Grpc.IntegrationTesting public ClientRunnerImpl(List channels, ClientType clientType, RpcType rpcType, int outstandingRpcsPerChannel, LoadParams loadParams, PayloadConfig payloadConfig, HistogramParams histogramParams) { GrpcPreconditions.CheckArgument(outstandingRpcsPerChannel > 0, "outstandingRpcsPerChannel"); + GrpcPreconditions.CheckNotNull(histogramParams, "histogramParams"); this.channels = new List(channels); this.clientType = clientType; this.rpcType = rpcType; From c578e44ab9b54f86df21abf9ffd79d5c37a2e0da Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Tue, 12 Apr 2016 16:09:35 -0700 Subject: [PATCH 23/38] add some console messages to node qps worker --- src/node/performance/worker.js | 2 ++ src/node/performance/worker_service_impl.js | 3 +++ 2 files changed, 5 insertions(+) diff --git a/src/node/performance/worker.js b/src/node/performance/worker.js index 98577bdbc97..7ef9b84fe76 100644 --- a/src/node/performance/worker.js +++ b/src/node/performance/worker.js @@ -33,6 +33,7 @@ 'use strict'; +var console = require('console'); var worker_service_impl = require('./worker_service_impl'); var grpc = require('../../../'); @@ -48,6 +49,7 @@ function runServer(port) { var address = '0.0.0.0:' + port; server.bind(address, server_creds); server.start(); + console.log('running QPS worker on %s', address); return server; } diff --git a/src/node/performance/worker_service_impl.js b/src/node/performance/worker_service_impl.js index 17458e4b933..4b5cb8f9c21 100644 --- a/src/node/performance/worker_service_impl.js +++ b/src/node/performance/worker_service_impl.js @@ -34,6 +34,7 @@ 'use strict'; var os = require('os'); +var console = require('console'); var BenchmarkClient = require('./benchmark_client'); var BenchmarkServer = require('./benchmark_server'); @@ -49,6 +50,7 @@ exports.runClient = function runClient(call) { switch (request.argtype) { case 'setup': var setup = request.setup; + console.log('ClientConfig %j', setup); client = new BenchmarkClient(setup.server_targets, setup.client_channels, setup.histogram_params, @@ -118,6 +120,7 @@ exports.runServer = function runServer(call) { var stats; switch (request.argtype) { case 'setup': + console.log('ServerConfig %j', request.setup); server = new BenchmarkServer('[::]', request.setup.port, request.setup.security_params); server.start(); From 4fec48b831694f856adddabe924cf8b3bfcdf7d5 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Tue, 12 Apr 2016 15:12:54 -0700 Subject: [PATCH 24/38] tiny fixes to scenario_config --- .../run_tests/performance/scenario_config.py | 43 ++++++++++++------- 1 file changed, 28 insertions(+), 15 deletions(-) diff --git a/tools/run_tests/performance/scenario_config.py b/tools/run_tests/performance/scenario_config.py index 7a82d257e40..e8f2fffaa25 100644 --- a/tools/run_tests/performance/scenario_config.py +++ b/tools/run_tests/performance/scenario_config.py @@ -33,6 +33,11 @@ SINGLE_MACHINE_CORES=8 WARMUP_SECONDS=5 BENCHMARK_SECONDS=30 +HISTOGRAM_PARAMS = { + 'resolution': 0.01, + 'max_possible': 60e9, +} + EMPTY_GENERIC_PAYLOAD = { 'bytebuf_params': { 'req_size': 0, @@ -83,7 +88,7 @@ class CXXLanguage: secargs = None yield { - 'name': 'generic_async_streaming_ping_pong_%s' + 'name': 'cpp_generic_async_streaming_ping_pong_%s' % secstr, 'num_servers': 1, 'num_clients': 1, @@ -98,6 +103,7 @@ class CXXLanguage: 'closed_loop': {} }, 'payload_config': EMPTY_GENERIC_PAYLOAD, + 'histogram_params': HISTOGRAM_PARAMS, }, 'server_config': { 'server_type': 'ASYNC_GENERIC_SERVER', @@ -110,7 +116,7 @@ class CXXLanguage: 'benchmark_seconds': BENCHMARK_SECONDS } yield { - 'name': 'generic_async_streaming_qps_unconstrained_%s' + 'name': 'cpp_generic_async_streaming_qps_unconstrained_%s' % secstr, 'num_servers': 1, 'num_clients': 0, @@ -125,6 +131,7 @@ class CXXLanguage: 'closed_loop': {} }, 'payload_config': EMPTY_GENERIC_PAYLOAD, + 'histogram_params': HISTOGRAM_PARAMS, }, 'server_config': { 'server_type': 'ASYNC_GENERIC_SERVER', @@ -137,7 +144,7 @@ class CXXLanguage: 'benchmark_seconds': BENCHMARK_SECONDS } yield { - 'name': 'generic_async_streaming_qps_one_server_core_%s' + 'name': 'cpp_generic_async_streaming_qps_one_server_core_%s' % secstr, 'num_servers': 1, 'num_clients': 0, @@ -152,6 +159,7 @@ class CXXLanguage: 'closed_loop': {} }, 'payload_config': EMPTY_GENERIC_PAYLOAD, + 'histogram_params': HISTOGRAM_PARAMS, }, 'server_config': { 'server_type': 'ASYNC_GENERIC_SERVER', @@ -164,7 +172,7 @@ class CXXLanguage: 'benchmark_seconds': BENCHMARK_SECONDS } yield { - 'name': 'protobuf_async_qps_unconstrained_%s' + 'name': 'cpp_generic_async_qps_unconstrained_%s' % secstr, 'num_servers': 1, 'num_clients': 0, @@ -179,6 +187,7 @@ class CXXLanguage: 'closed_loop': {} }, 'payload_config': EMPTY_GENERIC_PAYLOAD, + 'histogram_params': HISTOGRAM_PARAMS, }, 'server_config': { 'server_type': 'ASYNC_GENERIC_SERVER', @@ -191,7 +200,7 @@ class CXXLanguage: 'benchmark_seconds': BENCHMARK_SECONDS } yield { - 'name': 'single_channel_throughput_%s' + 'name': 'cpp_single_channel_throughput_%s' % secstr, 'num_servers': 1, 'num_clients': 1, @@ -206,6 +215,7 @@ class CXXLanguage: 'closed_loop': {} }, 'payload_config': BIG_GENERIC_PAYLOAD, + 'histogram_params': HISTOGRAM_PARAMS, }, 'server_config': { 'server_type': 'ASYNC_GENERIC_SERVER', @@ -218,7 +228,7 @@ class CXXLanguage: 'benchmark_seconds': BENCHMARK_SECONDS } yield { - 'name': 'protobuf_async_ping_pong_%s' + 'name': 'cpp_protobuf_async_ping_pong_%s' % secstr, 'num_servers': 1, 'num_clients': 1, @@ -233,13 +243,13 @@ class CXXLanguage: 'closed_loop': {} }, 'payload_config': EMPTY_PROTO_PAYLOAD, + 'histogram_params': HISTOGRAM_PARAMS, }, 'server_config': { - 'server_type': 'ASYNC_GENERIC_SERVER', + 'server_type': 'ASYNC_SERVER', 'security_params': secargs, 'core_limit': SINGLE_MACHINE_CORES/2, 'async_server_threads': 1, - 'payload_config': EMPTY_PROTO_PAYLOAD, }, 'warmup_seconds': WARMUP_SECONDS, 'benchmark_seconds': BENCHMARK_SECONDS @@ -262,8 +272,9 @@ class CSharpLanguage: def scenarios(self): # TODO(jtattermusch): add more scenarios + secargs = None yield { - 'name': 'csharp_async_generic_streaming_ping_pong', + 'name': 'csharp_generic_async_streaming_ping_pong', 'num_servers': 1, 'num_clients': 1, 'client_config': { @@ -277,11 +288,12 @@ class CSharpLanguage: 'closed_loop': {} }, 'payload_config': EMPTY_GENERIC_PAYLOAD, + 'histogram_params': HISTOGRAM_PARAMS, }, 'server_config': { 'server_type': 'ASYNC_GENERIC_SERVER', 'security_params': secargs, - 'core_limit': SINGLE_MACHINE_CORES/2, + 'core_limit': 0, 'async_server_threads': 1, 'payload_config': EMPTY_GENERIC_PAYLOAD, }, @@ -307,8 +319,9 @@ class NodeLanguage: def scenarios(self): # TODO(jtattermusch): add more scenarios + secargs = None yield { - 'name': 'node_sync_unary_ping_pong_protobuf', + 'name': 'node_protobuf_unary_ping_pong', 'num_servers': 1, 'num_clients': 1, 'client_config': { @@ -317,18 +330,18 @@ class NodeLanguage: 'outstanding_rpcs_per_channel': 1, 'client_channels': 1, 'async_client_threads': 1, - 'rpc_type': 'STREAMING', + 'rpc_type': 'UNARY', 'load_params': { 'closed_loop': {} }, 'payload_config': EMPTY_PROTO_PAYLOAD, + 'histogram_params': HISTOGRAM_PARAMS, }, 'server_config': { - 'server_type': 'ASYNC_GENERIC_SERVER', + 'server_type': 'ASYNC_SERVER', 'security_params': secargs, - 'core_limit': SINGLE_MACHINE_CORES/2, + 'core_limit': 0, 'async_server_threads': 1, - 'payload_config': EMPTY_PROTO_PAYLOAD, }, 'warmup_seconds': WARMUP_SECONDS, 'benchmark_seconds': BENCHMARK_SECONDS From 38becc28b0edb184f2a9a932733e301dd0e0d776 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Thu, 14 Apr 2016 08:00:35 -0700 Subject: [PATCH 25/38] added support for regex selection of scenarios --- tools/run_tests/run_performance_tests.py | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/tools/run_tests/run_performance_tests.py b/tools/run_tests/run_performance_tests.py index 51ed35f7608..477d32213aa 100755 --- a/tools/run_tests/run_performance_tests.py +++ b/tools/run_tests/run_performance_tests.py @@ -37,6 +37,7 @@ import json import multiprocessing import os import pipes +import re import subprocess import sys import tempfile @@ -82,6 +83,8 @@ def create_qpsworker_job(language, shortname=None, else: host_and_port='localhost:%s' % port + # TODO(jtattermusch): with some care, we can calculate the right timeout + # of a worker from the sum of warmup + benchmark times for all the scenarios jobspec = jobset.JobSpec( cmdline=cmdline, shortname=shortname, @@ -221,15 +224,16 @@ def start_qpsworkers(languages, worker_hosts): for worker_idx, worker in enumerate(workers)] -def create_scenarios(languages, workers_by_lang, remote_host=None): +def create_scenarios(languages, workers_by_lang, remote_host=None, regex='.*'): """Create jobspecs for scenarios to run.""" scenarios = [] for language in languages: for scenario_json in language.scenarios(): - scenario = create_scenario_jobspec(scenario_json, - workers_by_lang[str(language)], - remote_host=remote_host) - scenarios.append(scenario) + if re.search(args.regex, scenario_json['name']): + scenario = create_scenario_jobspec(scenario_json, + workers_by_lang[str(language)], + remote_host=remote_host) + scenarios.append(scenario) # the very last scenario requests shutting down the workers. all_workers = [worker @@ -268,6 +272,8 @@ argp.add_argument('--remote_worker_host', nargs='+', default=[], help='Worker hosts where to start QPS workers.') +argp.add_argument('-r', '--regex', default='.*', type=str, + help='Regex to select scenarios to run.') args = argp.parse_args() @@ -295,6 +301,9 @@ build_on_remote_hosts(remote_hosts, languages=[str(l) for l in languages], build qpsworker_jobs = start_qpsworkers(languages, args.remote_worker_host) +# TODO(jtattermusch): see https://github.com/grpc/grpc/issues/6174 +time.sleep(5) + # get list of worker addresses for each language. worker_addresses = dict([(str(language), []) for language in languages]) for job in qpsworker_jobs: @@ -303,7 +312,8 @@ for job in qpsworker_jobs: try: scenarios = create_scenarios(languages, workers_by_lang=worker_addresses, - remote_host=args.remote_driver_host) + remote_host=args.remote_driver_host, + regex=args.regex) if not scenarios: raise Exception('No scenarios to run') From ee9032c507aca8740e135a8cd6350a6f2a375153 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Thu, 14 Apr 2016 08:35:51 -0700 Subject: [PATCH 26/38] add support for cross-language tests --- tools/run_tests/run_performance_tests.py | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/tools/run_tests/run_performance_tests.py b/tools/run_tests/run_performance_tests.py index 477d32213aa..b3729acd9f5 100755 --- a/tools/run_tests/run_performance_tests.py +++ b/tools/run_tests/run_performance_tests.py @@ -42,6 +42,7 @@ import subprocess import sys import tempfile import time +import traceback import uuid import performance.scenario_config as scenario_config @@ -230,8 +231,22 @@ def create_scenarios(languages, workers_by_lang, remote_host=None, regex='.*'): for language in languages: for scenario_json in language.scenarios(): if re.search(args.regex, scenario_json['name']): + workers = workers_by_lang[str(language)] + # 'SERVER_LANGUAGE' is an indicator for this script to pick + # a server in different language. It doesn't belong to the Scenario + # schema, so we also need to remove it. + custom_server_lang = scenario_json.pop('SERVER_LANGUAGE', None) + if custom_server_lang: + if not workers_by_lang.get(custom_server_lang, []): + print 'Warning: Skipping scenario %s as' % scenario_json['name'] + print('SERVER_LANGUAGE is set to %s yet the language has ' + 'not been selected with -l' % custom_server_lang) + continue + for idx in range(0, scenario_json['num_servers']): + # replace first X workers by workers of a different language + workers[idx] = workers_by_lang[custom_server_lang][idx] scenario = create_scenario_jobspec(scenario_json, - workers_by_lang[str(language)], + workers, remote_host=remote_host) scenarios.append(scenario) @@ -328,5 +343,7 @@ try: jobset.message('FAILED', 'Some of the scenarios failed.', do_newline=True) sys.exit(1) +except: + traceback.print_exc() finally: finish_qps_workers(qpsworker_jobs) From d61c252f8e14fa6a3dc8c9d77064ad7394e0c87b Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Thu, 14 Apr 2016 08:36:25 -0700 Subject: [PATCH 27/38] add some more C# scenarios --- .../run_tests/performance/scenario_config.py | 79 +++++++++++++++++++ 1 file changed, 79 insertions(+) diff --git a/tools/run_tests/performance/scenario_config.py b/tools/run_tests/performance/scenario_config.py index e8f2fffaa25..2a94c93abea 100644 --- a/tools/run_tests/performance/scenario_config.py +++ b/tools/run_tests/performance/scenario_config.py @@ -300,6 +300,85 @@ class CSharpLanguage: 'warmup_seconds': WARMUP_SECONDS, 'benchmark_seconds': BENCHMARK_SECONDS } + yield { + 'name': 'csharp_protobuf_async_unary_ping_pong', + 'num_servers': 1, + 'num_clients': 1, + 'client_config': { + 'client_type': 'ASYNC_CLIENT', + 'security_params': secargs, + 'outstanding_rpcs_per_channel': 1, + 'client_channels': 1, + 'async_client_threads': 1, + 'rpc_type': 'UNARY', + 'load_params': { + 'closed_loop': {} + }, + 'payload_config': EMPTY_PROTO_PAYLOAD, + 'histogram_params': HISTOGRAM_PARAMS, + }, + 'server_config': { + 'server_type': 'ASYNC_SERVER', + 'security_params': secargs, + 'core_limit': 0, + 'async_server_threads': 1, + }, + 'warmup_seconds': WARMUP_SECONDS, + 'benchmark_seconds': BENCHMARK_SECONDS + } + yield { + 'name': 'csharp_protobuf_sync_to_async_unary_ping_pong', + 'num_servers': 1, + 'num_clients': 1, + 'client_config': { + 'client_type': 'SYNC_CLIENT', + 'security_params': secargs, + 'outstanding_rpcs_per_channel': 1, + 'client_channels': 1, + 'async_client_threads': 1, + 'rpc_type': 'UNARY', + 'load_params': { + 'closed_loop': {} + }, + 'payload_config': EMPTY_PROTO_PAYLOAD, + 'histogram_params': HISTOGRAM_PARAMS, + }, + 'server_config': { + 'server_type': 'ASYNC_SERVER', + 'security_params': secargs, + 'core_limit': 0, + 'async_server_threads': 1, + }, + 'warmup_seconds': WARMUP_SECONDS, + 'benchmark_seconds': BENCHMARK_SECONDS + } + yield { + 'name': 'csharp_to_cpp_protobuf_sync_unary_ping_pong', + 'num_servers': 1, + 'num_clients': 1, + 'client_config': { + 'client_type': 'SYNC_CLIENT', + 'security_params': secargs, + 'outstanding_rpcs_per_channel': 1, + 'client_channels': 1, + 'async_client_threads': 1, + 'rpc_type': 'UNARY', + 'load_params': { + 'closed_loop': {} + }, + 'payload_config': EMPTY_PROTO_PAYLOAD, + 'histogram_params': HISTOGRAM_PARAMS, + }, + 'server_config': { + 'server_type': 'SYNC_SERVER', + 'security_params': secargs, + 'core_limit': 0, + 'async_server_threads': 1, + }, + 'warmup_seconds': WARMUP_SECONDS, + 'benchmark_seconds': BENCHMARK_SECONDS, + 'SERVER_LANGUAGE': 'c++' # recognized by run_performance_tests.py + } def __str__(self): return 'csharp' From df14927146642fd32002232cb1fd54c128d6b057 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Thu, 14 Apr 2016 09:57:44 -0700 Subject: [PATCH 28/38] add ScenarioResult and ScenarioResultSummary proto messages --- src/proto/grpc/testing/control.proto | 39 ++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/src/proto/grpc/testing/control.proto b/src/proto/grpc/testing/control.proto index 458b19c4d6c..062c2a96c1f 100644 --- a/src/proto/grpc/testing/control.proto +++ b/src/proto/grpc/testing/control.proto @@ -194,3 +194,42 @@ message Scenario { message Scenarios { repeated Scenario scenarios = 1; } + +// Basic summary that can be computed from ClientStats and ServerStats +// once the scenario has finished. +message ScenarioResultSummary +{ + // Total number of operations per second over all clients. + double qps = 1; + // QPS per one server core. + double qps_per_server_core = 2; + // server load based on system_time (0.85 => 85%) + double server_system_time = 3; + // server load based on user_time (0.85 => 85%) + double server_user_time = 4; + // client load based on system_time (0.85 => 85%) + double client_system_time = 5; + // client load based on user_time (0.85 => 85%) + double client_user_time = 6; + + // X% latency percentiles (in seconds) + double latency_50 = 7; + double latency_90 = 8; + double latency_95 = 9; + double latency_99 = 10; + double latency_999 = 11; +} + +// Results of a single benchmark scenario. +message ScenarioResult { + // Inputs used to run the scenario. + Scenario scenario = 1; + // Histograms from all clients merged into one histogram. + HistogramData latencies = 2; + // Client stats for each client + repeated ClientStats client_stats = 3; + // Server stats for each server + repeated ServerStats server_stats = 4; + // An after-the-fact computed summary + ScenarioResultSummary summary = 5; +} From 7be244b6477f5f6d370a6e415d316819004965d0 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Thu, 14 Apr 2016 10:08:25 -0700 Subject: [PATCH 29/38] fix redundant C++ scenario --- tools/run_tests/performance/scenario_config.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/tools/run_tests/performance/scenario_config.py b/tools/run_tests/performance/scenario_config.py index 2a94c93abea..bd10b6f0324 100644 --- a/tools/run_tests/performance/scenario_config.py +++ b/tools/run_tests/performance/scenario_config.py @@ -172,7 +172,7 @@ class CXXLanguage: 'benchmark_seconds': BENCHMARK_SECONDS } yield { - 'name': 'cpp_generic_async_qps_unconstrained_%s' + 'name': 'cpp_protobuf_async_streaming_qps_unconstrained_%s' % secstr, 'num_servers': 1, 'num_clients': 0, @@ -186,15 +186,14 @@ class CXXLanguage: 'load_params': { 'closed_loop': {} }, - 'payload_config': EMPTY_GENERIC_PAYLOAD, + 'payload_config': EMPTY_PROTO_PAYLOAD, 'histogram_params': HISTOGRAM_PARAMS, }, 'server_config': { - 'server_type': 'ASYNC_GENERIC_SERVER', + 'server_type': 'ASYNC_SERVER', 'security_params': secargs, 'core_limit': SINGLE_MACHINE_CORES/2, 'async_server_threads': 1, - 'payload_config': EMPTY_GENERIC_PAYLOAD, }, 'warmup_seconds': WARMUP_SECONDS, 'benchmark_seconds': BENCHMARK_SECONDS From 05bb5b9326f1844d4c0ce4f4e81c3833a6c4ccca Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Thu, 14 Apr 2016 10:12:33 -0700 Subject: [PATCH 30/38] regenerate tests.json with C++ perf scenarios --- src/proto/grpc/testing/control.proto | 6 ++-- tools/run_tests/tests.json | 48 ++++++++++++++-------------- 2 files changed, 28 insertions(+), 26 deletions(-) diff --git a/src/proto/grpc/testing/control.proto b/src/proto/grpc/testing/control.proto index 062c2a96c1f..5db39d0298e 100644 --- a/src/proto/grpc/testing/control.proto +++ b/src/proto/grpc/testing/control.proto @@ -212,7 +212,7 @@ message ScenarioResultSummary // client load based on user_time (0.85 => 85%) double client_user_time = 6; - // X% latency percentiles (in seconds) + // X% latency percentiles (in nanoseconds) double latency_50 = 7; double latency_90 = 8; double latency_95 = 9; @@ -230,6 +230,8 @@ message ScenarioResult { repeated ClientStats client_stats = 3; // Server stats for each server repeated ServerStats server_stats = 4; + // Number of cores available to each server + repeated int32 server_cores = 5; // An after-the-fact computed summary - ScenarioResultSummary summary = 5; + ScenarioResultSummary summary = 6; } diff --git a/tools/run_tests/tests.json b/tools/run_tests/tests.json index f8c658672b8..ad03f16420c 100644 --- a/tools/run_tests/tests.json +++ b/tools/run_tests/tests.json @@ -22035,7 +22035,7 @@ { "args": [ "--scenario_json", - "'{\"name\": \"generic_async_streaming_ping_pong_secure\", \"warmup_seconds\": 5, \"benchmark_seconds\": 30, \"num_servers\": 1, \"server_config\": {\"async_server_threads\": 1, \"core_limit\": 4, \"security_params\": {\"use_test_ca\": true, \"server_host_override\": \"foo.test.google.fr\"}, \"payload_config\": {\"bytebuf_params\": {\"resp_size\": 0, \"req_size\": 0}}, \"server_type\": \"ASYNC_GENERIC_SERVER\"}, \"client_config\": {\"client_type\": \"ASYNC_CLIENT\", \"security_params\": {\"use_test_ca\": true, \"server_host_override\": \"foo.test.google.fr\"}, \"payload_config\": {\"bytebuf_params\": {\"resp_size\": 0, \"req_size\": 0}}, \"client_channels\": 1, \"async_client_threads\": 1, \"outstanding_rpcs_per_channel\": 1, \"rpc_type\": \"STREAMING\", \"load_params\": {\"closed_loop\": {}}}, \"num_clients\": 1}'" + "'{\"name\": \"cpp_generic_async_streaming_ping_pong_secure\", \"warmup_seconds\": 5, \"benchmark_seconds\": 30, \"num_servers\": 1, \"server_config\": {\"async_server_threads\": 1, \"core_limit\": 4, \"security_params\": {\"use_test_ca\": true, \"server_host_override\": \"foo.test.google.fr\"}, \"payload_config\": {\"bytebuf_params\": {\"resp_size\": 0, \"req_size\": 0}}, \"server_type\": \"ASYNC_GENERIC_SERVER\"}, \"client_config\": {\"client_type\": \"ASYNC_CLIENT\", \"security_params\": {\"use_test_ca\": true, \"server_host_override\": \"foo.test.google.fr\"}, \"payload_config\": {\"bytebuf_params\": {\"resp_size\": 0, \"req_size\": 0}}, \"client_channels\": 1, \"async_client_threads\": 1, \"outstanding_rpcs_per_channel\": 1, \"rpc_type\": \"STREAMING\", \"load_params\": {\"closed_loop\": {}}, \"histogram_params\": {\"max_possible\": 60000000000.0, \"resolution\": 0.01}}, \"num_clients\": 1}'" ], "boringssl": true, "ci_platforms": [ @@ -22056,12 +22056,12 @@ "posix", "windows" ], - "shortname": "json_run_localhost:generic_async_streaming_ping_pong_secure" + "shortname": "json_run_localhost:cpp_generic_async_streaming_ping_pong_secure" }, { "args": [ "--scenario_json", - "'{\"name\": \"generic_async_streaming_qps_unconstrained_secure\", \"warmup_seconds\": 5, \"benchmark_seconds\": 30, \"num_servers\": 1, \"server_config\": {\"async_server_threads\": 1, \"core_limit\": 4, \"security_params\": {\"use_test_ca\": true, \"server_host_override\": \"foo.test.google.fr\"}, \"payload_config\": {\"bytebuf_params\": {\"resp_size\": 0, \"req_size\": 0}}, \"server_type\": \"ASYNC_GENERIC_SERVER\"}, \"client_config\": {\"client_type\": \"ASYNC_CLIENT\", \"security_params\": {\"use_test_ca\": true, \"server_host_override\": \"foo.test.google.fr\"}, \"payload_config\": {\"bytebuf_params\": {\"resp_size\": 0, \"req_size\": 0}}, \"client_channels\": 64, \"async_client_threads\": 1, \"outstanding_rpcs_per_channel\": 100, \"rpc_type\": \"STREAMING\", \"load_params\": {\"closed_loop\": {}}}, \"num_clients\": 0}'" + "'{\"name\": \"cpp_generic_async_streaming_qps_unconstrained_secure\", \"warmup_seconds\": 5, \"benchmark_seconds\": 30, \"num_servers\": 1, \"server_config\": {\"async_server_threads\": 1, \"core_limit\": 4, \"security_params\": {\"use_test_ca\": true, \"server_host_override\": \"foo.test.google.fr\"}, \"payload_config\": {\"bytebuf_params\": {\"resp_size\": 0, \"req_size\": 0}}, \"server_type\": \"ASYNC_GENERIC_SERVER\"}, \"client_config\": {\"client_type\": \"ASYNC_CLIENT\", \"security_params\": {\"use_test_ca\": true, \"server_host_override\": \"foo.test.google.fr\"}, \"payload_config\": {\"bytebuf_params\": {\"resp_size\": 0, \"req_size\": 0}}, \"client_channels\": 64, \"async_client_threads\": 1, \"outstanding_rpcs_per_channel\": 100, \"rpc_type\": \"STREAMING\", \"load_params\": {\"closed_loop\": {}}, \"histogram_params\": {\"max_possible\": 60000000000.0, \"resolution\": 0.01}}, \"num_clients\": 0}'" ], "boringssl": true, "ci_platforms": [ @@ -22082,12 +22082,12 @@ "posix", "windows" ], - "shortname": "json_run_localhost:generic_async_streaming_qps_unconstrained_secure" + "shortname": "json_run_localhost:cpp_generic_async_streaming_qps_unconstrained_secure" }, { "args": [ "--scenario_json", - "'{\"name\": \"generic_async_streaming_qps_one_server_core_secure\", \"warmup_seconds\": 5, \"benchmark_seconds\": 30, \"num_servers\": 1, \"server_config\": {\"async_server_threads\": 1, \"core_limit\": 1, \"security_params\": {\"use_test_ca\": true, \"server_host_override\": \"foo.test.google.fr\"}, \"payload_config\": {\"bytebuf_params\": {\"resp_size\": 0, \"req_size\": 0}}, \"server_type\": \"ASYNC_GENERIC_SERVER\"}, \"client_config\": {\"client_type\": \"ASYNC_CLIENT\", \"security_params\": {\"use_test_ca\": true, \"server_host_override\": \"foo.test.google.fr\"}, \"payload_config\": {\"bytebuf_params\": {\"resp_size\": 0, \"req_size\": 0}}, \"client_channels\": 64, \"async_client_threads\": 1, \"outstanding_rpcs_per_channel\": 100, \"rpc_type\": \"STREAMING\", \"load_params\": {\"closed_loop\": {}}}, \"num_clients\": 0}'" + "'{\"name\": \"cpp_generic_async_streaming_qps_one_server_core_secure\", \"warmup_seconds\": 5, \"benchmark_seconds\": 30, \"num_servers\": 1, \"server_config\": {\"async_server_threads\": 1, \"core_limit\": 1, \"security_params\": {\"use_test_ca\": true, \"server_host_override\": \"foo.test.google.fr\"}, \"payload_config\": {\"bytebuf_params\": {\"resp_size\": 0, \"req_size\": 0}}, \"server_type\": \"ASYNC_GENERIC_SERVER\"}, \"client_config\": {\"client_type\": \"ASYNC_CLIENT\", \"security_params\": {\"use_test_ca\": true, \"server_host_override\": \"foo.test.google.fr\"}, \"payload_config\": {\"bytebuf_params\": {\"resp_size\": 0, \"req_size\": 0}}, \"client_channels\": 64, \"async_client_threads\": 1, \"outstanding_rpcs_per_channel\": 100, \"rpc_type\": \"STREAMING\", \"load_params\": {\"closed_loop\": {}}, \"histogram_params\": {\"max_possible\": 60000000000.0, \"resolution\": 0.01}}, \"num_clients\": 0}'" ], "boringssl": true, "ci_platforms": [ @@ -22108,12 +22108,12 @@ "posix", "windows" ], - "shortname": "json_run_localhost:generic_async_streaming_qps_one_server_core_secure" + "shortname": "json_run_localhost:cpp_generic_async_streaming_qps_one_server_core_secure" }, { "args": [ "--scenario_json", - "'{\"name\": \"protobuf_async_qps_unconstrained_secure\", \"warmup_seconds\": 5, \"benchmark_seconds\": 30, \"num_servers\": 1, \"server_config\": {\"async_server_threads\": 1, \"core_limit\": 4, \"security_params\": {\"use_test_ca\": true, \"server_host_override\": \"foo.test.google.fr\"}, \"payload_config\": {\"bytebuf_params\": {\"resp_size\": 0, \"req_size\": 0}}, \"server_type\": \"ASYNC_GENERIC_SERVER\"}, \"client_config\": {\"client_type\": \"ASYNC_CLIENT\", \"security_params\": {\"use_test_ca\": true, \"server_host_override\": \"foo.test.google.fr\"}, \"payload_config\": {\"bytebuf_params\": {\"resp_size\": 0, \"req_size\": 0}}, \"client_channels\": 64, \"async_client_threads\": 1, \"outstanding_rpcs_per_channel\": 100, \"rpc_type\": \"STREAMING\", \"load_params\": {\"closed_loop\": {}}}, \"num_clients\": 0}'" + "'{\"name\": \"cpp_protobuf_async_streaming_qps_unconstrained_secure\", \"warmup_seconds\": 5, \"benchmark_seconds\": 30, \"num_servers\": 1, \"server_config\": {\"async_server_threads\": 1, \"core_limit\": 4, \"security_params\": {\"use_test_ca\": true, \"server_host_override\": \"foo.test.google.fr\"}, \"server_type\": \"ASYNC_SERVER\"}, \"client_config\": {\"client_type\": \"ASYNC_CLIENT\", \"security_params\": {\"use_test_ca\": true, \"server_host_override\": \"foo.test.google.fr\"}, \"payload_config\": {\"simple_params\": {\"resp_size\": 0, \"req_size\": 0}}, \"client_channels\": 64, \"async_client_threads\": 1, \"outstanding_rpcs_per_channel\": 100, \"rpc_type\": \"STREAMING\", \"load_params\": {\"closed_loop\": {}}, \"histogram_params\": {\"max_possible\": 60000000000.0, \"resolution\": 0.01}}, \"num_clients\": 0}'" ], "boringssl": true, "ci_platforms": [ @@ -22134,12 +22134,12 @@ "posix", "windows" ], - "shortname": "json_run_localhost:protobuf_async_qps_unconstrained_secure" + "shortname": "json_run_localhost:cpp_protobuf_async_streaming_qps_unconstrained_secure" }, { "args": [ "--scenario_json", - "'{\"name\": \"single_channel_throughput_secure\", \"warmup_seconds\": 5, \"benchmark_seconds\": 30, \"num_servers\": 1, \"server_config\": {\"async_server_threads\": 1, \"core_limit\": 4, \"security_params\": {\"use_test_ca\": true, \"server_host_override\": \"foo.test.google.fr\"}, \"payload_config\": {\"bytebuf_params\": {\"resp_size\": 65536, \"req_size\": 65536}}, \"server_type\": \"ASYNC_GENERIC_SERVER\"}, \"client_config\": {\"client_type\": \"ASYNC_CLIENT\", \"security_params\": {\"use_test_ca\": true, \"server_host_override\": \"foo.test.google.fr\"}, \"payload_config\": {\"bytebuf_params\": {\"resp_size\": 65536, \"req_size\": 65536}}, \"client_channels\": 1, \"async_client_threads\": 1, \"outstanding_rpcs_per_channel\": 1, \"rpc_type\": \"STREAMING\", \"load_params\": {\"closed_loop\": {}}}, \"num_clients\": 1}'" + "'{\"name\": \"cpp_single_channel_throughput_secure\", \"warmup_seconds\": 5, \"benchmark_seconds\": 30, \"num_servers\": 1, \"server_config\": {\"async_server_threads\": 1, \"core_limit\": 4, \"security_params\": {\"use_test_ca\": true, \"server_host_override\": \"foo.test.google.fr\"}, \"payload_config\": {\"bytebuf_params\": {\"resp_size\": 65536, \"req_size\": 65536}}, \"server_type\": \"ASYNC_GENERIC_SERVER\"}, \"client_config\": {\"client_type\": \"ASYNC_CLIENT\", \"security_params\": {\"use_test_ca\": true, \"server_host_override\": \"foo.test.google.fr\"}, \"payload_config\": {\"bytebuf_params\": {\"resp_size\": 65536, \"req_size\": 65536}}, \"client_channels\": 1, \"async_client_threads\": 1, \"outstanding_rpcs_per_channel\": 1, \"rpc_type\": \"STREAMING\", \"load_params\": {\"closed_loop\": {}}, \"histogram_params\": {\"max_possible\": 60000000000.0, \"resolution\": 0.01}}, \"num_clients\": 1}'" ], "boringssl": true, "ci_platforms": [ @@ -22160,12 +22160,12 @@ "posix", "windows" ], - "shortname": "json_run_localhost:single_channel_throughput_secure" + "shortname": "json_run_localhost:cpp_single_channel_throughput_secure" }, { "args": [ "--scenario_json", - "'{\"name\": \"protobuf_async_ping_pong_secure\", \"warmup_seconds\": 5, \"benchmark_seconds\": 30, \"num_servers\": 1, \"server_config\": {\"async_server_threads\": 1, \"core_limit\": 4, \"security_params\": {\"use_test_ca\": true, \"server_host_override\": \"foo.test.google.fr\"}, \"payload_config\": {\"simple_params\": {\"resp_size\": 0, \"req_size\": 0}}, \"server_type\": \"ASYNC_GENERIC_SERVER\"}, \"client_config\": {\"client_type\": \"ASYNC_CLIENT\", \"security_params\": {\"use_test_ca\": true, \"server_host_override\": \"foo.test.google.fr\"}, \"payload_config\": {\"simple_params\": {\"resp_size\": 0, \"req_size\": 0}}, \"client_channels\": 1, \"async_client_threads\": 1, \"outstanding_rpcs_per_channel\": 1, \"rpc_type\": \"STREAMING\", \"load_params\": {\"closed_loop\": {}}}, \"num_clients\": 1}'" + "'{\"name\": \"cpp_protobuf_async_ping_pong_secure\", \"warmup_seconds\": 5, \"benchmark_seconds\": 30, \"num_servers\": 1, \"server_config\": {\"async_server_threads\": 1, \"core_limit\": 4, \"security_params\": {\"use_test_ca\": true, \"server_host_override\": \"foo.test.google.fr\"}, \"server_type\": \"ASYNC_SERVER\"}, \"client_config\": {\"client_type\": \"ASYNC_CLIENT\", \"security_params\": {\"use_test_ca\": true, \"server_host_override\": \"foo.test.google.fr\"}, \"payload_config\": {\"simple_params\": {\"resp_size\": 0, \"req_size\": 0}}, \"client_channels\": 1, \"async_client_threads\": 1, \"outstanding_rpcs_per_channel\": 1, \"rpc_type\": \"STREAMING\", \"load_params\": {\"closed_loop\": {}}, \"histogram_params\": {\"max_possible\": 60000000000.0, \"resolution\": 0.01}}, \"num_clients\": 1}'" ], "boringssl": true, "ci_platforms": [ @@ -22186,12 +22186,12 @@ "posix", "windows" ], - "shortname": "json_run_localhost:protobuf_async_ping_pong_secure" + "shortname": "json_run_localhost:cpp_protobuf_async_ping_pong_secure" }, { "args": [ "--scenario_json", - "'{\"name\": \"generic_async_streaming_ping_pong_insecure\", \"warmup_seconds\": 5, \"benchmark_seconds\": 30, \"num_servers\": 1, \"server_config\": {\"async_server_threads\": 1, \"core_limit\": 4, \"security_params\": null, \"payload_config\": {\"bytebuf_params\": {\"resp_size\": 0, \"req_size\": 0}}, \"server_type\": \"ASYNC_GENERIC_SERVER\"}, \"client_config\": {\"client_type\": \"ASYNC_CLIENT\", \"security_params\": null, \"payload_config\": {\"bytebuf_params\": {\"resp_size\": 0, \"req_size\": 0}}, \"client_channels\": 1, \"async_client_threads\": 1, \"outstanding_rpcs_per_channel\": 1, \"rpc_type\": \"STREAMING\", \"load_params\": {\"closed_loop\": {}}}, \"num_clients\": 1}'" + "'{\"name\": \"cpp_generic_async_streaming_ping_pong_insecure\", \"warmup_seconds\": 5, \"benchmark_seconds\": 30, \"num_servers\": 1, \"server_config\": {\"async_server_threads\": 1, \"core_limit\": 4, \"security_params\": null, \"payload_config\": {\"bytebuf_params\": {\"resp_size\": 0, \"req_size\": 0}}, \"server_type\": \"ASYNC_GENERIC_SERVER\"}, \"client_config\": {\"client_type\": \"ASYNC_CLIENT\", \"security_params\": null, \"payload_config\": {\"bytebuf_params\": {\"resp_size\": 0, \"req_size\": 0}}, \"client_channels\": 1, \"async_client_threads\": 1, \"outstanding_rpcs_per_channel\": 1, \"rpc_type\": \"STREAMING\", \"load_params\": {\"closed_loop\": {}}, \"histogram_params\": {\"max_possible\": 60000000000.0, \"resolution\": 0.01}}, \"num_clients\": 1}'" ], "boringssl": true, "ci_platforms": [ @@ -22212,12 +22212,12 @@ "posix", "windows" ], - "shortname": "json_run_localhost:generic_async_streaming_ping_pong_insecure" + "shortname": "json_run_localhost:cpp_generic_async_streaming_ping_pong_insecure" }, { "args": [ "--scenario_json", - "'{\"name\": \"generic_async_streaming_qps_unconstrained_insecure\", \"warmup_seconds\": 5, \"benchmark_seconds\": 30, \"num_servers\": 1, \"server_config\": {\"async_server_threads\": 1, \"core_limit\": 4, \"security_params\": null, \"payload_config\": {\"bytebuf_params\": {\"resp_size\": 0, \"req_size\": 0}}, \"server_type\": \"ASYNC_GENERIC_SERVER\"}, \"client_config\": {\"client_type\": \"ASYNC_CLIENT\", \"security_params\": null, \"payload_config\": {\"bytebuf_params\": {\"resp_size\": 0, \"req_size\": 0}}, \"client_channels\": 64, \"async_client_threads\": 1, \"outstanding_rpcs_per_channel\": 100, \"rpc_type\": \"STREAMING\", \"load_params\": {\"closed_loop\": {}}}, \"num_clients\": 0}'" + "'{\"name\": \"cpp_generic_async_streaming_qps_unconstrained_insecure\", \"warmup_seconds\": 5, \"benchmark_seconds\": 30, \"num_servers\": 1, \"server_config\": {\"async_server_threads\": 1, \"core_limit\": 4, \"security_params\": null, \"payload_config\": {\"bytebuf_params\": {\"resp_size\": 0, \"req_size\": 0}}, \"server_type\": \"ASYNC_GENERIC_SERVER\"}, \"client_config\": {\"client_type\": \"ASYNC_CLIENT\", \"security_params\": null, \"payload_config\": {\"bytebuf_params\": {\"resp_size\": 0, \"req_size\": 0}}, \"client_channels\": 64, \"async_client_threads\": 1, \"outstanding_rpcs_per_channel\": 100, \"rpc_type\": \"STREAMING\", \"load_params\": {\"closed_loop\": {}}, \"histogram_params\": {\"max_possible\": 60000000000.0, \"resolution\": 0.01}}, \"num_clients\": 0}'" ], "boringssl": true, "ci_platforms": [ @@ -22238,12 +22238,12 @@ "posix", "windows" ], - "shortname": "json_run_localhost:generic_async_streaming_qps_unconstrained_insecure" + "shortname": "json_run_localhost:cpp_generic_async_streaming_qps_unconstrained_insecure" }, { "args": [ "--scenario_json", - "'{\"name\": \"generic_async_streaming_qps_one_server_core_insecure\", \"warmup_seconds\": 5, \"benchmark_seconds\": 30, \"num_servers\": 1, \"server_config\": {\"async_server_threads\": 1, \"core_limit\": 1, \"security_params\": null, \"payload_config\": {\"bytebuf_params\": {\"resp_size\": 0, \"req_size\": 0}}, \"server_type\": \"ASYNC_GENERIC_SERVER\"}, \"client_config\": {\"client_type\": \"ASYNC_CLIENT\", \"security_params\": null, \"payload_config\": {\"bytebuf_params\": {\"resp_size\": 0, \"req_size\": 0}}, \"client_channels\": 64, \"async_client_threads\": 1, \"outstanding_rpcs_per_channel\": 100, \"rpc_type\": \"STREAMING\", \"load_params\": {\"closed_loop\": {}}}, \"num_clients\": 0}'" + "'{\"name\": \"cpp_generic_async_streaming_qps_one_server_core_insecure\", \"warmup_seconds\": 5, \"benchmark_seconds\": 30, \"num_servers\": 1, \"server_config\": {\"async_server_threads\": 1, \"core_limit\": 1, \"security_params\": null, \"payload_config\": {\"bytebuf_params\": {\"resp_size\": 0, \"req_size\": 0}}, \"server_type\": \"ASYNC_GENERIC_SERVER\"}, \"client_config\": {\"client_type\": \"ASYNC_CLIENT\", \"security_params\": null, \"payload_config\": {\"bytebuf_params\": {\"resp_size\": 0, \"req_size\": 0}}, \"client_channels\": 64, \"async_client_threads\": 1, \"outstanding_rpcs_per_channel\": 100, \"rpc_type\": \"STREAMING\", \"load_params\": {\"closed_loop\": {}}, \"histogram_params\": {\"max_possible\": 60000000000.0, \"resolution\": 0.01}}, \"num_clients\": 0}'" ], "boringssl": true, "ci_platforms": [ @@ -22264,12 +22264,12 @@ "posix", "windows" ], - "shortname": "json_run_localhost:generic_async_streaming_qps_one_server_core_insecure" + "shortname": "json_run_localhost:cpp_generic_async_streaming_qps_one_server_core_insecure" }, { "args": [ "--scenario_json", - "'{\"name\": \"protobuf_async_qps_unconstrained_insecure\", \"warmup_seconds\": 5, \"benchmark_seconds\": 30, \"num_servers\": 1, \"server_config\": {\"async_server_threads\": 1, \"core_limit\": 4, \"security_params\": null, \"payload_config\": {\"bytebuf_params\": {\"resp_size\": 0, \"req_size\": 0}}, \"server_type\": \"ASYNC_GENERIC_SERVER\"}, \"client_config\": {\"client_type\": \"ASYNC_CLIENT\", \"security_params\": null, \"payload_config\": {\"bytebuf_params\": {\"resp_size\": 0, \"req_size\": 0}}, \"client_channels\": 64, \"async_client_threads\": 1, \"outstanding_rpcs_per_channel\": 100, \"rpc_type\": \"STREAMING\", \"load_params\": {\"closed_loop\": {}}}, \"num_clients\": 0}'" + "'{\"name\": \"cpp_protobuf_async_streaming_qps_unconstrained_insecure\", \"warmup_seconds\": 5, \"benchmark_seconds\": 30, \"num_servers\": 1, \"server_config\": {\"async_server_threads\": 1, \"core_limit\": 4, \"security_params\": null, \"server_type\": \"ASYNC_SERVER\"}, \"client_config\": {\"client_type\": \"ASYNC_CLIENT\", \"security_params\": null, \"payload_config\": {\"simple_params\": {\"resp_size\": 0, \"req_size\": 0}}, \"client_channels\": 64, \"async_client_threads\": 1, \"outstanding_rpcs_per_channel\": 100, \"rpc_type\": \"STREAMING\", \"load_params\": {\"closed_loop\": {}}, \"histogram_params\": {\"max_possible\": 60000000000.0, \"resolution\": 0.01}}, \"num_clients\": 0}'" ], "boringssl": true, "ci_platforms": [ @@ -22290,12 +22290,12 @@ "posix", "windows" ], - "shortname": "json_run_localhost:protobuf_async_qps_unconstrained_insecure" + "shortname": "json_run_localhost:cpp_protobuf_async_streaming_qps_unconstrained_insecure" }, { "args": [ "--scenario_json", - "'{\"name\": \"single_channel_throughput_insecure\", \"warmup_seconds\": 5, \"benchmark_seconds\": 30, \"num_servers\": 1, \"server_config\": {\"async_server_threads\": 1, \"core_limit\": 4, \"security_params\": null, \"payload_config\": {\"bytebuf_params\": {\"resp_size\": 65536, \"req_size\": 65536}}, \"server_type\": \"ASYNC_GENERIC_SERVER\"}, \"client_config\": {\"client_type\": \"ASYNC_CLIENT\", \"security_params\": null, \"payload_config\": {\"bytebuf_params\": {\"resp_size\": 65536, \"req_size\": 65536}}, \"client_channels\": 1, \"async_client_threads\": 1, \"outstanding_rpcs_per_channel\": 1, \"rpc_type\": \"STREAMING\", \"load_params\": {\"closed_loop\": {}}}, \"num_clients\": 1}'" + "'{\"name\": \"cpp_single_channel_throughput_insecure\", \"warmup_seconds\": 5, \"benchmark_seconds\": 30, \"num_servers\": 1, \"server_config\": {\"async_server_threads\": 1, \"core_limit\": 4, \"security_params\": null, \"payload_config\": {\"bytebuf_params\": {\"resp_size\": 65536, \"req_size\": 65536}}, \"server_type\": \"ASYNC_GENERIC_SERVER\"}, \"client_config\": {\"client_type\": \"ASYNC_CLIENT\", \"security_params\": null, \"payload_config\": {\"bytebuf_params\": {\"resp_size\": 65536, \"req_size\": 65536}}, \"client_channels\": 1, \"async_client_threads\": 1, \"outstanding_rpcs_per_channel\": 1, \"rpc_type\": \"STREAMING\", \"load_params\": {\"closed_loop\": {}}, \"histogram_params\": {\"max_possible\": 60000000000.0, \"resolution\": 0.01}}, \"num_clients\": 1}'" ], "boringssl": true, "ci_platforms": [ @@ -22316,12 +22316,12 @@ "posix", "windows" ], - "shortname": "json_run_localhost:single_channel_throughput_insecure" + "shortname": "json_run_localhost:cpp_single_channel_throughput_insecure" }, { "args": [ "--scenario_json", - "'{\"name\": \"protobuf_async_ping_pong_insecure\", \"warmup_seconds\": 5, \"benchmark_seconds\": 30, \"num_servers\": 1, \"server_config\": {\"async_server_threads\": 1, \"core_limit\": 4, \"security_params\": null, \"payload_config\": {\"simple_params\": {\"resp_size\": 0, \"req_size\": 0}}, \"server_type\": \"ASYNC_GENERIC_SERVER\"}, \"client_config\": {\"client_type\": \"ASYNC_CLIENT\", \"security_params\": null, \"payload_config\": {\"simple_params\": {\"resp_size\": 0, \"req_size\": 0}}, \"client_channels\": 1, \"async_client_threads\": 1, \"outstanding_rpcs_per_channel\": 1, \"rpc_type\": \"STREAMING\", \"load_params\": {\"closed_loop\": {}}}, \"num_clients\": 1}'" + "'{\"name\": \"cpp_protobuf_async_ping_pong_insecure\", \"warmup_seconds\": 5, \"benchmark_seconds\": 30, \"num_servers\": 1, \"server_config\": {\"async_server_threads\": 1, \"core_limit\": 4, \"security_params\": null, \"server_type\": \"ASYNC_SERVER\"}, \"client_config\": {\"client_type\": \"ASYNC_CLIENT\", \"security_params\": null, \"payload_config\": {\"simple_params\": {\"resp_size\": 0, \"req_size\": 0}}, \"client_channels\": 1, \"async_client_threads\": 1, \"outstanding_rpcs_per_channel\": 1, \"rpc_type\": \"STREAMING\", \"load_params\": {\"closed_loop\": {}}, \"histogram_params\": {\"max_possible\": 60000000000.0, \"resolution\": 0.01}}, \"num_clients\": 1}'" ], "boringssl": true, "ci_platforms": [ @@ -22342,7 +22342,7 @@ "posix", "windows" ], - "shortname": "json_run_localhost:protobuf_async_ping_pong_insecure" + "shortname": "json_run_localhost:cpp_protobuf_async_ping_pong_insecure" }, { "args": [ From f2ba7fe037b4e97d727ba38603364c464144e2e9 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Thu, 14 Apr 2016 12:14:17 -0700 Subject: [PATCH 31/38] integrate ScenarioResult proto into qps driver --- test/cpp/qps/driver.cc | 17 +++-- test/cpp/qps/driver.h | 23 ------- test/cpp/qps/qps_driver.cc | 1 - test/cpp/qps/report.cc | 111 ++++++++++-------------------- test/cpp/qps/report.h | 23 +------ test/cpp/util/benchmark_config.cc | 8 --- 6 files changed, 48 insertions(+), 135 deletions(-) diff --git a/test/cpp/qps/driver.cc b/test/cpp/qps/driver.cc index c87ad6461d1..9c0649cae68 100644 --- a/test/cpp/qps/driver.cc +++ b/test/cpp/qps/driver.cc @@ -343,8 +343,8 @@ std::unique_ptr RunScenario( // Finish a run std::unique_ptr result(new ScenarioResult); - result->client_config = result_client_config; - result->server_config = result_server_config; + Histogram merged_latencies; + gpr_log(GPR_INFO, "Finishing clients"); for (auto client = &clients[0]; client != &clients[num_clients]; client++) { GPR_ASSERT(client->stream->Write(client_mark)); @@ -353,9 +353,8 @@ std::unique_ptr RunScenario( for (auto client = &clients[0]; client != &clients[num_clients]; client++) { GPR_ASSERT(client->stream->Read(&client_status)); const auto& stats = client_status.stats(); - result->latencies.MergeProto(stats.latencies()); - result->client_resources.emplace_back( - stats.time_elapsed(), stats.time_user(), stats.time_system(), -1); + merged_latencies.MergeProto(stats.latencies()); + result->add_client_stats()->CopyFrom(stats); GPR_ASSERT(!client->stream->Read(&client_status)); } for (auto client = &clients[0]; client != &clients[num_clients]; client++) { @@ -363,6 +362,8 @@ std::unique_ptr RunScenario( } delete[] clients; + merged_latencies.FillProto(result->mutable_latencies()); + gpr_log(GPR_INFO, "Finishing servers"); for (auto server = &servers[0]; server != &servers[num_servers]; server++) { GPR_ASSERT(server->stream->Write(server_mark)); @@ -370,10 +371,8 @@ std::unique_ptr RunScenario( } for (auto server = &servers[0]; server != &servers[num_servers]; server++) { GPR_ASSERT(server->stream->Read(&server_status)); - const auto& stats = server_status.stats(); - result->server_resources.emplace_back( - stats.time_elapsed(), stats.time_user(), stats.time_system(), - server_status.cores()); + result->add_server_stats()->CopyFrom(server_status.stats()); + result->add_server_cores(server_status.cores()); GPR_ASSERT(!server->stream->Read(&server_status)); } for (auto server = &servers[0]; server != &servers[num_servers]; server++) { diff --git a/test/cpp/qps/driver.h b/test/cpp/qps/driver.h index 21e51529d59..3a5cf138f11 100644 --- a/test/cpp/qps/driver.h +++ b/test/cpp/qps/driver.h @@ -41,29 +41,6 @@ namespace grpc { namespace testing { -class ResourceUsage { - public: - ResourceUsage(double w, double u, double s, int c) - : wall_time_(w), user_time_(u), system_time_(s), cores_(c) {} - double wall_time() const { return wall_time_; } - double user_time() const { return user_time_; } - double system_time() const { return system_time_; } - int cores() const { return cores_; } - - private: - double wall_time_; - double user_time_; - double system_time_; - int cores_; -}; - -struct ScenarioResult { - Histogram latencies; - std::vector client_resources; - std::vector server_resources; - ClientConfig client_config; - ServerConfig server_config; -}; std::unique_ptr RunScenario( const grpc::testing::ClientConfig& client_config, size_t num_clients, diff --git a/test/cpp/qps/qps_driver.cc b/test/cpp/qps/qps_driver.cc index e412c6919af..608181f77fb 100644 --- a/test/cpp/qps/qps_driver.cc +++ b/test/cpp/qps/qps_driver.cc @@ -85,7 +85,6 @@ using grpc::testing::ServerConfig; using grpc::testing::ClientType; using grpc::testing::ServerType; using grpc::testing::RpcType; -using grpc::testing::ResourceUsage; using grpc::testing::SecurityParams; namespace grpc { diff --git a/test/cpp/qps/report.cc b/test/cpp/qps/report.cc index b230eb441e8..6037ac76038 100644 --- a/test/cpp/qps/report.cc +++ b/test/cpp/qps/report.cc @@ -40,10 +40,13 @@ namespace grpc { namespace testing { -static double WallTime(ResourceUsage u) { return u.wall_time(); } -static double UserTime(ResourceUsage u) { return u.user_time(); } -static double SystemTime(ResourceUsage u) { return u.system_time(); } -static int Cores(ResourceUsage u) { return u.cores(); } +static double WallTime(ClientStats s) { return s.time_elapsed(); } +static double SystemTime(ClientStats s) { return s.time_system(); } +static double UserTime(ClientStats s) { return s.time_user(); } +static double ServerWallTime(ServerStats s) { return s.time_elapsed(); } +static double ServerSystemTime(ServerStats s) { return s.time_system(); } +static double ServerUserTime(ServerStats s) { return s.time_user(); } +static int Cores(int n) { return n; } void CompositeReporter::add(std::unique_ptr reporter) { reporters_.emplace_back(std::move(reporter)); @@ -74,102 +77,62 @@ void CompositeReporter::ReportTimes(const ScenarioResult& result) { } void GprLogReporter::ReportQPS(const ScenarioResult& result) { - gpr_log( - GPR_INFO, "QPS: %.1f", - result.latencies.Count() / average(result.client_resources, WallTime)); + Histogram histogram; + histogram.MergeProto(result.latencies()); + gpr_log(GPR_INFO, "QPS: %.1f", + histogram.Count() / average(result.client_stats(), WallTime)); } void GprLogReporter::ReportQPSPerCore(const ScenarioResult& result) { - auto qps = - result.latencies.Count() / average(result.client_resources, WallTime); + Histogram histogram; + histogram.MergeProto(result.latencies()); + auto qps = histogram.Count() / average(result.client_stats(), WallTime); gpr_log(GPR_INFO, "QPS: %.1f (%.1f/server core)", qps, - qps / sum(result.server_resources, Cores)); + qps / sum(result.server_cores(), Cores)); } void GprLogReporter::ReportLatency(const ScenarioResult& result) { + Histogram histogram; + histogram.MergeProto(result.latencies()); gpr_log(GPR_INFO, "Latencies (50/90/95/99/99.9%%-ile): %.1f/%.1f/%.1f/%.1f/%.1f us", - result.latencies.Percentile(50) / 1000, - result.latencies.Percentile(90) / 1000, - result.latencies.Percentile(95) / 1000, - result.latencies.Percentile(99) / 1000, - result.latencies.Percentile(99.9) / 1000); + histogram.Percentile(50) / 1000, + histogram.Percentile(90) / 1000, + histogram.Percentile(95) / 1000, + histogram.Percentile(99) / 1000, + histogram.Percentile(99.9) / 1000); } void GprLogReporter::ReportTimes(const ScenarioResult& result) { gpr_log(GPR_INFO, "Server system time: %.2f%%", - 100.0 * sum(result.server_resources, SystemTime) / - sum(result.server_resources, WallTime)); + 100.0 * sum(result.server_stats(), ServerSystemTime) / + sum(result.server_stats(), ServerWallTime)); gpr_log(GPR_INFO, "Server user time: %.2f%%", - 100.0 * sum(result.server_resources, UserTime) / - sum(result.server_resources, WallTime)); + 100.0 * sum(result.server_stats(), ServerUserTime) / + sum(result.server_stats(), ServerWallTime)); gpr_log(GPR_INFO, "Client system time: %.2f%%", - 100.0 * sum(result.client_resources, SystemTime) / - sum(result.client_resources, WallTime)); + 100.0 * sum(result.client_stats(), SystemTime) / + sum(result.client_stats(), WallTime)); gpr_log(GPR_INFO, "Client user time: %.2f%%", - 100.0 * sum(result.client_resources, UserTime) / - sum(result.client_resources, WallTime)); + 100.0 * sum(result.client_stats(), UserTime) / + sum(result.client_stats(), WallTime)); } -void PerfDbReporter::ReportQPS(const ScenarioResult& result) { - auto qps = - result.latencies.Count() / average(result.client_resources, WallTime); +void JsonReporter::ReportQPS(const ScenarioResult& result) { - perf_db_client_.setQps(qps); - perf_db_client_.setConfigs(result.client_config, result.server_config); } -void PerfDbReporter::ReportQPSPerCore(const ScenarioResult& result) { - auto qps = - result.latencies.Count() / average(result.client_resources, WallTime); - - auto qps_per_core = qps / sum(result.server_resources, Cores); - - perf_db_client_.setQps(qps); - perf_db_client_.setQpsPerCore(qps_per_core); - perf_db_client_.setConfigs(result.client_config, result.server_config); -} - -void PerfDbReporter::ReportLatency(const ScenarioResult& result) { - perf_db_client_.setLatencies(result.latencies.Percentile(50) / 1000, - result.latencies.Percentile(90) / 1000, - result.latencies.Percentile(95) / 1000, - result.latencies.Percentile(99) / 1000, - result.latencies.Percentile(99.9) / 1000); - perf_db_client_.setConfigs(result.client_config, result.server_config); +void JsonReporter::ReportQPSPerCore(const ScenarioResult& result) { + // NOP - all reporting is handled by ReportQPS. } -void PerfDbReporter::ReportTimes(const ScenarioResult& result) { - const double server_system_time = 100.0 * - sum(result.server_resources, SystemTime) / - sum(result.server_resources, WallTime); - const double server_user_time = 100.0 * - sum(result.server_resources, UserTime) / - sum(result.server_resources, WallTime); - const double client_system_time = 100.0 * - sum(result.client_resources, SystemTime) / - sum(result.client_resources, WallTime); - const double client_user_time = 100.0 * - sum(result.client_resources, UserTime) / - sum(result.client_resources, WallTime); - - perf_db_client_.setTimes(server_system_time, server_user_time, - client_system_time, client_user_time); - perf_db_client_.setConfigs(result.client_config, result.server_config); +void JsonReporter::ReportLatency(const ScenarioResult& result) { + // NOP - all reporting is handled by ReportQPS. } -void PerfDbReporter::SendData() { - // send data to performance database - bool data_state = - perf_db_client_.sendData(hashed_id_, test_name_, sys_info_, tag_); - - // check state of data sending - if (data_state) { - gpr_log(GPR_INFO, "Data sent to performance database successfully"); - } else { - gpr_log(GPR_INFO, "Data could not be sent to performance database"); - } +void JsonReporter::ReportTimes(const ScenarioResult& result) { + // NOP - all reporting is handled by ReportQPS. } } // namespace testing diff --git a/test/cpp/qps/report.h b/test/cpp/qps/report.h index 5caf3fe69a2..b299e415f18 100644 --- a/test/cpp/qps/report.h +++ b/test/cpp/qps/report.h @@ -104,33 +104,16 @@ class GprLogReporter : public Reporter { void ReportTimes(const ScenarioResult& result) GRPC_OVERRIDE; }; -/** Reporter for performance database tool */ -class PerfDbReporter : public Reporter { +/** Dumps the report to a JSON file. */ +class JsonReporter : public Reporter { public: - PerfDbReporter(const string& name, const string& hashed_id, - const string& test_name, const string& sys_info, - const string& server_address, const string& tag) - : Reporter(name), - hashed_id_(hashed_id), - test_name_(test_name), - sys_info_(sys_info), - tag_(tag) { - perf_db_client_.init(grpc::CreateChannel( - server_address, grpc::InsecureChannelCredentials())); - } - ~PerfDbReporter() GRPC_OVERRIDE { SendData(); }; + JsonReporter(const string& name) : Reporter(name) {} private: - PerfDbClient perf_db_client_; - std::string hashed_id_; - std::string test_name_; - std::string sys_info_; - std::string tag_; void ReportQPS(const ScenarioResult& result) GRPC_OVERRIDE; void ReportQPSPerCore(const ScenarioResult& result) GRPC_OVERRIDE; void ReportLatency(const ScenarioResult& result) GRPC_OVERRIDE; void ReportTimes(const ScenarioResult& result) GRPC_OVERRIDE; - void SendData(); }; } // namespace testing diff --git a/test/cpp/util/benchmark_config.cc b/test/cpp/util/benchmark_config.cc index 746d3d7ae65..ca4c27ec355 100644 --- a/test/cpp/util/benchmark_config.cc +++ b/test/cpp/util/benchmark_config.cc @@ -37,9 +37,6 @@ DEFINE_bool(enable_log_reporter, true, "Enable reporting of benchmark results through GprLog"); -DEFINE_bool(report_metrics_db, false, - "True if metrics to be reported to performance database"); - DEFINE_string(hashed_id, "", "Hash of the user id"); DEFINE_string(test_name, "", "Name of the test being executed"); @@ -71,11 +68,6 @@ static std::shared_ptr InitBenchmarkReporters() { composite_reporter->add( std::unique_ptr(new GprLogReporter("LogReporter"))); } - if (FLAGS_report_metrics_db) { - composite_reporter->add(std::unique_ptr( - new PerfDbReporter("PerfDbReporter", FLAGS_hashed_id, FLAGS_test_name, - FLAGS_sys_info, FLAGS_server_address, FLAGS_tag))); - } return std::shared_ptr(composite_reporter); } From 969ffaf5c61aeec0c52a721d64b39528da233fd3 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Thu, 14 Apr 2016 12:59:42 -0700 Subject: [PATCH 32/38] Enable JSON reports for qps drivers --- test/cpp/qps/qps_json_driver.cc | 6 +++++- test/cpp/qps/report.cc | 21 ++++++++++++++++++++- test/cpp/qps/report.h | 6 +++++- test/cpp/util/benchmark_config.cc | 7 +++++++ tools/run_tests/run_performance_tests.py | 1 + 5 files changed, 38 insertions(+), 3 deletions(-) diff --git a/test/cpp/qps/qps_json_driver.cc b/test/cpp/qps/qps_json_driver.cc index 8943a43ba87..e9266a5711b 100644 --- a/test/cpp/qps/qps_json_driver.cc +++ b/test/cpp/qps/qps_json_driver.cc @@ -102,12 +102,16 @@ static void QpsDriver() { for (int i = 0; i < scenarios.scenarios_size(); i++) { const Scenario &scenario = scenarios.scenarios(i); std::cerr << "RUNNING SCENARIO: " << scenario.name() << "\n"; - const auto result = + auto result = RunScenario(scenario.client_config(), scenario.num_clients(), scenario.server_config(), scenario.num_servers(), scenario.warmup_seconds(), scenario.benchmark_seconds(), scenario.spawn_local_worker_count()); + // Amend the result with scenario config. Eventually we should adjust + // RunScenario contract so we don't need to touch the result here. + result->mutable_scenario()->CopyFrom(scenario); + GetReporter()->ReportQPS(*result); GetReporter()->ReportQPSPerCore(*result); GetReporter()->ReportLatency(*result); diff --git a/test/cpp/qps/report.cc b/test/cpp/qps/report.cc index 6037ac76038..05fb1111206 100644 --- a/test/cpp/qps/report.cc +++ b/test/cpp/qps/report.cc @@ -33,6 +33,11 @@ #include "test/cpp/qps/report.h" +#include + +#include +#include + #include #include "test/cpp/qps/driver.h" #include "test/cpp/qps/stats.h" @@ -120,7 +125,21 @@ void GprLogReporter::ReportTimes(const ScenarioResult& result) { } void JsonReporter::ReportQPS(const ScenarioResult& result) { - + std::unique_ptr type_resolver( + google::protobuf::util::NewTypeResolverForDescriptorPool( + "type.googleapis.com", + google::protobuf::DescriptorPool::generated_pool())); + grpc::string binary; + grpc::string json_string; + result.SerializeToString(&binary); + auto status = BinaryToJsonString(type_resolver.get(), + "type.googleapis.com/grpc.testing.ScenarioResult", + binary, &json_string); + GPR_ASSERT(status.ok()); + + std::ofstream output_file(report_file_); + output_file << json_string; + output_file.close(); } void JsonReporter::ReportQPSPerCore(const ScenarioResult& result) { diff --git a/test/cpp/qps/report.h b/test/cpp/qps/report.h index b299e415f18..e32a129e76a 100644 --- a/test/cpp/qps/report.h +++ b/test/cpp/qps/report.h @@ -107,13 +107,17 @@ class GprLogReporter : public Reporter { /** Dumps the report to a JSON file. */ class JsonReporter : public Reporter { public: - JsonReporter(const string& name) : Reporter(name) {} + JsonReporter(const string& name, const string& report_file) : + Reporter(name), + report_file_(report_file) {} private: void ReportQPS(const ScenarioResult& result) GRPC_OVERRIDE; void ReportQPSPerCore(const ScenarioResult& result) GRPC_OVERRIDE; void ReportLatency(const ScenarioResult& result) GRPC_OVERRIDE; void ReportTimes(const ScenarioResult& result) GRPC_OVERRIDE; + + const string report_file_; }; } // namespace testing diff --git a/test/cpp/util/benchmark_config.cc b/test/cpp/util/benchmark_config.cc index ca4c27ec355..6fc864069ef 100644 --- a/test/cpp/util/benchmark_config.cc +++ b/test/cpp/util/benchmark_config.cc @@ -37,6 +37,9 @@ DEFINE_bool(enable_log_reporter, true, "Enable reporting of benchmark results through GprLog"); +DEFINE_string(scenario_result_file, "", + "Write JSON benchmark report to the file specified."); + DEFINE_string(hashed_id, "", "Hash of the user id"); DEFINE_string(test_name, "", "Name of the test being executed"); @@ -68,6 +71,10 @@ static std::shared_ptr InitBenchmarkReporters() { composite_reporter->add( std::unique_ptr(new GprLogReporter("LogReporter"))); } + if (FLAGS_scenario_result_file != "") { + composite_reporter->add(std::unique_ptr( + new JsonReporter("JsonReporter", FLAGS_scenario_result_file))); + } return std::shared_ptr(composite_reporter); } diff --git a/tools/run_tests/run_performance_tests.py b/tools/run_tests/run_performance_tests.py index b3729acd9f5..b62a4287477 100755 --- a/tools/run_tests/run_performance_tests.py +++ b/tools/run_tests/run_performance_tests.py @@ -98,6 +98,7 @@ def create_scenario_jobspec(scenario_json, workers, remote_host=None): # setting QPS_WORKERS env variable here makes sure it works with SSH too. cmd = 'QPS_WORKERS="%s" bins/opt/qps_json_driver ' % ','.join(workers) cmd += '--scenarios_json=%s' % pipes.quote(json.dumps({'scenarios': [scenario_json]})) + cmd += ' --scenario_result_file=scenario_result.json' if remote_host: user_at_host = '%s@%s' % (_REMOTE_HOST_USERNAME, remote_host) cmd = 'ssh %s "cd ~/performance_workspace/grpc/ && %s"' % (user_at_host, cmd) From 453442eefb742a7a9b4b87e434da5d135d4fa2f4 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Thu, 14 Apr 2016 18:05:42 -0700 Subject: [PATCH 33/38] fix formatting --- test/cpp/qps/report.cc | 12 +++++------- test/cpp/qps/report.h | 5 ++--- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/test/cpp/qps/report.cc b/test/cpp/qps/report.cc index 05fb1111206..9cef7bf52f8 100644 --- a/test/cpp/qps/report.cc +++ b/test/cpp/qps/report.cc @@ -85,7 +85,7 @@ void GprLogReporter::ReportQPS(const ScenarioResult& result) { Histogram histogram; histogram.MergeProto(result.latencies()); gpr_log(GPR_INFO, "QPS: %.1f", - histogram.Count() / average(result.client_stats(), WallTime)); + histogram.Count() / average(result.client_stats(), WallTime)); } void GprLogReporter::ReportQPSPerCore(const ScenarioResult& result) { @@ -102,10 +102,8 @@ void GprLogReporter::ReportLatency(const ScenarioResult& result) { histogram.MergeProto(result.latencies()); gpr_log(GPR_INFO, "Latencies (50/90/95/99/99.9%%-ile): %.1f/%.1f/%.1f/%.1f/%.1f us", - histogram.Percentile(50) / 1000, - histogram.Percentile(90) / 1000, - histogram.Percentile(95) / 1000, - histogram.Percentile(99) / 1000, + histogram.Percentile(50) / 1000, histogram.Percentile(90) / 1000, + histogram.Percentile(95) / 1000, histogram.Percentile(99) / 1000, histogram.Percentile(99.9) / 1000); } @@ -133,8 +131,8 @@ void JsonReporter::ReportQPS(const ScenarioResult& result) { grpc::string json_string; result.SerializeToString(&binary); auto status = BinaryToJsonString(type_resolver.get(), - "type.googleapis.com/grpc.testing.ScenarioResult", - binary, &json_string); + "type.googleapis.com/grpc.testing.ScenarioResult", + binary, &json_string); GPR_ASSERT(status.ok()); std::ofstream output_file(report_file_); diff --git a/test/cpp/qps/report.h b/test/cpp/qps/report.h index e32a129e76a..8f04d841245 100644 --- a/test/cpp/qps/report.h +++ b/test/cpp/qps/report.h @@ -107,9 +107,8 @@ class GprLogReporter : public Reporter { /** Dumps the report to a JSON file. */ class JsonReporter : public Reporter { public: - JsonReporter(const string& name, const string& report_file) : - Reporter(name), - report_file_(report_file) {} + JsonReporter(const string& name, const string& report_file) + : Reporter(name), report_file_(report_file) {} private: void ReportQPS(const ScenarioResult& result) GRPC_OVERRIDE; From 6321cec58e708ded42ae736b6caf56b0df868735 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Thu, 14 Apr 2016 20:00:29 -0700 Subject: [PATCH 34/38] fix formatting --- test/cpp/qps/report.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/cpp/qps/report.cc b/test/cpp/qps/report.cc index 9cef7bf52f8..07ab0a8f284 100644 --- a/test/cpp/qps/report.cc +++ b/test/cpp/qps/report.cc @@ -130,8 +130,8 @@ void JsonReporter::ReportQPS(const ScenarioResult& result) { grpc::string binary; grpc::string json_string; result.SerializeToString(&binary); - auto status = BinaryToJsonString(type_resolver.get(), - "type.googleapis.com/grpc.testing.ScenarioResult", + auto status = BinaryToJsonString( + type_resolver.get(), "type.googleapis.com/grpc.testing.ScenarioResult", binary, &json_string); GPR_ASSERT(status.ok()); From efd9803be5dfb367d0649987136a02be0b70ea0b Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Thu, 14 Apr 2016 16:29:24 -0700 Subject: [PATCH 35/38] Uploading results to big query --- tools/run_tests/performance/export_utils.py | 88 +++++++++++++++ .../performance/scenario_result_schema.json | 103 ++++++++++++++++++ 2 files changed, 191 insertions(+) create mode 100644 tools/run_tests/performance/export_utils.py create mode 100644 tools/run_tests/performance/scenario_result_schema.json diff --git a/tools/run_tests/performance/export_utils.py b/tools/run_tests/performance/export_utils.py new file mode 100644 index 00000000000..bdb36a5e822 --- /dev/null +++ b/tools/run_tests/performance/export_utils.py @@ -0,0 +1,88 @@ +# 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. + +# utilities for exporting benchmark results + +import json +import os +import sys +import uuid + + +gcp_utils_dir = os.path.abspath(os.path.join( + os.path.dirname(__file__), '../../gcp/utils')) +sys.path.append(gcp_utils_dir) +import big_query_utils + + +_PROJECT_ID='grpc-testing' +_DATASET_ID='test_dataset' +_RESULTS_TABLE_ID='scenario_results' + + +def upload_scenario_result_to_bigquery(result_file): + bq = big_query_utils.create_big_query() + _create_results_table(bq) + + with open(result_file, 'r') as f: + scenario_result = json.loads(f.read()) + _insert_result(bq, scenario_result) + + +def _insert_result(bq, scenario_result): + _flatten_result_inplace(scenario_result) + + # TODO: handle errors... + row = big_query_utils.make_row(str(uuid.uuid4()), scenario_result) + return big_query_utils.insert_rows(bq, + _PROJECT_ID, + _DATASET_ID, + _RESULTS_TABLE_ID, + [row]) + + +def _create_results_table(bq): + with open(os.path.dirname(__file__) + '/scenario_result_schema.json', 'r') as f: + table_schema = json.loads(f.read()) + desc = 'Results of performance benchmarks.' + return big_query_utils.create_table2(bq, _PROJECT_ID, _DATASET_ID, + _RESULTS_TABLE_ID, table_schema, desc) + + +def _flatten_result_inplace(scenario_result): + """Bigquery is not really great for handling deeply nested data + and repeated fields. To maintain values of some fields while keeping + the schema relatively simple, we artificially leave some of the fields + as JSON strings. + """ + scenario_result['scenario']['clientConfig'] = json.dumps(scenario_result['scenario']['clientConfig']) + scenario_result['scenario']['serverConfig'] = json.dumps(scenario_result['scenario']['serverConfig']) + scenario_result['latencies'] = json.dumps(scenario_result['latencies']) + for stats in scenario_result['clientStats']: + stats['latencies'] = json.dumps(stats['latencies']) diff --git a/tools/run_tests/performance/scenario_result_schema.json b/tools/run_tests/performance/scenario_result_schema.json new file mode 100644 index 00000000000..39aba21b0c2 --- /dev/null +++ b/tools/run_tests/performance/scenario_result_schema.json @@ -0,0 +1,103 @@ +[ + { + "name": "scenario", + "type": "RECORD", + "mode": "NULLABLE", + "fields": [ + { + "name": "name", + "type": "STRING", + "mode": "NULLABLE" + }, + { + "name": "clientConfig", + "type": "STRING", + "mode": "NULLABLE" + }, + { + "name": "numClients", + "type": "INTEGER", + "mode": "NULLABLE" + }, + { + "name": "serverConfig", + "type": "STRING", + "mode": "NULLABLE" + }, + { + "name": "numServers", + "type": "INTEGER", + "mode": "NULLABLE" + }, + { + "name": "warmupSeconds", + "type": "INTEGER", + "mode": "NULLABLE" + }, + { + "name": "benchmarkSeconds", + "type": "INTEGER", + "mode": "NULLABLE" + } + ] + }, + { + "name": "latencies", + "type": "STRING", + "mode": "NULLABLE" + }, + { + "name": "clientStats", + "type": "RECORD", + "mode": "REPEATED", + "fields": [ + { + "name": "latencies", + "type": "STRING", + "mode": "NULLABLE" + }, + { + "name": "timeElapsed", + "type": "FLOAT", + "mode": "NULLABLE" + }, + { + "name": "timeUser", + "type": "FLOAT", + "mode": "NULLABLE" + }, + { + "name": "timeSystem", + "type": "FLOAT", + "mode": "NULLABLE" + } + ] + }, + { + "name": "serverStats", + "type": "RECORD", + "mode": "REPEATED", + "fields": [ + { + "name": "timeElapsed", + "type": "FLOAT", + "mode": "NULLABLE" + }, + { + "name": "timeUser", + "type": "FLOAT", + "mode": "NULLABLE" + }, + { + "name": "timeSystem", + "type": "FLOAT", + "mode": "NULLABLE" + } + ] + }, + { + "name": "serverCores", + "type": "INTEGER", + "mode": "REPEATED" + } +] From 7d54db8d490b986bd5b704241d38ff0e28141eac Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Thu, 14 Apr 2016 16:57:45 -0700 Subject: [PATCH 36/38] minor refactoring of biq_query_utils --- tools/gcp/utils/big_query_utils.py | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/tools/gcp/utils/big_query_utils.py b/tools/gcp/utils/big_query_utils.py index c331a679422..913afd059eb 100755 --- a/tools/gcp/utils/big_query_utils.py +++ b/tools/gcp/utils/big_query_utils.py @@ -71,16 +71,22 @@ def create_dataset(biq_query, project_id, dataset_id): def create_table(big_query, project_id, dataset_id, table_id, table_schema, description): + fields = [{'name': field_name, + 'type': field_type, + 'description': field_description + } for (field_name, field_type, field_description) in table_schema] + return create_table2(big_query, project_id, dataset_id, table_id, + fields, description) + + +def create_table2(big_query, project_id, dataset_id, table_id, fields_schema, + description): is_success = True body = { 'description': description, 'schema': { - 'fields': [{ - 'name': field_name, - 'type': field_type, - 'description': field_description - } for (field_name, field_type, field_description) in table_schema] + 'fields': fields_schema }, 'tableReference': { 'datasetId': dataset_id, @@ -112,9 +118,7 @@ def insert_rows(big_query, project_id, dataset_id, table_id, rows_list): datasetId=dataset_id, tableId=table_id, body=body) - print body res = insert_req.execute(num_retries=NUM_RETRIES) - print res except HttpError as http_error: print 'Error in inserting rows in the table %s' % table_id is_success = False From 88cc4e26edc158b95f5aa2d54d59e3ffff1a2fc0 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Thu, 14 Apr 2016 16:58:50 -0700 Subject: [PATCH 37/38] minor changes to schema --- tools/run_tests/performance/export_utils.py | 1 + .../performance/scenario_result_schema.json | 103 +++++++++++++++++- 2 files changed, 102 insertions(+), 2 deletions(-) diff --git a/tools/run_tests/performance/export_utils.py b/tools/run_tests/performance/export_utils.py index bdb36a5e822..6df64cca1f1 100644 --- a/tools/run_tests/performance/export_utils.py +++ b/tools/run_tests/performance/export_utils.py @@ -86,3 +86,4 @@ def _flatten_result_inplace(scenario_result): scenario_result['latencies'] = json.dumps(scenario_result['latencies']) for stats in scenario_result['clientStats']: stats['latencies'] = json.dumps(stats['latencies']) + scenario_result['serverCores'] = json.dumps(scenario_result['serverCores']) diff --git a/tools/run_tests/performance/scenario_result_schema.json b/tools/run_tests/performance/scenario_result_schema.json index 39aba21b0c2..10d24a25177 100644 --- a/tools/run_tests/performance/scenario_result_schema.json +++ b/tools/run_tests/performance/scenario_result_schema.json @@ -1,4 +1,41 @@ [ + { + "name": "metadata", + "type": "RECORD", + "mode": "NULLABLE", + "fields": [ + { + "name": "buildNumber", + "type": "INTEGER", + "mode": "NULLABLE" + }, + { + "name": "buildUrl", + "type": "STRING", + "mode": "NULLABLE" + }, + { + "name": "jobName", + "type": "STRING", + "mode": "NULLABLE" + }, + { + "name": "gitCommit", + "type": "STRING", + "mode": "NULLABLE" + }, + { + "name": "gitActualCommit", + "type": "STRING", + "mode": "NULLABLE" + }, + { + "name": "created", + "type": "TIMESTAMP", + "mode": "NULLABLE" + } + ] + }, { "name": "scenario", "type": "RECORD", @@ -97,7 +134,69 @@ }, { "name": "serverCores", - "type": "INTEGER", - "mode": "REPEATED" + "type": "STRING", + "mode": "NULLABLE" + }, + { + "name": "summary", + "type": "RECORD", + "mode": "NULLABLE", + "fields": [ + { + "name": "qps", + "type": "FLOAT", + "mode": "NULLABLE" + }, + { + "name": "qps_per_server_core", + "type": "FLOAT", + "mode": "NULLABLE" + }, + { + "name": "server_system_time", + "type": "FLOAT", + "mode": "NULLABLE" + }, + { + "name": "server_user_time", + "type": "FLOAT", + "mode": "NULLABLE" + }, + { + "name": "client_system_time", + "type": "FLOAT", + "mode": "NULLABLE" + }, + { + "name": "client_user_time", + "type": "FLOAT", + "mode": "NULLABLE" + }, + { + "name": "latency_50", + "type": "FLOAT", + "mode": "NULLABLE" + }, + { + "name": "latency_90", + "type": "FLOAT", + "mode": "NULLABLE" + }, + { + "name": "latency_95", + "type": "FLOAT", + "mode": "NULLABLE" + }, + { + "name": "latency_99", + "type": "FLOAT", + "mode": "NULLABLE" + }, + { + "name": "latency_999", + "type": "FLOAT", + "mode": "NULLABLE" + } + ] } ] From 6d7fa5572e507f6d541b3bca035ba19ad3761e42 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Thu, 14 Apr 2016 17:42:54 -0700 Subject: [PATCH 38/38] result uploading --- .../{export_utils.py => bq_upload_result.py} | 42 ++++++++++++------- tools/run_tests/performance/run_qps_driver.sh | 40 ++++++++++++++++++ tools/run_tests/run_performance_tests.py | 28 +++++++++---- 3 files changed, 87 insertions(+), 23 deletions(-) rename tools/run_tests/performance/{export_utils.py => bq_upload_result.py} (70%) mode change 100644 => 100755 create mode 100755 tools/run_tests/performance/run_qps_driver.sh diff --git a/tools/run_tests/performance/export_utils.py b/tools/run_tests/performance/bq_upload_result.py old mode 100644 new mode 100755 similarity index 70% rename from tools/run_tests/performance/export_utils.py rename to tools/run_tests/performance/bq_upload_result.py index 6df64cca1f1..0f53ba5d02d --- a/tools/run_tests/performance/export_utils.py +++ b/tools/run_tests/performance/bq_upload_result.py @@ -1,3 +1,4 @@ +#!/usr/bin/env python2.7 # Copyright 2016, Google Inc. # All rights reserved. # @@ -27,8 +28,9 @@ # (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. -# utilities for exporting benchmark results +# Uploads performance benchmark result file to bigquery. +import argparse import json import os import sys @@ -42,37 +44,36 @@ import big_query_utils _PROJECT_ID='grpc-testing' -_DATASET_ID='test_dataset' -_RESULTS_TABLE_ID='scenario_results' -def upload_scenario_result_to_bigquery(result_file): +def _upload_scenario_result_to_bigquery(dataset_id, table_id, result_file): bq = big_query_utils.create_big_query() - _create_results_table(bq) + _create_results_table(bq, dataset_id, table_id) with open(result_file, 'r') as f: scenario_result = json.loads(f.read()) - _insert_result(bq, scenario_result) + if not _insert_result(bq, dataset_id, table_id, scenario_result): + print 'Error uploading result to bigquery.' + sys.exit(1) -def _insert_result(bq, scenario_result): - _flatten_result_inplace(scenario_result) - # TODO: handle errors... +def _insert_result(bq, dataset_id, table_id, scenario_result): + _flatten_result_inplace(scenario_result) row = big_query_utils.make_row(str(uuid.uuid4()), scenario_result) return big_query_utils.insert_rows(bq, _PROJECT_ID, - _DATASET_ID, - _RESULTS_TABLE_ID, + dataset_id, + table_id, [row]) -def _create_results_table(bq): +def _create_results_table(bq, dataset_id, table_id): with open(os.path.dirname(__file__) + '/scenario_result_schema.json', 'r') as f: table_schema = json.loads(f.read()) desc = 'Results of performance benchmarks.' - return big_query_utils.create_table2(bq, _PROJECT_ID, _DATASET_ID, - _RESULTS_TABLE_ID, table_schema, desc) + return big_query_utils.create_table2(bq, _PROJECT_ID, dataset_id, + table_id, table_schema, desc) def _flatten_result_inplace(scenario_result): @@ -87,3 +88,16 @@ def _flatten_result_inplace(scenario_result): for stats in scenario_result['clientStats']: stats['latencies'] = json.dumps(stats['latencies']) scenario_result['serverCores'] = json.dumps(scenario_result['serverCores']) + + +argp = argparse.ArgumentParser(description='Upload result to big query.') +argp.add_argument('--bq_result_table', required=True, default=None, type=str, + help='Bigquery "dataset.table" to upload results to.') +argp.add_argument('--file_to_upload', default='scenario_result.json', type=str, + help='Report file to upload.') + +args = argp.parse_args() + +dataset_id, table_id = args.bq_result_table.split('.', 2) +_upload_scenario_result_to_bigquery(dataset_id, table_id, args.file_to_upload) +print 'Successfully uploaded %s to BigQuery.\n' % args.file_to_upload diff --git a/tools/run_tests/performance/run_qps_driver.sh b/tools/run_tests/performance/run_qps_driver.sh new file mode 100755 index 00000000000..c8c6890df9d --- /dev/null +++ b/tools/run_tests/performance/run_qps_driver.sh @@ -0,0 +1,40 @@ +#!/bin/bash +# 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. + +set -ex + +cd $(dirname $0)/../../.. + +bins/opt/qps_json_driver "$@" + +if [ "$BQ_RESULT_TABLE" != "" ] +then + tools/run_tests/performance/bq_upload_result.py --bq_result_table="$BQ_RESULT_TABLE" +fi diff --git a/tools/run_tests/run_performance_tests.py b/tools/run_tests/run_performance_tests.py index b62a4287477..beedd819ad0 100755 --- a/tools/run_tests/run_performance_tests.py +++ b/tools/run_tests/run_performance_tests.py @@ -93,15 +93,19 @@ def create_qpsworker_job(language, shortname=None, return QpsWorkerJob(jobspec, language, host_and_port) -def create_scenario_jobspec(scenario_json, workers, remote_host=None): +def create_scenario_jobspec(scenario_json, workers, remote_host=None, + bq_result_table=None): """Runs one scenario using QPS driver.""" # setting QPS_WORKERS env variable here makes sure it works with SSH too. - cmd = 'QPS_WORKERS="%s" bins/opt/qps_json_driver ' % ','.join(workers) - cmd += '--scenarios_json=%s' % pipes.quote(json.dumps({'scenarios': [scenario_json]})) - cmd += ' --scenario_result_file=scenario_result.json' + cmd = 'QPS_WORKERS="%s" ' % ','.join(workers) + if bq_result_table: + cmd += 'BQ_RESULT_TABLE="%s" ' % bq_result_table + cmd += 'tools/run_tests/performance/run_qps_driver.sh ' + cmd += '--scenarios_json=%s ' % pipes.quote(json.dumps({'scenarios': [scenario_json]})) + cmd += '--scenario_result_file=scenario_result.json' if remote_host: user_at_host = '%s@%s' % (_REMOTE_HOST_USERNAME, remote_host) - cmd = 'ssh %s "cd ~/performance_workspace/grpc/ && %s"' % (user_at_host, cmd) + cmd = 'ssh %s "cd ~/performance_workspace/grpc/ && "%s' % (user_at_host, pipes.quote(cmd)) return jobset.JobSpec( cmdline=[cmd], @@ -117,7 +121,7 @@ def create_quit_jobspec(workers, remote_host=None): cmd = 'QPS_WORKERS="%s" bins/opt/qps_driver --quit' % ','.join(workers) if remote_host: user_at_host = '%s@%s' % (_REMOTE_HOST_USERNAME, remote_host) - cmd = 'ssh %s "cd ~/performance_workspace/grpc/ && %s"' % (user_at_host, cmd) + cmd = 'ssh %s "cd ~/performance_workspace/grpc/ && "%s' % (user_at_host, pipes.quote(cmd)) return jobset.JobSpec( cmdline=[cmd], @@ -226,7 +230,8 @@ def start_qpsworkers(languages, worker_hosts): for worker_idx, worker in enumerate(workers)] -def create_scenarios(languages, workers_by_lang, remote_host=None, regex='.*'): +def create_scenarios(languages, workers_by_lang, remote_host=None, regex='.*', + bq_result_table=None): """Create jobspecs for scenarios to run.""" scenarios = [] for language in languages: @@ -248,7 +253,8 @@ def create_scenarios(languages, workers_by_lang, remote_host=None, regex='.*'): workers[idx] = workers_by_lang[custom_server_lang][idx] scenario = create_scenario_jobspec(scenario_json, workers, - remote_host=remote_host) + remote_host=remote_host, + bq_result_table=bq_result_table) scenarios.append(scenario) # the very last scenario requests shutting down the workers. @@ -290,6 +296,8 @@ argp.add_argument('--remote_worker_host', help='Worker hosts where to start QPS workers.') argp.add_argument('-r', '--regex', default='.*', type=str, help='Regex to select scenarios to run.') +argp.add_argument('--bq_result_table', default=None, type=str, + help='Bigquery "dataset.table" to upload results to.') args = argp.parse_args() @@ -298,6 +306,7 @@ languages = set(scenario_config.LANGUAGES[l] scenario_config.LANGUAGES.iterkeys() if x == 'all' else [x] for x in args.language)) + # Put together set of remote hosts where to run and build remote_hosts = set() if args.remote_worker_host: @@ -329,7 +338,8 @@ try: scenarios = create_scenarios(languages, workers_by_lang=worker_addresses, remote_host=args.remote_driver_host, - regex=args.regex) + regex=args.regex, + bq_result_table=args.bq_result_table) if not scenarios: raise Exception('No scenarios to run')