From e3018e6f7d86cea35b2bc7f76e0b722b1ed7851b Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Fri, 13 Feb 2015 17:05:19 -0800 Subject: [PATCH] Avoid four allocations per stream --- Makefile | 2 +- src/core/transport/chttp2_transport.c | 18 ++++------- src/core/transport/stream_op.c | 46 ++++++++++++++++----------- src/core/transport/stream_op.h | 6 ++++ templates/Makefile.template | 2 +- 5 files changed, 42 insertions(+), 32 deletions(-) diff --git a/Makefile b/Makefile index fe0f7035e67..824d99468ad 100644 --- a/Makefile +++ b/Makefile @@ -69,7 +69,7 @@ CC_msan = clang CXX_msan = clang++ LD_msan = clang LDXX_msan = clang++ -CPPFLAGS_msan = -O1 -fsanitize=memory -fno-omit-frame-pointer +CPPFLAGS_msan = -O1 -fsanitize=memory -fno-omit-frame-pointer -fsanitize-memory-track-origins OPENSSL_CFLAGS_msan = -DPURIFY OPENSSL_CONFIG_msan = no-asm LDFLAGS_msan = -fsanitize=memory diff --git a/src/core/transport/chttp2_transport.c b/src/core/transport/chttp2_transport.c index ea579cf4a52..8b1fb789172 100644 --- a/src/core/transport/chttp2_transport.c +++ b/src/core/transport/chttp2_transport.c @@ -684,10 +684,11 @@ static void unlock(transport *t) { int i; pending_goaway *goaways = NULL; grpc_endpoint *ep = t->ep; - grpc_stream_op_buffer nuke_now = t->nuke_later_sopb; - - if (nuke_now.nops) { - memset(&t->nuke_later_sopb, 0, sizeof(t->nuke_later_sopb)); + grpc_stream_op_buffer nuke_now; + + grpc_sopb_init(&nuke_now); + if (t->nuke_later_sopb.nops) { + grpc_sopb_swap(&nuke_now, &t->nuke_later_sopb); } /* see if we need to trigger a write - and if so, get the data ready */ @@ -757,9 +758,7 @@ static void unlock(transport *t) { unref_transport(t); } - if (nuke_now.nops) { - grpc_sopb_destroy(&nuke_now); - } + grpc_sopb_destroy(&nuke_now); gpr_free(goaways); } @@ -1698,13 +1697,10 @@ static grpc_stream_state compute_state(gpr_uint8 write_closed, static int prepare_callbacks(transport *t) { stream *s; - grpc_stream_op_buffer temp_sopb; int n = 0; while ((s = stream_list_remove_head(t, PENDING_CALLBACKS))) { int execute = 1; - temp_sopb = s->parser.incoming_sopb; - s->parser.incoming_sopb = s->callback_sopb; - s->callback_sopb = temp_sopb; + grpc_sopb_swap(&s->parser.incoming_sopb, &s->callback_sopb); s->callback_state = compute_state(s->sent_write_closed, s->read_closed); if (s->callback_state == GRPC_STREAM_CLOSED) { diff --git a/src/core/transport/stream_op.c b/src/core/transport/stream_op.c index 555543fc4b8..03a338bd9be 100644 --- a/src/core/transport/stream_op.c +++ b/src/core/transport/stream_op.c @@ -38,23 +38,19 @@ #include -/* Initial number of operations to allocate */ -#define INITIAL_SLOTS 8 /* Exponential growth function: Given x, return a larger x. - Currently we grow by 1.5 times upon reallocation. - Assumes INITIAL_SLOTS > 1 */ + Currently we grow by 1.5 times upon reallocation. */ #define GROW(x) (3 * (x) / 2) void grpc_sopb_init(grpc_stream_op_buffer *sopb) { - sopb->ops = gpr_malloc(sizeof(grpc_stream_op) * INITIAL_SLOTS); - GPR_ASSERT(sopb->ops); + sopb->ops = sopb->inlined_ops; sopb->nops = 0; - sopb->capacity = INITIAL_SLOTS; + sopb->capacity = GRPC_SOPB_INLINE_ELEMENTS; } void grpc_sopb_destroy(grpc_stream_op_buffer *sopb) { grpc_stream_ops_unref_owned_objects(sopb->ops, sopb->nops); - gpr_free(sopb->ops); + if (sopb->ops != sopb->inlined_ops) gpr_free(sopb->ops); } void grpc_sopb_reset(grpc_stream_op_buffer *sopb) { @@ -62,6 +58,19 @@ void grpc_sopb_reset(grpc_stream_op_buffer *sopb) { sopb->nops = 0; } +void grpc_sopb_swap(grpc_stream_op_buffer *a, grpc_stream_op_buffer *b) { + grpc_stream_op_buffer temp = *a; + *a = *b; + *b = temp; + + if (a->ops == b->inlined_ops) { + a->ops = a->inlined_ops; + } + if (b->ops == a->inlined_ops) { + b->ops = b->inlined_ops; + } +} + void grpc_stream_ops_unref_owned_objects(grpc_stream_op *ops, size_t nops) { size_t i; for (i = 0; i < nops; i++) { @@ -84,17 +93,21 @@ void grpc_stream_ops_unref_owned_objects(grpc_stream_op *ops, size_t nops) { } } -static void expand(grpc_stream_op_buffer *sopb) { - sopb->capacity = GROW(sopb->capacity); - sopb->ops = gpr_realloc(sopb->ops, sizeof(grpc_stream_op) * sopb->capacity); - GPR_ASSERT(sopb->ops); +static void expandto(grpc_stream_op_buffer *sopb, size_t new_capacity) { + sopb->capacity = new_capacity; + if (sopb->ops == sopb->inlined_ops) { + sopb->ops = gpr_malloc(sizeof(grpc_stream_op) * new_capacity); + memcpy(sopb->ops, sopb->inlined_ops, sopb->nops * sizeof(grpc_stream_op)); + } else { + sopb->ops = gpr_realloc(sopb->ops, sizeof(grpc_stream_op) * new_capacity); + } } static grpc_stream_op *add(grpc_stream_op_buffer *sopb) { grpc_stream_op *out; if (sopb->nops == sopb->capacity) { - expand(sopb); + expandto(sopb, GROW(sopb->capacity)); } out = sopb->ops + sopb->nops; sopb->nops++; @@ -152,12 +165,7 @@ void grpc_sopb_append(grpc_stream_op_buffer *sopb, grpc_stream_op *ops, size_t new_nops = orig_nops + nops; if (new_nops > sopb->capacity) { - size_t new_capacity = GROW(sopb->capacity); - if (new_capacity < new_nops) { - new_capacity = new_nops; - } - sopb->ops = gpr_realloc(sopb->ops, sizeof(grpc_stream_op) * new_capacity); - sopb->capacity = new_capacity; + expandto(sopb, GPR_MAX(GROW(sopb->capacity), new_nops)); } memcpy(sopb->ops + orig_nops, ops, sizeof(grpc_stream_op) * nops); diff --git a/src/core/transport/stream_op.h b/src/core/transport/stream_op.h index 20d609133f8..d4d7515c4f2 100644 --- a/src/core/transport/stream_op.h +++ b/src/core/transport/stream_op.h @@ -40,6 +40,9 @@ #include #include "src/core/transport/metadata.h" +/* this many stream ops are inlined into a sopb before allocating */ +#define GRPC_SOPB_INLINE_ELEMENTS 16 + /* Operations that can be performed on a stream. Used by grpc_stream_op. */ typedef enum grpc_stream_op_code { @@ -96,6 +99,7 @@ typedef struct grpc_stream_op_buffer { grpc_stream_op *ops; size_t nops; size_t capacity; + grpc_stream_op inlined_ops[GRPC_SOPB_INLINE_ELEMENTS]; } grpc_stream_op_buffer; /* Initialize a stream op buffer */ @@ -104,6 +108,8 @@ void grpc_sopb_init(grpc_stream_op_buffer *sopb); void grpc_sopb_destroy(grpc_stream_op_buffer *sopb); /* Reset a sopb to no elements */ void grpc_sopb_reset(grpc_stream_op_buffer *sopb); +/* Swap two sopbs */ +void grpc_sopb_swap(grpc_stream_op_buffer *a, grpc_stream_op_buffer *b); void grpc_stream_ops_unref_owned_objects(grpc_stream_op *ops, size_t nops); diff --git a/templates/Makefile.template b/templates/Makefile.template index b9ae217054e..0a453cbd8bc 100644 --- a/templates/Makefile.template +++ b/templates/Makefile.template @@ -86,7 +86,7 @@ CC_msan = clang CXX_msan = clang++ LD_msan = clang LDXX_msan = clang++ -CPPFLAGS_msan = -O1 -fsanitize=memory -fno-omit-frame-pointer +CPPFLAGS_msan = -O1 -fsanitize=memory -fno-omit-frame-pointer -fsanitize-memory-track-origins OPENSSL_CFLAGS_msan = -DPURIFY OPENSSL_CONFIG_msan = no-asm LDFLAGS_msan = -fsanitize=memory