Skip to content
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.

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

#[inline(always)] on basic pointer methods #85218

Merged
merged 1 commit into from
May 13, 2021

Conversation

kornelski
Copy link
Contributor

Retryng #85201 with only inlining pointer methods. The goal is to make pointers behave just like pointers in O0, mainly to reduce overhead in debug builds.

cc @scottmcm

@rust-highfive
Copy link
Collaborator

r? @yaahc

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 12, 2021
@lqd
Copy link
Member

lqd commented May 12, 2021

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion.

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

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label May 12, 2021
@bors
Copy link
Contributor

bors commented May 12, 2021

⌛ Trying commit 3773740 with merge 28719fcb7d5f5e5565adb081bd8333b720d2b12e...

@bors
Copy link
Contributor

bors commented May 12, 2021

☀️ Try build successful - checks-actions
Build commit: 28719fcb7d5f5e5565adb081bd8333b720d2b12e (28719fcb7d5f5e5565adb081bd8333b720d2b12e)

@rust-timer
Copy link
Collaborator

Queued 28719fcb7d5f5e5565adb081bd8333b720d2b12e with parent ac923d9, future comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking try commit (28719fcb7d5f5e5565adb081bd8333b720d2b12e): comparison url.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. Please note that if the perf results are neutral, you should likely undo the rollup=never given below by specifying rollup- to bors.

Importantly, though, if the results of this run are non-neutral do not roll this PR up -- it will mask other regressions or improvements in the roll up.

@bors rollup=never
@rustbot label: +S-waiting-on-review -S-waiting-on-perf

@rustbot rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label May 12, 2021
@bjorn3
Copy link
Member

bjorn3 commented May 12, 2021

Improvements up to 0.7% with an outlier to 1.7% improvement. A couple of small regressions up to 0.2%. Overall a nice win.

@scottmcm
Copy link
Member

Sounds good. I agree it makes sense for things like not pessimizing the method versions of pointer functions.

r? @scottmcm
@bors r+ rollup=never

@bors
Copy link
Contributor

bors commented May 12, 2021

📌 Commit 3773740 has been approved by scottmcm

@rust-highfive rust-highfive assigned scottmcm and unassigned yaahc May 12, 2021
@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 12, 2021
@bors
Copy link
Contributor

bors commented May 12, 2021

⌛ Testing commit 3773740 with merge 31bd868...

@bors
Copy link
Contributor

bors commented May 13, 2021

☀️ Test successful - checks-actions
Approved by: scottmcm
Pushing 31bd868 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label May 13, 2021
@bors bors merged commit 31bd868 into rust-lang:master May 13, 2021
@rustbot rustbot added this to the 1.54.0 milestone May 13, 2021
@kornelski kornelski deleted the pointerinline branch May 13, 2021 00:35
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 9, 2022
Make some trivial functions `#[inline(always)]`

This is some kind of follow-up of PRs like rust-lang#85218, rust-lang#84061, rust-lang#87150. Functions that do very basic operations are made `#[inline(always)]` to avoid pessimizing them in debug builds when compared to using built-in operations directly.
RalfJung pushed a commit to RalfJung/miri that referenced this pull request Dec 11, 2022
Make some trivial functions `#[inline(always)]`

This is some kind of follow-up of PRs like rust-lang/rust#85218, rust-lang/rust#84061, rust-lang/rust#87150. Functions that do very basic operations are made `#[inline(always)]` to avoid pessimizing them in debug builds when compared to using built-in operations directly.
thomcc pushed a commit to tcdi/postgrestd that referenced this pull request Feb 10, 2023
Make some trivial functions `#[inline(always)]`

This is some kind of follow-up of PRs like rust-lang/rust#85218, rust-lang/rust#84061, rust-lang/rust#87150. Functions that do very basic operations are made `#[inline(always)]` to avoid pessimizing them in debug builds when compared to using built-in operations directly.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants