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

ConflictStoreTrie: Faster filtered search #6366

Merged
merged 6 commits into from
Dec 2, 2018

Conversation

Eh2406
Copy link
Contributor

@Eh2406 Eh2406 commented Dec 1, 2018

This is an optimization that I was thinking of doing in #6283. I did not then as this is optimizing a not particularly hot path. I wish to do it now as it is stuck in my head, and I need that head space for more important things.

This also "accidentally" fixes the indeterminacy introduced in #6283, by replacing the HashMap.iter().find() like code with BTreeMap.iter().find() like code. This is not strictly needed, as @alexcrichton pointed out (In most use cases the index will change between any two invocations anyway), but does make it much easier to deal with (fuzz) test cases.

@Eh2406
Copy link
Contributor Author

Eh2406 commented Dec 2, 2018

@rust-highfive seems to be asleep.
@dwijnand, @ishitatsuyuki, @Zekka, thanks for the input on discord.
I am working on finding the cause of the test failures.

@Eh2406 Eh2406 force-pushed the faster-filtered-search branch from 03094db to eeaebfc Compare December 2, 2018 00:46
@Eh2406
Copy link
Contributor Author

Eh2406 commented Dec 2, 2018

And CI is green.

@alexcrichton
Copy link
Member

@bors: r+

Nice find!

@bors
Copy link
Contributor

bors commented Dec 2, 2018

📌 Commit eeaebfc has been approved by alexcrichton

@bors
Copy link
Contributor

bors commented Dec 2, 2018

⌛ Testing commit eeaebfc with merge 5e85ba1...

bors added a commit that referenced this pull request Dec 2, 2018
ConflictStoreTrie: Faster filtered search

This is an optimization that I was thinking of doing in #6283. I did not then as this is optimizing a not particularly hot path. I wish to do it now as it is stuck in my head, and I need that head space for more important things.

This also "accidentally" fixes the [indeterminacy](#6283 (comment)) introduced in #6283, by replacing the `HashMap.iter().find()` like code with `BTreeMap.iter().find()` like code. This is not strictly needed, as @alexcrichton pointed out (In most use cases the index will change between any two invocations anyway), but does make it much easier to deal with (fuzz) test cases.
@bors
Copy link
Contributor

bors commented Dec 2, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: alexcrichton
Pushing 5e85ba1 to master...

@bors bors merged commit eeaebfc into rust-lang:master Dec 2, 2018
bors added a commit to rust-lang/rust that referenced this pull request Dec 4, 2018
Update cargo, rls

26 commits in b3d0b2e545b61d4cd08096911724b7d49d213f73..5e85ba14aaa20f8133863373404cb0af69eeef2c
2018-11-15 19:13:04 +0000 to 2018-12-02 14:37:25 +0000
- ConflictStoreTrie: Faster filtered search (rust-lang/cargo#6366)
- Remove `cmake` as a requirement (rust-lang/cargo#6368)
- progress: display "Downloading 1 crate" instead of "Downloading 1 crates" (rust-lang/cargo#6369)
- Use expect over unwrap, for panic-in-panic aborts (rust-lang/cargo#6364)
- Switch to pretty_env_logger, under --features pretty-env-logger (rust-lang/cargo#6362)
- use allow-dirty option in `cargo package` to skip vcs checks (rust-lang/cargo#6280)
- remove clones made redundant by Intern PackageId (rust-lang/cargo#6352)
- docs: correct profile usage of `cargo test --release` (rust-lang/cargo#6345)
- Improve doc for `cargo install` (rust-lang/cargo#6354)
- Intern PackageId (rust-lang/cargo#6332)
- Clean only release artifacts if --release option is set (rust-lang/cargo#6349)
- remove clones made redundant by Intern SourceId (rust-lang/cargo#6347)
- Intern SourceId (rust-lang/cargo#6342)
- Tweak Layout to allow for non json file targets with internal "." (rust-lang/cargo#6255)
- Correct Target Directory command-line option (rust-lang/cargo#6343)
- Persistent data structures by im-rs (rust-lang/cargo#6336)
- Move command prelude into main library (rust-lang/cargo#6335)
- Distinguish custom build invocations (rust-lang/cargo#6331)
- Allow crate_type=bin examples to run (rust-lang/cargo#6330)
- Make verify-project honour unstable features (rust-lang/cargo#6326)
- Make autodiscovery disable inferred targets (rust-lang/cargo#6329)
- Add `c` alias for `check` (rust-lang/cargo#6218)
- Allow user aliases to override built-in aliases (rust-lang/cargo#6259)
- Fix renaming directory project using build scripts with cross-compiling. (rust-lang/cargo#6328)
- Fix add_plugin_deps-related tests. (rust-lang/cargo#6327)
- Add a glossary. (rust-lang/cargo#6321)
@Eh2406 Eh2406 deleted the faster-filtered-search branch December 15, 2018 19:24
@ehuss ehuss added this to the 1.32.0 milestone Feb 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants