Skip to content

Conversation

bjorn3
Copy link
Member

@bjorn3 bjorn3 commented Mar 30, 2021

This wrapper implements StableAddress and falls back to directly reading the file on wasm32.

Taken from #83640, which I will close due to the perf regression.

This wrapper implements StableAddress and falls back to directly reading
the file on wasm32
@rust-highfive
Copy link
Contributor

Some changes occured to rustc_codegen_cranelift

cc @bjorn3

@rust-highfive
Copy link
Contributor

r? @lcnr

(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 Mar 30, 2021
@bjorn3
Copy link
Member Author

bjorn3 commented Mar 30, 2021

I don't think this is the cause of the perf regression in #83640, but just in case.

@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 Mar 30, 2021
@bors
Copy link
Collaborator

bors commented Mar 30, 2021

⌛ Trying commit 8331dbe with merge 2d883ace5ff221db332b7155122d99e1cb8a3375...

@cjgillot
Copy link
Contributor

Commit 8fcd03e7358ccb8f568ab3b34f3f31fac4b6ede9 also seems interessant to keep.

@bjorn3
Copy link
Member Author

bjorn3 commented Mar 30, 2021

It seemed to be a tiny (<0.1%) regression on the other PR. I can to benchmark after this commit.

@bors
Copy link
Collaborator

bors commented Mar 30, 2021

☀️ Try build successful - checks-actions
Build commit: 2d883ace5ff221db332b7155122d99e1cb8a3375 (2d883ace5ff221db332b7155122d99e1cb8a3375)

@rust-timer
Copy link
Collaborator

Queued 2d883ace5ff221db332b7155122d99e1cb8a3375 with parent 16156fb, future comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking try commit (2d883ace5ff221db332b7155122d99e1cb8a3375): 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 Mar 30, 2021
@bjorn3
Copy link
Member Author

bjorn3 commented Mar 31, 2021

This seems to be a slight regression on a few benchmarks. Trying with a few methods inlined.

@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 Mar 31, 2021
@bors
Copy link
Collaborator

bors commented Mar 31, 2021

⌛ Trying commit 5773e51 with merge d95b884a112e6be1fc0a5c178a5404c031f491a8...

@bors
Copy link
Collaborator

bors commented Mar 31, 2021

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

@rust-timer
Copy link
Collaborator

Queued d95b884a112e6be1fc0a5c178a5404c031f491a8 with parent 6ff482b, future comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking try commit (d95b884a112e6be1fc0a5c178a5404c031f491a8): 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 Mar 31, 2021
@bjorn3
Copy link
Member Author

bjorn3 commented Mar 31, 2021

That seems to be slightly better. Now trying with 8fcd03e7358ccb8f568ab3b34f3f31fac4b6ede9 cherry-picked.

@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 Mar 31, 2021
@bors
Copy link
Collaborator

bors commented Mar 31, 2021

⌛ Trying commit 88a7e8f2248260f4af7910a0e14e8d672afede9b with merge c6b3d6133f84d4c800af3b32f46663a91f0ab432...

@bors
Copy link
Collaborator

bors commented Mar 31, 2021

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

@rust-timer
Copy link
Collaborator

Queued c6b3d6133f84d4c800af3b32f46663a91f0ab432 with parent a5029ac, future comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking try commit (c6b3d6133f84d4c800af3b32f46663a91f0ab432): 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

@bjorn3
Copy link
Member Author

bjorn3 commented Mar 31, 2021

Nope, that is a regression. Maybe due to the double metadata call by both this code and memmap2? I reverted it again. Ready for review.

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

bjorn3 commented Apr 3, 2021

Added the safety comment.

@bors r=cjgillot

@bors
Copy link
Collaborator

bors commented Apr 3, 2021

📌 Commit bda6d1f has been approved by cjgillot

@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 Apr 3, 2021
@bors
Copy link
Collaborator

bors commented Apr 3, 2021

⌛ Testing commit bda6d1f with merge 97717a5...

@bors
Copy link
Collaborator

bors commented Apr 3, 2021

☀️ Test successful - checks-actions
Approved by: cjgillot
Pushing 97717a5 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Apr 3, 2021
@bors bors merged commit 97717a5 into rust-lang:master Apr 3, 2021
@rustbot rustbot added this to the 1.53.0 milestone Apr 3, 2021
@bjorn3 bjorn3 deleted the mmap_wrapper branch April 3, 2021 16:05
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.

7 participants