From 29a7f40af3d08c70ab8ba23fe33eb073a3da7b7f Mon Sep 17 00:00:00 2001 From: "Mark D. Roth" Date: Wed, 19 Oct 2016 13:46:38 -0700 Subject: [PATCH] Convert method config to a struct for use in the fast path. --- src/core/ext/client_config/client_channel.c | 6 +- src/core/ext/client_config/method_config.c | 56 ++++++++++++++-- src/core/ext/client_config/method_config.h | 16 ++++- src/core/lib/channel/message_size_filter.c | 74 +++++++++++++++------ src/core/lib/transport/mdstr_hash_table.c | 35 +++++++--- src/core/lib/transport/mdstr_hash_table.h | 9 +++ 6 files changed, 155 insertions(+), 41 deletions(-) diff --git a/src/core/ext/client_config/client_channel.c b/src/core/ext/client_config/client_channel.c index beaa9637c3b..613e98922b3 100644 --- a/src/core/ext/client_config/client_channel.c +++ b/src/core/ext/client_config/client_channel.c @@ -855,8 +855,7 @@ static void read_service_config(grpc_exec_ctx *exec_ctx, void *arg, // If the method config table was present, use it. if (method_config_table != NULL) { const grpc_method_config *method_config = - grpc_method_config_table_get_method_config(method_config_table, - calld->path); + grpc_method_config_table_get(method_config_table, calld->path); if (method_config != NULL) { const gpr_timespec *per_method_timeout = grpc_method_config_get_timeout(method_config); @@ -922,8 +921,7 @@ static grpc_error *cc_init_call_elem(grpc_exec_ctx *exec_ctx, grpc_method_config_table_ref(chand->method_config_table); gpr_mu_unlock(&chand->mu); grpc_method_config *method_config = - grpc_method_config_table_get_method_config(method_config_table, - args->path); + grpc_method_config_table_get(method_config_table, args->path); if (method_config != NULL) { const gpr_timespec *per_method_timeout = grpc_method_config_get_timeout(method_config); diff --git a/src/core/ext/client_config/method_config.c b/src/core/ext/client_config/method_config.c index f8a82323e70..a42fe2bfd91 100644 --- a/src/core/ext/client_config/method_config.c +++ b/src/core/ext/client_config/method_config.c @@ -254,12 +254,12 @@ int grpc_method_config_table_cmp(const grpc_method_config_table* table1, return grpc_mdstr_hash_table_cmp(table1, table2); } -grpc_method_config* grpc_method_config_table_get_method_config( - const grpc_method_config_table* table, const grpc_mdstr* path) { - grpc_method_config* method_config = grpc_mdstr_hash_table_get(table, path); +void* grpc_method_config_table_get(const grpc_mdstr_hash_table* table, + const grpc_mdstr* path) { + void* value = grpc_mdstr_hash_table_get(table, path); // If we didn't find a match for the path, try looking for a wildcard // entry (i.e., change "/service/method" to "/service/*"). - if (method_config == NULL) { + if (value == NULL) { const char* path_str = grpc_mdstr_as_c_string(path); const char* sep = strrchr(path_str, '/') + 1; const size_t len = (size_t)(sep - path_str); @@ -269,10 +269,10 @@ grpc_method_config* grpc_method_config_table_get_method_config( buf[len + 1] = '\0'; grpc_mdstr* wildcard_path = grpc_mdstr_from_string(buf); gpr_free(buf); - method_config = grpc_mdstr_hash_table_get(table, wildcard_path); + value = grpc_mdstr_hash_table_get(table, wildcard_path); GRPC_MDSTR_UNREF(wildcard_path); } - return method_config; + return value; } static void* copy_arg(void* p) { return grpc_method_config_table_ref(p); } @@ -294,3 +294,47 @@ grpc_arg grpc_method_config_table_create_channel_arg( arg.value.pointer.vtable = &arg_vtable; return arg; } + +// State used by convert_entry() below. +typedef struct conversion_state { + void* (*convert_value)(const grpc_method_config* method_config); + const grpc_mdstr_hash_table_vtable* vtable; + size_t num_entries; + grpc_mdstr_hash_table_entry* entries; +} conversion_state; + +// A function to be passed to grpc_mdstr_hash_table_iterate() to create +// a copy of the entries. +static void convert_entry(const grpc_mdstr_hash_table_entry* entry, + void* user_data) { + conversion_state* state = user_data; + state->entries[state->num_entries].key = GRPC_MDSTR_REF(entry->key); + state->entries[state->num_entries].value = state->convert_value(entry->value); + state->entries[state->num_entries].vtable = state->vtable; + ++state->num_entries; +} + +grpc_mdstr_hash_table* grpc_method_config_table_convert( + const grpc_method_config_table* table, + void* (*convert_value)(const grpc_method_config* method_config), + const grpc_mdstr_hash_table_vtable* vtable) { + // Create an array of the entries in the table with converted values. + conversion_state state; + state.convert_value = convert_value; + state.vtable = vtable; + state.num_entries = 0; + state.entries = gpr_malloc(sizeof(grpc_mdstr_hash_table_entry) * + grpc_mdstr_hash_table_num_entries(table)); + grpc_mdstr_hash_table_iterate(table, convert_entry, &state); + // Create a new table based on the array we just constructed. + grpc_mdstr_hash_table* new_table = + grpc_mdstr_hash_table_create(state.num_entries, state.entries); + // Clean up the array. + for (size_t i = 0; i < state.num_entries; ++i) { + GRPC_MDSTR_UNREF(state.entries[i].key); + vtable->destroy_value(state.entries[i].value); + } + gpr_free(state.entries); + // Return the new table. + return new_table; +} diff --git a/src/core/ext/client_config/method_config.h b/src/core/ext/client_config/method_config.h index 1302ac425d5..87c72ec0647 100644 --- a/src/core/ext/client_config/method_config.h +++ b/src/core/ext/client_config/method_config.h @@ -106,11 +106,23 @@ int grpc_method_config_table_cmp(const grpc_method_config_table* table1, /// the form "/service/method". /// Returns NULL if the method has no config. /// Caller does NOT own a reference to the result. -grpc_method_config* grpc_method_config_table_get_method_config( - const grpc_method_config_table* table, const grpc_mdstr* path); +/// +/// Note: This returns a void* instead of a grpc_method_config* so that +/// it can also be used for tables constructed via +/// grpc_method_config_table_convert(). +void* grpc_method_config_table_get(const grpc_mdstr_hash_table* table, + const grpc_mdstr* path); /// Returns a channel arg containing \a table. grpc_arg grpc_method_config_table_create_channel_arg( grpc_method_config_table* table); +/// Generates a new table from \a table whose values are converted to a +/// new form via the \a convert_value function. The new table will use +/// \a vtable for its values. +grpc_mdstr_hash_table* grpc_method_config_table_convert( + const grpc_method_config_table* table, + void* (*convert_value)(const grpc_method_config* method_config), + const grpc_mdstr_hash_table_vtable* vtable); + #endif /* GRPC_CORE_EXT_CLIENT_CONFIG_METHOD_CONFIG_H */ diff --git a/src/core/lib/channel/message_size_filter.c b/src/core/lib/channel/message_size_filter.c index 1382f199453..c402cf78a36 100644 --- a/src/core/lib/channel/message_size_filter.c +++ b/src/core/lib/channel/message_size_filter.c @@ -45,6 +45,44 @@ // The protobuf library will (by default) start warning at 100 megs. #define DEFAULT_MAX_RECV_MESSAGE_LENGTH (4 * 1024 * 1024) +typedef struct message_size_limits { + int max_send_size; + int max_recv_size; +} message_size_limits; + +static void* message_size_limits_copy(void* value) { + void* new_value = gpr_malloc(sizeof(message_size_limits)); + memcpy(new_value, value, sizeof(message_size_limits)); + return new_value; +} + +static int message_size_limits_cmp(void* value1, void* value2) { + const message_size_limits* v1 = value1; + const message_size_limits* v2 = value2; + if (v1->max_send_size > v2->max_send_size) return 1; + if (v1->max_send_size < v2->max_send_size) return -1; + if (v1->max_recv_size > v2->max_recv_size) return 1; + if (v1->max_recv_size < v2->max_recv_size) return -1; + return 0; +} + +static const grpc_mdstr_hash_table_vtable message_size_limits_vtable = { + gpr_free, message_size_limits_copy, message_size_limits_cmp}; + +static void* method_config_convert_value( + const grpc_method_config* method_config) { + message_size_limits* value = gpr_malloc(sizeof(message_size_limits)); + const int32_t* max_request_message_bytes = + grpc_method_config_get_max_request_message_bytes(method_config); + value->max_send_size = + max_request_message_bytes != NULL ? *max_request_message_bytes : -1; + const int32_t* max_response_message_bytes = + grpc_method_config_get_max_response_message_bytes(method_config); + value->max_recv_size = + max_response_message_bytes != NULL ? *max_response_message_bytes : -1; + return value; +} + typedef struct call_data { int max_send_size; int max_recv_size; @@ -61,8 +99,8 @@ typedef struct call_data { typedef struct channel_data { int max_send_size; int max_recv_size; - // Method config table. - grpc_method_config_table* method_config_table; + // Maps path names to message_size_limits structs. + grpc_mdstr_hash_table* method_limit_table; } channel_data; // Callback invoked when we receive a message. Here we check the max @@ -132,24 +170,19 @@ static grpc_error* init_call_elem(grpc_exec_ctx* exec_ctx, // size to the receive limit. calld->max_send_size = chand->max_send_size; calld->max_recv_size = chand->max_recv_size; - if (chand->method_config_table != NULL) { - grpc_method_config* method_config = - grpc_method_config_table_get_method_config(chand->method_config_table, - args->path); - if (method_config != NULL) { - const int32_t* max_request_message_bytes = - grpc_method_config_get_max_request_message_bytes(method_config); - if (max_request_message_bytes != NULL && - (*max_request_message_bytes < calld->max_send_size || + if (chand->method_limit_table != NULL) { + message_size_limits* limits = + grpc_method_config_table_get(chand->method_limit_table, args->path); + if (limits != NULL) { + if (limits->max_send_size >= 0 && + (limits->max_send_size < calld->max_send_size || calld->max_send_size < 0)) { - calld->max_send_size = *max_request_message_bytes; + calld->max_send_size = limits->max_send_size; } - const int32_t* max_response_message_bytes = - grpc_method_config_get_max_response_message_bytes(method_config); - if (max_response_message_bytes != NULL && - (*max_response_message_bytes < calld->max_recv_size || + if (limits->max_recv_size >= 0 && + (limits->max_recv_size < calld->max_recv_size || calld->max_recv_size < 0)) { - calld->max_recv_size = *max_response_message_bytes; + calld->max_recv_size = limits->max_recv_size; } } } @@ -191,8 +224,9 @@ static void init_channel_elem(grpc_exec_ctx* exec_ctx, grpc_channel_args_find(args->channel_args, GRPC_ARG_SERVICE_CONFIG); if (channel_arg != NULL) { GPR_ASSERT(channel_arg->type == GRPC_ARG_POINTER); - chand->method_config_table = grpc_method_config_table_ref( - (grpc_method_config_table*)channel_arg->value.pointer.p); + chand->method_limit_table = grpc_method_config_table_convert( + (grpc_method_config_table*)channel_arg->value.pointer.p, + method_config_convert_value, &message_size_limits_vtable); } } @@ -200,7 +234,7 @@ static void init_channel_elem(grpc_exec_ctx* exec_ctx, static void destroy_channel_elem(grpc_exec_ctx* exec_ctx, grpc_channel_element* elem) { channel_data* chand = elem->channel_data; - grpc_method_config_table_unref(chand->method_config_table); + grpc_mdstr_hash_table_unref(chand->method_limit_table); } const grpc_channel_filter grpc_message_size_filter = { diff --git a/src/core/lib/transport/mdstr_hash_table.c b/src/core/lib/transport/mdstr_hash_table.c index 4be0536dd76..30968e91ff9 100644 --- a/src/core/lib/transport/mdstr_hash_table.c +++ b/src/core/lib/transport/mdstr_hash_table.c @@ -42,6 +42,7 @@ struct grpc_mdstr_hash_table { gpr_refcount refs; size_t num_entries; + size_t size; grpc_mdstr_hash_table_entry* entries; }; @@ -50,13 +51,13 @@ struct grpc_mdstr_hash_table { static size_t grpc_mdstr_hash_table_find_index( const grpc_mdstr_hash_table* table, const grpc_mdstr* key, bool find_empty) { - for (size_t i = 0; i < table->num_entries; ++i) { - const size_t idx = (key->hash + i * i) % table->num_entries; + for (size_t i = 0; i < table->size; ++i) { + const size_t idx = (key->hash + i * i) % table->size; if (table->entries[idx].key == NULL) - return find_empty ? idx : table->num_entries; + return find_empty ? idx : table->size; if (table->entries[idx].key == key) return idx; } - return table->num_entries; // Not found. + return table->size; // Not found. } static void grpc_mdstr_hash_table_add( @@ -65,7 +66,7 @@ static void grpc_mdstr_hash_table_add( GPR_ASSERT(value != NULL); const size_t idx = grpc_mdstr_hash_table_find_index(table, key, true /* find_empty */); - GPR_ASSERT(idx != table->num_entries); // Table should never be full. + GPR_ASSERT(idx != table->size); // Table should never be full. grpc_mdstr_hash_table_entry* entry = &table->entries[idx]; entry->key = GRPC_MDSTR_REF(key); entry->value = vtable->copy_value(value); @@ -77,11 +78,12 @@ grpc_mdstr_hash_table* grpc_mdstr_hash_table_create( grpc_mdstr_hash_table* table = gpr_malloc(sizeof(*table)); memset(table, 0, sizeof(*table)); gpr_ref_init(&table->refs, 1); + table->num_entries = num_entries; // Quadratic probing gets best performance when the table is no more // than half full. - table->num_entries = num_entries * 2; + table->size = num_entries * 2; const size_t entry_size = - sizeof(grpc_mdstr_hash_table_entry) * table->num_entries; + sizeof(grpc_mdstr_hash_table_entry) * table->size; table->entries = gpr_malloc(entry_size); memset(table->entries, 0, entry_size); for (size_t i = 0; i < num_entries; ++i) { @@ -98,7 +100,7 @@ grpc_mdstr_hash_table* grpc_mdstr_hash_table_ref(grpc_mdstr_hash_table* table) { int grpc_mdstr_hash_table_unref(grpc_mdstr_hash_table* table) { if (table != NULL && gpr_unref(&table->refs)) { - for (size_t i = 0; i < table->num_entries; ++i) { + for (size_t i = 0; i < table->size; ++i) { grpc_mdstr_hash_table_entry* entry = &table->entries[i]; if (entry->key != NULL) { GRPC_MDSTR_UNREF(entry->key); @@ -112,11 +114,15 @@ int grpc_mdstr_hash_table_unref(grpc_mdstr_hash_table* table) { return 0; } +size_t grpc_mdstr_hash_table_num_entries(const grpc_mdstr_hash_table* table) { + return table->num_entries; +} + void* grpc_mdstr_hash_table_get(const grpc_mdstr_hash_table* table, const grpc_mdstr* key) { const size_t idx = grpc_mdstr_hash_table_find_index(table, key, false /* find_empty */); - if (idx == table->num_entries) return NULL; // Not found. + if (idx == table->size) return NULL; // Not found. return table->entries[idx].value; } @@ -140,3 +146,14 @@ int grpc_mdstr_hash_table_cmp(const grpc_mdstr_hash_table* table1, } return 0; } + +void grpc_mdstr_hash_table_iterate( + const grpc_mdstr_hash_table* table, + void (*func)(const grpc_mdstr_hash_table_entry* entry, void* user_data), + void* user_data) { + for (size_t i = 0; i < table->size; ++i) { + if (table->entries[i].key != NULL) { + func(&table->entries[i], user_data); + } + } +} diff --git a/src/core/lib/transport/mdstr_hash_table.h b/src/core/lib/transport/mdstr_hash_table.h index 52e5b023db0..bceb4df93de 100644 --- a/src/core/lib/transport/mdstr_hash_table.h +++ b/src/core/lib/transport/mdstr_hash_table.h @@ -70,6 +70,9 @@ grpc_mdstr_hash_table* grpc_mdstr_hash_table_ref(grpc_mdstr_hash_table* table); /** Returns 1 when \a table is destroyed. */ int grpc_mdstr_hash_table_unref(grpc_mdstr_hash_table* table); +/** Returns the number of entries in \a table. */ +size_t grpc_mdstr_hash_table_num_entries(const grpc_mdstr_hash_table* table); + /** Returns the value from \a table associated with \a key. Returns NULL if \a key is not found. */ void* grpc_mdstr_hash_table_get(const grpc_mdstr_hash_table* table, @@ -80,4 +83,10 @@ void* grpc_mdstr_hash_table_get(const grpc_mdstr_hash_table* table, int grpc_mdstr_hash_table_cmp(const grpc_mdstr_hash_table* table1, const grpc_mdstr_hash_table* table2); +/** Iterates over the entries in \a table, calling \a func for each entry. */ +void grpc_mdstr_hash_table_iterate( + const grpc_mdstr_hash_table* table, + void (*func)(const grpc_mdstr_hash_table_entry* entry, void* user_data), + void* user_data); + #endif /* GRPC_CORE_LIB_TRANSPORT_MDSTR_HASH_TABLE_H */