I've been experimenting locally with removing
TaskDeps.reads, and just using
TaskDeps.read_set. However, it appears that something is relying on the insertion order being preserved. If I just rely on the iteration order of the
FxHashSet (where we previously iterated over
TaskDeps.reads, then two incremental tests fail. If I switch it to use
linked_hash_set::LinkedHashSet, then the tests pass
the insertion order is itself pretty arbitrary (whatever order the query access its dependencies in), so I think that the problem arises from the order changing. The failing tests both fail to recover a
DefId, which suggests that we're incorrectly marking a query as green (e.g. we are somehow losing track of edges). However, I don't see anything in the code that relies on the order of edges being unchanged
Never mind - nothing was getting marked green incorrectly. Since the edges were in a different order during loading, marking the node green tried to load a different query before the
tcx.hir() dependency. When the edges are loaded in order, the 'parent' dependency (e.g.
hir().local_def_id_to_hir_id will always be run first, which prevents this problem)
Can you elaborate on your reasoning? I am not sure I follow.
tcx.hir().local_def_id_to_hir_id are queries.
I didn't look to closely into it - the failing query was
I see - I forgot that
tcx.hir() doesn't actually invoke any queries
what I think happened was something like
let res = tcx.first_query(val); tcx.second_query(res) inside of a query impl
when we tried to mark the deps of that query as green, we started with
normally, we would have started with
first_query, which would have failed, so we would never get to
This could probably be reproduced without any of the hash set changes by inserting
tasl+deps.reads.reverse() in the proper place
Preserving the order of dependencies is needed for correctness. A query provider is still just a regular procedure. I don't have time right now but I'll try to come up with a good example tomorrow.
Last updated: Oct 21 2021 at 20:03 UTC