From 423d6fd7ed418aead7d494e8d3c31ecd6e2743f1 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Wed, 12 Apr 2017 13:15:45 -0700 Subject: [PATCH 1/9] Optimize framing a little - rely on the fact that data-to-come holds a reference to data-being-written, so there's no need to add a ref for every frame written - provide an 'inlined' version of grpc_slice_malloc (via a #define) that gives the compiler more information about small allocations to enable better optimization --- include/grpc/slice.h | 10 ++ include/grpc/slice_buffer.h | 4 + .../lb_policy/grpclb/load_balancer_api.c | 2 +- .../transport/chttp2/transport/bin_decoder.c | 4 +- .../transport/chttp2/transport/bin_encoder.c | 6 +- .../chttp2/transport/chttp2_transport.c | 8 +- .../transport/chttp2/transport/frame_data.c | 4 +- .../transport/chttp2/transport/frame_goaway.c | 2 +- .../transport/chttp2/transport/frame_ping.c | 2 +- .../chttp2/transport/frame_rst_stream.c | 2 +- .../chttp2/transport/frame_settings.c | 4 +- .../chttp2/transport/frame_window_update.c | 2 +- .../chttp2/transport/hpack_encoder.c | 2 +- .../cronet/transport/cronet_transport.c | 2 +- src/core/lib/channel/http_client_filter.c | 2 +- src/core/lib/compression/message_compress.c | 4 +- src/core/lib/iomgr/tcp_windows.c | 2 +- .../lib/security/transport/secure_endpoint.c | 8 +- src/core/lib/slice/b64.c | 2 +- src/core/lib/slice/percent_encoding.c | 6 +- src/core/lib/slice/slice.c | 95 +++++++++++++------ src/core/lib/slice/slice_buffer.c | 36 ++++++- src/core/lib/surface/byte_buffer_reader.c | 2 +- 23 files changed, 150 insertions(+), 61 deletions(-) diff --git a/include/grpc/slice.h b/include/grpc/slice.h index ea66e094e98..4dce6da9155 100644 --- a/include/grpc/slice.h +++ b/include/grpc/slice.h @@ -75,6 +75,13 @@ GPRAPI grpc_slice grpc_slice_new_with_len(void *p, size_t len, call. Aborts if malloc() fails. */ GPRAPI grpc_slice grpc_slice_malloc(size_t length); +GPRAPI grpc_slice grpc_slice_malloc_large(size_t length); + +#define GRPC_SLICE_MALLOC(len) \ + ((len) <= GRPC_SLICE_INLINED_SIZE \ + ? (grpc_slice){.refcount = NULL, \ + .data.inlined = {.length = (uint8_t)(len)}} \ + : grpc_slice_malloc_large((len))) /* Intern a slice: @@ -117,6 +124,9 @@ GPRAPI grpc_slice grpc_slice_sub_no_ref(grpc_slice s, size_t begin, size_t end); Requires s intialized, split <= s.length */ GPRAPI grpc_slice grpc_slice_split_tail(grpc_slice *s, size_t split); +/* The same as grpc_slice_split_tail, but without altering refcounts */ +GPRAPI grpc_slice grpc_slice_split_tail_no_ref(grpc_slice *s, size_t split); + /* Splits s into two: modifies s to be s[split:s.length], and returns a new slice, sharing a refcount with s, that contains s[0:split]. Requires s intialized, split <= s.length */ diff --git a/include/grpc/slice_buffer.h b/include/grpc/slice_buffer.h index 2ed896645b4..cdbd74776c6 100644 --- a/include/grpc/slice_buffer.h +++ b/include/grpc/slice_buffer.h @@ -77,6 +77,10 @@ GPRAPI void grpc_slice_buffer_trim_end(grpc_slice_buffer *src, size_t n, /* move the first n bytes of src into dst */ GPRAPI void grpc_slice_buffer_move_first(grpc_slice_buffer *src, size_t n, grpc_slice_buffer *dst); +/* move the first n bytes of src into dst without adding references */ +GPRAPI void grpc_slice_buffer_move_first_no_ref(grpc_slice_buffer *src, + size_t n, + grpc_slice_buffer *dst); /* move the first n bytes of src into dst (copying them) */ GPRAPI void grpc_slice_buffer_move_first_into_buffer(grpc_exec_ctx *exec_ctx, grpc_slice_buffer *src, diff --git a/src/core/ext/filters/client_channel/lb_policy/grpclb/load_balancer_api.c b/src/core/ext/filters/client_channel/lb_policy/grpclb/load_balancer_api.c index 10af2525313..87549b78f0e 100644 --- a/src/core/ext/filters/client_channel/lb_policy/grpclb/load_balancer_api.c +++ b/src/core/ext/filters/client_channel/lb_policy/grpclb/load_balancer_api.c @@ -98,7 +98,7 @@ grpc_slice grpc_grpclb_request_encode(const grpc_grpclb_request *request) { pb_encode(&sizestream, grpc_lb_v1_LoadBalanceRequest_fields, request); encoded_length = sizestream.bytes_written; - slice = grpc_slice_malloc(encoded_length); + slice = GRPC_SLICE_MALLOC(encoded_length); outputstream = pb_ostream_from_buffer(GRPC_SLICE_START_PTR(slice), encoded_length); GPR_ASSERT(pb_encode(&outputstream, grpc_lb_v1_LoadBalanceRequest_fields, diff --git a/src/core/ext/transport/chttp2/transport/bin_decoder.c b/src/core/ext/transport/chttp2/transport/bin_decoder.c index 8c87de112eb..21bed18c9a7 100644 --- a/src/core/ext/transport/chttp2/transport/bin_decoder.c +++ b/src/core/ext/transport/chttp2/transport/bin_decoder.c @@ -169,7 +169,7 @@ grpc_slice grpc_chttp2_base64_decode(grpc_exec_ctx *exec_ctx, } } } - output = grpc_slice_malloc(output_length); + output = GRPC_SLICE_MALLOC(output_length); ctx.input_cur = GRPC_SLICE_START_PTR(input); ctx.input_end = GRPC_SLICE_END_PTR(input); @@ -193,7 +193,7 @@ grpc_slice grpc_chttp2_base64_decode_with_length(grpc_exec_ctx *exec_ctx, grpc_slice input, size_t output_length) { size_t input_length = GRPC_SLICE_LENGTH(input); - grpc_slice output = grpc_slice_malloc(output_length); + grpc_slice output = GRPC_SLICE_MALLOC(output_length); struct grpc_base64_decode_context ctx; // The length of a base64 string cannot be 4 * n + 1 diff --git a/src/core/ext/transport/chttp2/transport/bin_encoder.c b/src/core/ext/transport/chttp2/transport/bin_encoder.c index e301c073f37..3b8ab1f17aa 100644 --- a/src/core/ext/transport/chttp2/transport/bin_encoder.c +++ b/src/core/ext/transport/chttp2/transport/bin_encoder.c @@ -66,7 +66,7 @@ grpc_slice grpc_chttp2_base64_encode(grpc_slice input) { size_t input_triplets = input_length / 3; size_t tail_case = input_length % 3; size_t output_length = input_triplets * 4 + tail_xtra[tail_case]; - grpc_slice output = grpc_slice_malloc(output_length); + grpc_slice output = GRPC_SLICE_MALLOC(output_length); uint8_t *in = GRPC_SLICE_START_PTR(input); char *out = (char *)GRPC_SLICE_START_PTR(output); size_t i; @@ -119,7 +119,7 @@ grpc_slice grpc_chttp2_huffman_compress(grpc_slice input) { nbits += grpc_chttp2_huffsyms[*in].length; } - output = grpc_slice_malloc(nbits / 8 + (nbits % 8 != 0)); + output = GRPC_SLICE_MALLOC(nbits / 8 + (nbits % 8 != 0)); out = GRPC_SLICE_START_PTR(output); for (in = GRPC_SLICE_START_PTR(input); in != GRPC_SLICE_END_PTR(input); ++in) { @@ -184,7 +184,7 @@ grpc_slice grpc_chttp2_base64_encode_and_huffman_compress(grpc_slice input) { size_t output_syms = input_triplets * 4 + tail_xtra[tail_case]; size_t max_output_bits = 11 * output_syms; size_t max_output_length = max_output_bits / 8 + (max_output_bits % 8 != 0); - grpc_slice output = grpc_slice_malloc(max_output_length); + grpc_slice output = GRPC_SLICE_MALLOC(max_output_length); uint8_t *in = GRPC_SLICE_START_PTR(input); uint8_t *start_out = GRPC_SLICE_START_PTR(output); huff_out out; diff --git a/src/core/ext/transport/chttp2/transport/chttp2_transport.c b/src/core/ext/transport/chttp2/transport/chttp2_transport.c index e2816b0e045..f7d78026b7b 100644 --- a/src/core/ext/transport/chttp2/transport/chttp2_transport.c +++ b/src/core/ext/transport/chttp2/transport/chttp2_transport.c @@ -1906,7 +1906,7 @@ static void close_from_api(grpc_exec_ctx *exec_ctx, grpc_chttp2_transport *t, the time we got around to sending this, so instead we ignore HPACK compression and just write the uncompressed bytes onto the wire. */ if (!s->sent_initial_metadata) { - http_status_hdr = grpc_slice_malloc(13); + http_status_hdr = GRPC_SLICE_MALLOC(13); p = GRPC_SLICE_START_PTR(http_status_hdr); *p++ = 0x00; *p++ = 7; @@ -1925,7 +1925,7 @@ static void close_from_api(grpc_exec_ctx *exec_ctx, grpc_chttp2_transport *t, len += (uint32_t)GRPC_SLICE_LENGTH(http_status_hdr); } - status_hdr = grpc_slice_malloc(15 + (grpc_status >= 10)); + status_hdr = GRPC_SLICE_MALLOC(15 + (grpc_status >= 10)); p = GRPC_SLICE_START_PTR(status_hdr); *p++ = 0x00; /* literal header, not indexed */ *p++ = 11; /* len(grpc-status) */ @@ -1954,7 +1954,7 @@ static void close_from_api(grpc_exec_ctx *exec_ctx, grpc_chttp2_transport *t, size_t msg_len = GRPC_SLICE_LENGTH(slice); GPR_ASSERT(msg_len <= UINT32_MAX); uint32_t msg_len_len = GRPC_CHTTP2_VARINT_LENGTH((uint32_t)msg_len, 1); - message_pfx = grpc_slice_malloc(14 + msg_len_len); + message_pfx = GRPC_SLICE_MALLOC(14 + msg_len_len); p = GRPC_SLICE_START_PTR(message_pfx); *p++ = 0x00; /* literal header, not indexed */ *p++ = 12; /* len(grpc-message) */ @@ -1976,7 +1976,7 @@ static void close_from_api(grpc_exec_ctx *exec_ctx, grpc_chttp2_transport *t, len += (uint32_t)GRPC_SLICE_LENGTH(message_pfx); len += (uint32_t)msg_len; - hdr = grpc_slice_malloc(9); + hdr = GRPC_SLICE_MALLOC(9); p = GRPC_SLICE_START_PTR(hdr); *p++ = (uint8_t)(len >> 16); *p++ = (uint8_t)(len >> 8); diff --git a/src/core/ext/transport/chttp2/transport/frame_data.c b/src/core/ext/transport/chttp2/transport/frame_data.c index 6e9258ee7ee..e9cf5324b0c 100644 --- a/src/core/ext/transport/chttp2/transport/frame_data.c +++ b/src/core/ext/transport/chttp2/transport/frame_data.c @@ -123,7 +123,7 @@ void grpc_chttp2_encode_data(uint32_t id, grpc_slice_buffer *inbuf, uint8_t *p; static const size_t header_size = 9; - hdr = grpc_slice_malloc(header_size); + hdr = GRPC_SLICE_MALLOC(header_size); p = GRPC_SLICE_START_PTR(hdr); GPR_ASSERT(write_bytes < (1 << 24)); *p++ = (uint8_t)(write_bytes >> 16); @@ -137,7 +137,7 @@ void grpc_chttp2_encode_data(uint32_t id, grpc_slice_buffer *inbuf, *p++ = (uint8_t)(id); grpc_slice_buffer_add(outbuf, hdr); - grpc_slice_buffer_move_first(inbuf, write_bytes, outbuf); + grpc_slice_buffer_move_first_no_ref(inbuf, write_bytes, outbuf); stats->framing_bytes += header_size; stats->data_bytes += write_bytes; diff --git a/src/core/ext/transport/chttp2/transport/frame_goaway.c b/src/core/ext/transport/chttp2/transport/frame_goaway.c index 001271dd228..0f1c8b0772f 100644 --- a/src/core/ext/transport/chttp2/transport/frame_goaway.c +++ b/src/core/ext/transport/chttp2/transport/frame_goaway.c @@ -163,7 +163,7 @@ grpc_error *grpc_chttp2_goaway_parser_parse(grpc_exec_ctx *exec_ctx, void grpc_chttp2_goaway_append(uint32_t last_stream_id, uint32_t error_code, grpc_slice debug_data, grpc_slice_buffer *slice_buffer) { - grpc_slice header = grpc_slice_malloc(9 + 4 + 4); + grpc_slice header = GRPC_SLICE_MALLOC(9 + 4 + 4); uint8_t *p = GRPC_SLICE_START_PTR(header); uint32_t frame_length; GPR_ASSERT(GRPC_SLICE_LENGTH(debug_data) < UINT32_MAX - 4 - 4); diff --git a/src/core/ext/transport/chttp2/transport/frame_ping.c b/src/core/ext/transport/chttp2/transport/frame_ping.c index 6016e43127c..f09ca607397 100644 --- a/src/core/ext/transport/chttp2/transport/frame_ping.c +++ b/src/core/ext/transport/chttp2/transport/frame_ping.c @@ -43,7 +43,7 @@ static bool g_disable_ping_ack = false; grpc_slice grpc_chttp2_ping_create(uint8_t ack, uint64_t opaque_8bytes) { - grpc_slice slice = grpc_slice_malloc(9 + 8); + grpc_slice slice = GRPC_SLICE_MALLOC(9 + 8); uint8_t *p = GRPC_SLICE_START_PTR(slice); *p++ = 0; diff --git a/src/core/ext/transport/chttp2/transport/frame_rst_stream.c b/src/core/ext/transport/chttp2/transport/frame_rst_stream.c index 225f15c77c9..e0caa50e92e 100644 --- a/src/core/ext/transport/chttp2/transport/frame_rst_stream.c +++ b/src/core/ext/transport/chttp2/transport/frame_rst_stream.c @@ -44,7 +44,7 @@ grpc_slice grpc_chttp2_rst_stream_create(uint32_t id, uint32_t code, grpc_transport_one_way_stats *stats) { static const size_t frame_size = 13; - grpc_slice slice = grpc_slice_malloc(frame_size); + grpc_slice slice = GRPC_SLICE_MALLOC(frame_size); stats->framing_bytes += frame_size; uint8_t *p = GRPC_SLICE_START_PTR(slice); diff --git a/src/core/ext/transport/chttp2/transport/frame_settings.c b/src/core/ext/transport/chttp2/transport/frame_settings.c index 16881c0707a..2a2986301d2 100644 --- a/src/core/ext/transport/chttp2/transport/frame_settings.c +++ b/src/core/ext/transport/chttp2/transport/frame_settings.c @@ -93,7 +93,7 @@ grpc_slice grpc_chttp2_settings_create(uint32_t *old, const uint32_t *new, n += (new[i] != old[i] || (force_mask & (1u << i)) != 0); } - output = grpc_slice_malloc(9 + 6 * n); + output = GRPC_SLICE_MALLOC(9 + 6 * n); p = fill_header(GRPC_SLICE_START_PTR(output), 6 * n, 0); for (i = 0; i < count; i++) { @@ -115,7 +115,7 @@ grpc_slice grpc_chttp2_settings_create(uint32_t *old, const uint32_t *new, } grpc_slice grpc_chttp2_settings_ack_create(void) { - grpc_slice output = grpc_slice_malloc(9); + grpc_slice output = GRPC_SLICE_MALLOC(9); fill_header(GRPC_SLICE_START_PTR(output), 0, GRPC_CHTTP2_FLAG_ACK); return output; } diff --git a/src/core/ext/transport/chttp2/transport/frame_window_update.c b/src/core/ext/transport/chttp2/transport/frame_window_update.c index b76b6f6f477..8ed72dddca0 100644 --- a/src/core/ext/transport/chttp2/transport/frame_window_update.c +++ b/src/core/ext/transport/chttp2/transport/frame_window_update.c @@ -41,7 +41,7 @@ grpc_slice grpc_chttp2_window_update_create( uint32_t id, uint32_t window_update, grpc_transport_one_way_stats *stats) { static const size_t frame_size = 13; - grpc_slice slice = grpc_slice_malloc(frame_size); + grpc_slice slice = GRPC_SLICE_MALLOC(frame_size); stats->header_bytes += frame_size; uint8_t *p = GRPC_SLICE_START_PTR(slice); diff --git a/src/core/ext/transport/chttp2/transport/hpack_encoder.c b/src/core/ext/transport/chttp2/transport/hpack_encoder.c index 84586cd9988..b9b0f5cab0e 100644 --- a/src/core/ext/transport/chttp2/transport/hpack_encoder.c +++ b/src/core/ext/transport/chttp2/transport/hpack_encoder.c @@ -122,7 +122,7 @@ static void finish_frame(framer_state *st, int is_header_boundary, output before beginning */ static void begin_frame(framer_state *st) { st->header_idx = - grpc_slice_buffer_add_indexed(st->output, grpc_slice_malloc(9)); + grpc_slice_buffer_add_indexed(st->output, GRPC_SLICE_MALLOC(9)); st->output_length_at_start_of_frame = st->output->length; } diff --git a/src/core/ext/transport/cronet/transport/cronet_transport.c b/src/core/ext/transport/cronet/transport/cronet_transport.c index 0b9189558f8..c0b09e752ca 100644 --- a/src/core/ext/transport/cronet/transport/cronet_transport.c +++ b/src/core/ext/transport/cronet/transport/cronet_transport.c @@ -1165,7 +1165,7 @@ static enum e_op_result execute_stream_op(grpc_exec_ctx *exec_ctx, } else if (stream_state->rs.remaining_bytes == 0) { CRONET_LOG(GPR_DEBUG, "read operation complete"); grpc_slice read_data_slice = - grpc_slice_malloc((uint32_t)stream_state->rs.length_field); + GRPC_SLICE_MALLOC((uint32_t)stream_state->rs.length_field); uint8_t *dst_p = GRPC_SLICE_START_PTR(read_data_slice); memcpy(dst_p, stream_state->rs.read_buffer, (size_t)stream_state->rs.length_field); diff --git a/src/core/lib/channel/http_client_filter.c b/src/core/lib/channel/http_client_filter.c index 4e47c5c658f..570ec8608ba 100644 --- a/src/core/lib/channel/http_client_filter.c +++ b/src/core/lib/channel/http_client_filter.c @@ -312,7 +312,7 @@ static grpc_error *hc_mutate_op(grpc_exec_ctx *exec_ctx, op->payload->send_message.send_message->length, k_url_safe, k_multi_line); estimated_len += 1; /* for the trailing 0 */ - grpc_slice path_with_query_slice = grpc_slice_malloc(estimated_len); + grpc_slice path_with_query_slice = GRPC_SLICE_MALLOC(estimated_len); /* memcopy individual pieces into this slice */ uint8_t *write_ptr = diff --git a/src/core/lib/compression/message_compress.c b/src/core/lib/compression/message_compress.c index 49beb953b00..fd3d1e6fcc0 100644 --- a/src/core/lib/compression/message_compress.c +++ b/src/core/lib/compression/message_compress.c @@ -50,7 +50,7 @@ static int zlib_body(grpc_exec_ctx* exec_ctx, z_stream* zs, int r; int flush; size_t i; - grpc_slice outbuf = grpc_slice_malloc(OUTPUT_BLOCK_SIZE); + grpc_slice outbuf = GRPC_SLICE_MALLOC(OUTPUT_BLOCK_SIZE); const uInt uint_max = ~(uInt)0; GPR_ASSERT(GRPC_SLICE_LENGTH(outbuf) <= uint_max); @@ -65,7 +65,7 @@ static int zlib_body(grpc_exec_ctx* exec_ctx, z_stream* zs, do { if (zs->avail_out == 0) { grpc_slice_buffer_add_indexed(output, outbuf); - outbuf = grpc_slice_malloc(OUTPUT_BLOCK_SIZE); + outbuf = GRPC_SLICE_MALLOC(OUTPUT_BLOCK_SIZE); GPR_ASSERT(GRPC_SLICE_LENGTH(outbuf) <= uint_max); zs->avail_out = (uInt)GRPC_SLICE_LENGTH(outbuf); zs->next_out = GRPC_SLICE_START_PTR(outbuf); diff --git a/src/core/lib/iomgr/tcp_windows.c b/src/core/lib/iomgr/tcp_windows.c index f74aa687936..bdd4dd07afc 100644 --- a/src/core/lib/iomgr/tcp_windows.c +++ b/src/core/lib/iomgr/tcp_windows.c @@ -219,7 +219,7 @@ static void win_read(grpc_exec_ctx *exec_ctx, grpc_endpoint *ep, tcp->read_slices = read_slices; grpc_slice_buffer_reset_and_unref_internal(exec_ctx, read_slices); - tcp->read_slice = grpc_slice_malloc(8192); + tcp->read_slice = GRPC_SLICE_MALLOC(8192); buffer.len = (ULONG)GRPC_SLICE_LENGTH( tcp->read_slice); // we know slice size fits in 32bit. diff --git a/src/core/lib/security/transport/secure_endpoint.c b/src/core/lib/security/transport/secure_endpoint.c index 24da949e486..0d5c7432c64 100644 --- a/src/core/lib/security/transport/secure_endpoint.c +++ b/src/core/lib/security/transport/secure_endpoint.c @@ -130,7 +130,7 @@ static void secure_endpoint_ref(secure_endpoint *ep) { gpr_ref(&ep->ref); } static void flush_read_staging_buffer(secure_endpoint *ep, uint8_t **cur, uint8_t **end) { grpc_slice_buffer_add(ep->read_buffer, ep->read_staging_buffer); - ep->read_staging_buffer = grpc_slice_malloc(STAGING_BUFFER_SIZE); + ep->read_staging_buffer = GRPC_SLICE_MALLOC(STAGING_BUFFER_SIZE); *cur = GRPC_SLICE_START_PTR(ep->read_staging_buffer); *end = GRPC_SLICE_END_PTR(ep->read_staging_buffer); } @@ -252,7 +252,7 @@ static void endpoint_read(grpc_exec_ctx *exec_ctx, grpc_endpoint *secure_ep, static void flush_write_staging_buffer(secure_endpoint *ep, uint8_t **cur, uint8_t **end) { grpc_slice_buffer_add(&ep->output_buffer, ep->write_staging_buffer); - ep->write_staging_buffer = grpc_slice_malloc(STAGING_BUFFER_SIZE); + ep->write_staging_buffer = GRPC_SLICE_MALLOC(STAGING_BUFFER_SIZE); *cur = GRPC_SLICE_START_PTR(ep->write_staging_buffer); *end = GRPC_SLICE_END_PTR(ep->write_staging_buffer); } @@ -415,8 +415,8 @@ grpc_endpoint *grpc_secure_endpoint_create( grpc_slice_buffer_add(&ep->leftover_bytes, grpc_slice_ref_internal(leftover_slices[i])); } - ep->write_staging_buffer = grpc_slice_malloc(STAGING_BUFFER_SIZE); - ep->read_staging_buffer = grpc_slice_malloc(STAGING_BUFFER_SIZE); + ep->write_staging_buffer = GRPC_SLICE_MALLOC(STAGING_BUFFER_SIZE); + ep->read_staging_buffer = GRPC_SLICE_MALLOC(STAGING_BUFFER_SIZE); grpc_slice_buffer_init(&ep->output_buffer); grpc_slice_buffer_init(&ep->source_buffer); ep->read_buffer = NULL; diff --git a/src/core/lib/slice/b64.c b/src/core/lib/slice/b64.c index 2007cc48109..d9091646e06 100644 --- a/src/core/lib/slice/b64.c +++ b/src/core/lib/slice/b64.c @@ -202,7 +202,7 @@ static int decode_group(const unsigned char *codes, size_t num_codes, grpc_slice grpc_base64_decode_with_len(grpc_exec_ctx *exec_ctx, const char *b64, size_t b64_len, int url_safe) { - grpc_slice result = grpc_slice_malloc(b64_len); + grpc_slice result = GRPC_SLICE_MALLOC(b64_len); unsigned char *current = GRPC_SLICE_START_PTR(result); size_t result_size = 0; unsigned char codes[4]; diff --git a/src/core/lib/slice/percent_encoding.c b/src/core/lib/slice/percent_encoding.c index c76c58d371e..a77c69763f1 100644 --- a/src/core/lib/slice/percent_encoding.c +++ b/src/core/lib/slice/percent_encoding.c @@ -71,7 +71,7 @@ grpc_slice grpc_percent_encode_slice(grpc_slice slice, return grpc_slice_ref_internal(slice); } // second pass: actually encode - grpc_slice out = grpc_slice_malloc(output_length); + grpc_slice out = GRPC_SLICE_MALLOC(output_length); uint8_t *q = GRPC_SLICE_START_PTR(out); for (p = slice_start; p < slice_end; p++) { if (is_unreserved_character(*p, unreserved_bytes)) { @@ -125,7 +125,7 @@ bool grpc_strict_percent_decode_slice(grpc_slice slice_in, return true; } p = GRPC_SLICE_START_PTR(slice_in); - *slice_out = grpc_slice_malloc(out_length); + *slice_out = GRPC_SLICE_MALLOC(out_length); uint8_t *q = GRPC_SLICE_START_PTR(*slice_out); while (p != in_end) { if (*p == '%') { @@ -163,7 +163,7 @@ grpc_slice grpc_permissive_percent_decode_slice(grpc_slice slice_in) { return grpc_slice_ref_internal(slice_in); } p = GRPC_SLICE_START_PTR(slice_in); - grpc_slice out = grpc_slice_malloc(out_length); + grpc_slice out = GRPC_SLICE_MALLOC(out_length); uint8_t *q = GRPC_SLICE_START_PTR(out); while (p != in_end) { if (*p == '%') { diff --git a/src/core/lib/slice/slice.c b/src/core/lib/slice/slice.c index 1cddf062cd4..8896f1dfd3f 100644 --- a/src/core/lib/slice/slice.c +++ b/src/core/lib/slice/slice.c @@ -197,7 +197,7 @@ grpc_slice grpc_slice_new_with_len(void *p, size_t len, } grpc_slice grpc_slice_from_copied_buffer(const char *source, size_t length) { - grpc_slice slice = grpc_slice_malloc(length); + grpc_slice slice = GRPC_SLICE_MALLOC(length); memcpy(GRPC_SLICE_START_PTR(slice), source, length); return slice; } @@ -227,35 +227,42 @@ static const grpc_slice_refcount_vtable malloc_vtable = { malloc_ref, malloc_unref, grpc_slice_default_eq_impl, grpc_slice_default_hash_impl}; +grpc_slice grpc_slice_malloc_large(size_t length) { + grpc_slice slice; + + /* Memory layout used by the slice created here: + + +-----------+----------------------------------------------------------+ + | refcount | bytes | + +-----------+----------------------------------------------------------+ + + refcount is a malloc_refcount + bytes is an array of bytes of the requested length + Both parts are placed in the same allocation returned from gpr_malloc */ + malloc_refcount *rc = gpr_malloc(sizeof(malloc_refcount) + length); + + /* Initial refcount on rc is 1 - and it's up to the caller to release + this reference. */ + gpr_ref_init(&rc->refs, 1); + + rc->base.vtable = &malloc_vtable; + rc->base.sub_refcount = &rc->base; + + /* Build up the slice to be returned. */ + /* The slices refcount points back to the allocated block. */ + slice.refcount = &rc->base; + /* The data bytes are placed immediately after the refcount struct */ + slice.data.refcounted.bytes = (uint8_t *)(rc + 1); + /* And the length of the block is set to the requested length */ + slice.data.refcounted.length = length; + return slice; +} + grpc_slice grpc_slice_malloc(size_t length) { grpc_slice slice; if (length > sizeof(slice.data.inlined.bytes)) { - /* Memory layout used by the slice created here: - - +-----------+----------------------------------------------------------+ - | refcount | bytes | - +-----------+----------------------------------------------------------+ - - refcount is a malloc_refcount - bytes is an array of bytes of the requested length - Both parts are placed in the same allocation returned from gpr_malloc */ - malloc_refcount *rc = gpr_malloc(sizeof(malloc_refcount) + length); - - /* Initial refcount on rc is 1 - and it's up to the caller to release - this reference. */ - gpr_ref_init(&rc->refs, 1); - - rc->base.vtable = &malloc_vtable; - rc->base.sub_refcount = &rc->base; - - /* Build up the slice to be returned. */ - /* The slices refcount points back to the allocated block. */ - slice.refcount = &rc->base; - /* The data bytes are placed immediately after the refcount struct */ - slice.data.refcounted.bytes = (uint8_t *)(rc + 1); - /* And the length of the block is set to the requested length */ - slice.data.refcounted.length = length; + return grpc_slice_malloc_large(length); } else { /* small slice: just inline the data */ slice.refcount = NULL; @@ -341,6 +348,40 @@ grpc_slice grpc_slice_split_tail(grpc_slice *source, size_t split) { return tail; } +grpc_slice grpc_slice_split_tail_no_ref(grpc_slice *source, size_t split) { + grpc_slice tail; + + if (source->refcount == NULL) { + /* inlined data, copy it out */ + GPR_ASSERT(source->data.inlined.length >= split); + tail.refcount = NULL; + tail.data.inlined.length = (uint8_t)(source->data.inlined.length - split); + memcpy(tail.data.inlined.bytes, source->data.inlined.bytes + split, + tail.data.inlined.length); + source->data.inlined.length = (uint8_t)split; + } else { + size_t tail_length = source->data.refcounted.length - split; + GPR_ASSERT(source->data.refcounted.length >= split); + if (tail_length < sizeof(tail.data.inlined.bytes)) { + /* Copy out the bytes - it'll be cheaper than refcounting */ + tail.refcount = NULL; + tail.data.inlined.length = (uint8_t)tail_length; + memcpy(tail.data.inlined.bytes, source->data.refcounted.bytes + split, + tail_length); + } else { + /* Build the result */ + tail.refcount = &noop_refcount; + /* Point into the source array */ + tail.data.refcounted.bytes = source->data.refcounted.bytes + split; + tail.data.refcounted.length = tail_length; + } + source->refcount = source->refcount->sub_refcount; + source->data.refcounted.length = split; + } + + return tail; +} + grpc_slice grpc_slice_split_head(grpc_slice *source, size_t split) { grpc_slice head; @@ -457,7 +498,7 @@ int grpc_slice_slice(grpc_slice haystack, grpc_slice needle) { } grpc_slice grpc_slice_dup(grpc_slice a) { - grpc_slice copy = grpc_slice_malloc(GRPC_SLICE_LENGTH(a)); + grpc_slice copy = GRPC_SLICE_MALLOC(GRPC_SLICE_LENGTH(a)); memcpy(GRPC_SLICE_START_PTR(copy), GRPC_SLICE_START_PTR(a), GRPC_SLICE_LENGTH(a)); return copy; diff --git a/src/core/lib/slice/slice_buffer.c b/src/core/lib/slice/slice_buffer.c index c96b9c3b281..a13941e5ac8 100644 --- a/src/core/lib/slice/slice_buffer.c +++ b/src/core/lib/slice/slice_buffer.c @@ -255,14 +255,47 @@ void grpc_slice_buffer_move_into(grpc_slice_buffer *src, void grpc_slice_buffer_move_first(grpc_slice_buffer *src, size_t n, grpc_slice_buffer *dst) { + GPR_ASSERT(src->length >= n); + if (src->length == n) { + grpc_slice_buffer_move_into(src, dst); + return; + } + size_t output_len = dst->length + n; size_t new_input_len = src->length - n; + + while (src->count > 0) { + grpc_slice slice = grpc_slice_buffer_take_first(src); + size_t slice_len = GRPC_SLICE_LENGTH(slice); + if (n > slice_len) { + grpc_slice_buffer_add(dst, slice); + n -= slice_len; + } else if (n == slice_len) { + grpc_slice_buffer_add(dst, slice); + break; + } else { /* n < slice_len */ + grpc_slice_buffer_undo_take_first(src, grpc_slice_split_tail(&slice, n)); + GPR_ASSERT(GRPC_SLICE_LENGTH(slice) == n); + grpc_slice_buffer_add(dst, slice); + break; + } + } + GPR_ASSERT(dst->length == output_len); + GPR_ASSERT(src->length == new_input_len); + GPR_ASSERT(src->count > 0); +} + +void grpc_slice_buffer_move_first_no_ref(grpc_slice_buffer *src, size_t n, + grpc_slice_buffer *dst) { GPR_ASSERT(src->length >= n); if (src->length == n) { grpc_slice_buffer_move_into(src, dst); return; } + size_t output_len = dst->length + n; + size_t new_input_len = src->length - n; + while (src->count > 0) { grpc_slice slice = grpc_slice_buffer_take_first(src); size_t slice_len = GRPC_SLICE_LENGTH(slice); @@ -273,7 +306,8 @@ void grpc_slice_buffer_move_first(grpc_slice_buffer *src, size_t n, grpc_slice_buffer_add(dst, slice); break; } else { /* n < slice_len */ - grpc_slice_buffer_undo_take_first(src, grpc_slice_split_tail(&slice, n)); + grpc_slice_buffer_undo_take_first( + src, grpc_slice_split_tail_no_ref(&slice, n)); GPR_ASSERT(GRPC_SLICE_LENGTH(slice) == n); grpc_slice_buffer_add(dst, slice); break; diff --git a/src/core/lib/surface/byte_buffer_reader.c b/src/core/lib/surface/byte_buffer_reader.c index 1a6ccdaddb6..539b0142783 100644 --- a/src/core/lib/surface/byte_buffer_reader.c +++ b/src/core/lib/surface/byte_buffer_reader.c @@ -124,7 +124,7 @@ grpc_slice grpc_byte_buffer_reader_readall(grpc_byte_buffer_reader *reader) { grpc_slice in_slice; size_t bytes_read = 0; const size_t input_size = grpc_byte_buffer_length(reader->buffer_out); - grpc_slice out_slice = grpc_slice_malloc(input_size); + grpc_slice out_slice = GRPC_SLICE_MALLOC(input_size); uint8_t *const outbuf = GRPC_SLICE_START_PTR(out_slice); /* just an alias */ grpc_exec_ctx exec_ctx = GRPC_EXEC_CTX_INIT; From 255ea13aa381f905b88a0816a0a29ee0315d251f Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Wed, 12 Apr 2017 17:18:56 -0700 Subject: [PATCH 2/9] Reduce duplication --- include/grpc/slice.h | 7 +++-- src/core/lib/slice/slice.c | 47 ++++++++----------------------- src/core/lib/slice/slice_buffer.c | 43 ++++++++-------------------- 3 files changed, 27 insertions(+), 70 deletions(-) diff --git a/include/grpc/slice.h b/include/grpc/slice.h index 4dce6da9155..fe6379c867d 100644 --- a/include/grpc/slice.h +++ b/include/grpc/slice.h @@ -124,8 +124,11 @@ GPRAPI grpc_slice grpc_slice_sub_no_ref(grpc_slice s, size_t begin, size_t end); Requires s intialized, split <= s.length */ GPRAPI grpc_slice grpc_slice_split_tail(grpc_slice *s, size_t split); -/* The same as grpc_slice_split_tail, but without altering refcounts */ -GPRAPI grpc_slice grpc_slice_split_tail_no_ref(grpc_slice *s, size_t split); +/* The same as grpc_slice_split_tail, but with an option to skip altering + * refcounts (grpc_slice_split_tail_maybe_ref(..., true) is equivalent to + * grpc_slice_split_tail(...)) */ +GPRAPI grpc_slice grpc_slice_split_tail_maybe_ref(grpc_slice *s, size_t split, + bool inc_refs); /* Splits s into two: modifies s to be s[split:s.length], and returns a new slice, sharing a refcount with s, that contains s[0:split]. diff --git a/src/core/lib/slice/slice.c b/src/core/lib/slice/slice.c index 8896f1dfd3f..285218837ac 100644 --- a/src/core/lib/slice/slice.c +++ b/src/core/lib/slice/slice.c @@ -312,7 +312,8 @@ grpc_slice grpc_slice_sub(grpc_slice source, size_t begin, size_t end) { return subset; } -grpc_slice grpc_slice_split_tail(grpc_slice *source, size_t split) { +grpc_slice grpc_slice_split_tail_maybe_ref(grpc_slice *source, size_t split, + bool incref) { grpc_slice tail; if (source->refcount == NULL) { @@ -334,9 +335,13 @@ grpc_slice grpc_slice_split_tail(grpc_slice *source, size_t split) { tail_length); } else { /* Build the result */ - tail.refcount = source->refcount->sub_refcount; - /* Bump the refcount */ - tail.refcount->vtable->ref(tail.refcount); + if (incref) { + tail.refcount = source->refcount->sub_refcount; + /* Bump the refcount */ + tail.refcount->vtable->ref(tail.refcount); + } else { + tail.refcount = &noop_refcount; + } /* Point into the source array */ tail.data.refcounted.bytes = source->data.refcounted.bytes + split; tail.data.refcounted.length = tail_length; @@ -348,38 +353,8 @@ grpc_slice grpc_slice_split_tail(grpc_slice *source, size_t split) { return tail; } -grpc_slice grpc_slice_split_tail_no_ref(grpc_slice *source, size_t split) { - grpc_slice tail; - - if (source->refcount == NULL) { - /* inlined data, copy it out */ - GPR_ASSERT(source->data.inlined.length >= split); - tail.refcount = NULL; - tail.data.inlined.length = (uint8_t)(source->data.inlined.length - split); - memcpy(tail.data.inlined.bytes, source->data.inlined.bytes + split, - tail.data.inlined.length); - source->data.inlined.length = (uint8_t)split; - } else { - size_t tail_length = source->data.refcounted.length - split; - GPR_ASSERT(source->data.refcounted.length >= split); - if (tail_length < sizeof(tail.data.inlined.bytes)) { - /* Copy out the bytes - it'll be cheaper than refcounting */ - tail.refcount = NULL; - tail.data.inlined.length = (uint8_t)tail_length; - memcpy(tail.data.inlined.bytes, source->data.refcounted.bytes + split, - tail_length); - } else { - /* Build the result */ - tail.refcount = &noop_refcount; - /* Point into the source array */ - tail.data.refcounted.bytes = source->data.refcounted.bytes + split; - tail.data.refcounted.length = tail_length; - } - source->refcount = source->refcount->sub_refcount; - source->data.refcounted.length = split; - } - - return tail; +grpc_slice grpc_slice_split_tail(grpc_slice *source, size_t split) { + return grpc_slice_split_tail_maybe_ref(source, split, true); } grpc_slice grpc_slice_split_head(grpc_slice *source, size_t split) { diff --git a/src/core/lib/slice/slice_buffer.c b/src/core/lib/slice/slice_buffer.c index a13941e5ac8..5a4b434d28c 100644 --- a/src/core/lib/slice/slice_buffer.c +++ b/src/core/lib/slice/slice_buffer.c @@ -253,8 +253,9 @@ void grpc_slice_buffer_move_into(grpc_slice_buffer *src, src->length = 0; } -void grpc_slice_buffer_move_first(grpc_slice_buffer *src, size_t n, - grpc_slice_buffer *dst) { +static void slice_buffer_move_first_maybe_ref(grpc_slice_buffer *src, size_t n, + grpc_slice_buffer *dst, + bool incref) { GPR_ASSERT(src->length >= n); if (src->length == n) { grpc_slice_buffer_move_into(src, dst); @@ -274,7 +275,8 @@ void grpc_slice_buffer_move_first(grpc_slice_buffer *src, size_t n, grpc_slice_buffer_add(dst, slice); break; } else { /* n < slice_len */ - grpc_slice_buffer_undo_take_first(src, grpc_slice_split_tail(&slice, n)); + grpc_slice_buffer_undo_take_first( + src, grpc_slice_split_tail_maybe_ref(&slice, n, incref)); GPR_ASSERT(GRPC_SLICE_LENGTH(slice) == n); grpc_slice_buffer_add(dst, slice); break; @@ -285,37 +287,14 @@ void grpc_slice_buffer_move_first(grpc_slice_buffer *src, size_t n, GPR_ASSERT(src->count > 0); } +void grpc_slice_buffer_move_first(grpc_slice_buffer *src, size_t n, + grpc_slice_buffer *dst) { + slice_buffer_move_first_maybe_ref(src, n, dst, true); +} + void grpc_slice_buffer_move_first_no_ref(grpc_slice_buffer *src, size_t n, grpc_slice_buffer *dst) { - GPR_ASSERT(src->length >= n); - if (src->length == n) { - grpc_slice_buffer_move_into(src, dst); - return; - } - - size_t output_len = dst->length + n; - size_t new_input_len = src->length - n; - - while (src->count > 0) { - grpc_slice slice = grpc_slice_buffer_take_first(src); - size_t slice_len = GRPC_SLICE_LENGTH(slice); - if (n > slice_len) { - grpc_slice_buffer_add(dst, slice); - n -= slice_len; - } else if (n == slice_len) { - grpc_slice_buffer_add(dst, slice); - break; - } else { /* n < slice_len */ - grpc_slice_buffer_undo_take_first( - src, grpc_slice_split_tail_no_ref(&slice, n)); - GPR_ASSERT(GRPC_SLICE_LENGTH(slice) == n); - grpc_slice_buffer_add(dst, slice); - break; - } - } - GPR_ASSERT(dst->length == output_len); - GPR_ASSERT(src->length == new_input_len); - GPR_ASSERT(src->count > 0); + slice_buffer_move_first_maybe_ref(src, n, dst, false); } void grpc_slice_buffer_move_first_into_buffer(grpc_exec_ctx *exec_ctx, From a3583b22eaf5448daff914a786d1e5214fe9aaec Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Thu, 13 Apr 2017 06:42:47 -0700 Subject: [PATCH 3/9] Fix compilation, add a check for no slow usages of grpc_slice_malloc --- include/grpc/slice.h | 2 +- src/core/lib/slice/slice.c | 2 +- tools/run_tests/sanity/core_banned_functions.py | 1 + 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/include/grpc/slice.h b/include/grpc/slice.h index fe6379c867d..86a455b42c2 100644 --- a/include/grpc/slice.h +++ b/include/grpc/slice.h @@ -128,7 +128,7 @@ GPRAPI grpc_slice grpc_slice_split_tail(grpc_slice *s, size_t split); * refcounts (grpc_slice_split_tail_maybe_ref(..., true) is equivalent to * grpc_slice_split_tail(...)) */ GPRAPI grpc_slice grpc_slice_split_tail_maybe_ref(grpc_slice *s, size_t split, - bool inc_refs); + int inc_refs); /* Splits s into two: modifies s to be s[split:s.length], and returns a new slice, sharing a refcount with s, that contains s[0:split]. diff --git a/src/core/lib/slice/slice.c b/src/core/lib/slice/slice.c index 285218837ac..e3d030235c8 100644 --- a/src/core/lib/slice/slice.c +++ b/src/core/lib/slice/slice.c @@ -313,7 +313,7 @@ grpc_slice grpc_slice_sub(grpc_slice source, size_t begin, size_t end) { } grpc_slice grpc_slice_split_tail_maybe_ref(grpc_slice *source, size_t split, - bool incref) { + int incref) { grpc_slice tail; if (source->refcount == NULL) { diff --git a/tools/run_tests/sanity/core_banned_functions.py b/tools/run_tests/sanity/core_banned_functions.py index c3c3cbec767..2387c5f1da0 100755 --- a/tools/run_tests/sanity/core_banned_functions.py +++ b/tools/run_tests/sanity/core_banned_functions.py @@ -50,6 +50,7 @@ BANNED_EXCEPT = { 'grpc_os_error(': ['src/core/lib/iomgr/error.c'], 'grpc_wsa_error(': ['src/core/lib/iomgr/error.c'], 'grpc_log_if_error(': ['src/core/lib/iomgr/error.c'], + 'grpc_slice_malloc(': ['src/core/lib/slice/slice.c'], } errors = 0 From 0d23d8954f2ee9b809d21ec4e156b2347c8c340c Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Thu, 20 Apr 2017 14:46:37 -0700 Subject: [PATCH 4/9] Add flexibility on *which* slice gets reffed Use it to ensure that sb_move_first acts predictably --- include/grpc/slice.h | 8 +++++++- src/core/lib/slice/slice.c | 30 ++++++++++++++++++++---------- src/core/lib/slice/slice_buffer.c | 10 ++++++++-- 3 files changed, 35 insertions(+), 13 deletions(-) diff --git a/include/grpc/slice.h b/include/grpc/slice.h index d65bde49ee6..039347f17e3 100644 --- a/include/grpc/slice.h +++ b/include/grpc/slice.h @@ -124,11 +124,17 @@ GPRAPI grpc_slice grpc_slice_sub_no_ref(grpc_slice s, size_t begin, size_t end); Requires s intialized, split <= s.length */ GPRAPI grpc_slice grpc_slice_split_tail(grpc_slice *s, size_t split); +typedef enum { + GRPC_SLICE_REF_TAIL = 1, + GRPC_SLICE_REF_HEAD = 2, + GRPC_SLICE_REF_BOTH = 1 + 2 +} grpc_slice_ref_whom; + /* The same as grpc_slice_split_tail, but with an option to skip altering * refcounts (grpc_slice_split_tail_maybe_ref(..., true) is equivalent to * grpc_slice_split_tail(...)) */ GPRAPI grpc_slice grpc_slice_split_tail_maybe_ref(grpc_slice *s, size_t split, - int inc_refs); + grpc_slice_ref_whom ref_whom); /* Splits s into two: modifies s to be s[split:s.length], and returns a new slice, sharing a refcount with s, that contains s[0:split]. diff --git a/src/core/lib/slice/slice.c b/src/core/lib/slice/slice.c index 219d1a4efcf..9e69b443fe1 100644 --- a/src/core/lib/slice/slice.c +++ b/src/core/lib/slice/slice.c @@ -314,7 +314,7 @@ grpc_slice grpc_slice_sub(grpc_slice source, size_t begin, size_t end) { } grpc_slice grpc_slice_split_tail_maybe_ref(grpc_slice *source, size_t split, - int incref) { + grpc_slice_ref_whom ref_whom) { grpc_slice tail; if (source->refcount == NULL) { @@ -328,26 +328,36 @@ grpc_slice grpc_slice_split_tail_maybe_ref(grpc_slice *source, size_t split, } else { size_t tail_length = source->data.refcounted.length - split; GPR_ASSERT(source->data.refcounted.length >= split); - if (tail_length < sizeof(tail.data.inlined.bytes)) { + if (tail_length < sizeof(tail.data.inlined.bytes) && + ref_whom != GRPC_SLICE_REF_TAIL) { /* Copy out the bytes - it'll be cheaper than refcounting */ tail.refcount = NULL; tail.data.inlined.length = (uint8_t)tail_length; memcpy(tail.data.inlined.bytes, source->data.refcounted.bytes + split, tail_length); + source->refcount = source->refcount->sub_refcount; } else { /* Build the result */ - if (incref) { - tail.refcount = source->refcount->sub_refcount; - /* Bump the refcount */ - tail.refcount->vtable->ref(tail.refcount); - } else { - tail.refcount = &noop_refcount; + switch (ref_whom) { + case GRPC_SLICE_REF_TAIL: + tail.refcount = source->refcount->sub_refcount; + source->refcount = &noop_refcount; + break; + case GRPC_SLICE_REF_HEAD: + tail.refcount = &noop_refcount; + source->refcount = source->refcount->sub_refcount; + break; + case GRPC_SLICE_REF_BOTH: + tail.refcount = source->refcount->sub_refcount; + source->refcount = source->refcount->sub_refcount; + /* Bump the refcount */ + tail.refcount->vtable->ref(tail.refcount); + break; } /* Point into the source array */ tail.data.refcounted.bytes = source->data.refcounted.bytes + split; tail.data.refcounted.length = tail_length; } - source->refcount = source->refcount->sub_refcount; source->data.refcounted.length = split; } @@ -355,7 +365,7 @@ grpc_slice grpc_slice_split_tail_maybe_ref(grpc_slice *source, size_t split, } grpc_slice grpc_slice_split_tail(grpc_slice *source, size_t split) { - return grpc_slice_split_tail_maybe_ref(source, split, true); + return grpc_slice_split_tail_maybe_ref(source, split, GRPC_SLICE_REF_BOTH); } grpc_slice grpc_slice_split_head(grpc_slice *source, size_t split) { diff --git a/src/core/lib/slice/slice_buffer.c b/src/core/lib/slice/slice_buffer.c index 5a4b434d28c..e8d41ca0f7d 100644 --- a/src/core/lib/slice/slice_buffer.c +++ b/src/core/lib/slice/slice_buffer.c @@ -274,12 +274,18 @@ static void slice_buffer_move_first_maybe_ref(grpc_slice_buffer *src, size_t n, } else if (n == slice_len) { grpc_slice_buffer_add(dst, slice); break; - } else { /* n < slice_len */ + } else if (incref) { /* n < slice_len */ grpc_slice_buffer_undo_take_first( - src, grpc_slice_split_tail_maybe_ref(&slice, n, incref)); + src, grpc_slice_split_tail_maybe_ref(&slice, n, GRPC_SLICE_REF_BOTH)); GPR_ASSERT(GRPC_SLICE_LENGTH(slice) == n); grpc_slice_buffer_add(dst, slice); break; + } else { /* n < slice_len */ + grpc_slice_buffer_undo_take_first( + src, grpc_slice_split_tail_maybe_ref(&slice, n, GRPC_SLICE_REF_TAIL)); + GPR_ASSERT(GRPC_SLICE_LENGTH(slice) == n); + grpc_slice_buffer_add_indexed(dst, slice); + break; } } GPR_ASSERT(dst->length == output_len); From 85bf34a4a8d0b8299ddda1b1abca401d8f2dd4a6 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Fri, 21 Apr 2017 16:14:59 -0700 Subject: [PATCH 5/9] Copy slices for in-process transports: its no longer safe to just ref --- include/grpc/slice.h | 3 +++ src/core/lib/slice/slice.c | 7 +++++++ test/core/util/passthru_endpoint.c | 4 ++-- test/core/util/trickle_endpoint.c | 7 +++---- 4 files changed, 15 insertions(+), 6 deletions(-) diff --git a/include/grpc/slice.h b/include/grpc/slice.h index 039347f17e3..9c4b158ae8d 100644 --- a/include/grpc/slice.h +++ b/include/grpc/slice.h @@ -53,6 +53,9 @@ GPRAPI grpc_slice grpc_slice_ref(grpc_slice s); where dest!=NULL , then (*dest)(start, len). Requires s initialized. */ GPRAPI void grpc_slice_unref(grpc_slice s); +/* Copy slice - create a new slice that contains the same data as s */ +GPRAPI grpc_slice grpc_slice_copy(grpc_slice s); + /* Create a slice pointing at some data. Calls malloc to allocate a refcount for the object, and arranges that destroy will be called with the pointer passed in at destruction. */ diff --git a/src/core/lib/slice/slice.c b/src/core/lib/slice/slice.c index 9e69b443fe1..b90738fd1aa 100644 --- a/src/core/lib/slice/slice.c +++ b/src/core/lib/slice/slice.c @@ -55,6 +55,13 @@ grpc_slice grpc_empty_slice(void) { return out; } +grpc_slice grpc_slice_copy(grpc_slice s) { + grpc_slice out = GRPC_SLICE_MALLOC(GRPC_SLICE_LENGTH(s)); + memcpy(GRPC_SLICE_START_PTR(out), GRPC_SLICE_START_PTR(s), + GRPC_SLICE_LENGTH(s)); + return out; +} + grpc_slice grpc_slice_ref_internal(grpc_slice slice) { if (slice.refcount) { slice.refcount->vtable->ref(slice.refcount); diff --git a/test/core/util/passthru_endpoint.c b/test/core/util/passthru_endpoint.c index 121567fc0d2..ad718c75d1f 100644 --- a/test/core/util/passthru_endpoint.c +++ b/test/core/util/passthru_endpoint.c @@ -102,13 +102,13 @@ static void me_write(grpc_exec_ctx *exec_ctx, grpc_endpoint *ep, error = GRPC_ERROR_CREATE_FROM_STATIC_STRING("Endpoint already shutdown"); } else if (m->on_read != NULL) { for (size_t i = 0; i < slices->count; i++) { - grpc_slice_buffer_add(m->on_read_out, grpc_slice_ref(slices->slices[i])); + grpc_slice_buffer_add(m->on_read_out, grpc_slice_copy(slices->slices[i])); } grpc_closure_sched(exec_ctx, m->on_read, GRPC_ERROR_NONE); m->on_read = NULL; } else { for (size_t i = 0; i < slices->count; i++) { - grpc_slice_buffer_add(&m->read_buffer, grpc_slice_ref(slices->slices[i])); + grpc_slice_buffer_add(&m->read_buffer, grpc_slice_copy(slices->slices[i])); } } gpr_mu_unlock(&m->parent->mu); diff --git a/test/core/util/trickle_endpoint.c b/test/core/util/trickle_endpoint.c index 0848147158a..66f30c8e65d 100644 --- a/test/core/util/trickle_endpoint.c +++ b/test/core/util/trickle_endpoint.c @@ -66,14 +66,13 @@ static void te_read(grpc_exec_ctx *exec_ctx, grpc_endpoint *ep, static void te_write(grpc_exec_ctx *exec_ctx, grpc_endpoint *ep, grpc_slice_buffer *slices, grpc_closure *cb) { trickle_endpoint *te = (trickle_endpoint *)ep; - for (size_t i = 0; i < slices->count; i++) { - grpc_slice_ref_internal(slices->slices[i]); - } gpr_mu_lock(&te->mu); if (te->write_buffer.length == 0) { te->last_write = gpr_now(GPR_CLOCK_MONOTONIC); } - grpc_slice_buffer_addn(&te->write_buffer, slices->slices, slices->count); + for (size_t i = 0; i < slices->count; i++) { + grpc_slice_buffer_add(&te->write_buffer, grpc_slice_copy(slices->slices[i])); + } grpc_closure_sched(exec_ctx, cb, GRPC_ERROR_REF(te->error)); gpr_mu_unlock(&te->mu); } From e38948a1cdd44554095ad8b277e86877dfda1301 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Wed, 26 Apr 2017 08:53:25 -0700 Subject: [PATCH 6/9] clang-format --- test/core/util/passthru_endpoint.c | 3 ++- test/core/util/trickle_endpoint.c | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/test/core/util/passthru_endpoint.c b/test/core/util/passthru_endpoint.c index ad718c75d1f..6400845d235 100644 --- a/test/core/util/passthru_endpoint.c +++ b/test/core/util/passthru_endpoint.c @@ -108,7 +108,8 @@ static void me_write(grpc_exec_ctx *exec_ctx, grpc_endpoint *ep, m->on_read = NULL; } else { for (size_t i = 0; i < slices->count; i++) { - grpc_slice_buffer_add(&m->read_buffer, grpc_slice_copy(slices->slices[i])); + grpc_slice_buffer_add(&m->read_buffer, + grpc_slice_copy(slices->slices[i])); } } gpr_mu_unlock(&m->parent->mu); diff --git a/test/core/util/trickle_endpoint.c b/test/core/util/trickle_endpoint.c index 66f30c8e65d..58ac59711be 100644 --- a/test/core/util/trickle_endpoint.c +++ b/test/core/util/trickle_endpoint.c @@ -71,7 +71,8 @@ static void te_write(grpc_exec_ctx *exec_ctx, grpc_endpoint *ep, te->last_write = gpr_now(GPR_CLOCK_MONOTONIC); } for (size_t i = 0; i < slices->count; i++) { - grpc_slice_buffer_add(&te->write_buffer, grpc_slice_copy(slices->slices[i])); + grpc_slice_buffer_add(&te->write_buffer, + grpc_slice_copy(slices->slices[i])); } grpc_closure_sched(exec_ctx, cb, GRPC_ERROR_REF(te->error)); gpr_mu_unlock(&te->mu); From bb4046cd771344f69b3268a86c48b01cd34a921d Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Wed, 26 Apr 2017 08:56:30 -0700 Subject: [PATCH 7/9] Regenerate projects --- grpc.def | 1 + src/ruby/ext/grpc/rb_grpc_imports.generated.c | 2 ++ src/ruby/ext/grpc/rb_grpc_imports.generated.h | 5 ++++- 3 files changed, 7 insertions(+), 1 deletion(-) diff --git a/grpc.def b/grpc.def index 54f9609bc2a..8b64b37ff34 100644 --- a/grpc.def +++ b/grpc.def @@ -142,6 +142,7 @@ EXPORTS grpc_server_credentials_set_auth_metadata_processor grpc_slice_ref grpc_slice_unref + grpc_slice_copy grpc_slice_new grpc_slice_new_with_user_data grpc_slice_new_with_len diff --git a/src/ruby/ext/grpc/rb_grpc_imports.generated.c b/src/ruby/ext/grpc/rb_grpc_imports.generated.c index b6360f2d4c9..d11f7f6a78a 100644 --- a/src/ruby/ext/grpc/rb_grpc_imports.generated.c +++ b/src/ruby/ext/grpc/rb_grpc_imports.generated.c @@ -180,6 +180,7 @@ grpc_call_set_credentials_type grpc_call_set_credentials_import; grpc_server_credentials_set_auth_metadata_processor_type grpc_server_credentials_set_auth_metadata_processor_import; grpc_slice_ref_type grpc_slice_ref_import; grpc_slice_unref_type grpc_slice_unref_import; +grpc_slice_copy_type grpc_slice_copy_import; grpc_slice_new_type grpc_slice_new_import; grpc_slice_new_with_user_data_type grpc_slice_new_with_user_data_import; grpc_slice_new_with_len_type grpc_slice_new_with_len_import; @@ -481,6 +482,7 @@ void grpc_rb_load_imports(HMODULE library) { grpc_server_credentials_set_auth_metadata_processor_import = (grpc_server_credentials_set_auth_metadata_processor_type) GetProcAddress(library, "grpc_server_credentials_set_auth_metadata_processor"); grpc_slice_ref_import = (grpc_slice_ref_type) GetProcAddress(library, "grpc_slice_ref"); grpc_slice_unref_import = (grpc_slice_unref_type) GetProcAddress(library, "grpc_slice_unref"); + grpc_slice_copy_import = (grpc_slice_copy_type) GetProcAddress(library, "grpc_slice_copy"); grpc_slice_new_import = (grpc_slice_new_type) GetProcAddress(library, "grpc_slice_new"); grpc_slice_new_with_user_data_import = (grpc_slice_new_with_user_data_type) GetProcAddress(library, "grpc_slice_new_with_user_data"); grpc_slice_new_with_len_import = (grpc_slice_new_with_len_type) GetProcAddress(library, "grpc_slice_new_with_len"); diff --git a/src/ruby/ext/grpc/rb_grpc_imports.generated.h b/src/ruby/ext/grpc/rb_grpc_imports.generated.h index 539b7de9bd3..dc50b87baf2 100644 --- a/src/ruby/ext/grpc/rb_grpc_imports.generated.h +++ b/src/ruby/ext/grpc/rb_grpc_imports.generated.h @@ -491,6 +491,9 @@ extern grpc_slice_ref_type grpc_slice_ref_import; typedef void(*grpc_slice_unref_type)(grpc_slice s); extern grpc_slice_unref_type grpc_slice_unref_import; #define grpc_slice_unref grpc_slice_unref_import +typedef grpc_slice(*grpc_slice_copy_type)(grpc_slice s); +extern grpc_slice_copy_type grpc_slice_copy_import; +#define grpc_slice_copy grpc_slice_copy_import typedef grpc_slice(*grpc_slice_new_type)(void *p, size_t len, void (*destroy)(void *)); extern grpc_slice_new_type grpc_slice_new_import; #define grpc_slice_new grpc_slice_new_import @@ -530,7 +533,7 @@ extern grpc_slice_sub_no_ref_type grpc_slice_sub_no_ref_import; typedef grpc_slice(*grpc_slice_split_tail_type)(grpc_slice *s, size_t split); extern grpc_slice_split_tail_type grpc_slice_split_tail_import; #define grpc_slice_split_tail grpc_slice_split_tail_import -typedef grpc_slice(*grpc_slice_split_tail_maybe_ref_type)(grpc_slice *s, size_t split, int inc_refs); +typedef grpc_slice(*grpc_slice_split_tail_maybe_ref_type)(grpc_slice *s, size_t split, grpc_slice_ref_whom ref_whom); extern grpc_slice_split_tail_maybe_ref_type grpc_slice_split_tail_maybe_ref_import; #define grpc_slice_split_tail_maybe_ref grpc_slice_split_tail_maybe_ref_import typedef grpc_slice(*grpc_slice_split_head_type)(grpc_slice *s, size_t split); From 6ae9175690f26b21b2888c780821482074d4a1bb Mon Sep 17 00:00:00 2001 From: Matt Kwong Date: Wed, 26 Apr 2017 19:03:18 -0700 Subject: [PATCH 8/9] Update performance VM init script --- tools/gce/create_linux_performance_worker.sh | 4 +- tools/gce/linux_performance_worker_init.sh | 50 +++++++++++--------- 2 files changed, 29 insertions(+), 25 deletions(-) diff --git a/tools/gce/create_linux_performance_worker.sh b/tools/gce/create_linux_performance_worker.sh index 2c8cf0b96b3..68710e13b07 100755 --- a/tools/gce/create_linux_performance_worker.sh +++ b/tools/gce/create_linux_performance_worker.sh @@ -42,14 +42,14 @@ CLOUD_PROJECT=grpc-testing ZONE=us-central1-b # this zone allows 32core machines INSTANCE_NAME="${1:-grpc-performance-server1}" -MACHINE_TYPE=n1-standard-8 +MACHINE_TYPE=n1-standard-32 gcloud compute instances create $INSTANCE_NAME \ --project="$CLOUD_PROJECT" \ --zone "$ZONE" \ --machine-type $MACHINE_TYPE \ --image-project ubuntu-os-cloud \ - --image-family ubuntu-1604-lts \ + --image-family ubuntu-1610 \ --boot-disk-size 300 \ --scopes https://www.googleapis.com/auth/bigquery diff --git a/tools/gce/linux_performance_worker_init.sh b/tools/gce/linux_performance_worker_init.sh index 17f36fb4ff8..78cdd31f0b0 100755 --- a/tools/gce/linux_performance_worker_init.sh +++ b/tools/gce/linux_performance_worker_init.sh @@ -55,7 +55,10 @@ sudo apt-get install -y \ libc6 \ libc6-dbg \ libc6-dev \ + libcurl4-openssl-dev \ libgtest-dev \ + libreadline-dev \ + libssl-dev \ libtool \ make \ strace \ @@ -71,7 +74,8 @@ sudo apt-get install -y \ telnet \ unzip \ wget \ - zip + zip \ + zlib1g-dev # perftools sudo apt-get install -y google-perftools libgoogle-perftools-dev @@ -87,14 +91,15 @@ sudo pip install tabulate sudo pip install google-api-python-client sudo pip install virtualenv -# TODO(jtattermusch): For some reason, building gRPC Python depends on python3.4 -# being installed, but python3.4 is not available on Ubuntu 16.04. -# Temporarily fixing this by adding a PPA with python3.4, but we should -# really remove this hack once possible. -sudo add-apt-repository -y ppa:fkrull/deadsnakes -sudo apt-get update -sudo apt-get install -y python3.4 python3.4-dev -python3.4 -m pip install virtualenv +# Building gRPC Python depends on python3.4 being installed, but python3.4 +# is not available on Ubuntu 16.10, so install from source +curl -O https://www.python.org/ftp/python/3.4.6/Python-3.4.6.tgz +tar xzvf Python-3.4.6.tgz +cd Python-3.4.6 +./configure --enable-shared --prefix=/usr/local LDFLAGS="-Wl,--rpath=/usr/local/lib" +sudo make altinstall +cd .. +rm Python-3.4.6.tgz curl -O https://bootstrap.pypa.io/get-pip.py sudo pypy get-pip.py @@ -117,18 +122,25 @@ sudo apt-get update sudo apt-get install -y mono-devel nuget # C# .NET Core dependencies (https://www.microsoft.com/net/core#ubuntu) -sudo sh -c 'echo "deb [arch=amd64] https://apt-mo.trafficmanager.net/repos/dotnet-release/ xenial main" > /etc/apt/sources.list.d/dotnetdev.list' +sudo sh -c 'echo "deb [arch=amd64] https://apt-mo.trafficmanager.net/repos/dotnet-release/ yakkety main" > /etc/apt/sources.list.d/dotnetdev.list' sudo apt-key adv --keyserver apt-mo.trafficmanager.net --recv-keys 417A0893 sudo apt-get update -sudo apt-get install -y dotnet-dev-1.0.0-preview2-003131 +sudo apt-get install -y dotnet-dev-1.0.0-preview2.1-003155 sudo apt-get install -y dotnet-dev-1.0.1 # Ruby dependencies -gpg --keyserver hkp://keys.gnupg.net --recv-keys 409B6B1796C275462A1703113804BB82D39DC0E3 -curl -sSL https://get.rvm.io | bash -s stable --ruby +git clone https://github.com/rbenv/rbenv.git ~/.rbenv +export PATH="$HOME/.rbenv/bin:$PATH" +eval "$(rbenv init -)" + +git clone https://github.com/rbenv/ruby-build.git ~/.rbenv/plugins/ruby-build +export PATH="$HOME/.rbenv/plugins/ruby-build/bin:$PATH" + +rbenv install 2.4.0 +rbenv global 2.4.0 +ruby -v # Install bundler (prerequisite for gRPC Ruby) -source ~/.rvm/scripts/rvm gem install bundler # Java dependencies - nothing as we already have Java JDK 8 @@ -163,15 +175,7 @@ echo 4096 | sudo tee /proc/sys/kernel/perf_event_mlock_kb git clone -v https://github.com/brendangregg/FlameGraph ~/FlameGraph # Install scipy and numpy for benchmarking scripts -sudo apt-get install python-scipy python-numpy - -# Update Linux kernel to 4.9 -wget \ - kernel.ubuntu.com/~kernel-ppa/mainline/v4.9.20/linux-headers-4.9.20-040920_4.9.20-040920.201703310531_all.deb \ - kernel.ubuntu.com/~kernel-ppa/mainline/v4.9.20/linux-headers-4.9.20-040920-generic_4.9.20-040920.201703310531_amd64.deb \ - kernel.ubuntu.com/~kernel-ppa/mainline/v4.9.20/linux-image-4.9.20-040920-generic_4.9.20-040920.201703310531_amd64.deb -sudo dpkg -i linux-headers-4.9*.deb linux-image-4.9*.deb -rm linux-* +sudo apt-get install -y python-scipy python-numpy # Add pubkey of jenkins@grpc-jenkins-master to authorized keys of jenkins@ # This needs to happen as the last step to prevent Jenkins master from connecting From 38279ea2e0d6c1704365a1507b9e8d487e5d7837 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ivan=20Vu=C4=8Dica?= Date: Wed, 18 Jan 2017 00:04:16 +0000 Subject: [PATCH 9/9] Bazel rule for building grpc_cli. --- test/cpp/util/BUILD | 30 +++++++++++++++++++++++++++--- 1 file changed, 27 insertions(+), 3 deletions(-) diff --git a/test/cpp/util/BUILD b/test/cpp/util/BUILD index c83f89eb906..9dde22b4d1b 100644 --- a/test/cpp/util/BUILD +++ b/test/cpp/util/BUILD @@ -34,8 +34,8 @@ licenses(["notice"]) # 3-clause BSD cc_binary( name = "testso.so", srcs = [], - deps = ["//:grpc++_unsecure"], linkshared = 1, + deps = ["//:grpc++_unsecure"], ) cc_library( @@ -104,5 +104,29 @@ cc_test( ], ) - - +cc_binary( + name = "grpc_cli", + srcs = [ + "cli_call.cc", + "cli_call.h", + "cli_credentials.cc", + "cli_credentials.h", + "config_grpc_cli.h", + "grpc_cli.cc", + "grpc_tool.cc", + "grpc_tool.h", + "proto_file_parser.cc", + "proto_file_parser.h", + "proto_reflection_descriptor_database.cc", + "proto_reflection_descriptor_database.h", + "service_describer.cc", + "service_describer.h", + "test_config.h", + "test_config_cc.cc", + ], + deps = [ + "//:grpc++", + "//external:gflags", + "//src/proto/grpc/reflection/v1alpha:reflection_proto", + ], +)