-
Notifications
You must be signed in to change notification settings - Fork 163
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
Struct field ingredients #481
Struct field ingredients #481
Conversation
First step towards the new tracked struct design. SUBTLE: This is actually broken if the `#[id]` fields have a broken hash. We'll fix that in the next commit and add a test.
✅ Deploy Preview for salsa-rs canceled.
|
We do care about the order of things in this set, in general, so use `shift_remove`.
c631651
to
8772961
Compare
revision: crate::Revision, | ||
) -> bool { | ||
let id = <C::Id>::from_id(input.key_index.unwrap()); | ||
eprintln!("maybe_changed_after({id:?}, {revision:?})"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @nikomatsakis , I noticed that an eprintln!() statement was accidentally left in the PR. Could you please check and remove it if it's not needed? Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whoops! Good catch.
This branch overhauls how tracked structs are implemented to make them more efficient -- it also adds tests for some edge cases along the way. In the new design, instead of treating tracked structs like a bunch of functions with specified values (omg the hashing) it creates a single shared hashmap to map the tracked struct id to its fields.
The plan is to eventually transition to a "Salsa 3.0" design in which tracked structs can be literal pointers into this shared value, but that will require more extensive changes that I have to write-up and think through (there are some edge cases in the unsafe code that are a bit subtle). This seemed like a useful stopping point regardless.