aboutsummaryrefslogtreecommitdiffhomepage
path: root/src/core/ext/transport
diff options
context:
space:
mode:
authorGravatar Soheil Hassas Yeganeh <soheil@google.com>2018-11-05 12:11:47 -0500
committerGravatar Soheil Hassas Yeganeh <soheil@google.com>2018-11-05 21:40:56 -0500
commit6c49b3dad750168a5a5d72f777c29599dd0068cd (patch)
tree0e8b6f3a4b58873c5c47a4bfb7d06462b7c6a705 /src/core/ext/transport
parentc9b65f2bdea5c50da3564b6441b0d055000f56ca (diff)
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.
Diffstat (limited to 'src/core/ext/transport')
-rw-r--r--src/core/ext/transport/chttp2/transport/hpack_encoder.cc53
1 files changed, 28 insertions, 25 deletions
diff --git a/src/core/ext/transport/chttp2/transport/hpack_encoder.cc b/src/core/ext/transport/chttp2/transport/hpack_encoder.cc
index 920d52770f..dbe9df6ae3 100644
--- a/src/core/ext/transport/chttp2/transport/hpack_encoder.cc
+++ b/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)