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.

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

Rollup of 10 pull requests #75060

Merged
merged 25 commits into from
Aug 2, 2020
Merged

Rollup of 10 pull requests #75060

merged 25 commits into from
Aug 2, 2020

Conversation

JohnTitor
Copy link
Member

Successful merges:

Failed merges:

r? @ghost

ssomers and others added 25 commits July 28, 2020 15:21
This removes all dependencies on pre-1.0 proc-macro ecosystem crates
(syn, quote, and proc-macro2)
These are quite long, usually, and in most cases not interesting. On smaller
terminals they can take up more than a full page of output, hiding the error
diagnostics emitted.
…ulacrum

BTreeMap: remove into_slices and its unsafe block

A small tweak to make BTreeMap code shorter and less unsafe.

r? @Mark-Simulacrum
…tracking, r=Mark-Simulacrum

BTreeMap::drain_filter should not touch the root during iteration

Although Miri doesn't point it out, I believe there is undefined behaviour using `drain_filter` when draining the 11th-last element from a tree that was larger. When this happens, the last remaining child nodes are merged, the root becomes empty and is popped from the tree. That last step establishes a mutable reference to the node elected root and writes a pointer in `node::Root`, while iteration continues to visit the same node.

This is mostly code from rust-lang#74437, slightly adapted.
…ulacrum

BTreeMap: define forget_type only when relevant

Similar to `forget_node_type` for handles.
No effect on generated code, apart maybe from the superfluous calls that might not have been optimized away.

r? @Mark-Simulacrum
Make tests faster in Miri

Reduce some test iteration counts in Miri.
…r=Mark-Simulacrum

Update elasticlunr-rs and ammonia transitive deps

This removes all dependencies on pre-1.0 proc-macro ecosystem crates
(syn, quote, and proc-macro2)
…th-tracing, r=oli-obk

Replaced log with tracing crate

Issue rust-lang#74747
…nkov

Rename rustc_middle::cstore::DepKind to CrateDepKind

It is ambiguous with DepGraph's own DepKind.
…=alexcrichton

Avoid dumping rustc invocations to stdout

These are quite long, usually, and in most cases not interesting. On smaller
terminals they can take up more than a full page of output, hiding the error
diagnostics emitted.
@JohnTitor
Copy link
Member Author

@rustbot modify labels: +rollup
@bors r+ rollup=never p=5

@bors
Copy link
Contributor

bors commented Aug 2, 2020

📌 Commit 50f2b5d has been approved by JohnTitor

@rustbot rustbot added the rollup A PR which is a rollup label Aug 2, 2020
@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Aug 2, 2020
@bors
Copy link
Contributor

bors commented Aug 2, 2020

⌛ Testing commit 50f2b5d with merge a99ae95...

@bors
Copy link
Contributor

bors commented Aug 2, 2020

☀️ Test successful - checks-actions, checks-azure
Approved by: JohnTitor
Pushing a99ae95 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Aug 2, 2020
@bors bors merged commit a99ae95 into rust-lang:master Aug 2, 2020
@JohnTitor JohnTitor deleted the rollup-aq8sfxf branch August 2, 2020 17:57
@Mark-Simulacrum
Copy link
Member

This was a slight performance regression, of up to 1%. Perhaps one of the BTree PRs are at fault? I recall there being some known regressions there.

@JohnTitor
Copy link
Member Author

Hmm, I'm not familiar with that, cc @ssomers in case.

@ssomers
Copy link
Contributor

ssomers commented Aug 4, 2020

#74762 is the only one that shows a penalty, benchmarking with stage 0. I.e.

benchcmp o o1 --threshold 5
 name                                           o ns/iter  o1 ns/iter  diff ns/iter  diff %  speedup
 btree::map::clone_fat_100_and_drain_all        172,930    155,862          -17,068  -9.87%   x 1.11
 btree::map::clone_fat_100_and_remove_all       173,350    218,235           44,885  25.89%   x 0.79
 btree::map::clone_fat_100_and_remove_half      118,970    138,497           19,527  16.41%   x 0.86
 btree::map::clone_fat_val_100_and_remove_all   81,776     96,849            15,073  18.43%   x 0.84
 btree::map::clone_fat_val_100_and_remove_half  57,137     64,924             7,787  13.63%   x 0.88
 btree::map::clone_slim_100_and_clear           2,258      2,137               -121  -5.36%   x 1.06
 btree::map::clone_slim_100_and_remove_all      4,873      5,172                299   6.14%   x 0.94
 btree::map::first_and_last_0                   30         32                     2   6.67%   x 0.94
 btree::map::insert_rand_100                    41         44                     3   7.32%   x 0.93
 btree::map::insert_rand_10_000                 41         45                     4   9.76%   x 0.91
 btree::map::insert_seq_10_000                  95         100                    5   5.26%   x 0.95
 btree::map::iter_0                             1,485      1,734                249  16.77%   x 0.86
 btree::set::is_subset_100_vs_10k               1,223      1,292                 69   5.64%   x 0.95

They're mostly benchmarks that keep changing their tune up and down whenever you touch some code, except the clone_fat tests, which are brand new. Looking closer and without threshold, all of the remove benchmarks are doing worse, and remove is the only genuine change in the PR. But amost all the drain_filter ones do better. I would think that the introduction of remove_kv_tracking to support drain_filter months ago would have had much more effect. I don't mind reverting the genuine change or all of it. It's not an ideal implementation for drain_filter anyway and I think by now I might be able to write one, without interfering with remove.

@JohnTitor
Copy link
Member Author

@ssomers Thanks for investigating it!

@Mark-Simulacrum Is it worth to revert it and check perf run?

@ssomers
Copy link
Contributor

ssomers commented Aug 5, 2020

#75182 may be a better thing to check the perf of

@cuviper cuviper added this to the 1.47.0 milestone May 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. rollup A PR which is a rollup S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.