Merge pull request #13045 from yang-g/hpack_parse

Only allocate cached_timeout when md is interned
pull/12178/merge
Yang Gao 7 years ago committed by GitHub
commit 7702ba37e3
  1. 10
      src/core/ext/transport/chttp2/transport/hpack_parser.cc
  2. 20
      src/core/ext/transport/chttp2/transport/parsing.cc
  3. 84
      test/cpp/microbenchmarks/bm_chttp2_hpack.cc

@ -33,6 +33,7 @@
#include "src/core/lib/debug/stats.h"
#include "src/core/lib/profiling/timers.h"
#include "src/core/lib/slice/slice_internal.h"
#include "src/core/lib/slice/slice_string_helpers.h"
#include "src/core/lib/support/string.h"
#include "src/core/lib/transport/http2_errors.h"
@ -650,9 +651,14 @@ static const uint8_t inverse_base64[256] = {
/* emission helpers */
static grpc_error *on_hdr(grpc_exec_ctx *exec_ctx, grpc_chttp2_hpack_parser *p,
grpc_mdelem md, int add_to_table) {
if (GRPC_TRACER_ON(grpc_http_trace) && !GRPC_MDELEM_IS_INTERNED(md)) {
if (GRPC_TRACER_ON(grpc_http_trace)) {
char *k = grpc_slice_to_c_string(GRPC_MDKEY(md));
char *v = grpc_slice_to_c_string(GRPC_MDVALUE(md));
char *v = NULL;
if (grpc_is_binary_header(GRPC_MDKEY(md))) {
v = grpc_dump_slice(GRPC_MDVALUE(md), GPR_DUMP_HEX);
} else {
v = grpc_slice_to_c_string(GRPC_MDVALUE(md));
}
gpr_log(
GPR_DEBUG,
"Decode: '%s: %s', elem_interned=%d [%d], k_interned=%d, v_interned=%d",

@ -435,19 +435,21 @@ static void on_initial_header(grpc_exec_ctx *exec_ctx, void *tp,
grpc_millis *cached_timeout =
static_cast<grpc_millis *>(grpc_mdelem_get_user_data(md, free_timeout));
grpc_millis timeout;
if (cached_timeout == NULL) {
/* not already parsed: parse it now, and store the result away */
cached_timeout = (grpc_millis *)gpr_malloc(sizeof(grpc_millis));
if (!grpc_http2_decode_timeout(GRPC_MDVALUE(md), cached_timeout)) {
if (cached_timeout != NULL) {
timeout = *cached_timeout;
} else {
if (!grpc_http2_decode_timeout(GRPC_MDVALUE(md), &timeout)) {
char *val = grpc_slice_to_c_string(GRPC_MDVALUE(md));
gpr_log(GPR_ERROR, "Ignoring bad timeout value '%s'", val);
gpr_free(val);
*cached_timeout = GRPC_MILLIS_INF_FUTURE;
timeout = GRPC_MILLIS_INF_FUTURE;
}
if (GRPC_MDELEM_IS_INTERNED(md)) {
/* store the result */
cached_timeout = (grpc_millis *)gpr_malloc(sizeof(grpc_millis));
*cached_timeout = timeout;
grpc_mdelem_set_user_data(md, free_timeout, cached_timeout);
}
timeout = *cached_timeout;
grpc_mdelem_set_user_data(md, free_timeout, cached_timeout);
} else {
timeout = *cached_timeout;
}
if (timeout != GRPC_MILLIS_INF_FUTURE) {
grpc_chttp2_incoming_metadata_buffer_set_deadline(

@ -28,6 +28,7 @@ extern "C" {
#include "src/core/lib/slice/slice_internal.h"
#include "src/core/lib/slice/slice_string_helpers.h"
#include "src/core/lib/transport/static_metadata.h"
#include "src/core/lib/transport/timeout_encoding.h"
}
#include "test/cpp/microbenchmarks/helpers.h"
#include "third_party/benchmark/include/benchmark/benchmark.h"
@ -441,7 +442,8 @@ static void UnrefHeader(grpc_exec_ctx *exec_ctx, void *user_data,
GRPC_MDELEM_UNREF(exec_ctx, md);
}
template <class Fixture>
template <class Fixture,
void (*OnHeader)(grpc_exec_ctx *, void *, grpc_mdelem) = UnrefHeader>
static void BM_HpackParserParseHeader(benchmark::State &state) {
TrackCounters track_counters;
grpc_exec_ctx exec_ctx = GRPC_EXEC_CTX_INIT;
@ -449,7 +451,7 @@ static void BM_HpackParserParseHeader(benchmark::State &state) {
std::vector<grpc_slice> benchmark_slices = Fixture::GetBenchmarkSlices();
grpc_chttp2_hpack_parser p;
grpc_chttp2_hpack_parser_init(&exec_ctx, &p);
p.on_header = UnrefHeader;
p.on_header = OnHeader;
p.on_header_user_data = nullptr;
for (auto slice : init_slices) {
GPR_ASSERT(GRPC_ERROR_NONE ==
@ -759,6 +761,81 @@ class RepresentativeServerTrailingMetadata {
}
};
static void free_timeout(void *p) { gpr_free(p); }
// New implementation.
static void OnHeaderNew(grpc_exec_ctx *exec_ctx, void *user_data,
grpc_mdelem md) {
if (grpc_slice_eq(GRPC_MDKEY(md), GRPC_MDSTR_GRPC_TIMEOUT)) {
grpc_millis *cached_timeout =
static_cast<grpc_millis *>(grpc_mdelem_get_user_data(md, free_timeout));
grpc_millis timeout;
if (cached_timeout != NULL) {
timeout = *cached_timeout;
} else {
if (!grpc_http2_decode_timeout(GRPC_MDVALUE(md), &timeout)) {
char *val = grpc_slice_to_c_string(GRPC_MDVALUE(md));
gpr_log(GPR_ERROR, "Ignoring bad timeout value '%s'", val);
gpr_free(val);
timeout = GRPC_MILLIS_INF_FUTURE;
}
if (GRPC_MDELEM_IS_INTERNED(md)) {
/* not already parsed: parse it now, and store the
* result away */
cached_timeout = (grpc_millis *)gpr_malloc(sizeof(grpc_millis));
*cached_timeout = timeout;
grpc_mdelem_set_user_data(md, free_timeout, cached_timeout);
}
}
benchmark::DoNotOptimize(timeout);
GRPC_MDELEM_UNREF(exec_ctx, md);
} else {
GPR_ASSERT(0);
}
}
// Current implementation.
static void OnHeaderOld(grpc_exec_ctx *exec_ctx, void *user_data,
grpc_mdelem md) {
if (grpc_slice_eq(GRPC_MDKEY(md), GRPC_MDSTR_GRPC_TIMEOUT)) {
grpc_millis *cached_timeout =
static_cast<grpc_millis *>(grpc_mdelem_get_user_data(md, free_timeout));
grpc_millis timeout;
if (cached_timeout == NULL) {
/* not already parsed: parse it now, and store the result away */
cached_timeout = (grpc_millis *)gpr_malloc(sizeof(grpc_millis));
if (!grpc_http2_decode_timeout(GRPC_MDVALUE(md), cached_timeout)) {
char *val = grpc_slice_to_c_string(GRPC_MDVALUE(md));
gpr_log(GPR_ERROR, "Ignoring bad timeout value '%s'", val);
gpr_free(val);
*cached_timeout = GRPC_MILLIS_INF_FUTURE;
}
timeout = *cached_timeout;
grpc_mdelem_set_user_data(md, free_timeout, cached_timeout);
} else {
timeout = *cached_timeout;
}
benchmark::DoNotOptimize(timeout);
GRPC_MDELEM_UNREF(exec_ctx, md);
} else {
GPR_ASSERT(0);
}
}
// Send the same deadline repeatedly
class SameDeadline {
public:
static std::vector<grpc_slice> GetInitSlices() {
return {
grpc_slice_from_static_string("@\x0cgrpc-timeout\x03"
"30S")};
}
static std::vector<grpc_slice> GetBenchmarkSlices() {
// Use saved key and literal value.
return {MakeSlice({0x0f, 0x2f, 0x03, '3', '0', 'S'})};
}
};
BENCHMARK_TEMPLATE(BM_HpackParserParseHeader, EmptyBatch);
BENCHMARK_TEMPLATE(BM_HpackParserParseHeader, IndexedSingleStaticElem);
BENCHMARK_TEMPLATE(BM_HpackParserParseHeader, AddIndexedSingleStaticElem);
@ -786,6 +863,9 @@ BENCHMARK_TEMPLATE(BM_HpackParserParseHeader,
BENCHMARK_TEMPLATE(BM_HpackParserParseHeader,
RepresentativeServerTrailingMetadata);
BENCHMARK_TEMPLATE(BM_HpackParserParseHeader, SameDeadline, OnHeaderOld);
BENCHMARK_TEMPLATE(BM_HpackParserParseHeader, SameDeadline, OnHeaderNew);
} // namespace hpack_parser_fixtures
BENCHMARK_MAIN();

Loading…
Cancel
Save