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

Convert ShardedHashMap to use hashbrown::HashTable #137701

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

cuviper
Copy link
Member

@cuviper cuviper commented Feb 27, 2025

The hash_raw_entry feature (#56167) has finished fcp-close, so the compiler
should stop using it to allow its removal. Several Sharded maps were
using raw entries to avoid re-hashing between shard and map lookup, and
we can do that with hashbrown::HashTable instead.

@rustbot
Copy link
Collaborator

rustbot commented Feb 27, 2025

r? @fmease

rustbot has assigned @fmease.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Feb 27, 2025
@rustbot
Copy link
Collaborator

rustbot commented Feb 27, 2025

These commits modify the Cargo.lock file. Unintentional changes to Cargo.lock can be introduced when switching branches and rebasing PRs.

If this was unintentional then you should revert the changes before this PR is merged.
Otherwise, you can ignore this comment.

Some changes occurred to the CTFE / Miri interpreter

cc @rust-lang/miri, @rust-lang/wg-const-eval

@bors

This comment was marked as resolved.

@cuviper cuviper force-pushed the sharded-hashtable branch from b437273 to 547eaca Compare March 4, 2025 16:27
@fmease
Copy link
Member

fmease commented Mar 10, 2025

for visibility @rustbot ping wg-parallel-rustc

@bors r+

@rustbot
Copy link
Collaborator

rustbot commented Mar 10, 2025

Error: This team (wg-parallel-rustc) cannot be pinged via this command; it may need to be added to triagebot.toml on the default branch.

Please file an issue on GitHub at triagebot if there's a problem with this bot, or reach out on #t-infra on Zulip.

@bors
Copy link
Contributor

bors commented Mar 10, 2025

📌 Commit 547eaca has been approved by fmease

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 10, 2025
@fmease
Copy link
Member

fmease commented Mar 10, 2025

cc some members of WG-parallel-rustc for visibility @SparrowLii @bjorn3 since triagebot ping groups failed on me (and since there's no proper GH team)

jieyouxu added a commit to jieyouxu/rust that referenced this pull request Mar 10, 2025
Convert `ShardedHashMap` to use `hashbrown::HashTable`

The `hash_raw_entry` feature (rust-lang#56167) has finished fcp-close, so the compiler
should stop using it to allow its removal. Several `Sharded` maps were
using raw entries to avoid re-hashing between shard and map lookup, and
we can do that with `hashbrown::HashTable` instead.
jieyouxu added a commit to jieyouxu/rust that referenced this pull request Mar 10, 2025
Convert `ShardedHashMap` to use `hashbrown::HashTable`

The `hash_raw_entry` feature (rust-lang#56167) has finished fcp-close, so the compiler
should stop using it to allow its removal. Several `Sharded` maps were
using raw entries to avoid re-hashing between shard and map lookup, and
we can do that with `hashbrown::HashTable` instead.
@jieyouxu
Copy link
Member

Missing mem import

error[E0433]: failed to resolve: use of unresolved module or unlinked crate `mem`
   --> compiler/rustc_data_structures/src/sharded.rs:193:32
    |
193 |                 let previous = mem::replace(&mut e.into_mut().1, value);
    |                                ^^^ use of unresolved module or unlinked crate `mem`
    |
    = help: if you wanted to use a crate named `mem`, use `cargo add mem` to add it to your `Cargo.toml`
help: consider importing this module

@bors r-

@jieyouxu jieyouxu closed this Mar 11, 2025
@jieyouxu jieyouxu reopened this Mar 11, 2025
@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Mar 11, 2025
@cuviper
Copy link
Member Author

cuviper commented Mar 11, 2025

Oh, I see, the import was removed by #138040. I'll rebase and fix it.

The `hash_raw_entry` feature has finished fcp-close, so the compiler
should stop using it to allow its removal. Several `Sharded` maps were
using raw entries to avoid re-hashing between shard and map lookup, and
we can do that with `hashbrown::HashTable` instead.
@cuviper cuviper force-pushed the sharded-hashtable branch from 547eaca to 3b0c258 Compare March 11, 2025 00:09
@cuviper
Copy link
Member Author

cuviper commented Mar 11, 2025

PR checks are passing, so we should be good to go!

@bors r=fmease

@bors
Copy link
Contributor

bors commented Mar 11, 2025

📌 Commit 3b0c258 has been approved by fmease

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 11, 2025
jieyouxu added a commit to jieyouxu/rust that referenced this pull request Mar 11, 2025
Convert `ShardedHashMap` to use `hashbrown::HashTable`

The `hash_raw_entry` feature (rust-lang#56167) has finished fcp-close, so the compiler
should stop using it to allow its removal. Several `Sharded` maps were
using raw entries to avoid re-hashing between shard and map lookup, and
we can do that with `hashbrown::HashTable` instead.
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 11, 2025
Rollup of 18 pull requests

Successful merges:

 - rust-lang#126856 (remove deprecated tool `rls`)
 - rust-lang#137314 (change definitely unproductive cycles to error)
 - rust-lang#137504 (Move methods from Map to TyCtxt, part 4.)
 - rust-lang#137701 (Convert `ShardedHashMap` to use `hashbrown::HashTable`)
 - rust-lang#137967 ([AIX] Fix hangs during testing)
 - rust-lang#138002 (Disable CFI for weakly linked syscalls)
 - rust-lang#138052 (strip `-Wlinker-messages` wrappers from `rust-lld` rmake test)
 - rust-lang#138063 (Improve `-Zunpretty=hir` for parsed attrs)
 - rust-lang#138109 (make precise capturing args in rustdoc Json typed)
 - rust-lang#138147 (Add maintainers for powerpc64le-unknown-linux-gnu)
 - rust-lang#138245 (stabilize `ci_rustc_if_unchanged_logic` test for local environments)
 - rust-lang#138296 (Remove `AdtFlags::IS_ANONYMOUS` and `Copy`/`Clone` condition for anonymous ADT)
 - rust-lang#138300 (add tracking issue for unqualified_local_imports)
 - rust-lang#138307 (Allow specifying glob patterns for try jobs)
 - rust-lang#138313 (Update books)
 - rust-lang#138315 (use next_back() instead of last() on DoubleEndedIterator)
 - rust-lang#138318 (Rustdoc: remove a bunch of `@ts-expect-error` from main.js)
 - rust-lang#138330 (Remove unnecessary `[lints.rust]` sections.)

Failed merges:

 - rust-lang#137147 (Add exclude to config.toml)

r? `@ghost`
`@rustbot` modify labels: rollup
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants