Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Stop Copying Every Http Request in Message Handler (#44564) #49809

Merged
merged 1 commit into from
Dec 4, 2019
Merged

Stop Copying Every Http Request in Message Handler (#44564) #49809

merged 1 commit into from
Dec 4, 2019

Conversation

original-brownbear
Copy link
Member

  • Copying the request is not necessary here. We can simply release it once the response has been generated and a lot of Unpooled allocations that way
  • Relates Reduce garbage for requests with unpooled buffers #32228
    • I think the issue that preventet that PR that PR from being merged was solved by Optimize Bulk Message Parsing and Message Length Parsing #39634 that moved the bulk index marker search to ByteBuf bulk access so the composite buffer shouldn't require many additional bounds checks (I'd argue the bounds checks we add, we save when copying the composite buffer)
  • I couldn't neccessarily reproduce much of a speedup from this change, but I could reproduce a very measureable reduction in GC time with e.g. Rally's PMC (4g heap node and bulk requests of size 5k saw a reduction in young GC time by ~10% for me)

backport of #44564

@original-brownbear
Stop Copying Every Http Request in Message Handler (elastic#44564)
f60f814 
* Copying the request is not necessary here. We can simply release it once the response has been generated and a lot of `Unpooled` allocations that way
* Relates elastic#32228
   * I think the issue that preventet that PR  that PR from being merged was solved by elastic#39634 that moved the bulk index marker search to ByteBuf bulk access so the composite buffer shouldn't require many additional bounds checks  (I'd argue the bounds checks we add, we save when copying the composite buffer)
* I couldn't neccessarily reproduce much of a speedup from this change, but I could reproduce a very measureable reduction in GC time with e.g. Rally's PMC (4g heap node and bulk requests of size 5k saw a reduction in young GC time by ~10% for me)
@original-brownbear original-brownbear added :Distributed Coordination/Network Http and internode communication implementations backport labels Dec 3, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-distributed (:Distributed/Network)

@original-brownbear original-brownbear merged commit 996cddd into elastic:7.x Dec 4, 2019
@original-brownbear original-brownbear deleted the 44564-7.x branch December 4, 2019 07:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
backport :Distributed Coordination/Network Http and internode communication implementations
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@original-brownbear @elasticmachine