Add name_dependents reverse index for incremental invalidation#646
Add name_dependents reverse index for incremental invalidation#646
name_dependents reverse index for incremental invalidation#646Conversation
st0012
force-pushed
the
add-name-dependents-index
branch
from
0993425 to
4fd9a14
Compare
| /// This name's `parent_scope` is the key name — structural dependency. | ||
| ChildName(NameId), | ||
| /// This name's `nesting` is the key name — reference-only dependency. | ||
| NestedName(NameId), |
There was a problem hiding this comment.
Is there a logic need for this distinction? Or could we just merge them into Name for dependent names?
There was a problem hiding this comment.
In the next PR (#641), we will have something like:
NameDependent::ChildName(id) => queue.push(InvalidationItem::UnresolveName(*id)),
NameDependent::NestedName(id) => queue.push(InvalidationItem::UnresolveReferences(*id)),The main difference is that UnresolveReferences will only unresolve constant references:
class Foo; end
class Bar
Foo
class Baz; end
# Bar has [NestedName(Foo), ChildName(Baz)]
endWhen Bar's ancestors changes:
ChildName(Baz)will trigger a total invalidation onBazNestedName(Foo)will only invalide theFooreference
There was a problem hiding this comment.
If the ancestors of Bar change, all constant references inside of the namespace have to be invalidated. I suspect that we can merge these two because the information of what needs to be invalidated is already encoded in the hashmap.
There was a problem hiding this comment.
If it's ok, I'd like to merge this as it is, and see if the invalidation algo looks better/worse without the 2nd enum after some rounds of reviews.
| } | ||
|
|
||
| #[cfg(test)] | ||
| mod name_dependent_tests { |
There was a problem hiding this comment.
Not opposed to it, but is it common in Rust to split tests groups in different modules?
There was a problem hiding this comment.
Yes. ZJIT does this as well. IMO it's a nice way to scope test helpers.
There was a problem hiding this comment.
It's too bad we didn't start out like that. We should've created modules for indexing each individual type of thing. Same for resolution, all ancestors tests could be separate.
Anyway, not worth the investment to refactor immediately, but something we may want later.
There was a problem hiding this comment.
I opened #649 in case anyway wants to give it a try.
st0012
force-pushed
the
add-name-dependents-index
branch
4 times, most recently
from
19b1837 to
eac46bb
Compare
st0012
force-pushed
the
add-name-dependents-index
branch
2 times, most recently
from
3fc7d48 to
21785c9
Compare
Build a reverse index during indexing that tracks which definitions, references, and names depend on each NameId. Name-to-name edges encode the dependency type at registration time: - ChildName: parent_scope relationship (structural dependency) - NestedName: nesting relationship (reference-only dependency) - Definition/Reference: direct dependents of the name This index will be consumed by the incremental invalidation engine to efficiently cascade changes without scanning the full graph.
st0012
force-pushed
the
add-name-dependents-index
branch
from
21785c9 to
7ff0dbd
Compare
2 participants
Build a reverse index during indexing that tracks which definitions, references, and names depend on each
NameId. Name-to-name edges encode the dependency type at registration time:ChildName:parent_scoperelationship (structural dependency)NestedName:nestingrelationship (reference-only dependency)Definition/Reference: direct dependents of the nameThis index will be consumed by the incremental invalidation engine to efficiently cascade changes without scanning the full graph.