-
Notifications
You must be signed in to change notification settings - Fork 164
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
feat: Implement #599 #748
base: master
Are you sure you want to change the base?
feat: Implement #599 #748
Conversation
✅ Deploy Preview for salsa-rs canceled.
|
CodSpeed Performance ReportMerging #748 will improve performances by 3.28%Comparing Summary
Benchmarks breakdown
|
d1f6e52
to
5c7a7c5
Compare
So implementing the memo table part is actually trivial, the real issue is the dependency tracking as that goes through the type erased ingredient trait and now we have potentially more keys than just an |
I just realized, all of this is probably unnecessary! Instead of interning the additional arguments, can't we just synthesize a new tracked struct with each arg being a field? That would trivial fix things from what I can tell? |
That won't work because you can't create tracked structs from outside a query. Each tracked struct also gets a new id (it's uniqueness is defined by its instance not only its values). It would limit multi argument queries to only be callable from other queries. It's also not clear to me how that's different from interning? |
Currently looking into #599, this change makes things a bit simpler to overview removing some back and forth in code paths