Fast-path for no-error case for grpc_error_get_status.

For each client side call, we execute grpc_error_get_status; in the common case
that there is no error, we save roughly 30 instructions and a call to strlen.
pull/19220/head
Arjun Roy 6 years ago
parent 21c4e7d9f2
commit bff4dd1b2d
  1. 2
      src/core/ext/filters/http/client/http_client_filter.cc
  2. 4
      src/core/ext/filters/http/client_authority_filter.cc
  3. 10
      src/core/ext/transport/chttp2/transport/hpack_parser.cc
  4. 6
      src/core/ext/transport/chttp2/transport/hpack_table.cc
  5. 2
      src/core/ext/transport/inproc/inproc_transport.cc
  6. 24
      src/core/lib/iomgr/error.cc
  7. 49
      src/core/lib/slice/slice.cc
  8. 15
      src/core/lib/slice/slice_internal.h
  9. 11
      src/core/lib/transport/error_utils.cc
  10. 1
      tools/run_tests/sanity/core_banned_functions.py

@ -558,7 +558,7 @@ static grpc_slice user_agent_from_args(const grpc_channel_args* args,
tmp = gpr_strvec_flatten(&v, nullptr); tmp = gpr_strvec_flatten(&v, nullptr);
gpr_strvec_destroy(&v); gpr_strvec_destroy(&v);
result = grpc_slice_intern(grpc_slice_from_static_string(tmp)); result = grpc_slice_intern(grpc_slice_from_static_string_internal(tmp));
gpr_free(tmp); gpr_free(tmp);
return result; return result;

@ -101,8 +101,8 @@ grpc_error* init_channel_elem(grpc_channel_element* elem,
return GRPC_ERROR_CREATE_FROM_STATIC_STRING( return GRPC_ERROR_CREATE_FROM_STATIC_STRING(
"GRPC_ARG_DEFAULT_AUTHORITY channel arg. must be a string"); "GRPC_ARG_DEFAULT_AUTHORITY channel arg. must be a string");
} }
chand->default_authority = chand->default_authority = grpc_slice_intern(
grpc_slice_intern(grpc_slice_from_static_string(default_authority_str)); grpc_slice_from_static_string_internal(default_authority_str));
chand->default_authority_mdelem = grpc_mdelem_create( chand->default_authority_mdelem = grpc_mdelem_create(
GRPC_MDSTR_AUTHORITY, chand->default_authority, nullptr); GRPC_MDSTR_AUTHORITY, chand->default_authority, nullptr);
GPR_ASSERT(!args->is_last); GPR_ASSERT(!args->is_last);

@ -670,7 +670,7 @@ static grpc_slice take_string(grpc_chttp2_hpack_parser* p,
str->copied = true; str->copied = true;
str->data.referenced = grpc_empty_slice(); str->data.referenced = grpc_empty_slice();
} else if (intern) { } else if (intern) {
s = grpc_slice_intern(grpc_slice_from_static_buffer( s = grpc_slice_intern(grpc_slice_from_static_buffer_internal(
str->data.copied.str, str->data.copied.length)); str->data.copied.str, str->data.copied.length));
} else { } else {
s = grpc_slice_from_copied_buffer(str->data.copied.str, s = grpc_slice_from_copied_buffer(str->data.copied.str,
@ -1496,14 +1496,14 @@ static grpc_error* parse_key_string(grpc_chttp2_hpack_parser* p,
static bool is_binary_literal_header(grpc_chttp2_hpack_parser* p) { static bool is_binary_literal_header(grpc_chttp2_hpack_parser* p) {
/* We know that either argument here is a reference counter slice. /* We know that either argument here is a reference counter slice.
* 1. If a result of grpc_slice_from_static_buffer, the refcount is set to * 1. If a result of grpc_slice_from_static_buffer_internal, the refcount is
* NoopRefcount. * set to kNoopRefcount.
* 2. If it's p->key.data.referenced, then p->key.copied was set to false, * 2. If it's p->key.data.referenced, then p->key.copied was set to false,
* which occurs in begin_parse_string() - where the refcount is set to * which occurs in begin_parse_string() - where the refcount is set to
* p->current_slice_refcount, which is not null. */ * p->current_slice_refcount, which is not null. */
return grpc_is_refcounted_slice_binary_header( return grpc_is_refcounted_slice_binary_header(
p->key.copied ? grpc_slice_from_static_buffer(p->key.data.copied.str, p->key.copied ? grpc_slice_from_static_buffer_internal(
p->key.data.copied.length) p->key.data.copied.str, p->key.data.copied.length)
: p->key.data.referenced); : p->key.data.referenced);
} }

@ -29,6 +29,7 @@
#include "src/core/lib/debug/trace.h" #include "src/core/lib/debug/trace.h"
#include "src/core/lib/gpr/murmur_hash.h" #include "src/core/lib/gpr/murmur_hash.h"
#include "src/core/lib/slice/slice_internal.h"
#include "src/core/lib/surface/validate_metadata.h" #include "src/core/lib/surface/validate_metadata.h"
#include "src/core/lib/transport/static_metadata.h" #include "src/core/lib/transport/static_metadata.h"
@ -182,9 +183,10 @@ void grpc_chttp2_hptbl_init(grpc_chttp2_hptbl* tbl) {
memset(tbl->ents, 0, sizeof(*tbl->ents) * tbl->cap_entries); memset(tbl->ents, 0, sizeof(*tbl->ents) * tbl->cap_entries);
for (i = 1; i <= GRPC_CHTTP2_LAST_STATIC_ENTRY; i++) { for (i = 1; i <= GRPC_CHTTP2_LAST_STATIC_ENTRY; i++) {
tbl->static_ents[i - 1] = grpc_mdelem_from_slices( tbl->static_ents[i - 1] = grpc_mdelem_from_slices(
grpc_slice_intern(grpc_slice_from_static_string(static_table[i].key)),
grpc_slice_intern( grpc_slice_intern(
grpc_slice_from_static_string(static_table[i].value))); grpc_slice_from_static_string_internal(static_table[i].key)),
grpc_slice_intern(
grpc_slice_from_static_string_internal(static_table[i].value)));
} }
} }

@ -1203,7 +1203,7 @@ void inproc_transports_create(grpc_transport** server_transport,
*/ */
void grpc_inproc_transport_init(void) { void grpc_inproc_transport_init(void) {
grpc_core::ExecCtx exec_ctx; grpc_core::ExecCtx exec_ctx;
g_empty_slice = grpc_slice_from_static_buffer(nullptr, 0); g_empty_slice = grpc_slice_from_static_buffer_internal(nullptr, 0);
grpc_slice key_tmp = grpc_slice_from_static_string(":path"); grpc_slice key_tmp = grpc_slice_from_static_string(":path");
g_fake_path_key = grpc_slice_intern(key_tmp); g_fake_path_key = grpc_slice_intern(key_tmp);

@ -447,13 +447,17 @@ grpc_error* grpc_error_set_int(grpc_error* src, grpc_error_ints which,
typedef struct { typedef struct {
grpc_status_code code; grpc_status_code code;
const char* msg; const char* msg;
size_t len;
} special_error_status_map; } special_error_status_map;
static const special_error_status_map error_status_map[] = {
{GRPC_STATUS_OK, ""}, // GRPC_ERROR_NONE const special_error_status_map error_status_map[] = {
{GRPC_STATUS_INVALID_ARGUMENT, ""}, // GRPC_ERROR_RESERVED_1 {GRPC_STATUS_OK, "", 0}, // GRPC_ERROR_NONE
{GRPC_STATUS_RESOURCE_EXHAUSTED, "Out of memory"}, // GRPC_ERROR_OOM {GRPC_STATUS_INVALID_ARGUMENT, "", 0}, // GRPC_ERROR_RESERVED_1
{GRPC_STATUS_INVALID_ARGUMENT, ""}, // GRPC_ERROR_RESERVED_2 {GRPC_STATUS_RESOURCE_EXHAUSTED, "Out of memory",
{GRPC_STATUS_CANCELLED, "Cancelled"}, // GRPC_ERROR_CANCELLED strlen("Out of memory")}, // GRPC_ERROR_OOM
{GRPC_STATUS_INVALID_ARGUMENT, "", 0}, // GRPC_ERROR_RESERVED_2
{GRPC_STATUS_CANCELLED, "Cancelled",
strlen("Cancelled")}, // GRPC_ERROR_CANCELLED
}; };
bool grpc_error_get_int(grpc_error* err, grpc_error_ints which, intptr_t* p) { bool grpc_error_get_int(grpc_error* err, grpc_error_ints which, intptr_t* p) {
@ -483,8 +487,12 @@ bool grpc_error_get_str(grpc_error* err, grpc_error_strs which,
grpc_slice* str) { grpc_slice* str) {
if (grpc_error_is_special(err)) { if (grpc_error_is_special(err)) {
if (which != GRPC_ERROR_STR_GRPC_MESSAGE) return false; if (which != GRPC_ERROR_STR_GRPC_MESSAGE) return false;
*str = grpc_slice_from_static_string( const special_error_status_map& msg =
error_status_map[reinterpret_cast<size_t>(err)].msg); error_status_map[reinterpret_cast<size_t>(err)];
str->refcount = &grpc_core::kNoopRefcount;
str->data.refcounted.bytes =
reinterpret_cast<uint8_t*>(const_cast<char*>(msg.msg));
str->data.refcounted.length = msg.len;
return true; return true;
} }
uint8_t slot = err->strs[which]; uint8_t slot = err->strs[which];

@ -66,32 +66,13 @@ void grpc_slice_unref(grpc_slice slice) {
} }
} }
namespace grpc_core {
/* grpc_slice_from_static_string support structure - a refcount that does /* grpc_slice_from_static_string support structure - a refcount that does
nothing */ nothing */
static grpc_slice_refcount NoopRefcount = grpc_slice_refcount kNoopRefcount(grpc_slice_refcount::Type::NOP);
grpc_slice_refcount(grpc_slice_refcount::Type::NOP); static_assert(std::is_trivially_destructible<decltype(kNoopRefcount)>::value,
"kNoopRefcount must be trivially destructible.");
size_t grpc_slice_memory_usage(grpc_slice s) {
if (s.refcount == nullptr || s.refcount == &NoopRefcount) {
return 0;
} else {
return s.data.refcounted.length;
}
}
grpc_slice grpc_slice_from_static_buffer(const void* s, size_t len) {
grpc_slice slice;
slice.refcount = &NoopRefcount;
slice.data.refcounted.bytes = (uint8_t*)s;
slice.data.refcounted.length = len;
return slice;
}
grpc_slice grpc_slice_from_static_string(const char* s) {
return grpc_slice_from_static_buffer(s, strlen(s));
}
namespace grpc_core {
/* grpc_slice_new support structures - we create a refcount object extended /* grpc_slice_new support structures - we create a refcount object extended
with the user provided data pointer & destroy function */ with the user provided data pointer & destroy function */
@ -122,6 +103,22 @@ class NewSliceRefcount {
} // namespace grpc_core } // namespace grpc_core
size_t grpc_slice_memory_usage(grpc_slice s) {
if (s.refcount == nullptr || s.refcount == &grpc_core::kNoopRefcount) {
return 0;
} else {
return s.data.refcounted.length;
}
}
grpc_slice grpc_slice_from_static_buffer(const void* s, size_t len) {
return grpc_slice_from_static_buffer_internal(s, len);
}
grpc_slice grpc_slice_from_static_string(const char* s) {
return grpc_slice_from_static_buffer_internal(s, strlen(s));
}
grpc_slice grpc_slice_new_with_user_data(void* p, size_t len, grpc_slice grpc_slice_new_with_user_data(void* p, size_t len,
void (*destroy)(void*), void (*destroy)(void*),
void* user_data) { void* user_data) {
@ -375,10 +372,10 @@ grpc_slice grpc_slice_split_tail_maybe_ref(grpc_slice* source, size_t split,
switch (ref_whom) { switch (ref_whom) {
case GRPC_SLICE_REF_TAIL: case GRPC_SLICE_REF_TAIL:
tail.refcount = source->refcount->sub_refcount(); tail.refcount = source->refcount->sub_refcount();
source->refcount = &NoopRefcount; source->refcount = &grpc_core::kNoopRefcount;
break; break;
case GRPC_SLICE_REF_HEAD: case GRPC_SLICE_REF_HEAD:
tail.refcount = &NoopRefcount; tail.refcount = &grpc_core::kNoopRefcount;
source->refcount = source->refcount->sub_refcount(); source->refcount = source->refcount->sub_refcount();
break; break;
case GRPC_SLICE_REF_BOTH: case GRPC_SLICE_REF_BOTH:

@ -171,6 +171,8 @@ struct grpc_slice_refcount {
namespace grpc_core { namespace grpc_core {
extern grpc_slice_refcount kNoopRefcount;
struct InternedSliceRefcount { struct InternedSliceRefcount {
static void Destroy(void* arg) { static void Destroy(void* arg) {
auto* rc = static_cast<InternedSliceRefcount*>(arg); auto* rc = static_cast<InternedSliceRefcount*>(arg);
@ -312,4 +314,17 @@ grpc_slice grpc_slice_from_moved_string(grpc_core::UniquePtr<char> p);
// 0. All other slices will return the size of the allocated chars. // 0. All other slices will return the size of the allocated chars.
size_t grpc_slice_memory_usage(grpc_slice s); size_t grpc_slice_memory_usage(grpc_slice s);
inline grpc_slice grpc_slice_from_static_buffer_internal(const void* s,
size_t len) {
grpc_slice slice;
slice.refcount = &grpc_core::kNoopRefcount;
slice.data.refcounted.bytes = (uint8_t*)s;
slice.data.refcounted.length = len;
return slice;
}
inline grpc_slice grpc_slice_from_static_string_internal(const char* s) {
return grpc_slice_from_static_buffer_internal(s, strlen(s));
}
#endif /* GRPC_CORE_LIB_SLICE_SLICE_INTERNAL_H */ #endif /* GRPC_CORE_LIB_SLICE_SLICE_INTERNAL_H */

@ -22,6 +22,7 @@
#include <grpc/support/string_util.h> #include <grpc/support/string_util.h>
#include "src/core/lib/iomgr/error_internal.h" #include "src/core/lib/iomgr/error_internal.h"
#include "src/core/lib/slice/slice_internal.h"
#include "src/core/lib/transport/status_conversion.h" #include "src/core/lib/transport/status_conversion.h"
static grpc_error* recursively_find_error_with_field(grpc_error* error, static grpc_error* recursively_find_error_with_field(grpc_error* error,
@ -52,7 +53,15 @@ void grpc_error_get_status(grpc_error* error, grpc_millis deadline,
if (GPR_LIKELY(error == GRPC_ERROR_NONE)) { if (GPR_LIKELY(error == GRPC_ERROR_NONE)) {
if (code != nullptr) *code = GRPC_STATUS_OK; if (code != nullptr) *code = GRPC_STATUS_OK;
if (slice != nullptr) { if (slice != nullptr) {
grpc_error_get_str(error, GRPC_ERROR_STR_GRPC_MESSAGE, slice); // Normally, we call grpc_error_get_str(
// error, GRPC_ERROR_STR_GRPC_MESSAGE, slice).
// We can fastpath since we know that:
// 1) Error is null
// 2) which == GRPC_ERROR_STR_GRPC_MESSAGE
// 3) The resulting slice is statically known.
// 4) Said resulting slice is of length 0 ("").
// This means 3 movs, instead of 10s of instructions and a strlen.
*slice = grpc_slice_from_static_string_internal("");
} }
if (http_error != nullptr) { if (http_error != nullptr) {
*http_error = GRPC_HTTP2_NO_ERROR; *http_error = GRPC_HTTP2_NO_ERROR;

@ -23,6 +23,7 @@ os.chdir(os.path.join(os.path.dirname(sys.argv[0]), '../../..'))
# map of banned function signature to whitelist # map of banned function signature to whitelist
BANNED_EXCEPT = { BANNED_EXCEPT = {
'grpc_slice_from_static_buffer(': ['src/core/lib/slice/slice.cc'],
'grpc_resource_quota_ref(': ['src/core/lib/iomgr/resource_quota.cc'], 'grpc_resource_quota_ref(': ['src/core/lib/iomgr/resource_quota.cc'],
'grpc_resource_quota_unref(': 'grpc_resource_quota_unref(':
['src/core/lib/iomgr/resource_quota.cc', 'src/core/lib/surface/server.cc'], ['src/core/lib/iomgr/resource_quota.cc', 'src/core/lib/surface/server.cc'],

Loading…
Cancel
Save