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 `mark_neighbours_as_waiting_from`. #64420

Merged

Conversation

@nnethercote
Copy link
Contributor

nnethercote commented Sep 13, 2019

This function is very hot, doesn't get inlined because it's recursive,
and the function calls are significant.

This commit splits it into inlined and uninlined variants, and uses the
inlined variant for the hot call site. This wins several percent on a
few benchmarks.

r? @nikomatsakis

This function is very hot, doesn't get inlined because it's recursive,
and the function calls are significant.

This commit splits it into inlined and uninlined variants, and uses the
inlined variant for the hot call site. This wins several percent on a
few benchmarks.
@nnethercote

This comment has been minimized.

Copy link
Contributor Author

nnethercote commented Sep 13, 2019

I will do a perf run:
@bors try

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Sep 13, 2019

⌛️ Trying commit a2261ad with merge 3013cf5...

bors added a commit that referenced this pull request Sep 13, 2019
…from, r=<try>

Inline `mark_neighbours_as_waiting_from`.

This function is very hot, doesn't get inlined because it's recursive,
and the function calls are significant.

This commit splits it into inlined and uninlined variants, and uses the
inlined variant for the hot call site. This wins several percent on a
few benchmarks.

r? @nikomatsakis
@bors

This comment has been minimized.

Copy link
Contributor

bors commented Sep 13, 2019

☀️ Try build successful - checks-azure
Build commit: 3013cf5

@Centril

This comment has been minimized.

Copy link
Member

Centril commented Sep 13, 2019

@rust-timer build 3013cf5

@rust-timer

This comment has been minimized.

Copy link

rust-timer commented Sep 13, 2019

Success: Queued 3013cf5 with parent f43ac06, comparison URL.

@rust-timer

This comment has been minimized.

Copy link

rust-timer commented Sep 13, 2019

Finished benchmarking try commit 3013cf5, comparison URL.

@nnethercote

This comment has been minimized.

Copy link
Contributor Author

nnethercote commented Sep 13, 2019

Perf results are good, up to 2.8% instruction count wins.

@Mark-Simulacrum

This comment has been minimized.

Copy link
Member

Mark-Simulacrum commented Sep 13, 2019

@bors r+ rollup

Marking as rollup since we already have the perf statistics.

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Sep 13, 2019

📌 Commit a2261ad has been approved by Mark-Simulacrum

Centril added a commit to Centril/rust that referenced this pull request Sep 14, 2019
…as_waiting_from, r=Mark-Simulacrum

Inline `mark_neighbours_as_waiting_from`.

This function is very hot, doesn't get inlined because it's recursive,
and the function calls are significant.

This commit splits it into inlined and uninlined variants, and uses the
inlined variant for the hot call site. This wins several percent on a
few benchmarks.

r? @nikomatsakis
bors added a commit that referenced this pull request Sep 14, 2019
Rollup of 17 pull requests

Successful merges:

 - #64116 (Fix minor typo in docs.)
 - #64290 (Provide a span if main function is not present in crate)
 - #64302 (Shrink `ObligationCauseCode`)
 - #64372 (use randSecure and randABytes)
 - #64374 (Box `DiagnosticBuilder`.)
 - #64375 (Fast path for vec.clear/truncate )
 - #64378 (Fix inconsistent link formatting.)
 - #64384 (Trim rustc-workspace-hack)
 - #64393 ( declare EnvKey before use to fix build error)
 - #64420 (Inline `mark_neighbours_as_waiting_from`.)
 - #64422 (Remove raw string literal quotes from error index descriptions)
 - #64423 (Add self to .mailmap)
 - #64424 (Fix false "never constructed" warnings for `Self::` variant paths)
 - #64425 (typo fix)
 - #64431 (fn ptr is structural match)
 - #64435 (codegen: use "_N" (like for other locals) instead of "argN", for argument names.)
 - #64439 (fix #64430, confusing `owned_box` error message in no_std build)

Failed merges:

r? @ghost
Centril added a commit to Centril/rust that referenced this pull request Sep 14, 2019
…as_waiting_from, r=Mark-Simulacrum

Inline `mark_neighbours_as_waiting_from`.

This function is very hot, doesn't get inlined because it's recursive,
and the function calls are significant.

This commit splits it into inlined and uninlined variants, and uses the
inlined variant for the hot call site. This wins several percent on a
few benchmarks.

r? @nikomatsakis
bors added a commit that referenced this pull request Sep 14, 2019
Rollup of 17 pull requests

Successful merges:

 - #63846 (Added table containing the system calls used by Instant and SystemTime.)
 - #64116 (Fix minor typo in docs.)
 - #64290 (Provide a span if main function is not present in crate)
 - #64302 (Shrink `ObligationCauseCode`)
 - #64372 (use randSecure and randABytes)
 - #64374 (Box `DiagnosticBuilder`.)
 - #64375 (Fast path for vec.clear/truncate )
 - #64378 (Fix inconsistent link formatting.)
 - #64384 (Trim rustc-workspace-hack)
 - #64393 ( declare EnvKey before use to fix build error)
 - #64420 (Inline `mark_neighbours_as_waiting_from`.)
 - #64422 (Remove raw string literal quotes from error index descriptions)
 - #64423 (Add self to .mailmap)
 - #64425 (typo fix)
 - #64431 (fn ptr is structural match)
 - #64435 (codegen: use "_N" (like for other locals) instead of "argN", for argument names.)
 - #64439 (fix #64430, confusing `owned_box` error message in no_std build)

Failed merges:

r? @ghost
Centril added a commit to Centril/rust that referenced this pull request Sep 14, 2019
…as_waiting_from, r=Mark-Simulacrum

Inline `mark_neighbours_as_waiting_from`.

This function is very hot, doesn't get inlined because it's recursive,
and the function calls are significant.

This commit splits it into inlined and uninlined variants, and uses the
inlined variant for the hot call site. This wins several percent on a
few benchmarks.

r? @nikomatsakis
bors added a commit that referenced this pull request Sep 14, 2019
Rollup of 17 pull requests

Successful merges:

 - #63846 (Added table containing the system calls used by Instant and SystemTime.)
 - #64116 (Fix minor typo in docs.)
 - #64203 (A few cosmetic improvements to code & comments in liballoc and libcore)
 - #64302 (Shrink `ObligationCauseCode`)
 - #64372 (use randSecure and randABytes)
 - #64374 (Box `DiagnosticBuilder`.)
 - #64375 (Fast path for vec.clear/truncate )
 - #64378 (Fix inconsistent link formatting.)
 - #64384 (Trim rustc-workspace-hack)
 - #64393 ( declare EnvKey before use to fix build error)
 - #64420 (Inline `mark_neighbours_as_waiting_from`.)
 - #64422 (Remove raw string literal quotes from error index descriptions)
 - #64423 (Add self to .mailmap)
 - #64425 (typo fix)
 - #64431 (fn ptr is structural match)
 - #64435 (codegen: use "_N" (like for other locals) instead of "argN", for argument names.)
 - #64439 (fix #64430, confusing `owned_box` error message in no_std build)

Failed merges:

r? @ghost
@bors bors merged commit a2261ad into rust-lang:master Sep 14, 2019
5 checks passed
5 checks passed
homu Test successful
Details
pr Build #20190913.4 succeeded
Details
pr (Linux mingw-check) Linux mingw-check succeeded
Details
pr (Linux x86_64-gnu-llvm-6.0) Linux x86_64-gnu-llvm-6.0 succeeded
Details
pr (LinuxTools) LinuxTools succeeded
Details
@nnethercote nnethercote deleted the nnethercote:inline-mark_neighbours_as_waiting_from branch Sep 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants
You can’t perform that action at this time.