Skip to content

Mark NonNull read family and offset_from as inline(always) - #161004

Closed
zakrad wants to merge 1 commit into
rust-lang:mainfrom
zakrad:fix-nonnull-read-inline
Closed

Mark NonNull read family and offset_from as inline(always)#161004
zakrad wants to merge 1 commit into
rust-lang:mainfrom
zakrad:fix-nonnull-read-inline

Conversation

@zakrad

@zakrad zakrad commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

View all comments

These are one line wrappers that just forward to the raw pointer funcs that are all #[inline(always)], as are NonNull's own write and offset, The read and offset_from were left as plain #[inline]

read, read_volatile, read_unaligned, and offset_from are the only NonNull
pointer methods still marked inline; the other 17 are inline(always), as are
the raw const/mut pointer equivalents. They are one-line delegating wrappers,
so make them consistent -- they now inline in debug like their siblings.
@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Aug 12, 2026
@rustbot

rustbot commented Aug 12, 2026

Copy link
Copy Markdown
Collaborator

r? @clarfonthey

rustbot has assigned @clarfonthey.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: libs
  • libs expanded to 12 candidates
  • Random selection from JohnTitor, Mark-Simulacrum, clarfonthey, nia-e

@clarfonthey

Copy link
Copy Markdown
Contributor

Do you have any particular evidence this helps? I figure it probably does, but I could be wrong.

@zakrad

zakrad commented Aug 12, 2026

Copy link
Copy Markdown
Contributor Author

Do you have any particular evidence this helps? I figure it probably does, but I could be wrong.

The raw pointer funcs these wrap are #[inline(always)] #160816 NonNull read + offset_from left out, codegen wise the difference shows in debug, at -Copt-level=0 #[inline] stays an out of line call, while #[inline(always)] gets inlined, its not a release perf change but consistency plus tiny unoptimized codegen

@clarfonthey

Copy link
Copy Markdown
Contributor

Right, I know what inline(always) does, but I mostly am asking whether this actually noticeably improves performance without affecting build times, since that's not always clear.

@zakrad

zakrad commented Aug 13, 2026

Copy link
Copy Markdown
Contributor Author

No evidence it does, it was mostly for consistency, we can have perf run tho

@clarfonthey

clarfonthey commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

@bors try

@rust-bors

This comment has been minimized.

rust-bors Bot pushed a commit that referenced this pull request Aug 18, 2026
Mark NonNull read family and offset_from as inline(always)
@clarfonthey

Copy link
Copy Markdown
Contributor

@rust-timer queue

@rust-timer

This comment has been minimized.

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Aug 18, 2026
@rust-bors

rust-bors Bot commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

☀️ Try build successful (CI)
Build commit: 0a988f7 (0a988f73b92c408de20490bfee5e5a2c20460e80)
Base parent: e71c0f1 (e71c0f1e3395b10a8c331317be1a5c107bdf7b2e)

@rust-timer

This comment has been minimized.

@rust-timer

Copy link
Copy Markdown
Collaborator

Finished benchmarking commit (0a988f7): comparison URL.

Overall result: no relevant changes - no action needed

Benchmarking means the PR may be perf-sensitive. Consider adding rollup=never if this change is not fit for rolling up.

@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

This perf run didn't have relevant results for this metric.

Max RSS (memory usage)

Results (secondary 0.6%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
0.6% [0.6%, 0.6%] 1
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) - - 0

Cycles

This perf run didn't have relevant results for this metric.

Binary size

This perf run didn't have relevant results for this metric.

Bootstrap: 456.919s -> 458.765s (0.40%)
Artifact size: 398.99 MiB -> 399.01 MiB (0.01%)

@rustbot rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Aug 18, 2026
@clarfonthey

Copy link
Copy Markdown
Contributor

@rust-timer build 0a988f7 profiles=debug

@rust-timer

Copy link
Copy Markdown
Collaborator

Cannot attach SHAs to try benchmark request on PR 161004 and SHA 0a988f7: failed to attach SHAs to try benchmark request

Caused by:
0: db error
1: ERROR: duplicate key value violates unique constraint "benchmark_request_tag_key"
DETAIL: Key (tag)=(0a988f7) already exists.

@clarfonthey

Copy link
Copy Markdown
Contributor

@bors try
@rust-timer queue profiles=debug

@rust-timer

This comment has been minimized.

@rust-bors

This comment has been minimized.

rust-bors Bot pushed a commit that referenced this pull request Aug 18, 2026
Mark NonNull read family and offset_from as inline(always)
@clarfonthey

Copy link
Copy Markdown
Contributor

@rust-timer build 93f964f profiles=debug

@rust-timer

Copy link
Copy Markdown
Collaborator

Missing artifact for sha 93f964f3b6c9a41e2cc599d2699f5e1aab15459f (https://ci-artifacts.rust-lang.org/rustc-builds/93f964f3b6c9a41e2cc599d2699f5e1aab15459f/rustc-nightly-x86_64-unknown-linux-gnu.tar.xz); not built yet, try again later.

@rust-bors

rust-bors Bot commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

☀️ Try build successful (CI)
Build commit: 5e7f312 (5e7f312343b2fce41af58de929a6042926fb6715)
Base parent: e71c0f1 (e71c0f1e3395b10a8c331317be1a5c107bdf7b2e)

@rust-timer

This comment has been minimized.

@rust-timer

Copy link
Copy Markdown
Collaborator

Finished benchmarking commit (5e7f312): comparison URL.

Overall result: no relevant changes - no action needed

Benchmarking means the PR may be perf-sensitive. Consider adding rollup=never if this change is not fit for rolling up.

@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

This perf run didn't have relevant results for this metric.

Max RSS (memory usage)

Results (primary 1.8%, secondary 2.2%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)
1.8% [1.3%, 2.2%] 4
Regressions ❌
(secondary)
2.2% [1.1%, 4.1%] 11
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 1.8% [1.3%, 2.2%] 4

Cycles

Results (secondary 11.3%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
11.3% [11.3%, 11.3%] 1
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) - - 0

Binary size

This perf run didn't have relevant results for this metric.

Bootstrap: 456.919s -> 457.847s (0.20%)
Artifact size: 398.99 MiB -> 399.66 MiB (0.17%)

@clarfonthey

Copy link
Copy Markdown
Contributor

Unless you have any actual benchmarks to share I'm just going to close this tbh

@zakrad

zakrad commented Aug 19, 2026

Copy link
Copy Markdown
Contributor Author

no benchmarks, fine to close

@zakrad zakrad closed this Aug 19, 2026
@rustbot rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 19, 2026
@zakrad
zakrad deleted the fix-nonnull-read-inline branch August 19, 2026 18:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

T-libs Relevant to the library team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants