Repository navigation
Upgrade thin-vec to 0.2.21, including fix for .collect() capacity - #163959
Conversation
This includes mozilla/thin-vec#99 , which makes thin-vec respect size_hint when collecting. In particular, this causes collections of 1-3 items to not be grown to a capacity of 4. The compiler has a lot of `TinyVec`s in the 1-3 range, so this substantially helps memory, and seems neutral on instructions. For aws-sdk-ec2 full check, this saves 30.69 MiB max-rss (0.55% of peak RSS).
|
These commits modify the If this was unintentional then you should revert the changes before this PR is merged. Some changes occurred in compiler/rustc_attr_ir cc @jdonszelmann, @JonathanBrouwer Some changes occurred in compiler/rustc_attr_parsing cc @jdonszelmann, @JonathanBrouwer The parser was modified, potentially altering the grammar of (stable) Rust cc @fmease |
|
r? @wesleywiser rustbot has assigned @wesleywiser. Use Why was this reviewer chosen?The reviewer was selected based on:
|
|
@bors try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Upgrade `thin-vec` to 0.2.21, including fix for `.collect()` capacity
This comment has been minimized.
This comment has been minimized.
|
Finished benchmarking commit (bcfac1e): comparison URL. Overall result: ❌✅ regressions and improvements - please read:Benchmarking means the PR may be perf-sensitive. It's automatically marked not fit for rolling up. Overriding is possible but disadvised: it risks changing compiler perf. Next, please: If you can, justify the regressions found in this try perf run in writing along with @bors rollup=never rustc-perf Instruction countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
Max RSS (memory usage)Results (primary 0.4%, secondary 2.1%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (primary -2.1%, secondary -4.9%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeThis perf run didn't have relevant results for this metric. Bootstrap: 490.731s -> 491.187s (0.09%) |
|
Improvements outweigh regressions. @bors r+ |
This comment has been minimized.
This comment has been minimized.
Upgrade `thin-vec` to 0.2.21, including fix for `.collect()` capacity This includes mozilla/thin-vec#99 , which makes thin-vec respect size_hint when collecting. In particular, this causes collections of 1-3 items to not be grown to a capacity of 4. The compiler has a lot of `TinyVec`s in the 1-3 range, so this substantially helps memory, and seems neutral on instructions. For aws-sdk-ec2 full check, this saves 30.69 MiB max-rss (0.55% of peak RSS). --- Merging this avoids the need to work around it in other work (e.g. by replacing `.collect()` with something calling `ThinVec::with_capacity(...)` first).
|
The job Click to see the possible cause of the failure (guessed by this bot) |
|
💔 Test for 2e13aef failed: CI. Failed job:
|
|
Looks like a spurious network failure... @bors r=nnethercote |
This comment has been minimized.
This comment has been minimized.
What is this?This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.Comparing 65efa19 (parent) -> ae0fe51 (this PR) Test differencesNo test diffs found Test dashboardRun cargo run --manifest-path src/ci/citool/Cargo.toml -- \
test-dashboard ae0fe511b15ab760d7e963b13389d1bc2ab42b18 --output-dir test-dashboardAnd then open Job duration changes
How to interpret the job duration changes?Job durations can vary a lot, based on the actual runner instance |
|
Finished benchmarking commit (ae0fe51): comparison URL. Overall result: ❌✅ regressions and improvements - please read:Our benchmarks found a performance regression caused by this PR. Next Steps:
@rustbot label: +perf-regression Instruction countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
Max RSS (memory usage)Results (primary -0.5%, secondary 2.4%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (primary 2.2%, secondary -4.1%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeThis perf run didn't have relevant results for this metric. Bootstrap: 485.904s -> 487.56s (0.34%) |
|
Dependency update, so not much to do here, but the perf. result is a wash anyway. @rustbot label: +perf-regression-triaged |
View all comments
This includes mozilla/thin-vec#99 , which makes thin-vec respect size_hint when collecting.
In particular, this causes collections of 1-3 items to not be grown to a capacity of 4. The compiler has a lot of
TinyVecs in the 1-3 range, so this substantially helps memory, and seems neutral on instructions.For aws-sdk-ec2 full check, this saves 30.69 MiB max-rss (0.55% of peak RSS).
Merging this avoids the need to work around it in other work (e.g. by replacing
.collect()with something callingThinVec::with_capacity(...)first).