Faster sir decoding #116

bjorn3 · 2020-09-05T09:05:45Z

No description provided.

vext01 · 2020-09-05T15:21:56Z

yktrace/src/hwt/mapper.rs

-            .section_data_by_name(id.name())
-            .unwrap_or(borrow::Cow::Borrowed(&[][..])))
+            .section_by_name(id.name())
+            .map(|sec| sec.data().expect("failed to decompress section"))


What impact does compression have on performance?

I don't know. ykrustc is not emitting compressed elf sections. They are an ELF extension that is as far as I know only used for debug info in some cases.

vext01 · 2020-09-05T15:22:23Z

yktrace/src/sir.rs

@@ -64,7 +65,9 @@ lazy_static! {

 impl Sir {
    pub fn read_file(file: &Path) -> Result<Sir, ()> {
-        let ef = elf::File::open_path(file).unwrap();
+        // SAFETY: Not really, we hope that nobody changes the file underneath our feet.
+        let data = unsafe { Mmap::map(&File::open(file).unwrap()).unwrap() };


So mmap is faster?

There are two options when using object. Either you load the whole file into a Vec<u8> or you mmap the file. The former would also load the .text and .data sections, not just the .yksir_* sections. Rustc also uses mmap for metadata loading. I have not benchmarked both approaches though.

vext01 · 2020-09-05T15:22:50Z

Thanks for this.

How much time does all of this shave off?

bjorn3 · 2020-09-05T16:31:33Z

In debug mode:

Benchmark #1: ./ykview_master_debug ./yktrace_test_master
  Time (mean ± σ):      8.852 s ±  0.094 s    [User: 8.636 s, System: 0.216 s]
  Range (min … max):    8.777 s …  9.103 s    10 runs
 
Benchmark #2: ./ykview_faster_sir_decoding_debug ./yktrace_test_faster_sir_decoding
  Time (mean ± σ):      6.552 s ±  0.010 s    [User: 6.435 s, System: 0.117 s]
  Range (min … max):    6.536 s …  6.564 s    10 runs
 
Summary
  './ykview_faster_sir_decoding_debug ./yktrace_test_faster_sir_decoding' ran
    1.35 ± 0.01 times faster than './ykview_master_debug ./yktrace_test_master'

File size:

-rwxr-xr-x 1 bjorn3 bjorn3  84M Sep  5 17:14 yktrace_test_faster_sir_decoding
-rwxr-xr-x 1 bjorn3 bjorn3 110M Sep  5 16:55 yktrace_test_master

It is faster and produces smaller SIR.

Bonus benchmark

In release mode:

Benchmark #1: ./ykview_master ./yktrace_test_master
  Time (mean ± σ):      1.043 s ±  0.013 s    [User: 847.8 ms, System: 194.7 ms]
  Range (min … max):    1.035 s …  1.093 s    20 runs
 
  Warning: Statistical outliers were detected. Consider re-running this benchmark on a quiet PC without any interferences from other programs. It might help to use the '--warmup' or '--prepare' options.
 
Benchmark #2: ./ykview_faster_sir_decoding ./yktrace_test_faster_sir_decoding
  Time (mean ± σ):     666.1 ms ±   1.7 ms    [User: 558.0 ms, System: 108.0 ms]
  Range (min … max):   663.7 ms … 669.6 ms    20 runs
 
Summary
  './ykview_faster_sir_decoding ./yktrace_test_faster_sir_decoding' ran
    1.57 ± 0.02 times faster than './ykview_master ./yktrace_test_master'

vext01 · 2020-09-05T19:12:44Z

yktrace/src/sir.rs

 };
 use ykpack::{self, bodyflags, Body, CguHash, Decoder, Local, Pack, Ty};
+use memmap::Mmap;
+use object::{ObjectSection, Object}; // FIXME kill.


What's the FIXME about?

Huh, I can't remember writing that.

What is even weirder is that that line was created using the rust-analyzer import assist as far as I know.

vext01 · 2020-09-05T19:13:25Z

I wish we could build just the SIR loading stuff with optimisations. We never need to trace that so we can optimise to our heart's content!

vext01 · 2020-09-05T19:15:12Z

Related:
rust-lang/rust#54882

bjorn3 · 2020-09-05T19:18:37Z

Would it be possible to build everything except for ykrt without tracing and then make ykrt call into the rest of the crates using #[do_not_trace] wrappers when necessary?

vext01 · 2020-09-06T11:11:51Z

(I'm debugging a buildbot problem, please ignore these bors trys)

bors try

bors · 2020-09-06T11:12:34Z

try

Build failed:

buildbot/ykrustc-buildbot-build-script-hwt

vext01 · 2020-09-07T09:48:37Z

Would it be possible to build everything except for ykrt without tracing and then make ykrt call into the rest of the crates using #[do_not_trace] wrappers when necessary?

In theory yes, I guess we could force the optimisation flags in those crates (in Cargo.toml)?

vext01 · 2020-09-07T09:50:03Z

Please squash.

bjorn3 · 2020-09-07T09:54:13Z

Splat

vext01 · 2020-09-07T09:57:01Z

bors r+

116: Faster sir decoding r=vext01 a=bjorn3 Co-authored-by: bjorn3 <[email protected]>

bors · 2020-09-07T09:58:01Z

Build failed:

buildbot/ykrustc-buildbot-build-script-hwt

vext01 · 2020-09-07T09:59:47Z

bors r+

116: Faster sir decoding r=vext01 a=bjorn3 Co-authored-by: bjorn3 <[email protected]>

bjorn3 · 2020-09-07T10:00:17Z

Fixed rustfmt. This will need a ykrustc PR too because of the switch to bincode.

bors · 2020-09-07T10:01:42Z

Build failed:

buildbot/ykrustc-buildbot-build-script-hwt

vext01 · 2020-09-07T10:04:08Z

Fixed rustfmt. This will need a ykrustc PR too because of the switch to bincode.

Do you just need a rebuild of ykrustc, or are there code changes?

bjorn3 · 2020-09-07T10:07:44Z

Just a rebuild against the new ykpack. Opened as softdevteam/ykrustc#128. Can you check that I made all changes necessary for cycle breaking?

bjorn3 · 2020-09-07T10:08:05Z

Wait, I need to fix a warning in this PR.

vext01 · 2020-09-07T11:06:53Z

Can we wait for the upstream sync to go in before merging this?

I'm just stuck with:
https://rust-lang.zulipchat.com/#narrow/stream/182449-t-compiler.2Fhelp/topic/path.20trimming.20problem

bjorn3 · 2020-09-07T11:09:03Z

The ykrustc PR already got a bors try, so the cached rustc already this PR.

vext01 · 2020-09-07T11:11:41Z

OK. I'll probably end up having to redo the sync. Rebasing pretty much never works well for them.

bors r+

bors · 2020-09-07T11:14:48Z

Build succeeded:

buildbot/ykrustc-buildbot-build-script-hwt

Use bincode instead of msgpack for SIR serialization

eb50f54

vext01 reviewed Sep 5, 2020

View reviewed changes

bors bot added a commit that referenced this pull request Sep 6, 2020

Try #116:

54bb098

vext01 self-assigned this Sep 7, 2020

bjorn3 force-pushed the faster_sir_decoding branch from d7695b9 to 529fdb9 Compare September 7, 2020 09:53

bors bot added a commit that referenced this pull request Sep 7, 2020

Merge #116

9c52be9

116: Faster sir decoding r=vext01 a=bjorn3 Co-authored-by: bjorn3 <[email protected]>

bjorn3 force-pushed the faster_sir_decoding branch from 529fdb9 to 94224da Compare September 7, 2020 09:59

bors bot added a commit that referenced this pull request Sep 7, 2020

Merge #116

6734417

116: Faster sir decoding r=vext01 a=bjorn3 Co-authored-by: bjorn3 <[email protected]>

bjorn3 mentioned this pull request Sep 7, 2020

Faster sir decoding softdevteam/ykrustc#128

Merged

Use object instead of elf for object file reading

99637e6

bjorn3 force-pushed the faster_sir_decoding branch from 94224da to 99637e6 Compare September 7, 2020 10:09

bors bot merged commit 2c909e2 into ykjit:master Sep 7, 2020

bjorn3 deleted the faster_sir_decoding branch September 7, 2020 11:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Faster sir decoding #116

Faster sir decoding #116

bjorn3 commented Sep 5, 2020

vext01 Sep 5, 2020

bjorn3 Sep 5, 2020

vext01 Sep 5, 2020

bjorn3 Sep 5, 2020

vext01 commented Sep 5, 2020

bjorn3 commented Sep 5, 2020

vext01 Sep 5, 2020

bjorn3 Sep 5, 2020

bjorn3 Sep 5, 2020

vext01 commented Sep 5, 2020

vext01 commented Sep 5, 2020

bjorn3 commented Sep 5, 2020

vext01 commented Sep 6, 2020

bors bot commented Sep 6, 2020

vext01 commented Sep 7, 2020

vext01 commented Sep 7, 2020

bjorn3 commented Sep 7, 2020

vext01 commented Sep 7, 2020

bors bot commented Sep 7, 2020

vext01 commented Sep 7, 2020

bjorn3 commented Sep 7, 2020

bors bot commented Sep 7, 2020

vext01 commented Sep 7, 2020

bjorn3 commented Sep 7, 2020

bjorn3 commented Sep 7, 2020

vext01 commented Sep 7, 2020

bjorn3 commented Sep 7, 2020 •

edited

Loading

vext01 commented Sep 7, 2020

bors bot commented Sep 7, 2020

Faster sir decoding #116

Faster sir decoding #116

Conversation

bjorn3 commented Sep 5, 2020

vext01 Sep 5, 2020

Choose a reason for hiding this comment

bjorn3 Sep 5, 2020

Choose a reason for hiding this comment

vext01 Sep 5, 2020

Choose a reason for hiding this comment

bjorn3 Sep 5, 2020

Choose a reason for hiding this comment

vext01 commented Sep 5, 2020

bjorn3 commented Sep 5, 2020

vext01 Sep 5, 2020

Choose a reason for hiding this comment

bjorn3 Sep 5, 2020

Choose a reason for hiding this comment

bjorn3 Sep 5, 2020

Choose a reason for hiding this comment

vext01 commented Sep 5, 2020

vext01 commented Sep 5, 2020

bjorn3 commented Sep 5, 2020

vext01 commented Sep 6, 2020

bors bot commented Sep 6, 2020

try

vext01 commented Sep 7, 2020

vext01 commented Sep 7, 2020

bjorn3 commented Sep 7, 2020

vext01 commented Sep 7, 2020

bors bot commented Sep 7, 2020

vext01 commented Sep 7, 2020

bjorn3 commented Sep 7, 2020

bors bot commented Sep 7, 2020

vext01 commented Sep 7, 2020

bjorn3 commented Sep 7, 2020

bjorn3 commented Sep 7, 2020

vext01 commented Sep 7, 2020

bjorn3 commented Sep 7, 2020 • edited Loading

vext01 commented Sep 7, 2020

bors bot commented Sep 7, 2020

bjorn3 commented Sep 7, 2020 •

edited

Loading