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.

Sign up for GitHub

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

Jump to bottom

Compile time perf regression for packed-simd's max-rss #57432

Closed
memoryruins opened this issue Jan 8, 2019 · 2 comments · Fixed by #58207
Closed

Compile time perf regression for packed-simd's max-rss #57432

memoryruins opened this issue Jan 8, 2019 · 2 comments · Fixed by #58207
Assignees
@oli-obk
Labels
I-slow Issue: Problems and improvements with respect to performance of generated code.

Comments

@memoryruins
Copy link
Contributor

packed-simd's max-rss and faults regressed in #56723

https://perf.rust-lang.org/compare.html?start=ae167c91aa9abefcd4a9697b5560330ef18e2e3e&end=d6d32ac25df2984f66b6abd14c1096880e04179a&stat=max-rss

compare 2019-01-04 (ae167c9) 2019-01-04 (d6d32ac)

max-rss

packed-simd-check
	avg: 153.5%	min: 2.4%	max: 241.9%
packed-simd-opt
	avg: 119.0%	min: 2.2%	max: 229.7%
packed-simd-debug
	avg: 117.2%	min: -0.7%	max: 226.8%

faults

packed-simd-check
	avg: 149.2%	min: 0.6%	max: 235.8%
packed-simd-opt
	avg: 113.7%	min: 0.6%	max: 225.5%
packed-simd-debug
	avg: 113.6%	min: 0.6%	max: 223.3%

Other crates' max-rss were affected, although not to the extent above.

cc @oli-obk
@estebank estebank added the I-slow Issue: Problems and improvements with respect to performance of generated code. label Jan 8, 2019
@oli-obk oli-obk self-assigned this Jan 8, 2019
@oli-obk oli-obk mentioned this issue Jan 8, 2019
Merged
pietroalbini added a commit to pietroalbini/rust that referenced this issue Jan 12, 2019
@pietroalbini
Rollup merge of rust-lang#57442 - oli-obk:lazy_const, r=RalfJung
bff32fc 
Simplify `ConstValue::ScalarPair`

While looking at rust-lang#57432 I realized that some of our types for representing constants are very big. This reduces `LazyConst` to 3/4th of its original size and simplifies some code around slices at the same time.

r? @RalfJung
@memoryruins memoryruins mentioned this issue Jan 22, 2019
Closed
Centril added a commit to Centril/rust that referenced this issue Jan 26, 2019
@Centril
Rollup merge of rust-lang#57442 - oli-obk:lazy_const, r=RalfJung
1dcbcb9 
Simplify `ConstValue::ScalarPair`

While looking at rust-lang#57432 I realized that some of our types for representing constants are very big. This reduces `LazyConst` to 3/4th of its original size and simplifies some code around slices at the same time.

r? @RalfJung
Centril added a commit to Centril/rust that referenced this issue Jan 27, 2019
@Centril
Rollup merge of rust-lang#57442 - oli-obk:lazy_const, r=RalfJung
62c863d 
Simplify `ConstValue::ScalarPair`

While looking at rust-lang#57432 I realized that some of our types for representing constants are very big. This reduces `LazyConst` to 3/4th of its original size and simplifies some code around slices at the same time.

r? @RalfJung
bors added a commit that referenced this issue Jan 28, 2019
@bors
Auto merge of #57442 - oli-obk:lazy_const, r=RalfJung
ec504de 
Simplify `ConstValue::ScalarPair`

While looking at #57432 I realized that some of our types for representing constants are very big. This reduces `LazyConst` to 3/4th of its original size and simplifies some code around slices at the same time.

r? @RalfJung
@mati865
Copy link
Contributor

mati865 commented Jan 28, 2019

Looks like there were some issues with the perf but latest results show improvement for packed-simd.

@memoryruins
Copy link
Contributor Author

#57442 definitely helped :)

https://perf.rust-lang.org/compare.html?start=01af12008d63a64446a86d995e772f8a539a4202&end=ec504def3665b0bc2ec80bede6dba2c603928315&stat=max-rss

compare 2019-01-27 (01af120) 2019-01-28 (ec504de) % change

max-rss

packed-simd-check
	avg: -10.1%	min: -14.7%	max: 0.3%
packed-simd-debug
	avg: -8.5%	min: -14.2%	max: -0.1%
packed-simd-opt
	avg: -9.2%	min: -14.0%	max: -2.8%

image

nnethercote added a commit to nnethercote/rust that referenced this issue Feb 6, 2019
@nnethercote
Make intern_lazy_const actually intern its argument.
f2871a9 
Currently it just unconditionally allocates it in the arena.

For a "Clean Check" build of the the `packed-simd` benchmark, this
change reduces both the `max-rss` and `faults` counts by 59%; it
slightly (~3%) increases the instruction counts but the `wall-time` is
unchanged.

For the same builds of a few other benchmarks, `max-rss` and `faults`
drop by 1--5%, but instruction counts and `wall-time` changes are in the
noise.

Fixes rust-lang#57432, fixes rust-lang#57829.
@nnethercote nnethercote mentioned this issue Feb 6, 2019
Merged
bors added a commit that referenced this issue Feb 9, 2019
@bors
Auto merge of #58207 - nnethercote:intern_lazy_const, r=oli-obk
d329d46 
Make `intern_lazy_const` actually intern its argument.

Currently it just unconditionally allocates it in the arena.

For a "Clean Check" build of the the `packed-simd` benchmark, this
change reduces both the `max-rss` and `faults` counts by 59%; it
slightly (~3%) increases the instruction counts but the `wall-time` is
unchanged.

For the same builds of a few other benchmarks, `max-rss` and `faults`
drop by 1--5%, but instruction counts and `wall-time` changes are in the
noise.

Fixes #57432, fixes #57829.
pietroalbini pushed a commit to pietroalbini/rust that referenced this issue Feb 17, 2019
@nnethercote @pietroalbini
Make intern_lazy_const actually intern its argument.
94ca417 
Currently it just unconditionally allocates it in the arena.

For a "Clean Check" build of the the `packed-simd` benchmark, this
change reduces both the `max-rss` and `faults` counts by 59%; it
slightly (~3%) increases the instruction counts but the `wall-time` is
unchanged.

For the same builds of a few other benchmarks, `max-rss` and `faults`
drop by 1--5%, but instruction counts and `wall-time` changes are in the
noise.

Fixes rust-lang#57432, fixes rust-lang#57829.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@oli-obk oli-obk

Labels
I-slow Issue: Problems and improvements with respect to performance of generated code.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Make intern_lazy_const actually intern its argument. nnethercote/rust
4 participants
@oli-obk @mati865 @estebank @memoryruins