From e29feb2adb26b0d2b8707f95af01a082d742d241 Mon Sep 17 00:00:00 2001 From: David Garcia Quintas Date: Mon, 15 Jun 2015 18:08:13 -0700 Subject: [PATCH 1/6] Introduced compression levels as an abstraction for the actual algorithm. --- include/grpc/compression.h | 13 +++++++++++++ src/core/compression/algorithm.c | 19 +++++++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/include/grpc/compression.h b/include/grpc/compression.h index 630fa1656ae..207898f6054 100644 --- a/include/grpc/compression.h +++ b/include/grpc/compression.h @@ -34,6 +34,9 @@ #ifndef GRPC_COMPRESSION_H #define GRPC_COMPRESSION_H +/** To be used in channel arguments */ +#define GRPC_COMPRESSION_LEVEL_ARG "grpc.compression_level" + /* The various compression algorithms supported by GRPC */ typedef enum { GRPC_COMPRESS_NONE = 0, @@ -43,7 +46,17 @@ typedef enum { GRPC_COMPRESS_ALGORITHMS_COUNT } grpc_compression_algorithm; +typedef enum { + GRPC_COMPRESS_LEVEL_NONE = 0, + GRPC_COMPRESS_LEVEL_LOW, + GRPC_COMPRESS_LEVEL_MED, + GRPC_COMPRESS_LEVEL_HIGH +} grpc_compression_level; + const char *grpc_compression_algorithm_name( grpc_compression_algorithm algorithm); +grpc_compression_algorithm grpc_compression_algorithm_for_level( + grpc_compression_level level); + #endif /* GRPC_COMPRESSION_H */ diff --git a/src/core/compression/algorithm.c b/src/core/compression/algorithm.c index 36ead843d26..6c25733ba98 100644 --- a/src/core/compression/algorithm.c +++ b/src/core/compression/algorithm.c @@ -31,6 +31,7 @@ * */ +#include #include const char *grpc_compression_algorithm_name( @@ -47,3 +48,21 @@ const char *grpc_compression_algorithm_name( } return "error"; } + +/* TODO(dgq): Add the ability to specify parameters to the individual + * compression algorithms */ +grpc_compression_algorithm grpc_compression_algorithm_for_level( + grpc_compression_level level) { + switch (level) { + case GRPC_COMPRESS_NONE: + return GRPC_COMPRESS_NONE; + case GRPC_COMPRESS_LEVEL_LOW: + case GRPC_COMPRESS_LEVEL_MED: + return GRPC_COMPRESS_DEFLATE; + case GRPC_COMPRESS_LEVEL_HIGH: + return GRPC_COMPRESS_GZIP; + default: + /* we shouldn't be making it here */ + abort(); + } +} From fd62174190d55d880a3a0a3ae1f88d90525c0464 Mon Sep 17 00:00:00 2001 From: David Garcia Quintas Date: Mon, 15 Jun 2015 23:50:39 -0700 Subject: [PATCH 2/6] Mapped DEFLARE as a high compression level algorithm --- src/core/compression/algorithm.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/core/compression/algorithm.c b/src/core/compression/algorithm.c index 6c25733ba98..06b034dee75 100644 --- a/src/core/compression/algorithm.c +++ b/src/core/compression/algorithm.c @@ -58,9 +58,8 @@ grpc_compression_algorithm grpc_compression_algorithm_for_level( return GRPC_COMPRESS_NONE; case GRPC_COMPRESS_LEVEL_LOW: case GRPC_COMPRESS_LEVEL_MED: - return GRPC_COMPRESS_DEFLATE; case GRPC_COMPRESS_LEVEL_HIGH: - return GRPC_COMPRESS_GZIP; + return GRPC_COMPRESS_DEFLATE; default: /* we shouldn't be making it here */ abort(); From cc6c43c4961ec532835cebd4a395598cabcffbcc Mon Sep 17 00:00:00 2001 From: David Garcia Quintas Date: Tue, 16 Jun 2015 11:35:41 -0700 Subject: [PATCH 3/6] Exposed compression level in channel arguments (C and C++) --- include/grpc++/channel_arguments.h | 4 ++++ src/core/channel/channel_args.c | 24 ++++++++++++++++++++++++ src/core/channel/channel_args.h | 18 ++++++++++++++---- src/cpp/client/channel_arguments.cc | 5 +++++ 4 files changed, 47 insertions(+), 4 deletions(-) diff --git a/include/grpc++/channel_arguments.h b/include/grpc++/channel_arguments.h index 8d338c654ec..68f24cde4af 100644 --- a/include/grpc++/channel_arguments.h +++ b/include/grpc++/channel_arguments.h @@ -38,6 +38,7 @@ #include #include +#include #include namespace grpc { @@ -58,6 +59,9 @@ class ChannelArguments { void SetSslTargetNameOverride(const grpc::string& name); // TODO(yangg) add flow control options + // Set the compression level for the channel. + void SetCompressionLevel(grpc_compression_level level); + // Generic channel argument setters. Only for advanced use cases. void SetInt(const grpc::string& key, int value); void SetString(const grpc::string& key, const grpc::string& value); diff --git a/src/core/channel/channel_args.c b/src/core/channel/channel_args.c index 1b0e33b1232..166d559a456 100644 --- a/src/core/channel/channel_args.c +++ b/src/core/channel/channel_args.c @@ -115,3 +115,27 @@ int grpc_channel_args_is_census_enabled(const grpc_channel_args *a) { } return 0; } + +grpc_compression_level grpc_channel_args_get_compression_level( + const grpc_channel_args *a) { + size_t i; + if (a) { + for (i = 0; a && i < a->num_args; ++i) { + if (a->args[i].type == GRPC_ARG_INTEGER && + !strcmp(GRPC_COMPRESSION_LEVEL_ARG, a->args[i].key)) { + return a->args[i].value.integer; + break; + } + } + } + return GRPC_COMPRESS_LEVEL_NONE; +} + +void grpc_channel_args_set_compression_level( + grpc_channel_args **a, grpc_compression_level level) { + grpc_arg tmp; + tmp.type = GRPC_ARG_INTEGER; + tmp.key = GRPC_COMPRESSION_LEVEL_ARG; + tmp.value.integer = level; + *a = grpc_channel_args_copy_and_add(*a, &tmp); +} diff --git a/src/core/channel/channel_args.h b/src/core/channel/channel_args.h index eb5bf63986a..bf747b26e64 100644 --- a/src/core/channel/channel_args.h +++ b/src/core/channel/channel_args.h @@ -34,21 +34,31 @@ #ifndef GRPC_INTERNAL_CORE_CHANNEL_CHANNEL_ARGS_H #define GRPC_INTERNAL_CORE_CHANNEL_CHANNEL_ARGS_H +#include #include /* Copy some arguments */ grpc_channel_args *grpc_channel_args_copy(const grpc_channel_args *src); -/* Copy some arguments and add the to_add parameter in the end. +/** Copy some arguments and add the to_add parameter in the end. If to_add is NULL, it is equivalent to call grpc_channel_args_copy. */ grpc_channel_args *grpc_channel_args_copy_and_add(const grpc_channel_args *src, const grpc_arg *to_add); -/* Destroy arguments created by grpc_channel_args_copy */ +/** Destroy arguments created by grpc_channel_args_copy */ void grpc_channel_args_destroy(grpc_channel_args *a); -/* Reads census_enabled settings from channel args. Returns 1 if census_enabled - is specified in channel args, otherwise returns 0. */ +/** Reads census_enabled settings from channel args. Returns 1 if census_enabled + * is specified in channel args, otherwise returns 0. */ int grpc_channel_args_is_census_enabled(const grpc_channel_args *a); +/** Returns the compression level set in \a a. */ +grpc_compression_level grpc_channel_args_get_compression_level( + const grpc_channel_args *a); + +/** Sets the compression level in \a a to \a level. Setting it to + * GRPC_COMPRESS_LEVEL_NONE disables compression for the channel. */ +void grpc_channel_args_set_compression_level( + grpc_channel_args **a, grpc_compression_level level); + #endif /* GRPC_INTERNAL_CORE_CHANNEL_CHANNEL_ARGS_H */ diff --git a/src/cpp/client/channel_arguments.cc b/src/cpp/client/channel_arguments.cc index 87f8349eefd..679c4f1503d 100644 --- a/src/cpp/client/channel_arguments.cc +++ b/src/cpp/client/channel_arguments.cc @@ -34,6 +34,7 @@ #include #include +#include "src/core/channel/channel_args.h" namespace grpc { @@ -41,6 +42,10 @@ void ChannelArguments::SetSslTargetNameOverride(const grpc::string& name) { SetString(GRPC_SSL_TARGET_NAME_OVERRIDE_ARG, name); } +void ChannelArguments::SetCompressionLevel(grpc_compression_level level) { + SetInt(GRPC_COMPRESSION_LEVEL_ARG, level); +} + grpc::string ChannelArguments::GetSslTargetNameOverride() const { for (unsigned int i = 0; i < args_.size(); i++) { if (grpc::string(GRPC_SSL_TARGET_NAME_OVERRIDE_ARG) == args_[i].key) { From ccb20bb98ccf35ef1e3398dfd762abcf1dec6bb1 Mon Sep 17 00:00:00 2001 From: David Garcia Quintas Date: Tue, 16 Jun 2015 17:52:40 -0700 Subject: [PATCH 4/6] Fixed wrong switch label name --- src/core/compression/algorithm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/compression/algorithm.c b/src/core/compression/algorithm.c index 06b034dee75..4db48df6cbf 100644 --- a/src/core/compression/algorithm.c +++ b/src/core/compression/algorithm.c @@ -54,7 +54,7 @@ const char *grpc_compression_algorithm_name( grpc_compression_algorithm grpc_compression_algorithm_for_level( grpc_compression_level level) { switch (level) { - case GRPC_COMPRESS_NONE: + case GRPC_COMPRESS_LEVEL_NONE: return GRPC_COMPRESS_NONE; case GRPC_COMPRESS_LEVEL_LOW: case GRPC_COMPRESS_LEVEL_MED: From 821e4a757f20181a5a2fbc7be1e7865c283d02d1 Mon Sep 17 00:00:00 2001 From: Tim Emiola Date: Wed, 17 Jun 2015 11:57:46 -0700 Subject: [PATCH 5/6] Prints test names as ruby tests run --- src/ruby/.rspec | 2 ++ src/ruby/spec/spec_helper.rb | 2 ++ 2 files changed, 4 insertions(+) diff --git a/src/ruby/.rspec b/src/ruby/.rspec index dd579f7a137..cd7c5fb5b21 100755 --- a/src/ruby/.rspec +++ b/src/ruby/.rspec @@ -1,2 +1,4 @@ -I. --require spec_helper +--format documentation +--color diff --git a/src/ruby/spec/spec_helper.rb b/src/ruby/spec/spec_helper.rb index 101165c146f..270d2e97d32 100644 --- a/src/ruby/spec/spec_helper.rb +++ b/src/ruby/spec/spec_helper.rb @@ -53,3 +53,5 @@ RSpec.configure do |config| include RSpec::LoggingHelper config.capture_log_messages end + +RSpec::Expectations.configuration.warn_about_potential_false_positives = false From 6bbfcc3fec703438690f308f3a706f58c87380e1 Mon Sep 17 00:00:00 2001 From: Jorge Canizales Date: Wed, 17 Jun 2015 14:10:52 -0700 Subject: [PATCH 6/6] Disallow started request writers on GRPCCall init --- src/objective-c/GRPCClient/GRPCCall.m | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/objective-c/GRPCClient/GRPCCall.m b/src/objective-c/GRPCClient/GRPCCall.m index 279ef935d50..687932c9381 100644 --- a/src/objective-c/GRPCClient/GRPCCall.m +++ b/src/objective-c/GRPCClient/GRPCCall.m @@ -100,7 +100,9 @@ NSString * const kGRPCStatusMetadataKey = @"io.grpc.StatusMetadataKey"; if (!host || !method) { [NSException raise:NSInvalidArgumentException format:@"Neither host nor method can be nil."]; } - // TODO(jcanizales): Throw if the requestWriter was already started. + if (requestWriter.state != GRXWriterStateNotStarted) { + [NSException raise:NSInvalidArgumentException format:@"The requests writer can't be already started."]; + } if ((self = [super init])) { static dispatch_once_t initialization; dispatch_once(&initialization, ^{