-
Notifications
You must be signed in to change notification settings - Fork 10.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix unnecessary slice moving in slice_buffer.c:maybe_embiggen #10211
Conversation
src/core/lib/slice/slice_buffer.c
Outdated
} else { | ||
if (sb->base_slices != sb->slices) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be good to do this before growing the capacity: i.e line 54 above sb->capacity = GROW(sb->capacity)
Because after the memmove
, we might have some space and no longer have to do a gpr_realloc
So perhaps it would be just good do the memmove just before checking for if (slice_count == sb->capacity)
..
i.e something like:
static void maybe_embiggen(grpc_slice_buffer *sb) {
size_t slice_offset = (size_t)(sb->slices - sb->base_slices);
size_t slice_count = sb->count + slice_offset;
if (slice_count == sb->capacity && sb->base_slices != sb->slices) {
/* do the memmove */
/* re-calculate slice_offset or rather just reset slice_count to sb->count */
}
..
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That sounds great.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good. Thanks Muxi.
test this please |
|
gRPC_interop_pull_requests - #9011 |
Avoid moving slices unnecessarily when invoking
grpc_slice_buffer_add
that impacted performance.Cherry-picked from #9626 as it is still going to take some time to submit that PR, while this change is orthogonal and going to benefit widely.