Avoid copies while creating the metadata object file #89980

bjorn3 · 2021-10-17T12:58:13Z

This is now possible thanks to gimli-rs/object#369 and gimli-rs/object#370.

rust-highfive · 2021-10-17T12:58:16Z

r? @nagisa

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

bjorn3 · 2021-10-17T12:58:24Z

@bors try @rust-timer queue

rust-timer · 2021-10-17T12:58:26Z

Awaiting bors try build completion.

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

bors · 2021-10-17T12:58:32Z

⌛ Trying commit e94f6d53199fb5057549a7ac40d6789234627853 with merge 004de1ed6e374e4d0b15f135c005f65181a65388...

nagisa · 2021-10-17T13:12:55Z

Cargo.lock

@@ -4984,7 +4993,7 @@ dependencies = [
 "hermit-abi",
 "libc",
 "miniz_oxide",
- "object",
+ "object 0.26.2",


Consider updating this as well.

The backtrace crate will need to be updated first. While object 0.27 is mostly compatible with 0.26 I don't want to risk it breaking due to the backtrace crate depending on 0.26, but libstd then giving it the 0.27 version.

bors · 2021-10-17T14:27:11Z

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

rust-timer · 2021-10-17T14:27:12Z

Queued 004de1ed6e374e4d0b15f135c005f65181a65388 with parent 1d6f242, future comparison URL.

rust-timer · 2021-10-17T17:07:00Z

Finished benchmarking commit (004de1ed6e374e4d0b15f135c005f65181a65388): comparison url.

Summary: This change led to moderate relevant mixed results 🤷 in compiler performance.

Moderate improvement in instruction counts (up to -0.7% on incr-unchanged builds of deeply-nested-async)
Small regression in instruction counts (up to 0.2% on incr-full builds of clap-rs)

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR led to changes in compiler perf.

Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please fix the regressions and do another perf run. If the next run shows neutral or positive results, the label will be automatically removed.

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

This avoids a clone of the section data

bjorn3 · 2021-10-17T17:14:48Z

In the last commit I forgot to replace an occurrence of append_section_data with set_section_data causing this commit to only improve perf on windows and macos. Fixed it now.

@bors try @rust-timer queue

rust-timer · 2021-10-17T17:14:49Z

Awaiting bors try build completion.

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

bors · 2021-10-17T17:14:55Z

⌛ Trying commit 2e42950 with merge 1d2e2ee78a3c9f24af580e6d95c9ee90ff366a43...

bors · 2021-10-17T18:42:21Z

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

rust-timer · 2021-10-17T18:42:22Z

Queued 1d2e2ee78a3c9f24af580e6d95c9ee90ff366a43 with parent 6f53ddf, future comparison URL.

fbstj · 2021-10-17T19:29:50Z

compiler/rustc_codegen_ssa/src/back/link.rs

+            file
+        };
+        let out_file = match fs::File::create(out_filename) {
+            Ok(out_file) => io::BufWriter::new(out_file),


could this be an Result::and_then() chain? to deduplicate the error messages... or are the messages going to be "improved" somehow to be being different?

rust-timer · 2021-10-17T20:15:19Z

Finished benchmarking commit (1d2e2ee78a3c9f24af580e6d95c9ee90ff366a43): comparison url.

Summary: This change led to moderate relevant mixed results 🤷 in compiler performance.

Moderate improvement in instruction counts (up to -0.7% on incr-unchanged builds of deeply-nested-async)
Small regression in instruction counts (up to 0.6% on incr-full builds of ctfe-stress-4)

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR led to changes in compiler perf.

Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please fix the regressions and do another perf run. If the next run shows neutral or positive results, the label will be automatically removed.

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

This reverts commit 02f1adf.

bjorn3 · 2021-10-25T11:17:41Z

Let's try only the set_section_data change and not using write_stream.

@bors try @rust-timer queue

rust-timer · 2021-10-25T11:17:43Z

Awaiting bors try build completion.

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

bors · 2021-10-25T11:17:50Z

⌛ Trying commit a3a8a2b with merge 2129c06d54db38da7d32318e20dd5b9b4bedf6b9...

bors · 2021-10-25T14:09:47Z

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

rust-timer · 2021-10-25T14:09:48Z

Queued 2129c06d54db38da7d32318e20dd5b9b4bedf6b9 with parent 56694b0, future comparison URL.

Mark-Simulacrum · 2021-10-25T17:55:49Z

@bors try @rust-timer queue

rust-timer · 2021-10-25T17:55:50Z

Awaiting bors try build completion.

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

bors · 2021-10-25T17:55:57Z

⌛ Trying commit a3a8a2b with merge 3342d61e41045577b63c09dc68e4d2385b8a8e6e...

bors · 2021-10-25T19:27:58Z

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

bjorn3 · 2021-10-29T14:35:04Z

@rust-timer check 3342d61e41045577b63c09dc68e4d2385b8a8e6e

bjorn3 · 2021-10-29T14:35:58Z

@rust-timer build 3342d61e41045577b63c09dc68e4d2385b8a8e6e

rust-timer · 2021-10-29T14:36:00Z

Queued 3342d61e41045577b63c09dc68e4d2385b8a8e6e with parent 84c2a85, future comparison URL.

rust-timer · 2021-10-29T16:07:10Z

Finished benchmarking commit (3342d61e41045577b63c09dc68e4d2385b8a8e6e): comparison url.

Summary: This benchmark run did not return any relevant changes.

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR led to changes in compiler perf.

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

bjorn3 · 2021-10-29T16:16:08Z

set_section_data without write_stream is a small improvement on average it seems, but very close to noise.

nagisa · 2021-11-12T10:04:43Z

@bjorn3 do you think we should still land this despite no significant performance improvements? The changes seem non-intrusive enough that I'm happy to see this landing either way. (r=me, your call if you want to land this or not)

bjorn3 · 2021-11-12T12:14:40Z

I think it is worth it. It should wait for backtrace-rs to update it's object dependency first to avoid multiple versions of object being used I think.

bjorn3 added 2 commits October 17, 2021 14:53

Update object to 0.27

57ff5fa

Use Object::write_stream

02f1adf

bjorn3 added A-metadata Area: Crate metadata T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Oct 17, 2021

rust-highfive assigned nagisa Oct 17, 2021

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 17, 2021

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Oct 17, 2021

nagisa reviewed Oct 17, 2021

View reviewed changes

rustbot added perf-regression Performance regression. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels Oct 17, 2021

bjorn3 force-pushed the faster_metadata_writing branch from e94f6d5 to 22abfca Compare October 17, 2021 17:13

Use set_section_data instead of append_section_data

2e42950

This avoids a clone of the section data

bjorn3 force-pushed the faster_metadata_writing branch from 22abfca to 2e42950 Compare October 17, 2021 17:13

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Oct 17, 2021

fbstj reviewed Oct 17, 2021

View reviewed changes

rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Oct 17, 2021

Revert "Use Object::write_stream"

a3a8a2b

This reverts commit 02f1adf.

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Oct 25, 2021

rustbot removed S-waiting-on-perf Status: Waiting on a perf run to be completed. perf-regression Performance regression. labels Oct 29, 2021

nagisa added S-blocked Status: Marked as blocked ❌ on something else such as an RFC or other implementation work. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 19, 2021

bjorn3 closed this Feb 12, 2022

bjorn3 deleted the faster_metadata_writing branch February 12, 2022 12:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Avoid copies while creating the metadata object file #89980

Avoid copies while creating the metadata object file #89980

bjorn3 commented Oct 17, 2021

rust-highfive commented Oct 17, 2021

bjorn3 commented Oct 17, 2021

rust-timer commented Oct 17, 2021

bors commented Oct 17, 2021

nagisa Oct 17, 2021

bjorn3 Oct 17, 2021

bors commented Oct 17, 2021

rust-timer commented Oct 17, 2021

rust-timer commented Oct 17, 2021

bjorn3 commented Oct 17, 2021

rust-timer commented Oct 17, 2021

bors commented Oct 17, 2021

bors commented Oct 17, 2021

rust-timer commented Oct 17, 2021

fbstj Oct 17, 2021

rust-timer commented Oct 17, 2021

bjorn3 commented Oct 25, 2021

rust-timer commented Oct 25, 2021

bors commented Oct 25, 2021

bors commented Oct 25, 2021

rust-timer commented Oct 25, 2021

Mark-Simulacrum commented Oct 25, 2021

rust-timer commented Oct 25, 2021

bors commented Oct 25, 2021

bors commented Oct 25, 2021

bjorn3 commented Oct 29, 2021

bjorn3 commented Oct 29, 2021

rust-timer commented Oct 29, 2021

rust-timer commented Oct 29, 2021

bjorn3 commented Oct 29, 2021

nagisa commented Nov 12, 2021

bjorn3 commented Nov 12, 2021

Avoid copies while creating the metadata object file #89980

Avoid copies while creating the metadata object file #89980

Conversation

bjorn3 commented Oct 17, 2021

rust-highfive commented Oct 17, 2021

bjorn3 commented Oct 17, 2021

rust-timer commented Oct 17, 2021

bors commented Oct 17, 2021

nagisa Oct 17, 2021

Choose a reason for hiding this comment

bjorn3 Oct 17, 2021

Choose a reason for hiding this comment

bors commented Oct 17, 2021

rust-timer commented Oct 17, 2021

rust-timer commented Oct 17, 2021

bjorn3 commented Oct 17, 2021

rust-timer commented Oct 17, 2021

bors commented Oct 17, 2021

bors commented Oct 17, 2021

rust-timer commented Oct 17, 2021

fbstj Oct 17, 2021

Choose a reason for hiding this comment

rust-timer commented Oct 17, 2021

bjorn3 commented Oct 25, 2021

rust-timer commented Oct 25, 2021

bors commented Oct 25, 2021

bors commented Oct 25, 2021

rust-timer commented Oct 25, 2021

Mark-Simulacrum commented Oct 25, 2021

rust-timer commented Oct 25, 2021

bors commented Oct 25, 2021

bors commented Oct 25, 2021

bjorn3 commented Oct 29, 2021

bjorn3 commented Oct 29, 2021

rust-timer commented Oct 29, 2021

rust-timer commented Oct 29, 2021

bjorn3 commented Oct 29, 2021

nagisa commented Nov 12, 2021

bjorn3 commented Nov 12, 2021