From 55337bb317ab22e662ffebba71078a4e8def7127 Mon Sep 17 00:00:00 2001 From: ncteisen Date: Thu, 2 Mar 2017 15:12:33 -0800 Subject: Error arena optimization --- src/core/lib/iomgr/error_internal.h | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) (limited to 'src/core/lib/iomgr/error_internal.h') diff --git a/src/core/lib/iomgr/error_internal.h b/src/core/lib/iomgr/error_internal.h index 1c89ead4ed..bdebbe1792 100644 --- a/src/core/lib/iomgr/error_internal.h +++ b/src/core/lib/iomgr/error_internal.h @@ -35,18 +35,28 @@ #define GRPC_CORE_LIB_IOMGR_ERROR_INTERNAL_H #include -#include +#include // TODO, do we need this? -#include +#include + +typedef struct linked_error linked_error; + +struct linked_error { + grpc_error *err; + uint8_t next; +}; struct grpc_error { gpr_refcount refs; - gpr_avl ints; - gpr_avl strs; - gpr_avl times; - gpr_avl errs; - uintptr_t next_err; + uint8_t ints[GRPC_ERROR_INT_MAX]; + uint8_t strs[GRPC_ERROR_STR_MAX]; + uint8_t times[GRPC_ERROR_TIME_MAX]; + uint8_t first_err; + uint8_t last_err; gpr_atm error_string; + uint8_t arena_size; + uint8_t arena_capacity; + intptr_t arena[0]; }; bool grpc_error_is_special(grpc_error *err); -- cgit v1.2.3 From ceddd293919a9580cef928fa56a35480de278682 Mon Sep 17 00:00:00 2001 From: ncteisen Date: Thu, 9 Mar 2017 17:04:24 -0800 Subject: Address github comments --- src/core/lib/iomgr/error.c | 17 +++++++++-------- src/core/lib/iomgr/error_internal.h | 4 ++-- src/core/lib/transport/error_utils.c | 4 ++-- test/core/iomgr/error_test.c | 2 +- test/cpp/microbenchmarks/bm_error.cc | 24 ++++++++++++++++++++++++ 5 files changed, 38 insertions(+), 13 deletions(-) (limited to 'src/core/lib/iomgr/error_internal.h') diff --git a/src/core/lib/iomgr/error.c b/src/core/lib/iomgr/error.c index f0b7e01d49..c6851b0b6c 100644 --- a/src/core/lib/iomgr/error.c +++ b/src/core/lib/iomgr/error.c @@ -155,7 +155,7 @@ grpc_error *grpc_error_ref(grpc_error *err) { static void unref_errs(grpc_error *err) { uint8_t slot = err->first_err; while (slot != UINT8_MAX) { - linked_error *lerr = (linked_error *)(err->arena + slot); + grpc_linked_error *lerr = (grpc_linked_error *)(err->arena + slot); GRPC_ERROR_UNREF(lerr->err); GPR_ASSERT(err->last_err == slot ? lerr->next == UINT8_MAX : lerr->next != UINT8_MAX); @@ -254,25 +254,26 @@ static void internal_set_time(grpc_error **err, grpc_error_times which, } static void internal_add_error(grpc_error **err, grpc_error *new) { - linked_error new_last = {new, UINT8_MAX}; - uint8_t slot = get_placement(err, sizeof(linked_error)); + grpc_linked_error new_last = {new, UINT8_MAX}; + uint8_t slot = get_placement(err, sizeof(grpc_linked_error)); if ((*err)->first_err == UINT8_MAX) { GPR_ASSERT((*err)->last_err == UINT8_MAX); (*err)->last_err = slot; (*err)->first_err = slot; } else { GPR_ASSERT((*err)->last_err != UINT8_MAX); - linked_error *old_last = (linked_error *)((*err)->arena + (*err)->last_err); + grpc_linked_error *old_last = + (grpc_linked_error *)((*err)->arena + (*err)->last_err); old_last->next = slot; (*err)->last_err = slot; } - memcpy((*err)->arena + slot, &new_last, sizeof(linked_error)); + memcpy((*err)->arena + slot, &new_last, sizeof(grpc_linked_error)); } #define SLOTS_PER_INT (sizeof(intptr_t) / sizeof(intptr_t)) #define SLOTS_PER_STR (sizeof(grpc_slice) / sizeof(intptr_t)) #define SLOTS_PER_TIME (sizeof(gpr_timespec) / sizeof(intptr_t)) -#define SLOTS_PER_LINKED_ERROR (sizeof(linked_error) / sizeof(intptr_t)) +#define SLOTS_PER_LINKED_ERROR (sizeof(grpc_linked_error) / sizeof(intptr_t)) // size of storing one int and two slices and a timespec. For line, desc, file, // and time created @@ -345,7 +346,7 @@ static void ref_strs(grpc_error *err) { static void ref_errs(grpc_error *err) { uint8_t slot = err->first_err; while (slot != UINT8_MAX) { - linked_error *lerr = (linked_error *)(err->arena + slot); + grpc_linked_error *lerr = (grpc_linked_error *)(err->arena + slot); GRPC_ERROR_REF(lerr->err); slot = lerr->next; } @@ -636,7 +637,7 @@ static void add_errs(grpc_error *err, char **s, size_t *sz, size_t *cap) { uint8_t slot = err->first_err; bool first = true; while (slot != UINT8_MAX) { - linked_error *lerr = (linked_error *)(err->arena + slot); + grpc_linked_error *lerr = (grpc_linked_error *)(err->arena + slot); if (!first) append_chr(',', s, sz, cap); first = false; const char *e = grpc_error_string(lerr->err); diff --git a/src/core/lib/iomgr/error_internal.h b/src/core/lib/iomgr/error_internal.h index bdebbe1792..fb4814e41f 100644 --- a/src/core/lib/iomgr/error_internal.h +++ b/src/core/lib/iomgr/error_internal.h @@ -39,9 +39,9 @@ #include -typedef struct linked_error linked_error; +typedef struct grpc_linked_error grpc_linked_error; -struct linked_error { +struct grpc_linked_error { grpc_error *err; uint8_t next; }; diff --git a/src/core/lib/transport/error_utils.c b/src/core/lib/transport/error_utils.c index 707aac024b..ef55e561fb 100644 --- a/src/core/lib/transport/error_utils.c +++ b/src/core/lib/transport/error_utils.c @@ -46,7 +46,7 @@ static grpc_error *recursively_find_error_with_field(grpc_error *error, // Otherwise, search through its children. uint8_t slot = error->first_err; while (slot != UINT8_MAX) { - linked_error *lerr = (linked_error *)(error->arena + slot); + grpc_linked_error *lerr = (grpc_linked_error *)(error->arena + slot); grpc_error *result = recursively_find_error_with_field(lerr->err, which); if (result) return result; slot = lerr->next; @@ -114,7 +114,7 @@ bool grpc_error_has_clear_grpc_status(grpc_error *error) { } uint8_t slot = error->first_err; while (slot != UINT8_MAX) { - linked_error *lerr = (linked_error *)(error->arena + slot); + grpc_linked_error *lerr = (grpc_linked_error *)(error->arena + slot); if (grpc_error_has_clear_grpc_status(lerr->err)) { return true; } diff --git a/test/core/iomgr/error_test.c b/test/core/iomgr/error_test.c index d8289ac102..2a6b1b17fd 100644 --- a/test/core/iomgr/error_test.c +++ b/test/core/iomgr/error_test.c @@ -1,6 +1,6 @@ /* * - * Copyright 2015, Google Inc. + * Copyright 2017, Google Inc. * All rights reserved. * * Redistribution and use in source and binary forms, with or without diff --git a/test/cpp/microbenchmarks/bm_error.cc b/test/cpp/microbenchmarks/bm_error.cc index 76f168b75b..c4f6aa19d5 100644 --- a/test/cpp/microbenchmarks/bm_error.cc +++ b/test/cpp/microbenchmarks/bm_error.cc @@ -83,6 +83,30 @@ static void BM_ErrorCreateAndSetIntAndStr(benchmark::State& state) { } BENCHMARK(BM_ErrorCreateAndSetIntAndStr); +static void BM_ErrorCreateAndSetIntLoop(benchmark::State& state) { + TrackCounters track_counters; + grpc_error* error = GRPC_ERROR_CREATE("Error"); + int n = 0; + while (state.KeepRunning()) { + error = grpc_error_set_int(error, GRPC_ERROR_INT_GRPC_STATUS, n++); + } + GRPC_ERROR_UNREF(error); + track_counters.Finish(state); +} +BENCHMARK(BM_ErrorCreateAndSetIntLoop); + +static void BM_ErrorCreateAndSetStrLoop(benchmark::State& state) { + TrackCounters track_counters; + grpc_error* error = GRPC_ERROR_CREATE("Error"); + const char* str = "hello"; + while (state.KeepRunning()) { + error = grpc_error_set_str(error, GRPC_ERROR_STR_GRPC_MESSAGE, str); + } + GRPC_ERROR_UNREF(error); + track_counters.Finish(state); +} +BENCHMARK(BM_ErrorCreateAndSetStrLoop); + static void BM_ErrorRefUnref(benchmark::State& state) { TrackCounters track_counters; grpc_error* error = GRPC_ERROR_CREATE("Error"); -- cgit v1.2.3