perf(allocator/vec2): replace self.reserve(1) calls with self.grow_one() for better efficiency
#9856
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was referenced Mar 18, 2025
CodSpeed Instrumentation Performance ReportMerging #9856 will not alter performance
8000
Comparing Summary
|
|
Unbelievable 😢 EDIT: This unbelievable result is due to my mistake. |
a305372 to
b9a6398
Compare
4dc483f to
9d1db26
Compare
9d1db26 to
c94a0f0
Compare
b9a6398 to
9968328
Compare
83b077f to
43613e1
Compare
|
I've noticed the current |
149392c to
901e287
Compare
44dd469 to
aae3118
Compare
graphite-app bot
pushed a commit
that referenced
this pull request
May 3, 2025
…marking reserve as `#[cold]` and `#[inline(never)]` (#10675) I guess the performance regression reason is that the current implementation has more instructions than before. Here to use the lower of `size_hint` to reserve space, which is bloating the loop body. Also, the `for` loop is easier to optimize by the compiler. `reserve` inside `extend` is rarely taken, so mark it as `#[cold]` and `#[inline(never)]`, which can reduce the instructions in `while` loop. We got a 3%-4% performance improvement in the `minfier`, but the transformer performance did not fully get back to before #10670. Anyway, I think we can accept the less than 1% performance regression; this change can unblock us from pushing forward the `Vec` improvement; we will get it back in at the end of the stack! See #9856
cbd2cdb to
979432a
Compare
aae3118 to
a703a01
Compare
overlookmotel
approved these changes
May 3, 2025
Merge activity
|
graphite-app bot
pushed a commit
that referenced
this pull request
May 3, 2025
…_one()` for better efficiency (#9856) The places calling `self.reserve(1)` already checked `len == self.buf.cap()`, in order to avoid checking again in `reserve`, we should call `grow_one` instead which is a shortcut of `grow_amortized(len, 1)`.
a703a01 to
8f5911f
Compare
Base automatically changed from
03-18-feat_allocator_vec2_add_specialized_grow_one_method
to
main
May 3, 2025 15:00
…_one()` for better efficiency (#9856) The places calling `self.reserve(1)` already checked `len == self.buf.cap()`, in order to avoid checking again in `reserve`, we should call `grow_one` instead which is a shortcut of `grow_amortized(len, 1)`.
8f5911f to
04e0390
Compare
WalkthroughThis change updates the memory growth strategy in a custom vector implementation. Specifically, within the Possibly 8000 related PRs
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
⏰ Context from checks skipped due to timeout of 90000ms (16)
🔇 Additional comments (3)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The places calling
self.reserve(1)already checkedlen == self.buf.cap(), in order to avoid checking again inreserve, we should callgrow_oneinstead which is a shortcut ofgrow_amortized(len, 1).