From 3c3f3ad75a478e8e3eac8d25bf72701fae6c02f7 Mon Sep 17 00:00:00 2001 From: Juanli Shen Date: Fri, 1 Jun 2018 10:21:14 -0700 Subject: [PATCH] Decrease GRPC_CHTTP2_HPACKC_NUM_VALUES --- .../chttp2/transport/hpack_encoder.cc | 16 ++++++++----- .../chttp2/transport/hpack_encoder.h | 7 +++--- .../transport/chttp2/hpack_encoder_test.cc | 23 ++++++++----------- 3 files changed, 23 insertions(+), 23 deletions(-) diff --git a/src/core/ext/transport/chttp2/transport/hpack_encoder.cc b/src/core/ext/transport/chttp2/transport/hpack_encoder.cc index d5ef063883b..0eaf63f133b 100644 --- a/src/core/ext/transport/chttp2/transport/hpack_encoder.cc +++ b/src/core/ext/transport/chttp2/transport/hpack_encoder.cc @@ -41,14 +41,18 @@ #include "src/core/lib/transport/static_metadata.h" #include "src/core/lib/transport/timeout_encoding.h" -#define HASH_FRAGMENT_1(x) ((x)&255) -#define HASH_FRAGMENT_2(x) ((x >> 8) & 255) -#define HASH_FRAGMENT_3(x) ((x >> 16) & 255) -#define HASH_FRAGMENT_4(x) ((x >> 24) & 255) +#define HASH_FRAGMENT_MASK (GRPC_CHTTP2_HPACKC_NUM_VALUES - 1) +#define HASH_FRAGMENT_1(x) ((x)&HASH_FRAGMENT_MASK) +#define HASH_FRAGMENT_2(x) \ + (((x) >> GRPC_CHTTP2_HPACKC_NUM_VALUES_BITS) & HASH_FRAGMENT_MASK) +#define HASH_FRAGMENT_3(x) \ + (((x) >> (GRPC_CHTTP2_HPACKC_NUM_VALUES_BITS * 2)) & HASH_FRAGMENT_MASK) +#define HASH_FRAGMENT_4(x) \ + (((x) >> (GRPC_CHTTP2_HPACKC_NUM_VALUES_BITS * 3)) & HASH_FRAGMENT_MASK) /* if the probability of this item being seen again is < 1/x then don't add it to the table */ -#define ONE_ON_ADD_PROBABILITY 128 +#define ONE_ON_ADD_PROBABILITY (GRPC_CHTTP2_HPACKC_NUM_VALUES >> 1) /* don't consider adding anything bigger than this to the hpack table */ #define MAX_DECODER_SPACE_USAGE 512 @@ -135,7 +139,7 @@ static void inc_filter(uint8_t idx, uint32_t* sum, uint8_t* elems) { } else { int i; *sum = 0; - for (i = 0; i < GRPC_CHTTP2_HPACKC_NUM_FILTERS; i++) { + for (i = 0; i < GRPC_CHTTP2_HPACKC_NUM_VALUES; i++) { elems[i] /= 2; (*sum) += elems[i]; } diff --git a/src/core/ext/transport/chttp2/transport/hpack_encoder.h b/src/core/ext/transport/chttp2/transport/hpack_encoder.h index b3709321314..e31a7399d78 100644 --- a/src/core/ext/transport/chttp2/transport/hpack_encoder.h +++ b/src/core/ext/transport/chttp2/transport/hpack_encoder.h @@ -28,8 +28,9 @@ #include "src/core/lib/transport/metadata_batch.h" #include "src/core/lib/transport/transport.h" -#define GRPC_CHTTP2_HPACKC_NUM_FILTERS 256 -#define GRPC_CHTTP2_HPACKC_NUM_VALUES 256 +// This should be <= 8. We use 6 to save space. +#define GRPC_CHTTP2_HPACKC_NUM_VALUES_BITS 6 +#define GRPC_CHTTP2_HPACKC_NUM_VALUES (1 << GRPC_CHTTP2_HPACKC_NUM_VALUES_BITS) /* initial table size, per spec */ #define GRPC_CHTTP2_HPACKC_INITIAL_TABLE_SIZE 4096 /* maximum table size we'll actually use */ @@ -58,7 +59,7 @@ typedef struct { a new literal should be added to the compression table or not. They track a single integer that counts how often a particular value has been seen. When that count reaches max (255), all values are halved. */ - uint8_t filter_elems[GRPC_CHTTP2_HPACKC_NUM_FILTERS]; + uint8_t filter_elems[GRPC_CHTTP2_HPACKC_NUM_VALUES]; /* entry tables for keys & elems: these tables track values that have been seen and *may* be in the decompressor table */ diff --git a/test/core/transport/chttp2/hpack_encoder_test.cc b/test/core/transport/chttp2/hpack_encoder_test.cc index d3ba50a91ce..2a57198ab64 100644 --- a/test/core/transport/chttp2/hpack_encoder_test.cc +++ b/test/core/transport/chttp2/hpack_encoder_test.cc @@ -160,6 +160,8 @@ static void encode_int_to_str(int i, char* p) { } static void test_decode_table_overflow() { + // Decrease the default table size to make decode table overflow easier. + grpc_chttp2_hpack_compressor_set_max_table_size(&g_compressor, 1024); int i; char key[3], value[3]; char* expect; @@ -170,27 +172,20 @@ static void test_decode_table_overflow() { false, }; - for (i = 0; i < 114; i++) { + for (i = 0; i < 29; i++) { encode_int_to_str(i, key); encode_int_to_str(i + 1, value); - - if (i + 61 >= 127) { + if (i == 0) { + // 3fe107 corresponds to the table size update. gpr_asprintf(&expect, - "000009 0104 deadbeef ff%02x 40 02%02x%02x 02%02x%02x", - i + 61 - 127, key[0], key[1], value[0], value[1]); - } else if (i > 0) { + "00000a 0104 deadbeef 3fe107 40 02%02x%02x 02%02x%02x", + key[0], key[1], value[0], value[1]); + verify(params, expect, 1, key, value); + } else { gpr_asprintf(&expect, "000008 0104 deadbeef %02x 40 02%02x%02x 02%02x%02x", 0x80 + 61 + i, key[0], key[1], value[0], value[1]); - } else { - gpr_asprintf(&expect, "000007 0104 deadbeef 40 02%02x%02x 02%02x%02x", - key[0], key[1], value[0], value[1]); - } - - if (i > 0) { verify(params, expect, 2, "aa", "ba", key, value); - } else { - verify(params, expect, 1, key, value); } gpr_free(expect); }