From 987d526efc541ab0175c89558ce89441dd15ff9e Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Wed, 14 Jul 2021 12:16:17 -0700 Subject: [PATCH] Eliminate some HPACK LUTs (#26657) Eliminate HPACK table Use a switch statement instead of a table lookup for the first byte of HPACK parsing. This will lead the way to some other improvements down the track (I have a substantial overhaul here planned), but this is a necessary first step. --- .../chttp2/transport/hpack_parser.cc | 201 +++++++++--------- tools/codegen/core/gen_hpack_tables.cc | 87 -------- 2 files changed, 99 insertions(+), 189 deletions(-) diff --git a/src/core/ext/transport/chttp2/transport/hpack_parser.cc b/src/core/ext/transport/chttp2/transport/hpack_parser.cc index 7a872555f38..08f1a02d9dd 100644 --- a/src/core/ext/transport/chttp2/transport/hpack_parser.cc +++ b/src/core/ext/transport/chttp2/transport/hpack_parser.cc @@ -151,107 +151,6 @@ static grpc_error_handle parse_max_tbl_size_x(grpc_chttp2_hpack_parser* p, const uint8_t* cur, const uint8_t* end); -/* we translate the first byte of a hpack field into one of these decoding - cases, then use a lookup table to jump directly to the appropriate parser. - - _X => the integer index is all ones, meaning we need to do varint decoding - _V => the integer index is all zeros, meaning we need to decode an additional - string value */ -typedef enum { - INDEXED_FIELD, - INDEXED_FIELD_X, - LITHDR_INCIDX, - LITHDR_INCIDX_X, - LITHDR_INCIDX_V, - LITHDR_NOTIDX, - LITHDR_NOTIDX_X, - LITHDR_NOTIDX_V, - LITHDR_NVRIDX, - LITHDR_NVRIDX_X, - LITHDR_NVRIDX_V, - MAX_TBL_SIZE, - MAX_TBL_SIZE_X, - ILLEGAL -} first_byte_type; - -/* jump table of parse state functions -- order must match first_byte_type - above */ -static const grpc_chttp2_hpack_parser_state first_byte_action[] = { - parse_indexed_field, parse_indexed_field_x, parse_lithdr_incidx, - parse_lithdr_incidx_x, parse_lithdr_incidx_v, parse_lithdr_notidx, - parse_lithdr_notidx_x, parse_lithdr_notidx_v, parse_lithdr_nvridx, - parse_lithdr_nvridx_x, parse_lithdr_nvridx_v, parse_max_tbl_size, - parse_max_tbl_size_x, parse_illegal_op}; - -/* indexes the first byte to a parse state function - generated by - gen_hpack_tables.c */ -static const uint8_t first_byte_lut[256] = { - LITHDR_NOTIDX_V, LITHDR_NOTIDX, LITHDR_NOTIDX, LITHDR_NOTIDX, - LITHDR_NOTIDX, LITHDR_NOTIDX, LITHDR_NOTIDX, LITHDR_NOTIDX, - LITHDR_NOTIDX, LITHDR_NOTIDX, LITHDR_NOTIDX, LITHDR_NOTIDX, - LITHDR_NOTIDX, LITHDR_NOTIDX, LITHDR_NOTIDX, LITHDR_NOTIDX_X, - LITHDR_NVRIDX_V, LITHDR_NVRIDX, LITHDR_NVRIDX, LITHDR_NVRIDX, - LITHDR_NVRIDX, LITHDR_NVRIDX, LITHDR_NVRIDX, LITHDR_NVRIDX, - LITHDR_NVRIDX, LITHDR_NVRIDX, LITHDR_NVRIDX, LITHDR_NVRIDX, - LITHDR_NVRIDX, LITHDR_NVRIDX, LITHDR_NVRIDX, LITHDR_NVRIDX_X, - MAX_TBL_SIZE, MAX_TBL_SIZE, MAX_TBL_SIZE, MAX_TBL_SIZE, - MAX_TBL_SIZE, MAX_TBL_SIZE, MAX_TBL_SIZE, MAX_TBL_SIZE, - MAX_TBL_SIZE, MAX_TBL_SIZE, MAX_TBL_SIZE, MAX_TBL_SIZE, - MAX_TBL_SIZE, MAX_TBL_SIZE, MAX_TBL_SIZE, MAX_TBL_SIZE, - MAX_TBL_SIZE, MAX_TBL_SIZE, MAX_TBL_SIZE, MAX_TBL_SIZE, - MAX_TBL_SIZE, MAX_TBL_SIZE, MAX_TBL_SIZE, MAX_TBL_SIZE, - MAX_TBL_SIZE, MAX_TBL_SIZE, MAX_TBL_SIZE, MAX_TBL_SIZE, - MAX_TBL_SIZE, MAX_TBL_SIZE, MAX_TBL_SIZE, MAX_TBL_SIZE_X, - LITHDR_INCIDX_V, LITHDR_INCIDX, LITHDR_INCIDX, LITHDR_INCIDX, - LITHDR_INCIDX, LITHDR_INCIDX, LITHDR_INCIDX, LITHDR_INCIDX, - LITHDR_INCIDX, LITHDR_INCIDX, LITHDR_INCIDX, LITHDR_INCIDX, - LITHDR_INCIDX, LITHDR_INCIDX, LITHDR_INCIDX, LITHDR_INCIDX, - LITHDR_INCIDX, LITHDR_INCIDX, LITHDR_INCIDX, LITHDR_INCIDX, - LITHDR_INCIDX, LITHDR_INCIDX, LITHDR_INCIDX, LITHDR_INCIDX, - LITHDR_INCIDX, LITHDR_INCIDX, LITHDR_INCIDX, LITHDR_INCIDX, - LITHDR_INCIDX, LITHDR_INCIDX, LITHDR_INCIDX, LITHDR_INCIDX, - LITHDR_INCIDX, LITHDR_INCIDX, LITHDR_INCIDX, LITHDR_INCIDX, - LITHDR_INCIDX, LITHDR_INCIDX, LITHDR_INCIDX, LITHDR_INCIDX, - LITHDR_INCIDX, LITHDR_INCIDX, LITHDR_INCIDX, LITHDR_INCIDX, - LITHDR_INCIDX, LITHDR_INCIDX, LITHDR_INCIDX, LITHDR_INCIDX, - LITHDR_INCIDX, LITHDR_INCIDX, LITHDR_INCIDX, LITHDR_INCIDX, - LITHDR_INCIDX, LITHDR_INCIDX, LITHDR_INCIDX, LITHDR_INCIDX, - LITHDR_INCIDX, LITHDR_INCIDX, LITHDR_INCIDX, LITHDR_INCIDX, - LITHDR_INCIDX, LITHDR_INCIDX, LITHDR_INCIDX, LITHDR_INCIDX_X, - ILLEGAL, INDEXED_FIELD, INDEXED_FIELD, INDEXED_FIELD, - INDEXED_FIELD, INDEXED_FIELD, INDEXED_FIELD, INDEXED_FIELD, - INDEXED_FIELD, INDEXED_FIELD, INDEXED_FIELD, INDEXED_FIELD, - INDEXED_FIELD, INDEXED_FIELD, INDEXED_FIELD, INDEXED_FIELD, - INDEXED_FIELD, INDEXED_FIELD, INDEXED_FIELD, INDEXED_FIELD, - INDEXED_FIELD, INDEXED_FIELD, INDEXED_FIELD, INDEXED_FIELD, - INDEXED_FIELD, INDEXED_FIELD, INDEXED_FIELD, INDEXED_FIELD, - INDEXED_FIELD, INDEXED_FIELD, INDEXED_FIELD, INDEXED_FIELD, - INDEXED_FIELD, INDEXED_FIELD, INDEXED_FIELD, INDEXED_FIELD, - INDEXED_FIELD, INDEXED_FIELD, INDEXED_FIELD, INDEXED_FIELD, - INDEXED_FIELD, INDEXED_FIELD, INDEXED_FIELD, INDEXED_FIELD, - INDEXED_FIELD, INDEXED_FIELD, INDEXED_FIELD, INDEXED_FIELD, - INDEXED_FIELD, INDEXED_FIELD, INDEXED_FIELD, INDEXED_FIELD, - INDEXED_FIELD, INDEXED_FIELD, INDEXED_FIELD, INDEXED_FIELD, - INDEXED_FIELD, INDEXED_FIELD, INDEXED_FIELD, INDEXED_FIELD, - INDEXED_FIELD, INDEXED_FIELD, INDEXED_FIELD, INDEXED_FIELD, - INDEXED_FIELD, INDEXED_FIELD, INDEXED_FIELD, INDEXED_FIELD, - INDEXED_FIELD, INDEXED_FIELD, INDEXED_FIELD, INDEXED_FIELD, - INDEXED_FIELD, INDEXED_FIELD, INDEXED_FIELD, INDEXED_FIELD, - INDEXED_FIELD, INDEXED_FIELD, INDEXED_FIELD, INDEXED_FIELD, - INDEXED_FIELD, INDEXED_FIELD, INDEXED_FIELD, INDEXED_FIELD, - INDEXED_FIELD, INDEXED_FIELD, INDEXED_FIELD, INDEXED_FIELD, - INDEXED_FIELD, INDEXED_FIELD, INDEXED_FIELD, INDEXED_FIELD, - INDEXED_FIELD, INDEXED_FIELD, INDEXED_FIELD, INDEXED_FIELD, - INDEXED_FIELD, INDEXED_FIELD, INDEXED_FIELD, INDEXED_FIELD, - INDEXED_FIELD, INDEXED_FIELD, INDEXED_FIELD, INDEXED_FIELD, - INDEXED_FIELD, INDEXED_FIELD, INDEXED_FIELD, INDEXED_FIELD, - INDEXED_FIELD, INDEXED_FIELD, INDEXED_FIELD, INDEXED_FIELD, - INDEXED_FIELD, INDEXED_FIELD, INDEXED_FIELD, INDEXED_FIELD, - INDEXED_FIELD, INDEXED_FIELD, INDEXED_FIELD, INDEXED_FIELD, - INDEXED_FIELD, INDEXED_FIELD, INDEXED_FIELD, INDEXED_FIELD, - INDEXED_FIELD, INDEXED_FIELD, INDEXED_FIELD, INDEXED_FIELD_X, -}; - /* state table for huffman decoding: given a state, gives an index/16 into next_sub_tbl. Taking that index and adding the value of the nibble being considered returns the next state. @@ -731,7 +630,105 @@ static grpc_error_handle parse_begin(grpc_chttp2_hpack_parser* p, return GRPC_ERROR_NONE; } - return first_byte_action[first_byte_lut[*cur]](p, cur, end); + switch (*cur >> 4) { + // Literal header not indexed. + // First byte format: 0000xxxx + // Where xxxx: + // 0000 - literal key + // 1111 - indexed key, varint encoded index + // other - indexed key, inline encoded index + case 0: + switch (*cur & 0xf) { + case 0: // literal key + return parse_lithdr_notidx_v(p, cur, end); + case 0xf: // varint encoded key index + return parse_lithdr_notidx_x(p, cur, end); + default: // inline encoded key index + return parse_lithdr_notidx(p, cur, end); + } + // Literal header never indexed. + // First byte format: 0001xxxx + // Where xxxx: + // 0000 - literal key + // 1111 - indexed key, varint encoded index + // other - indexed key, inline encoded index + case 1: + switch (*cur & 0xf) { + case 0: // literal key + return parse_lithdr_nvridx_v(p, cur, end); + case 0xf: // varint encoded key index + return parse_lithdr_nvridx_x(p, cur, end); + default: // inline encoded key index + return parse_lithdr_nvridx(p, cur, end); + } + // Update max table size. + // First byte format: 001xxxxx + // Where xxxxx: + // 11111 - max size is varint encoded + // other - max size is stored inline + case 2: + // inline encoded max table size + return parse_max_tbl_size(p, cur, end); + case 3: + if (*cur == 0x3f) { + // varint encoded max table size + return parse_max_tbl_size_x(p, cur, end); + } else { + // inline encoded max table size + return parse_max_tbl_size(p, cur, end); + } + // Literal header with incremental indexing. + // First byte format: 01xxxxxx + // Where xxxxxx: + // 000000 - literal key + // 111111 - indexed key, varint encoded index + // other - indexed key, inline encoded index + case 4: + if (*cur == 0x40) { + // literal key + return parse_lithdr_incidx_v(p, cur, end); + } + case 5: + case 6: + // inline encoded key index + return parse_lithdr_incidx(p, cur, end); + case 7: + if (*cur == 0x7f) { + // varint encoded key index + return parse_lithdr_incidx_x(p, cur, end); + } else { + // inline encoded key index + return parse_lithdr_incidx(p, cur, end); + } + // Indexed Header Field Representation + // First byte format: 1xxxxxxx + // Where xxxxxxx: + // 0000000 - illegal + // 1111111 - varint encoded field index + // other - inline encoded field index + case 8: + if (*cur == 0x80) { + // illegal value. + return parse_illegal_op(p, cur, end); + } + case 9: + case 10: + case 11: + case 12: + case 13: + case 14: + // inline encoded field index + return parse_indexed_field(p, cur, end); + case 15: + if (*cur == 0xff) { + // varint encoded field index + return parse_indexed_field_x(p, cur, end); + } else { + // inline encoded field index + return parse_indexed_field(p, cur, end); + } + } + GPR_UNREACHABLE_CODE(abort()); } /* stream dependency and prioritization data: we just skip it */ diff --git a/tools/codegen/core/gen_hpack_tables.cc b/tools/codegen/core/gen_hpack_tables.cc index d25dafc03af..d01a90eb78f 100644 --- a/tools/codegen/core/gen_hpack_tables.cc +++ b/tools/codegen/core/gen_hpack_tables.cc @@ -26,92 +26,6 @@ #include #include "src/core/ext/transport/chttp2/transport/huffsyms.h" -/* - * first byte LUT generation - */ - -typedef struct { - const char *call; - /* bit prefix for the field type */ - unsigned char prefix; - /* length of the bit prefix for the field type */ - unsigned char prefix_length; - /* index value: 0 = all zeros, 2 = all ones, 1 otherwise */ - unsigned char index; -} spec; - -static const spec fields[] = { - {"INDEXED_FIELD", 0X80, 1, 1}, {"INDEXED_FIELD_X", 0X80, 1, 2}, - {"LITHDR_INCIDX", 0X40, 2, 1}, {"LITHDR_INCIDX_X", 0X40, 2, 2}, - {"LITHDR_INCIDX_V", 0X40, 2, 0}, {"LITHDR_NOTIDX", 0X00, 4, 1}, - {"LITHDR_NOTIDX_X", 0X00, 4, 2}, {"LITHDR_NOTIDX_V", 0X00, 4, 0}, - {"LITHDR_NVRIDX", 0X10, 4, 1}, {"LITHDR_NVRIDX_X", 0X10, 4, 2}, - {"LITHDR_NVRIDX_V", 0X10, 4, 0}, {"MAX_TBL_SIZE", 0X20, 3, 1}, - {"MAX_TBL_SIZE_X", 0X20, 3, 2}, -}; - -static const int num_fields = sizeof(fields) / sizeof(*fields); - -static unsigned char prefix_mask(unsigned char prefix_len) { - unsigned char i; - unsigned char out = 0; - for (i = 0; i < prefix_len; i++) { - /* NB: the following integer arithmetic operation needs to be in its - * expanded form due to the "integral promotion" performed (see section - * 3.2.1.1 of the C89 draft standard). A cast to the smaller container type - * is then required to avoid the compiler warning */ - out = (unsigned char)(out | (unsigned char)(1 << (7 - i))); - } - return out; -} - -static unsigned char suffix_mask(unsigned char prefix_len) { - return (unsigned char)~prefix_mask(prefix_len); -} - -static void generate_first_byte_lut(void) { - int i, j, n; - const spec *chrspec; - unsigned char suffix; - - n = printf("static CALLTYPE first_byte[256] = {"); - /* for each potential first byte of a header */ - for (i = 0; i < 256; i++) { - /* find the field type that matches it */ - chrspec = NULL; - for (j = 0; j < num_fields; j++) { - if ((prefix_mask(fields[j].prefix_length) & i) == fields[j].prefix) { - /* NB: the following integer arithmetic operation needs to be in its - * expanded form due to the "integral promotion" performed (see section - * 3.2.1.1 of the C89 draft standard). A cast to the smaller container - * type is then required to avoid the compiler warning */ - suffix = (unsigned char)(suffix_mask(fields[j].prefix_length) & - (unsigned char)i); - if (suffix == suffix_mask(fields[j].prefix_length)) { - if (fields[j].index != 2) continue; - } else if (suffix == 0) { - if (fields[j].index != 0) continue; - } else { - if (fields[j].index != 1) continue; - } - GPR_ASSERT(chrspec == NULL); - chrspec = &fields[j]; - } - } - if (chrspec) { - n += printf("%s, ", chrspec->call); - } else { - n += printf("ILLEGAL, "); - } - /* make some small effort towards readable output */ - if (n > 70) { - printf("\n "); - n = 2; - } - } - printf("};\n"); -} - /* * Huffman decoder table generation */ @@ -327,7 +241,6 @@ static void generate_base64_huff_encoder_table(void) { int main(void) { generate_huff_tables(); - generate_first_byte_lut(); generate_base64_huff_encoder_table(); return 0;