Skip to content

perf(allocator/vec2): calling Bump::grow or Bump::shrink at the call site directly instead of calling realloc#10686

Merged
graphite-app[bot] merged 1 commit intomainfrom
04-29-perf_allocator_vec2_calling_bump_grow_or_bump_shrink_at_the_call_site_directly
May 3, 2025
Merged

perf(allocator/vec2): calling Bump::grow or Bump::shrink at the call site directly instead of calling realloc#10686
graphite-app[bot] merged 1 commit intomainfrom
04-29-perf_allocator_vec2_calling_bump_grow_or_bump_shrink_at_the_call_site_directly

Conversation

@Dunqing
Copy link
Member

@Dunqing Dunqing commented Apr 29, 2025 •
edited by overlookmotel
Loading

This PR aims to remove the realloc method, which was copied from bumpalo. The realloc function is used to call the shrink or grow method of bumpalo by checking the new size. This is unnecessary because in the call site, we know which method we should call, and the ZST check is also unnecessary because it has been checked earlier in the call site.

Also, this is aligned with the standard library implementation.

@github-actions github-actions bot added the C-performance Category - Solution not expected to change functional behavior, only performance label Apr 29, 2025
This was referenced Apr 29, 2025
Merged
Merged
Merged
Merged
Merged
@Dunqing Graphite App
Copy link
Member Author

Dunqing commented Apr 29, 2025 •
edited
Loading

How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

  • 0-merge - adds this PR to the back of the merge queue
  • hotfix - for urgent hot fixes, skip the queue and merge this PR next

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

@Dunqing Dunqing mentioned this pull request Apr 29, 2025
Merged
@Dunqing Dunqing changed the title perf(allocator/vec2): calling Bump::grow or Bump::shrink at the call site directly perf(allocator/vec2): calling Bump::growor Bump::shrink at the call site directly Apr 29, 2025
@codspeed-hq
Copy link

codspeed-hq bot commented Apr 29, 2025 •
edited
Loading

CodSpeed Instrumentation Performance Report

Merging #10686 will create unknown performance changes

Comparing 04-29-perf_allocator_vec2_calling_bump_grow_or_bump_shrink_at_the_call_site_directly (2dc4779) with main (b4953b4)

Summary

🆕 36 new benchmarks

Benchmarks breakdown

Benchmark BASE HEAD Change
🆕 codegen[checker.ts] N/A 22.2 ms N/A
🆕 codegen_sourcemap[checker.ts] N/A 65.2 ms N/A
🆕 formatter[antd.js] N/A 714.7 ms N/A
🆕 formatter[react.development.js] N/A 8.1 ms N/A
🆕 formatter[typescript.js] N/A 1.1 s N/A
🆕 isolated-declarations[vue-id.ts] N/A 58.5 ms N/A
🆕 lexer[RadixUIAdoptionSection.jsx] N/A 21.3 µs N/A
🆕 lexer[antd.js] N/A 24.1 ms N/A
🆕 lexer[cal.com.tsx] N/A 5.8 ms N/A
🆕 lexer[checker.ts] N/A 14.4 ms N/A
🆕 lexer[pdf.mjs] N/A 3.9 ms N/A
🆕 linter[RadixUIAdoptionSection.jsx] N/A 2.7 ms N/A
🆕 linter[cal.com.tsx] N/A 1.2 s N/A
🆕 linter[checker.ts] N/A 3.1 s N/A
🆕 mangler[antd.js] N/A 16 ms N/A
🆕 mangler[react.development.js] N/A 294.9 µs N/A
🆕 mangler[typescript.js] N/A 39.6 ms N/A
🆕 minifier[antd.js] N/A 159.5 ms N/A
🆕 minifier[react.development.js] N/A 1.8 ms N/A
🆕 minifier[typescript.js] N/A 279.4 ms N/A
... ... ... ... ...

ℹ️ Only the first 20 benchmarks are displayed. Go to the app to view all benchmarks.

@github-actions github-actions bot added the A-parser Area - Parser label Apr 29, 2025
@Dunqing Dunqing force-pushed the 04-29-perf_allocator_vec2_calling_bump_grow_or_bump_shrink_at_the_call_site_directly branch 6 times, most recently from 2252ad9 to 1737415 Compare April 29, 2025 09:44
@Dunqing Dunqing changed the title perf(allocator/vec2): calling Bump::growor Bump::shrink at the call site directly perf(allocator/vec2): calling Bump::grow or Bump::shrink at the call site directly Apr 29, 2025
@Dunqing Dunqing force-pushed the 04-29-perf_allocator_vec2_calling_bump_grow_or_bump_shrink_at_the_call_site_directly branch 3 times, most recently from d753000 to 58393b8 Compare April 29, 2025 12:41
@Dunqing Dunqing changed the title perf(allocator/vec2): calling Bump::grow or Bump::shrink at the call site directly perf(allocator/vec2): calling Bump::grow or Bump::shrink at the call site directly instead of calling realloc Apr 29, 2025
@Dunqing Dunqing marked this pull request as ready for review April 29, 2025 15:41
@Dunqing Dunqing mentioned this pull request Apr 29, 2025
Merged
@graphite-app graphite-app bot force-pushed the 04-28-perf_allocator_vec2_resolve_performance_regression_for_extend branch from c70f032 to 04757c5 Compare April 29, 2025 16:10
@graphite-app graphite-app bot requested a review from overlookmotel as a code owner April 29, 2025 16:10
@graphite-app graphite-app bot force-pushed the 04-29-perf_allocator_vec2_calling_bump_grow_or_bump_shrink_at_the_call_site_directly branch from 58393b8 to 50aaab5 Compare April 29, 2025 16:11
@graphite-app graphite-app bot force-pushed the 04-28-perf_allocator_vec2_resolve_performance_regression_for_extend branch from 04757c5 to b56f8e1 Compare April 30, 2025 00:10
@graphite-app graphite-app bot force-pushed the 04-29-perf_allocator_vec2_calling_bump_grow_or_bump_shrink_at_the_call_site_directly branch from 50aaab5 to cdb5487 Compare April 30, 2025 00:10
@Dunqing Dunqing changed the base branch from 04-28-perf_allocator_vec2_resolve_performance_regression_for_extend to graphite-base/10686 April 30, 2025 00:35
@Dunqing Dunqing force-pushed the graphite-base/10686 branch from b56f8e1 to 4682891 Compare April 30, 2025 02:18
@Dunqing Dunqing force-pushed the 04-29-perf_allocator_vec2_calling_bump_grow_or_bump_shrink_at_the_call_site_directly branch from cdb5487 to c9bfd21 Compare April 30, 2025 02:18
@Dunqing Dunqing changed the base branch from graphite-base/10686 to 04-28-perf_allocator_vec2_resolve_performance_regression_for_extend April 30, 2025 02:18
@Dunqing Dunqing mentioned this pull request Apr 30, 2025
Merged
@Boshen Boshen added the 0-merge Merge with Graphite Merge Queue label May 3, 2025
@Boshen Graphite App
Copy link
Member

Boshen commented May 3, 2025 •
edited by graphite-app bot
Loading

Merge activity

@Dunqing
perf(allocator/vec2): calling Bump::grow or Bump::shrink at the c…
2dc4779 
…all site directly instead of calling `realloc` (#10686)

This PR aims to remove the `realloc` method, which was copied from `bumpalo`. The `realloc` function is used to call the `shrink` or `grow` method of bumpalo by checking the new size. This is unnecessary because in the call site, we know which method we should call, and the ZST check is also unnecessary because it has been checked earlier in the call site.

Also, this is aligned with the standard library implementation.
@graphite-app graphite-app bot force-pushed the 04-28-perf_allocator_vec2_resolve_performance_regression_for_extend branch from 4682891 to b4953b4 Compare May 3, 2025 13:03
@graphite-app graphite-app bot force-pushed the 04-29-perf_allocator_vec2_calling_bump_grow_or_bump_shrink_at_the_call_site_directly branch from c9bfd21 to 2dc4779 Compare May 3, 2025 13:04
@graphite-app graphite-app bot removed the 0-merge Merge with Graphite Merge Queue label May 3, 2025
Base automatically changed from 04-28-perf_allocator_vec2_resolve_performance_regression_for_extend to main May 3, 2025 13:10
@graphite-app graphite-app bot merged commit 2dc4779 into main May 3, 2025
28 checks passed
@graphite-app graphite-app bot deleted the 04-29-perf_allocator_vec2_calling_bump_grow_or_bump_shrink_at_the_call_site_directly branch May 3, 2025 13:11
@oxc-bot oxc-bot mentioned this pull request May 3, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@overlookmotel overlookmotel overlookmotel approved these changes

Assignees

No one assigned

Labels

A-parser Area - Parser C-performance Category - Solution not expected to change functional behavior, only performance

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Dunqing @Boshen @overlookmotel