From 9c02a3cca1bfa7fbb3fe4fe3b0a16bd2e18e2df5 Mon Sep 17 00:00:00 2001 From: Muxi Yan Date: Thu, 7 Dec 2017 19:02:22 -0800 Subject: [PATCH] Do not separate message/stream compression levels --- include/grpc/compression.h | 4 +- include/grpc/impl/codegen/compression_types.h | 9 ++-- src/core/lib/compression/compression.cc | 28 +++++----- .../lib/compression/compression_internal.cc | 54 +++---------------- .../lib/compression/compression_internal.h | 34 +----------- src/core/lib/surface/channel.cc | 8 +-- src/ruby/ext/grpc/rb_compression_options.c | 12 ++--- test/core/end2end/tests/compressed_payload.cc | 2 +- .../stream_compression_compressed_payload.cc | 23 -------- 9 files changed, 35 insertions(+), 139 deletions(-) diff --git a/include/grpc/compression.h b/include/grpc/compression.h index 4adfc7a2a7f..a4f6a8faf2b 100644 --- a/include/grpc/compression.h +++ b/include/grpc/compression.h @@ -50,9 +50,7 @@ GRPCAPI int grpc_compression_algorithm_name( grpc_compression_algorithm algorithm, const char** name); /** Returns the compression algorithm corresponding to \a level for the - * compression algorithms encoded in the \a accepted_encodings bitset. - * - * It abort()s for unknown levels. */ + * compression algorithms encoded in the \a accepted_encodings bitset.*/ GRPCAPI grpc_compression_algorithm grpc_compression_algorithm_for_level( grpc_compression_level level, uint32_t accepted_encodings); diff --git a/include/grpc/impl/codegen/compression_types.h b/include/grpc/impl/codegen/compression_types.h index dec37fa9679..ddc667fcdb1 100644 --- a/include/grpc/impl/codegen/compression_types.h +++ b/include/grpc/impl/codegen/compression_types.h @@ -68,12 +68,9 @@ typedef enum { * compression algorithms. */ typedef enum { GRPC_COMPRESS_LEVEL_NONE = 0, - GRPC_COMPRESS_LEVEL_MESSAGE_LOW, - GRPC_COMPRESS_LEVEL_MESSAGE_MED, - GRPC_COMPRESS_LEVEL_MESSAGE_HIGH, - GRPC_COMPRESS_LEVEL_STREAM_LOW, - GRPC_COMPRESS_LEVEL_STREAM_MED, - GRPC_COMPRESS_LEVEL_STREAM_HIGH, + GRPC_COMPRESS_LEVEL_LOW, + GRPC_COMPRESS_LEVEL_MED, + GRPC_COMPRESS_LEVEL_HIGH, GRPC_COMPRESS_LEVEL_COUNT } grpc_compression_level; diff --git a/src/core/lib/compression/compression.cc b/src/core/lib/compression/compression.cc index 88fb37ffe26..a5b123b4e2e 100644 --- a/src/core/lib/compression/compression.cc +++ b/src/core/lib/compression/compression.cc @@ -87,22 +87,18 @@ grpc_compression_algorithm grpc_compression_algorithm_for_level( grpc_compression_algorithm algo; if (level == GRPC_COMPRESS_LEVEL_NONE) { return GRPC_COMPRESS_NONE; - } else if (level <= GRPC_COMPRESS_LEVEL_MESSAGE_HIGH) { - GPR_ASSERT( - grpc_compression_algorithm_from_message_stream_compression_algorithm( - &algo, - grpc_message_compression_algorithm_for_level( - grpc_compression_level_to_message_compression_level(level), - grpc_compression_bitset_to_message_bitset(accepted_encodings)), - (grpc_stream_compression_algorithm)0)); - return algo; - } else if (level <= GRPC_COMPRESS_LEVEL_STREAM_HIGH) { - GPR_ASSERT( - grpc_compression_algorithm_from_message_stream_compression_algorithm( - &algo, (grpc_message_compression_algorithm)0, - grpc_stream_compression_algorithm_for_level( - grpc_compression_level_to_stream_compression_level(level), - grpc_compression_bitset_to_stream_bitset(accepted_encodings)))); + } else if (level <= GRPC_COMPRESS_LEVEL_HIGH) { + // TODO(mxyan): Design algorithm to select from all algorithms, including + // stream compression algorithm + if (!grpc_compression_algorithm_from_message_stream_compression_algorithm( + &algo, + grpc_message_compression_algorithm_for_level( + level, + grpc_compression_bitset_to_message_bitset(accepted_encodings)), + static_cast(0))){ + gpr_log(GPR_ERROR, "Parse compression level error"); + return GRPC_COMPRESS_NONE; + } return algo; } else { gpr_log(GPR_ERROR, "Unknown compression level: %d", level); diff --git a/src/core/lib/compression/compression_internal.cc b/src/core/lib/compression/compression_internal.cc index 8352ad255e5..263cdf06eb6 100644 --- a/src/core/lib/compression/compression_internal.cc +++ b/src/core/lib/compression/compression_internal.cc @@ -76,29 +76,6 @@ grpc_mdelem grpc_stream_compression_encoding_mdelem( /* Interfaces performing transformation between compression algorithms and * levels. */ -grpc_message_compression_level -grpc_compression_level_to_message_compression_level( - grpc_compression_level level) { - if (level >= GRPC_COMPRESS_LEVEL_COUNT) { - return GRPC_MESSAGE_COMPRESS_LEVEL_NONE; - } - return (grpc_message_compression_level)( - (uint32_t)(level - GRPC_COMPRESS_LEVEL_NONE) + - (uint32_t)GRPC_MESSAGE_COMPRESS_LEVEL_NONE); -} - -grpc_stream_compression_level -grpc_compression_level_to_stream_compression_level( - grpc_compression_level level) { - if (level >= GRPC_COMPRESS_LEVEL_COUNT) { - return GRPC_STREAM_COMPRESS_LEVEL_NONE; - } - return (grpc_stream_compression_level)( - (uint32_t)(level - (GRPC_MESSAGE_COMPRESS_LEVEL_COUNT - 1) - - GRPC_COMPRESS_LEVEL_NONE) + - (uint32_t)GRPC_STREAM_COMPRESS_LEVEL_NONE); -} - grpc_message_compression_algorithm grpc_compression_algorithm_to_message_compression_algorithm( grpc_compression_algorithm algo) { @@ -209,17 +186,17 @@ int grpc_message_compression_algorithm_name( /* TODO(dgq): Add the ability to specify parameters to the individual * compression algorithms */ grpc_message_compression_algorithm grpc_message_compression_algorithm_for_level( - grpc_message_compression_level level, uint32_t accepted_encodings) { + grpc_compression_level level, uint32_t accepted_encodings) { GRPC_API_TRACE("grpc_message_compression_algorithm_for_level(level=%d)", 1, ((int)level)); - if (level > GRPC_MESSAGE_COMPRESS_LEVEL_HIGH) { + if (level > GRPC_COMPRESS_LEVEL_HIGH) { gpr_log(GPR_ERROR, "Unknown message compression level %d.", (int)level); abort(); } const size_t num_supported = GPR_BITCOUNT(accepted_encodings) - 1; /* discard NONE */ - if (level == GRPC_MESSAGE_COMPRESS_LEVEL_NONE || num_supported == 0) { + if (level == GRPC_COMPRESS_LEVEL_NONE || num_supported == 0) { return GRPC_MESSAGE_COMPRESS_NONE; } @@ -249,13 +226,13 @@ grpc_message_compression_algorithm grpc_message_compression_algorithm_for_level( } switch (level) { - case GRPC_MESSAGE_COMPRESS_LEVEL_NONE: + case GRPC_COMPRESS_LEVEL_NONE: abort(); /* should have been handled already */ - case GRPC_MESSAGE_COMPRESS_LEVEL_LOW: + case GRPC_COMPRESS_LEVEL_LOW: return sorted_supported_algos[0]; - case GRPC_MESSAGE_COMPRESS_LEVEL_MED: + case GRPC_COMPRESS_LEVEL_MED: return sorted_supported_algos[num_supported / 2]; - case GRPC_MESSAGE_COMPRESS_LEVEL_HIGH: + case GRPC_COMPRESS_LEVEL_HIGH: return sorted_supported_algos[num_supported - 1]; default: abort(); @@ -281,23 +258,6 @@ int grpc_message_compression_algorithm_parse( /* Interfaces for stream compression. */ -grpc_stream_compression_algorithm grpc_stream_compression_algorithm_for_level( - grpc_stream_compression_level level, uint32_t accepted_encodings) { - GRPC_API_TRACE("grpc_stream_compression_algorithm_for_level(level=%d)", 1, - ((int)level)); - if (level > GRPC_STREAM_COMPRESS_LEVEL_HIGH) { - gpr_log(GPR_ERROR, "Unknown stream compression level %d.", (int)level); - abort(); - } - - /* TODO(mxyan): Use more sophisticated scheme when more algorithms added. */ - if (level != GRPC_STREAM_COMPRESS_LEVEL_NONE && - GPR_BITGET(accepted_encodings, GRPC_STREAM_COMPRESS_GZIP)) { - return GRPC_STREAM_COMPRESS_GZIP; - } - return GRPC_STREAM_COMPRESS_NONE; -} - int grpc_stream_compression_algorithm_parse( grpc_slice value, grpc_stream_compression_algorithm* algorithm) { if (grpc_slice_eq(value, GRPC_MDSTR_IDENTITY)) { diff --git a/src/core/lib/compression/compression_internal.h b/src/core/lib/compression/compression_internal.h index 4279d4f6974..72f01dd1b7b 100644 --- a/src/core/lib/compression/compression_internal.h +++ b/src/core/lib/compression/compression_internal.h @@ -40,38 +40,9 @@ typedef enum { GRPC_STREAM_COMPRESS_ALGORITHMS_COUNT } grpc_stream_compression_algorithm; -/** Compression levels allow a party with knowledge of its peer's accepted - * encodings to request compression in an abstract way. The level-algorithm - * mapping is performed internally and depends on the peer's supported - * compression algorithms. */ -typedef enum { - GRPC_MESSAGE_COMPRESS_LEVEL_NONE = 0, - GRPC_MESSAGE_COMPRESS_LEVEL_LOW, - GRPC_MESSAGE_COMPRESS_LEVEL_MED, - GRPC_MESSAGE_COMPRESS_LEVEL_HIGH, - GRPC_MESSAGE_COMPRESS_LEVEL_COUNT -} grpc_message_compression_level; - -/** Compression levels for stream compression algorithms */ -typedef enum { - GRPC_STREAM_COMPRESS_LEVEL_NONE = 0, - GRPC_STREAM_COMPRESS_LEVEL_LOW, - GRPC_STREAM_COMPRESS_LEVEL_MED, - GRPC_STREAM_COMPRESS_LEVEL_HIGH, - GRPC_STREAM_COMPRESS_LEVEL_COUNT -} grpc_stream_compression_level; - /* Interfaces performing transformation between compression algorithms and * levels. */ -grpc_message_compression_level -grpc_compression_level_to_message_compression_level( - grpc_compression_level level); - -grpc_stream_compression_level -grpc_compression_level_to_stream_compression_level( - grpc_compression_level level); - grpc_message_compression_algorithm grpc_compression_algorithm_to_message_compression_algorithm( grpc_compression_algorithm algo); @@ -98,16 +69,13 @@ int grpc_message_compression_algorithm_name( grpc_message_compression_algorithm algorithm, const char** name); grpc_message_compression_algorithm grpc_message_compression_algorithm_for_level( - grpc_message_compression_level level, uint32_t accepted_encodings); + grpc_compression_level level, uint32_t accepted_encodings); int grpc_message_compression_algorithm_parse( grpc_slice value, grpc_message_compression_algorithm* algorithm); /* Interfaces for stream compression. */ -grpc_stream_compression_algorithm grpc_stream_compression_algorithm_for_level( - grpc_stream_compression_level level, uint32_t accepted_encodings); - int grpc_stream_compression_algorithm_parse( grpc_slice value, grpc_stream_compression_algorithm* algorithm); diff --git a/src/core/lib/surface/channel.cc b/src/core/lib/surface/channel.cc index 826006f9b60..0936f9251b7 100644 --- a/src/core/lib/surface/channel.cc +++ b/src/core/lib/surface/channel.cc @@ -143,17 +143,17 @@ grpc_channel* grpc_channel_create_with_builder( GRPC_COMPRESSION_CHANNEL_DEFAULT_LEVEL)) { channel->compression_options.default_level.is_set = true; channel->compression_options.default_level.level = - (grpc_compression_level)grpc_channel_arg_get_integer( + static_cast(grpc_channel_arg_get_integer( &args->args[i], {GRPC_COMPRESS_LEVEL_NONE, GRPC_COMPRESS_LEVEL_NONE, - GRPC_COMPRESS_LEVEL_COUNT - 1}); + GRPC_COMPRESS_LEVEL_COUNT - 1})); } else if (0 == strcmp(args->args[i].key, GRPC_COMPRESSION_CHANNEL_DEFAULT_ALGORITHM)) { channel->compression_options.default_algorithm.is_set = true; channel->compression_options.default_algorithm.algorithm = - (grpc_compression_algorithm)grpc_channel_arg_get_integer( + static_cast(grpc_channel_arg_get_integer( &args->args[i], {GRPC_COMPRESS_NONE, GRPC_COMPRESS_NONE, - GRPC_COMPRESS_ALGORITHMS_COUNT - 1}); + GRPC_COMPRESS_ALGORITHMS_COUNT - 1})); } else if (0 == strcmp(args->args[i].key, GRPC_COMPRESSION_CHANNEL_ENABLED_ALGORITHMS_BITSET)) { diff --git a/src/ruby/ext/grpc/rb_compression_options.c b/src/ruby/ext/grpc/rb_compression_options.c index 466f032817c..a7e37099afc 100644 --- a/src/ruby/ext/grpc/rb_compression_options.c +++ b/src/ruby/ext/grpc/rb_compression_options.c @@ -128,11 +128,11 @@ grpc_compression_level grpc_rb_compression_options_level_name_to_value_internal( if (id_compress_level_none == SYM2ID(level_name)) { return GRPC_COMPRESS_LEVEL_NONE; } else if (id_compress_level_low == SYM2ID(level_name)) { - return GRPC_COMPRESS_LEVEL_MESSAGE_LOW; + return GRPC_COMPRESS_LEVEL_LOW; } else if (id_compress_level_medium == SYM2ID(level_name)) { - return GRPC_COMPRESS_LEVEL_MESSAGE_MED; + return GRPC_COMPRESS_LEVEL_MED; } else if (id_compress_level_high == SYM2ID(level_name)) { - return GRPC_COMPRESS_LEVEL_MESSAGE_HIGH; + return GRPC_COMPRESS_LEVEL_HIGH; } rb_raise(rb_eArgError, @@ -265,11 +265,11 @@ VALUE grpc_rb_compression_options_level_value_to_name_internal( switch (compression_value) { case GRPC_COMPRESS_LEVEL_NONE: return ID2SYM(id_compress_level_none); - case GRPC_COMPRESS_LEVEL_MESSAGE_LOW: + case GRPC_COMPRESS_LEVEL_LOW: return ID2SYM(id_compress_level_low); - case GRPC_COMPRESS_LEVEL_MESSAGE_MED: + case GRPC_COMPRESS_LEVEL_MED: return ID2SYM(id_compress_level_medium); - case GRPC_COMPRESS_LEVEL_MESSAGE_HIGH: + case GRPC_COMPRESS_LEVEL_HIGH: return ID2SYM(id_compress_level_high); default: rb_raise( diff --git a/test/core/end2end/tests/compressed_payload.cc b/test/core/end2end/tests/compressed_payload.cc index bfabd0f9c10..618839f03df 100644 --- a/test/core/end2end/tests/compressed_payload.cc +++ b/test/core/end2end/tests/compressed_payload.cc @@ -591,7 +591,7 @@ static void test_invoke_request_with_server_level( request_with_payload_template( config, "test_invoke_request_with_server_level", 0, GRPC_COMPRESS_NONE, GRPC_COMPRESS_NONE, GRPC_COMPRESS_NONE, GRPC_COMPRESS_NONE /* ignored */, - nullptr, true, GRPC_COMPRESS_LEVEL_MESSAGE_HIGH, false); + nullptr, true, GRPC_COMPRESS_LEVEL_HIGH, false); } static void test_invoke_request_with_compressed_payload_md_override( diff --git a/test/core/end2end/tests/stream_compression_compressed_payload.cc b/test/core/end2end/tests/stream_compression_compressed_payload.cc index 638149a1cf4..256059520da 100644 --- a/test/core/end2end/tests/stream_compression_compressed_payload.cc +++ b/test/core/end2end/tests/stream_compression_compressed_payload.cc @@ -569,15 +569,6 @@ static void test_invoke_request_with_send_message_before_initial_metadata( GRPC_COMPRESS_LEVEL_NONE, true, false, GRPC_COMPRESS_NONE); } -static void test_invoke_request_with_server_level( - grpc_end2end_test_config config) { - request_with_payload_template( - config, "test_invoke_request_with_server_level", 0, GRPC_COMPRESS_NONE, - GRPC_COMPRESS_NONE, GRPC_COMPRESS_NONE, GRPC_COMPRESS_STREAM_GZIP, - /* ignored */ nullptr, true, GRPC_COMPRESS_LEVEL_STREAM_HIGH, false, - false, GRPC_COMPRESS_NONE); -} - static void test_invoke_request_with_compressed_payload_md_override( grpc_end2end_test_config config) { grpc_metadata gzip_compression_override; @@ -620,25 +611,11 @@ static void test_invoke_request_with_disabled_algorithm( GRPC_STATUS_UNIMPLEMENTED, nullptr); } -static void test_stream_compression_override_message_compression( - grpc_end2end_test_config config) { - grpc_compression_level level = GRPC_COMPRESS_LEVEL_STREAM_MED; - request_with_payload_template( - config, "test_stream_compression_override_message_compression", 0, - GRPC_COMPRESS_NONE, GRPC_COMPRESS_NONE, GRPC_COMPRESS_NONE, - grpc_compression_algorithm_for_level( - level, (1u << GRPC_COMPRESS_ALGORITHMS_COUNT) - 1), - /* ignored */ nullptr, true, level, false, true, - GRPC_COMPRESS_MESSAGE_GZIP); -} - void stream_compression_compressed_payload(grpc_end2end_test_config config) { test_invoke_request_with_compressed_payload(config); test_invoke_request_with_send_message_before_initial_metadata(config); - test_invoke_request_with_server_level(config); test_invoke_request_with_compressed_payload_md_override(config); test_invoke_request_with_disabled_algorithm(config); - test_stream_compression_override_message_compression(config); } void stream_compression_compressed_payload_pre_init(void) {}