perf: Speedup coins.Sort() when coins is of length 1 #18875
Conversation
Coins.Sort() performs a heap allocation, even though sort.Sort() is in place.
This is because the compiler is doing {something} called runtime.convTSlice which internally makes a copy of the entire slice.
We should ideally find a solution that avoids this malloc for every Coins.Sort() of all sizes, but just eliminating it for slices of length <= 1 is already a big win, as Sort's are used throughout the Add and Sub logic.
|
Warning Rate Limit Exceeded@ValarDragon has exceeded the limit for the number of files or commits that can be reviewed per hour. Please wait 13 minutes and 59 seconds before requesting another review. How to resolve this issue?After the wait time has elapsed, a review can be triggered using the @coderabbitai review command. Alternatively, push new commits to this PR. We recommend that you space out your commits to avoid hitting the rate limit. How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. WalkthroughThe update optimizes the sorting process for coin collections. It introduces a conditional check to skip sorting when the collection contains one or zero elements, thereby saving computational resources by avoiding unnecessary operations on already sorted or singleton sets. Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on X ? TipsChat with CodeRabbit Bot (
|
This comment has been minimized.
This comment has been minimized.
|
Pprof before for a load in osmosis that led to sorts size 1 coins: |
julienrbrt
added
backport/v0.47.x
(cherry picked from commit bd04173) # Conflicts: # CHANGELOG.md
(cherry picked from commit bd04173) # Conflicts: # CHANGELOG.md
#18877) Co-authored-by: Dev Ojha <ValarDragon@users.noreply.github.com> Co-authored-by: Julien Robert <julien@rbrt.fr>
#18878) Co-authored-by: Dev Ojha <ValarDragon@users.noreply.github.com> Co-authored-by: Julien Robert <julien@rbrt.fr>
…#18875) (cosmos#18878) Co-authored-by: Dev Ojha <ValarDragon@users.noreply.github.com> Co-authored-by: Julien Robert <julien@rbrt.fr>
…#18875) (cosmos#18878) Co-authored-by: Dev Ojha <ValarDragon@users.noreply.github.com> Co-authored-by: Julien Robert <julien@rbrt.fr>
Coins.Sort() performs a heap allocation, even though sort.Sort() is in place.
This is because the compiler is doing {something} called runtime.convTSlice which internally makes a copy of the entire slice.
We should ideally find a solution that avoids this malloc for every Coins.Sort() of all sizes, but just eliminating it for slices of length <= 1 is already a big win, as Sort's are used throughout the Add and Sub logic.
(At least its a 2 second CPU savings win in Osmosis epoch code)
Author Checklist
!in the type prefix if API or client breaking changeCHANGELOG.mdReviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
I have...