Skip to content

Fix caching for normal allocations#10825

Merged
normanmaurer merged 1 commit into4.1from
fix_caches
Nov 25, 2020
Merged

Fix caching for normal allocations#10825
normanmaurer merged 1 commit into4.1from
fix_caches

Conversation

@normanmaurer
Copy link
Copy Markdown
Member

Motivation:

#10267 introduced a change that reduced the fragmentation. Unfortunally it also introduced a regression when it comes to caching of normal allocations. This can have a negative performance impact depending on the allocation sizes.

Modifications:

  • Fix algorithm to calculate the array size for normal allocation caches
  • Correctly calculate indeox for normal caches
  • Add unit test

Result:

Fixes #10805

Motivation:

#10267 introduced a change that reduced the fragmentation. Unfortunally it also introduced a regression when it comes to caching of normal allocations. This can have a negative performance impact depending on the allocation sizes.

Modifications:

- Fix algorithm to calculate the array size for normal allocation caches
- Correctly calculate indeox for normal caches
- Add unit test

Result:

Fixes #10805
@normanmaurer
Copy link
Copy Markdown
Member Author

/cc @martinfurmanski @yuanrw

@normanmaurer
Copy link
Copy Markdown
Member Author

Before the change:

PooledByteBufAllocatorBenchmark.allocateAndFree  thrpt   20  5839558.424 ± 145747.978  ops/s

After:

PooledByteBufAllocatorBenchmark.allocateAndFree  thrpt   20  15917107.067 ± 332551.316  ops/s

Copy link
Copy Markdown
Contributor

@franz1981 franz1981 left a comment

Choose a reason for hiding this comment

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

LGTM, well done ;)

@normanmaurer normanmaurer added this to the 4.1.55.Final milestone Nov 25, 2020
@normanmaurer
Copy link
Copy Markdown
Member Author

Beside this @chrisvest had some ideas how to make the implementation a bit more efficient in general. But thats for another pr :)

Copy link
Copy Markdown
Member

@chrisvest chrisvest left a comment

Choose a reason for hiding this comment

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

👍

@normanmaurer
Copy link
Copy Markdown
Member Author

The failure for java11 is not related to this pr and just some ci problem.

@normanmaurer normanmaurer merged commit 221c1a1 into 4.1 Nov 25, 2020
@normanmaurer normanmaurer deleted the fix_caches branch November 25, 2020 14:05
normanmaurer added a commit that referenced this pull request Nov 25, 2020
Motivation:

#10267 introduced a change that reduced the fragmentation. Unfortunally it also introduced a regression when it comes to caching of normal allocations. This can have a negative performance impact depending on the allocation sizes.

Modifications:

- Fix algorithm to calculate the array size for normal allocation caches
- Correctly calculate indeox for normal caches
- Add unit test

Result:

Fixes #10805
@martinfurmanski
Copy link
Copy Markdown
Contributor

Thanks for the quick analysis and turnaround everybody. I've confirmed the performance locally here as well.

raidyue pushed a commit to raidyue/netty that referenced this pull request Jul 8, 2022
Motivation:

netty#10267 introduced a change that reduced the fragmentation. Unfortunally it also introduced a regression when it comes to caching of normal allocations. This can have a negative performance impact depending on the allocation sizes.

Modifications:

- Fix algorithm to calculate the array size for normal allocation caches
- Correctly calculate indeox for normal caches
- Add unit test

Result:

Fixes netty#10805
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.

Performance regression in buffer pool

5 participants