Chiebot-Mirror
/
grpc
mirror of https://github.com/grpc/grpc.git
8
0
Fork 0
Code Issues Projects Releases Wiki Activity

Optimize error handling for special cases.

Browse Source 
This commit contains a few improvements:
1. Using a consequetive range of [0..4], will allow us to merge all
   branches of error_is_special into one comparison.
2. With (1), we can remove the for loops to find entries in
   error_status_map with a single O(1) lookup.
3. grpc_error_is_special() code paths should be inlined for ref
   and unref to avoid callq for the majority of cases where speical
   error is used.
4. grpc_error_get_int() should never accept a nullptr argument to
   avoid an expensive branch in the hot path. Callers should all
   allocate a dummy int on the stack when calling.
pull/16932/head
Soheil Hassas Yeganeh 6 years ago
parent dfcc96227c
commit 3a9411ca1e
5 changed files with 51 additions and 41 deletions
Whitespace
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Split View
Diff Options
Show Stats Download Patch File Download Diff File
  1. 6
      src/core/ext/transport/chttp2/transport/parsing.cc
  2. 47
      src/core/lib/iomgr/error.cc
  3. 31
      src/core/lib/iomgr/error.h
  4. 2
      src/core/lib/iomgr/error_internal.h
  5. 6
      src/core/lib/transport/error_utils.cc

6
src/core/ext/transport/chttp2/transport/parsing.cc
Unescape View File

@ -368,6 +368,7 @@ static grpc_error* init_data_frame_parser(grpc_chttp2_transport* t) {
&s->data_parser, t->incoming_frame_flags, s->id, s);
}
error_handler:
intptr_t unused;
if (err == GRPC_ERROR_NONE) {
t->incoming_stream = s;
/* t->parser = grpc_chttp2_data_parser_parse;*/
@ -375,7 +376,7 @@ error_handler:
t->parser_data = &s->data_parser;
t->ping_state.last_ping_sent_time = GRPC_MILLIS_INF_PAST;
return GRPC_ERROR_NONE;
} else if (grpc_error_get_int(err, GRPC_ERROR_INT_STREAM_ID, nullptr)) {
} else if (grpc_error_get_int(err, GRPC_ERROR_INT_STREAM_ID, &unused)) {
/* handle stream errors by closing the stream */
if (s != nullptr) {
grpc_chttp2_mark_stream_closed(t, s, true, false, err);
@ -756,9 +757,10 @@ static grpc_error* parse_frame_slice(grpc_chttp2_transport* t, grpc_slice slice,
int is_last) {
grpc_chttp2_stream* s = t->incoming_stream;
grpc_error* err = t->parser(t->parser_data, t, s, slice, is_last);
intptr_t unused;
if (GPR_LIKELY(err == GRPC_ERROR_NONE)) {
return err;
} else if (grpc_error_get_int(err, GRPC_ERROR_INT_STREAM_ID, nullptr)) {
} else if (grpc_error_get_int(err, GRPC_ERROR_INT_STREAM_ID, &unused)) {
if (grpc_http_trace.enabled()) {
const char* msg = grpc_error_string(err);
gpr_log(GPR_ERROR, "%s", msg);

47
src/core/lib/iomgr/error.cc
Unescape View File

@ -121,14 +121,8 @@ static const char* error_time_name(grpc_error_times key) {
GPR_UNREACHABLE_CODE(return "unknown");
}
bool grpc_error_is_special(grpc_error* err) {
return err == GRPC_ERROR_NONE || err == GRPC_ERROR_OOM ||
err == GRPC_ERROR_CANCELLED;
}
#ifndef NDEBUG
grpc_error* grpc_error_ref(grpc_error* err, const char* file, int line) {
if (grpc_error_is_special(err)) return err;
grpc_error* grpc_error_do_ref(grpc_error* err, const char* file, int line) {
if (grpc_trace_error_refcount.enabled()) {
gpr_log(GPR_DEBUG, "%p: %" PRIdPTR " -> %" PRIdPTR " [%s:%d]", err,
gpr_atm_no_barrier_load(&err->atomics.refs.count),
@ -138,8 +132,7 @@ grpc_error* grpc_error_ref(grpc_error* err, const char* file, int line) {
return err;
}
#else
grpc_error* grpc_error_ref(grpc_error* err) {
if (grpc_error_is_special(err)) return err;
grpc_error* grpc_error_do_ref(grpc_error* err) {
gpr_ref(&err->atomics.refs);
return err;
}
@ -177,8 +170,7 @@ static void error_destroy(grpc_error* err) {
}
#ifndef NDEBUG
void grpc_error_unref(grpc_error* err, const char* file, int line) {
if (grpc_error_is_special(err)) return;
void grpc_error_do_unref(grpc_error* err, const char* file, int line) {
if (grpc_trace_error_refcount.enabled()) {
gpr_log(GPR_DEBUG, "%p: %" PRIdPTR " -> %" PRIdPTR " [%s:%d]", err,
gpr_atm_no_barrier_load(&err->atomics.refs.count),
@ -189,8 +181,7 @@ void grpc_error_unref(grpc_error* err, const char* file, int line) {
}
}
#else
void grpc_error_unref(grpc_error* err) {
if (grpc_error_is_special(err)) return;
void grpc_error_do_unref(grpc_error* err) {
if (gpr_unref(&err->atomics.refs)) {
error_destroy(err);
}
@ -450,26 +441,23 @@ grpc_error* grpc_error_set_int(grpc_error* src, grpc_error_ints which,
}
typedef struct {
grpc_error* error;
grpc_status_code code;
const char* msg;
} special_error_status_map;
static const special_error_status_map error_status_map[] = {
{GRPC_ERROR_NONE, GRPC_STATUS_OK, ""},
{GRPC_ERROR_CANCELLED, GRPC_STATUS_CANCELLED, "Cancelled"},
{GRPC_ERROR_OOM, GRPC_STATUS_RESOURCE_EXHAUSTED, "Out of memory"},
{GRPC_STATUS_OK, ""}, // GRPC_ERROR_NONE
{GRPC_STATUS_INVALID_ARGUMENT, ""}, // GRPC_ERROR_RESERVED_1
{GRPC_STATUS_RESOURCE_EXHAUSTED, "Out of memory"}, // GRPC_ERROR_OOM
{GRPC_STATUS_INVALID_ARGUMENT, ""}, // GRPC_ERROR_RESERVED_2
{GRPC_STATUS_CANCELLED, "Cancelled"}, // GRPC_ERROR_CANCELLED
};
bool grpc_error_get_int(grpc_error* err, grpc_error_ints which, intptr_t* p) {
GPR_TIMER_SCOPE("grpc_error_get_int", 0);
if (grpc_error_is_special(err)) {
for (size_t i = 0; i < GPR_ARRAY_SIZE(error_status_map); i++) {
if (error_status_map[i].error == err) {
if (which != GRPC_ERROR_INT_GRPC_STATUS) return false;
if (p != nullptr) *p = error_status_map[i].code;
return true;
}
}
if (which != GRPC_ERROR_INT_GRPC_STATUS) return false;
*p = error_status_map[reinterpret_cast<size_t>(err)].code;
return true;
}
uint8_t slot = err->ints[which];
if (slot != UINT8_MAX) {
@ -490,13 +478,10 @@ grpc_error* grpc_error_set_str(grpc_error* src, grpc_error_strs which,
bool grpc_error_get_str(grpc_error* err, grpc_error_strs which,
grpc_slice* str) {
if (grpc_error_is_special(err)) {
for (size_t i = 0; i < GPR_ARRAY_SIZE(error_status_map); i++) {
if (error_status_map[i].error == err) {
if (which != GRPC_ERROR_STR_GRPC_MESSAGE) return false;
*str = grpc_slice_from_static_string(error_status_map[i].msg);
return true;
}
}
if (which != GRPC_ERROR_STR_GRPC_MESSAGE) return false;
*str = grpc_slice_from_static_string(
error_status_map[reinterpret_cast<size_t>(err)].msg);
return true;
}
uint8_t slot = err->strs[which];
if (slot != UINT8_MAX) {

31
src/core/lib/iomgr/error.h
Unescape View File

@ -120,8 +120,15 @@ typedef enum {
/// polling engines) can safely use the lower bit for themselves.
#define GRPC_ERROR_NONE ((grpc_error*)NULL)
#define GRPC_ERROR_RESERVED_1 ((grpc_error*)1)
#define GRPC_ERROR_OOM ((grpc_error*)2)
#define GRPC_ERROR_RESERVED_2 ((grpc_error*)3)
#define GRPC_ERROR_CANCELLED ((grpc_error*)4)
#define GRPC_ERROR_SPECIAL_MAX GRPC_ERROR_CANCELLED
inline bool grpc_error_is_special(struct grpc_error* err) {
return err <= GRPC_ERROR_SPECIAL_MAX;
}
// debug only toggles that allow for a sanity to check that ensures we will
// never create any errors in the per-RPC hotpath.
@ -158,13 +165,29 @@ grpc_error* grpc_error_create(const char* file, int line, grpc_slice desc,
errs, count)
#ifndef NDEBUG
grpc_error* grpc_error_ref(grpc_error* err, const char* file, int line);
void grpc_error_unref(grpc_error* err, const char* file, int line);
grpc_error* grpc_error_do_ref(grpc_error* err, const char* file, int line);
void grpc_error_do_unref(grpc_error* err, const char* file, int line);
inline grpc_error* grpc_error_ref(grpc_error* err, const char* file, int line) {
if (grpc_error_is_special(err)) return err;
return grpc_error_do_ref(err, file, line);
}
inline void grpc_error_unref(grpc_error* err, const char* file, int line) {
if (grpc_error_is_special(err)) return;
grpc_error_do_unref(err, file, line);
}
#define GRPC_ERROR_REF(err) grpc_error_ref(err, __FILE__, __LINE__)
#define GRPC_ERROR_UNREF(err) grpc_error_unref(err, __FILE__, __LINE__)
#else
grpc_error* grpc_error_ref(grpc_error* err);
void grpc_error_unref(grpc_error* err);
grpc_error* grpc_error_do_ref(grpc_error* err);
void grpc_error_do_unref(grpc_error* err);
inline grpc_error* grpc_error_ref(grpc_error* err) {
if (grpc_error_is_special(err)) return err;
return grpc_error_do_ref(err);
}
inline void grpc_error_unref(grpc_error* err) {
if (grpc_error_is_special(err)) return;
grpc_error_do_unref(err);
}
#define GRPC_ERROR_REF(err) grpc_error_ref(err)
#define GRPC_ERROR_UNREF(err) grpc_error_unref(err)
#endif

2
src/core/lib/iomgr/error_internal.h
Unescape View File

@ -58,6 +58,4 @@ struct grpc_error {
intptr_t arena[0];
};
bool grpc_error_is_special(struct grpc_error* err);
#endif /* GRPC_CORE_LIB_IOMGR_ERROR_INTERNAL_H */

6
src/core/lib/transport/error_utils.cc
Unescape View File

@ -26,8 +26,9 @@
static grpc_error* recursively_find_error_with_field(grpc_error* error,
grpc_error_ints which) {
intptr_t unused;
// If the error itself has a status code, return it.
if (grpc_error_get_int(error, which, nullptr)) {
if (grpc_error_get_int(error, which, &unused)) {
return error;
}
if (grpc_error_is_special(error)) return nullptr;
@ -102,7 +103,8 @@ void grpc_error_get_status(grpc_error* error, grpc_millis deadline,
}
bool grpc_error_has_clear_grpc_status(grpc_error* error) {
if (grpc_error_get_int(error, GRPC_ERROR_INT_GRPC_STATUS, nullptr)) {
intptr_t unused;
if (grpc_error_get_int(error, GRPC_ERROR_INT_GRPC_STATUS, &unused)) {
return true;
}
uint8_t slot = error->first_err;

Write Preview
Loading…
Cancel
Save