[release 1.8] backport of precompile: serialize the full edges graph (#46920) #47741
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This includes only the changes to `dump.c` for this change, but excludes
the functional part of the change (except for the additional bugfixes
mentioned below).
ORIGINAL COMMIT TEXT:
This fixes a long-standing issue with how we've handled `invoke` calls
with respect to method invalidation. When we load a package, we need
to ask whether a given MethodInstance would be compiled in the same
way now (aka, in the user's running session) as when the package was
precompiled; in practice, the way we do that is to test whether the
dispatches would be to the same methods in the current
world-age. `invoke` presents special challenges because it allows the
coder to deliberately select a different method than the one that
would be chosen by ordinary dispatch; if there is no record of how
this choice was made, it can look like it resolves to the wrong method
and this can trigger invalidation.
This allows a MethodInstance to store dispatch tuples as well as other
MethodInstances among their backedges.
Additionally:
- provide backedge-iterators for both C and Julia that abstracts
the specific storage mechanism.
- fix a bug in the CodeInstance `relocatability` field, where methods
that only return a constant (and hence store `nothing` for
`inferred`) were deemed non-relocatable.
- fix a bug in which #43990 should have checked that the method had
not been deleted. Tests passed formerly simply because we weren't
caching external CodeInstances that inferred down to a `Const`;
fixing that exposed the bug. This bug has been exposed since
merging #43990 for non-`Const` inference, and would affect Revise
etc.
Co-authored-by: Jameson Nash <[email protected]>
(cherry picked from commits dd375e1,
cb0721b, 9e39fe9)
(cherry picked from commit d4f0567)
SnoopCompile attempts to attribute invalidations to specific causes, but until now it has not generally been able to handle what it called "delayed" invalidations, which arose when a MethodInstance backedge wasn't valid anymore. This dumps more data to the reporting stream and should allow SnoopCompile to assemble the full chain of causes. This also adds invalidation of the backedges of methods that fail to validate their external edges. (cherry picked from commit b43bc62)
This fixes backedge-based invalidation when a precompiled `invoke` is followed by loading a package that adds new specializations for the `invoke`d method. An example is LowRankApprox.jl, where FillArrays adds a specialization to `unique`. (cherry picked from commit 698beed)
Previously, we would flatten the edges graph during serialization, to simplify the deserialization codes, but that now was adding complexity and confusion and uncertainty to the code paths. Clean that all up, so that we do not do that. Validation is performed while they are represented as forward edges, so avoids needing to interact with backedges at all. This uses the same algorithm now as #46749 for cycle convergence. (cherry picked from commit fbd5a72)
(cherry picked from commit e304cd5)
Co-authored-by: Jameson Nash <[email protected]> Co-authored-by: Valentin Churavy <[email protected]> (cherry picked from commit 0daab8a)
vtjnash
added
backport 1.8
26 tasks
KristofferC
changed the base branch from
release-1.8
to
backports-release-1.8
KristofferC
added a commit
that referenced
this pull request
Dec 14, 2022
[release 1.8] backport of precompile: serialize the full edges graph (#46920)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hopefully fixes #47685 (from brief local testing, it seems to)