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.

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

Reuse the same allocator as used by the ByteBuf that is used during… #10226

Merged
merged 2 commits into from Apr 29, 2020

Conversation

normanmaurer
Copy link
Member

… multipart decoding

Motivation:

We should not use Unpooled to allocate buffers if possible to ensure we can make use of pooling etc.

Modifications:

  • Only allocate a buffer if really needed
  • Use the ByteBufAllocator of the offered ByteBuf
  • Ensure we not use buffer.copy() but explicitly allocate a buffer and then copy into it to not hit the limit of maxCapacity()

Result:

Improve allocations

@normanmaurer
Copy link
Member Author

/cc @fabienrenaud

… multipart decoding

Motivation:

We should not use Unpooled to allocate buffers if possible to ensure we can make use of pooling etc.

Modifications:

- Only allocate a buffer if really needed
- Use the ByteBufAllocator of the offered ByteBuf
- Ensure we not use buffer.copy() but explicitly allocate a buffer and then copy into it to not hit the limit of maxCapacity()

Result:

Improve allocations
Copy link
Contributor

@slandelle slandelle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Except that I personally prefer the ternary operator (expression style) instead of if/else with multiple assignments, LGTM.

@normanmaurer
Copy link
Member Author

@slandelle addressed.... PTAL again

Copy link
Contributor

@slandelle slandelle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@normanmaurer normanmaurer merged commit 8f7ca2b into 4.1 Apr 29, 2020
@normanmaurer normanmaurer deleted the multi_part_alloc branch April 29, 2020 12:39
normanmaurer added a commit that referenced this pull request Apr 29, 2020
#10226)

Motivation:

We should not use Unpooled to allocate buffers if possible to ensure we can make use of pooling etc.

Modifications:

- Only allocate a buffer if really needed
- Use the ByteBufAllocator of the offered ByteBuf
- Ensure we not use buffer.copy() but explicitly allocate a buffer and then copy into it to not hit the limit of maxCapacity()

Result:

Improve allocations
Kvicii pushed a commit to Kvicii/netty that referenced this pull request May 4, 2020
* '4.1' of github.com:netty/netty: (24 commits)
  Rename testmethods to make these more clear (netty#10231)
  Specify algorithm for key pair in self signed certificate to generate EC or RSA based certificate. (netty#10223)
  Reuse the same allocator as used by the `ByteBuf` that is used during… (netty#10226)
  Remove some debugging cruft (netty#10229)
  Fix memory leak in HttpPostMultipartRequestDecoder (netty#10227)
  Don't log with warn level in the DnsNameResolver in most cases (netty#10225)
  Detect CNAME loops in the CNAME cache while trying to resolve (netty#10221)
  Update to latest Conscrypt release and add workarounds for bugs (netty#10211)
  HttpPostRequestDecoder: retain instead of copy when first buf is last (netty#10209)
  Move up the size check in AbstractDiskHttpData.setContent. (netty#10222)
  Dns resolver: honor resolv.conf timeout, rotate and attempts options (netty#10207)
  Add check for DefaultFileRegion to calculate size of msg in AbstractTrafficShapingHandler (netty#10215)
  Remove unused import in JsonObjectDecoder.java (netty#10213)
  Fix a potential fd leak in AbstractDiskHttpData.delete (netty#10212)
  Update testsuite / pom.xml to be able to build with Java15 (netty#10210)
  Add fastpath implementation for Unpooled.copiedBuffer(CharSequence, Charset) when UTF-8 or US-ASCII is used (netty#10206)
  Add epoll aarch64 maven config and Dockerfile (netty#9804)
  HTTP2: Guard against multiple ctx.close(...) calls with the same ChannelPromise (netty#10201)
  [maven-release-plugin] prepare for next development iteration
  [maven-release-plugin] prepare release netty-4.1.49.Final
  ...
ihanyong pushed a commit to ihanyong/netty that referenced this pull request Jul 31, 2020
netty#10226)

Motivation:

We should not use Unpooled to allocate buffers if possible to ensure we can make use of pooling etc.

Modifications:

- Only allocate a buffer if really needed
- Use the ByteBufAllocator of the offered ByteBuf
- Ensure we not use buffer.copy() but explicitly allocate a buffer and then copy into it to not hit the limit of maxCapacity()

Result:

Improve allocations
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants