Compress `Liveness` data some more. #54420

nnethercote · 2018-09-21T10:57:02Z

Profiling shows that the (reader, writer, used) triples used by
liveness analysis almost always have invalid reader and writer
fields. We can take advantage of this knowledge to use a compressed
representation for them, falling back to a secondary table for the
uncommon cases.

This change reduces instruction counts on numerous benchmarks, the best
by 16%. It also reduces max-rss on numerous benchmarks, the best by 38%.

The patch also renames these triples from Users to RWU, because it's
confusing having a type whose name is plural and then used within
vectors whose names are also plural.

r? @nikomatsakis

Profiling shows that the `(reader, writer, used)` triples used by liveness analysis almost always have invalid `reader` and `writer` fields. We can take advantage of this knowledge to use a compressed representation for them, falling back to a secondary table for the uncommon cases. This change reduces instruction counts on numerous benchmarks, the best by 16%. It also reduces max-rss on numerous benchmarks, the best by 38%. The patch also renames these triples from `Users` to `RWU`, because it's confusing having a type whose name is plural and then used within vectors whose names are also plural.

nnethercote · 2018-09-21T10:59:45Z

I know I just changed this code in #54211, but I came up with this better way of handling things and couldn't resist trying. It worked better than I expected. Instruction counts:

keccak-check
        avg: -5.0%      min: -16.2%     max: -1.2%
keccak-debug
        avg: -6.1%      min: -14.0%     max: -1.5%
keccak-opt
        avg: -5.5%      min: -14.0%     max: -1.2%
inflate-check
        avg: -2.4%      min: -8.3%      max: -0.9%
inflate-opt
        avg: -2.0%?     min: -6.8%?     max: -0.3%?
inflate-debug
        avg: -2.6%      min: -6.4%      max: -1.3%
clap-rs-check
        avg: -2.4%      min: -4.7%      max: -0.8%
clap-rs-debug
        avg: -2.6%      min: -3.7%      max: -2.0%
clap-rs-opt
        avg: -1.5%      min: -2.5%      max: -1.0%

max-rss:

keccak-check
        avg: -27.4%     min: -38.7%     max: -0.1%
keccak-debug
        avg: -12.7%     min: -38.0%     max: 0.0%
keccak-opt
        avg: -12.7%     min: -38.0%     max: 0.0%
inflate-check
        avg: -6.2%      min: -18.4%     max: 0.2%
inflate-opt
        avg: -2.1%?     min: -9.3%?     max: 0.6%?
inflate-debug
        avg: -1.6%      min: -8.8%      max: 1.6%
clap-rs-debug
        avg: 1.6%       min: -0.6%      max: 7.2%
clap-rs-opt
        avg: -1.4%      min: -6.6%      max: 0.8%
clap-rs-check
        avg: -0.6%      min: -0.9%      max: -0.1%

I also tried a different representation, where each packed RWU was a single byte and the secondary table was an FxHashMap. It saved some more memory, but the instruction counts and wall-times were a bit higher, so I finalized on this vector approach.

nnethercote · 2018-09-21T10:59:53Z

@bors try

bors · 2018-09-21T11:00:06Z

⌛ Trying commit b2f25e3 with merge b777974...

@nikomatsakis

Compress `Liveness` data some more. Profiling shows that the `(reader, writer, used)` triples used by liveness analysis almost always have invalid `reader` and `writer` fields. We can take advantage of this knowledge to use a compressed representation for them, falling back to a secondary table for the uncommon cases. This change reduces instruction counts on numerous benchmarks, the best by 16%. It also reduces max-rss on numerous benchmarks, the best by 38%. The patch also renames these triples from `Users` to `RWU`, because it's confusing having a type whose name is plural and then used within vectors whose names are also plural. r? @nikomatsakis

nnethercote · 2018-09-21T11:04:13Z

BTW, although the benefits (esp. max-rss) mostly accrue to non-NLL builds, the NLL builds do get some improvement.

bors · 2018-09-21T13:21:33Z

☀️ Test successful - status-travis
State: approved= try=True

nikomatsakis · 2018-09-21T19:11:44Z

@bors r+

@nnethercote what do you think about trying to rewrite this pass altogether to operate on MIR? By this point you've about rewritten it already a few times. =)

bors · 2018-09-21T19:11:45Z

📌 Commit b2f25e3 has been approved by nikomatsakis

nnethercote · 2018-09-21T20:22:54Z

LOL, I could give it a try, though you said previously that @wesleywiser was interested and I wouldn't want to steal it from them.

wesleywiser · 2018-09-21T21:12:37Z

@nnethercote Please, feel free to take that :) I've got a couple other things on my plate at the moment. I'll get to it at some point if you don't.

@nikomatsakis

…tsakis Compress `Liveness` data some more. Profiling shows that the `(reader, writer, used)` triples used by liveness analysis almost always have invalid `reader` and `writer` fields. We can take advantage of this knowledge to use a compressed representation for them, falling back to a secondary table for the uncommon cases. This change reduces instruction counts on numerous benchmarks, the best by 16%. It also reduces max-rss on numerous benchmarks, the best by 38%. The patch also renames these triples from `Users` to `RWU`, because it's confusing having a type whose name is plural and then used within vectors whose names are also plural. r? @nikomatsakis

@ghost

Rollup of 16 pull requests Successful merges: - #53652 (define copy_within on slices) - #54261 (Make `dyn` a keyword in the 2018 edition) - #54280 (remove (more) CAS API from Atomic* types where not natively supported) - #54323 (rustbuild: drop color handling) - #54350 (Support specifying edition in doc test) - #54370 (Improve handling of type bounds in `bit_set.rs`.) - #54371 (add -Zui-testing to rustdoc) - #54374 (Make 'proc_macro::MultiSpan' public.) - #54402 (Use no_default_libraries for all NetBSD flavors) - #54409 (Detect `for _ in in bar {}` typo) - #54412 (add applicability to span_suggestion call) - #54413 (Add UI test for deref recursion limit printing twice) - #54415 (parser: Tweak function parameter parsing to avoid rollback on succesfull path) - #54420 (Compress `Liveness` data some more.) - #54422 (Simplify slice's first(_mut) and last(_mut) with get) - #54446 (Unify christianpoveda's emails) Failed merges: - #54058 (Introduce the partition_dedup/by/by_key methods for slices) r? @ghost

@ghost

Rollup of 16 pull requests Successful merges: - #53652 (define copy_within on slices) - #54261 (Make `dyn` a keyword in the 2018 edition) - #54280 (remove (more) CAS API from Atomic* types where not natively supported) - #54323 (rustbuild: drop color handling) - #54350 (Support specifying edition in doc test) - #54370 (Improve handling of type bounds in `bit_set.rs`.) - #54371 (add -Zui-testing to rustdoc) - #54374 (Make 'proc_macro::MultiSpan' public.) - #54402 (Use no_default_libraries for all NetBSD flavors) - #54409 (Detect `for _ in in bar {}` typo) - #54412 (add applicability to span_suggestion call) - #54413 (Add UI test for deref recursion limit printing twice) - #54415 (parser: Tweak function parameter parsing to avoid rollback on succesfull path) - #54420 (Compress `Liveness` data some more.) - #54422 (Simplify slice's first(_mut) and last(_mut) with get) - #54446 (Unify christianpoveda's emails) Failed merges: - #54058 (Introduce the partition_dedup/by/by_key methods for slices) r? @ghost

nnethercote · 2018-09-25T02:07:59Z

Perf results look good, as expected.

rust-highfive assigned nikomatsakis Sep 21, 2018

rust-highfive added the S-waiting-on-review label Sep 21, 2018

nnethercote mentioned this pull request Sep 21, 2018

High memory usage compiling `keccak` benchmark #54208

Open

nikomatsakis approved these changes Sep 21, 2018

View changes

bors added S-waiting-on-bors and removed S-waiting-on-review labels Sep 21, 2018

pietroalbini mentioned this pull request Sep 22, 2018

Rollup of 16 pull requests #54457

Merged

bors merged commit b2f25e3 into rust-lang:master Sep 22, 2018
2 checks passed

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

homu Test successful
Details

nnethercote deleted the nnethercote:PackedRWU-Vec branch Sep 23, 2018

rust-lang / rust

Compress `Liveness` data some more. #54420

Compress `Liveness` data some more. #54420

nnethercote commented Sep 21, 2018

nnethercote commented Sep 21, 2018

nnethercote commented Sep 21, 2018

bors commented Sep 21, 2018

nnethercote commented Sep 21, 2018

bors commented Sep 21, 2018

nikomatsakis commented Sep 21, 2018

bors commented Sep 21, 2018

nnethercote commented Sep 21, 2018

wesleywiser commented Sep 21, 2018 •

edited

nnethercote commented Sep 25, 2018

rust-lang / rust

Join GitHub today

GitHub is where the world builds software

Compress `Liveness` data some more. #54420

Compress `Liveness` data some more. #54420

Conversation

nnethercote commented Sep 21, 2018

nnethercote commented Sep 21, 2018

nnethercote commented Sep 21, 2018

bors commented Sep 21, 2018

nnethercote commented Sep 21, 2018

bors commented Sep 21, 2018

nikomatsakis commented Sep 21, 2018

bors commented Sep 21, 2018

nnethercote commented Sep 21, 2018

wesleywiser commented Sep 21, 2018 • edited

nnethercote commented Sep 25, 2018

Essential cookies

Always active

Analytics cookies

wesleywiser commented Sep 21, 2018 •

edited