Eliminate function pointers in hpack_enc().

Using a conditional branch to set a function pointer will prevent the
compiler to inline the function and worse it would jump based on a
register value killing the pipline.

Here is a short example to demonstrate the variants:
https://godbolt.org/z/radfZg

Remove `add_nothing` and add a lambda when necessary to make sure
everything is inlined.
pull/17102/head
Soheil Hassas Yeganeh 6 years ago
parent c9b65f2bde
commit 6c49b3dad7
  1. 53
      src/core/ext/transport/chttp2/transport/hpack_encoder.cc

@ -212,10 +212,6 @@ static uint32_t prepare_space_for_new_elem(grpc_chttp2_hpack_compressor* c,
return new_index;
}
/* dummy function */
static void add_nothing(grpc_chttp2_hpack_compressor* c, grpc_mdelem elem,
size_t elem_size) {}
// Add a key to the dynamic table. Both key and value will be added to table at
// the decoder.
static void add_key_with_index(grpc_chttp2_hpack_compressor* c,
@ -524,17 +520,22 @@ static void hpack_enc(grpc_chttp2_hpack_compressor* c, grpc_mdelem elem,
uint32_t indices_key;
/* should this elem be in the table? */
size_t decoder_space_usage =
const size_t decoder_space_usage =
grpc_chttp2_get_size_in_hpack_table(elem, st->use_true_binary_metadata);
bool should_add_elem = elem_interned &&
decoder_space_usage < MAX_DECODER_SPACE_USAGE &&
c->filter_elems[HASH_FRAGMENT_1(elem_hash)] >=
c->filter_elems_sum / ONE_ON_ADD_PROBABILITY;
void (*maybe_add)(grpc_chttp2_hpack_compressor*, grpc_mdelem, size_t) =
should_add_elem ? add_elem : add_nothing;
void (*emit)(grpc_chttp2_hpack_compressor*, uint32_t, grpc_mdelem,
framer_state*) =
should_add_elem ? emit_lithdr_incidx : emit_lithdr_noidx;
const bool should_add_elem = elem_interned &&
decoder_space_usage < MAX_DECODER_SPACE_USAGE &&
c->filter_elems[HASH_FRAGMENT_1(elem_hash)] >=
c->filter_elems_sum / ONE_ON_ADD_PROBABILITY;
auto emit_maybe_add = [&should_add_elem, &elem, &st, &c, &indices_key,
&decoder_space_usage] {
if (should_add_elem) {
emit_lithdr_incidx(c, dynidx(c, indices_key), elem, st);
add_elem(c, elem, decoder_space_usage);
} else {
emit_lithdr_noidx(c, dynidx(c, indices_key), elem, st);
}
};
/* no hits for the elem... maybe there's a key? */
indices_key = c->indices_keys[HASH_FRAGMENT_2(key_hash)];
@ -542,8 +543,7 @@ static void hpack_enc(grpc_chttp2_hpack_compressor* c, grpc_mdelem elem,
GRPC_MDKEY(elem)) &&
indices_key > c->tail_remote_index) {
/* HIT: key (first cuckoo hash) */
emit(c, dynidx(c, indices_key), elem, st);
maybe_add(c, elem, decoder_space_usage);
emit_maybe_add();
return;
}
@ -552,20 +552,23 @@ static void hpack_enc(grpc_chttp2_hpack_compressor* c, grpc_mdelem elem,
GRPC_MDKEY(elem)) &&
indices_key > c->tail_remote_index) {
/* HIT: key (first cuckoo hash) */
emit(c, dynidx(c, indices_key), elem, st);
maybe_add(c, elem, decoder_space_usage);
emit_maybe_add();
return;
}
/* no elem, key in the table... fall back to literal emission */
bool should_add_key =
const bool should_add_key =
!elem_interned && decoder_space_usage < MAX_DECODER_SPACE_USAGE;
emit = (should_add_elem || should_add_key) ? emit_lithdr_incidx_v
: emit_lithdr_noidx_v;
maybe_add =
should_add_elem ? add_elem : (should_add_key ? add_key : add_nothing);
emit(c, 0, elem, st);
maybe_add(c, elem, decoder_space_usage);
if (should_add_elem || should_add_key) {
emit_lithdr_incidx_v(c, 0, elem, st);
} else {
emit_lithdr_noidx_v(c, 0, elem, st);
}
if (should_add_elem) {
add_elem(c, elem, decoder_space_usage);
} else if (should_add_key) {
add_key(c, elem, decoder_space_usage);
}
}
#define STRLEN_LIT(x) (sizeof(x) - 1)

Loading…
Cancel
Save