Skip to content

perf(span): add #[repr(transparent)] to Atom #16310

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.

Sign up for GitHub

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

Jump to bottom
Merged
graphite-app merged 1 commit into from
Dec 1, 2025
Merged

perf(span): add #[repr(transparent)] to Atom #16310

graphite-app merged 1 commit into from
Dec 1, 2025

Conversation

@overlookmotel
Copy link
Member

@overlookmotel overlookmotel commented Dec 1, 2025 •
edited
Loading

Atom is just a wrapper around a &str, so make it #[repr(transparent)]. This may help perf in some circumstances.

@overlookmotel Graphite App
Copy link
Member Author

overlookmotel commented Dec 1, 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.

@github-actions github-actions bot added the C-performance Category - Solution not expected to change functional behavior, only performance label Dec 1, 2025
@overlookmotel overlookmotel marked this pull request as ready for review December 1, 2025 00:43
Copilot AI review requested due to automatic review settings December 1, 2025 00:43
Copilot finished reviewing on behalf of overlookmotel December 1, 2025 00:44
Copilot AI reviewed Dec 1, 2025
View reviewed changes
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds a performance optimization to the Atom<'a> struct by marking it with #[repr(transparent)]. Since Atom is a single-field wrapper around &str, this attribute ensures the compiler treats it identically to &str in terms of memory layout and calling conventions, potentially improving performance in FFI boundaries and function calls. This change aligns with established patterns used throughout the codebase for similar wrapper types like Box, Vec, and other single-field structs in the allocator and traverse modules.

  • Adds #[repr(transparent)] attribute to Atom<'a> struct
  • Ensures optimal ABI representation for the single-field wrapper
  • Maintains consistency with codebase patterns for similar wrapper types

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@codspeed-hq
Copy link

codspeed-hq bot commented Dec 1, 2025

CodSpeed Performance Report

Merging #16310 will not alter performance

Comparing 12-01-perf_span_add_repr_transparent_to_atom_ (9efe73c) with main (43a6c32)

Summary

✅ 42 untouched
⏩ 3 skipped1

Footnotes

  1. 3 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

@overlookmotel overlookmotel self-assigned this Dec 1, 2025
@overlookmotel overlookmotel mentioned this pull request Dec 1, 2025
Merged
@overlookmotel
Copy link
Member Author

overlookmotel commented Dec 1, 2025

Some positive results on benchmarks. Unclear if that's just noise or not. But it certainly won't hurt, so merging.

@graphite-app graphite-app bot added the 0-merge Merge with Graphite Merge Queue label Dec 1, 2025
@graphite-app
Copy link
Contributor

graphite-app bot commented Dec 1, 2025 •
edited
Loading

Merge activity

@overlookmotel
perf(span): add #[repr(transparent)] to Atom (#16310)
ffca070 
`Atom` is just a wrapper around a `&str`, so make it `#[repr(transparent)]`. This *may* help perf in some circumstances.
@graphite-app graphite-app bot force-pushed the 12-01-perf_span_add_repr_transparent_to_atom_ branch from 9efe73c to ffca070 Compare December 1, 2025 02:30
@graphite-app graphite-app bot merged commit ffca070 into main Dec 1, 2025
20 checks passed
@graphite-app graphite-app bot deleted the 12-01-perf_span_add_repr_transparent_to_atom_ branch December 1, 2025 02:36
@graphite-app graphite-app bot removed the 0-merge Merge with Graphite Merge Queue label Dec 1, 2025
@Boshen Boshen mentioned this pull request Dec 1, 2025
Merged
overlookmotel pushed a commit that referenced this pull request Dec 1, 2025
@Boshen
release(crates): oxc v0.100.0 (#16331)
0872758 
### 💥 BREAKING CHANGES

- 74cf572 ast: [**BREAKING**] Make `source` field of `TSImportType` a
`StringLiteral` (#16114) (copilot-swe-agent)
- 43156ae ast: [**BREAKING**] Rename `TSImportType` `argument` field to
`source` (#16110) (overlookmotel)
- 934d873 napi: [**BREAKING**] Drop `armv7-unknown-linux-musleabihf`
support (#16105) (Boshen)

### 🚀 Features

- 669afe0 ast: Add `Expression::is_jsx` method (#16154) (Dunqing)
- 17a8caa parser: Add diagnostic for JSX identifiers with hyphens
(#16133) (camchenry)
- 0549ae5 parser: Add diagnostic for expected ident after optional chain
(#16132) (camchenry)
- db839ae parser: Improve diagnostic for unexpected optional
declarations (#16131) (camchenry)
- bab4bc8 napi/parser: Add type annotations to parse-raw-worker test
(#15998) (camc314)

### 🐛 Bug Fixes

- 35ed36c traverse: Fix panic when truncating non-ASCII variable names
(#16265) (peter)
- 9149a26 linter/plugins, napi/parser: Deep freeze visitor keys (#16293)
(overlookmotel)
- 6b54dab minifier: Incorrect non-null object condition simplification
with `&&` and `||` (#16161) (sapphi-red)
- 9cc20a1 minifier: Avoid merging side effectful expressions to next
assignment statement if the side effect may change the left hand side
reference (#16165) (sapphi-red)
- 91eb3f2 ast/estree: Convert `TSImportType` `argument` field to
`Literal` (#16109) (overlookmotel)
- 1199cee parser: Reject invalid modifiers on parameter properties with
binding patterns (#16083) (camc314)
- f376325 traverse: Remove `console.log` from build script (#16049)
(overlookmotel)

### ⚡ Performance

- 82d784f lexer: Reduce bounds checks in `Lexer::get_string` (#16317)
(overlookmotel)
- cc2f352 span: Add `#[inline]` to `Atom` methods (#16311)
(overlookmotel)
- ffca070 span: Add `#[repr(transparent)]` to `Atom` (#16310)
(overlookmotel)
- 02bdf90 linter/plugins, napi/parser: Reuse arrays in visitor keys
(#16294) (overlookmotel)

### 📚 Documentation

- 891e0b4 parser: Add note about falling back to parse TSType in
TSImportType (#16119) (camc314)

Co-authored-by: Boshen <1430279+Boshen@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

Assignees

@overlookmotel overlookmotel

Labels

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.

2 participants

@overlookmotel