aot: move jl_insert_backedges to Julia side
#56499
Merged
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
aviatesk
force-pushed
the
avi/move-staticdata_utils-julia
branch
from
4959411 to
2eafa61
Compare
|
Does this have any performance diff? |
|
Moving the implementation to Julia seems to result in a slight increase in allocations, but considering the potential for future improvements, it doesn’t seem like a significant issue julia> @time using Debugger
0.233097 seconds (626.92 k allocations: 33.375 MiB, 26.87% compilation time: 100% of which was recompilation) # master
0.228869 seconds (627.05 k allocations: 33.421 MiB, 27.19% compilation time: 100% of which was recompilation) |
aviatesk
force-pushed
the
avi/move-staticdata_utils-julia
branch
from
2eafa61 to
718aea8
Compare
aviatesk
force-pushed
the
avi/move-staticdata_utils-julia
branch
4 times, most recently
from
907d058 to
6b69260
Compare
aviatesk
force-pushed
the
avi/move-staticdata_utils-julia
branch
2 times, most recently
from
7b54cf5 to
87836bf
Compare
vtjnash
reviewed
Jan 6, 2025
vtjnash
reviewed
Jan 6, 2025
vtjnash
reviewed
Jan 6, 2025
aviatesk
force-pushed
the
avi/move-staticdata_utils-julia
branch
from
109748f to
929d53c
Compare
aviatesk
added a commit
that referenced
this pull request
Jan 8, 2025
If #56499 is merged, the functionality of `jl_reinit_ccallable` will become incomplete. However, it is questionable whether `jl_reinit_ccallable` is truly necessary. It seems to have been added in #44527, but all test cases appear to pass without it. Therefore, it might be safe to remove it altogether.
aviatesk
added a commit
that referenced
this pull request
Jan 8, 2025
If #56499 is merged, the functionality of `jl_reinit_ccallable` will become incomplete. However, it is questionable whether `jl_reinit_ccallable` is truly necessary. It seems to have been added in #44527, but all test cases appear to pass without it. Therefore, it might be safe to remove it altogether.
aviatesk
force-pushed
the
avi/move-staticdata_utils-julia
branch
from
929d53c to
8ed7f7f
Compare
Keno
added a commit
that referenced
this pull request
Jan 9, 2025
Our binding partion invalidation code scans the original source for any GlobalRefs that need to be invalidated. However, this is not the only source of access to binding partitions. Various intrinsics (in particular the `*global` ones) also operate on bindings. Since these are not manifest in the source, we instead use the existing `edges` mechanism to give them forward edges. This PR only includes the basic info type, and validation in the replacement case. There's a bit more work to do there, but I'm waiting on #56499 for that part, precompilation in particular.
Keno
added a commit
that referenced
this pull request
Jan 9, 2025
Our binding partion invalidation code scans the original source for any GlobalRefs that need to be invalidated. However, this is not the only source of access to binding partitions. Various intrinsics (in particular the `*global` ones) also operate on bindings. Since these are not manifest in the source, we instead use the existing `edges` mechanism to give them forward edges. This PR only includes the basic info type, and validation in the replacement case. There's a bit more work to do there, but I'm waiting on #56499 for that part, precompilation in particular.
Keno
added a commit
that referenced
this pull request
Jan 10, 2025
Our binding partion invalidation code scans the original source for any GlobalRefs that need to be invalidated. However, this is not the only source of access to binding partitions. Various intrinsics (in particular the `*global` ones) also operate on bindings. Since these are not manifest in the source, we instead use the existing `edges` mechanism to give them forward edges. This PR only includes the basic info type, and validation in the replacement case. There's a bit more work to do there, but I'm waiting on #56499 for that part, precompilation in particular.
Keno
added a commit
that referenced
this pull request
Jan 10, 2025
Our binding partion invalidation code scans the original source for any GlobalRefs that need to be invalidated. However, this is not the only source of access to binding partitions. Various intrinsics (in particular the `*global` ones) also operate on bindings. Since these are not manifest in the source, we instead use the existing `edges` mechanism to give them forward edges. This PR only includes the basic info type, and validation in the replacement case. There's a bit more work to do there, but I'm waiting on #56499 for that part, precompilation in particular.
aviatesk
force-pushed
the
avi/move-staticdata_utils-julia
branch
2 times, most recently
from
e1c3ce6 to
9340251
Compare
aviatesk
force-pushed
the
avi/move-staticdata_utils-julia
branch
from
9340251 to
b5d0227
Compare
|
This should be ready to go. |
aviatesk
force-pushed
the
avi/move-staticdata_utils-julia
branch
from
b5d0227 to
d9d83a6
Compare
aviatesk
force-pushed
the
avi/move-staticdata_utils-julia
branch
2 times, most recently
from
0b2575d to
2e7b0b2
Compare
vtjnash
reviewed
Jan 15, 2025
vtjnash
reviewed
Jan 15, 2025
vtjnash
reviewed
Jan 15, 2025
| @@ -0,0 +1,296 @@ | |||
| # This file is a part of Julia. License is MIT: https://julialang.org/license | |||
|
|
|||
| module StaticData | |||
There was a problem hiding this comment.
This eventually should be moved inside the Compiler module and locked to the Compiler world age, since broadly speaking this algorithm is just a slightly different re-implementation of that entire code. I guess that isn't blocking though for now, since this is a private module name
There was a problem hiding this comment.
Let's move it to Compiler in a separate PR.
With #56447, the dependency between `jl_insert_backedges` and method insertion has been eliminated, allowing `jl_insert_backedges` to be performed after loading. As a result, it is now possible to move `jl_insert_backedges` to the Julia side. Currently this commit simply moves the implementation without adding any new features.
Co-authored-by: Jameson Nash <[email protected]>
aviatesk
force-pushed
the
avi/move-staticdata_utils-julia
branch
from
d427164 to
6e8bd6b
Compare
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.
With #56447, the dependency between
jl_insert_backedgesand method insertion has been eliminated, allowingjl_insert_backedgesto be performed after loading. As a result, it is now possible to movejl_insert_backedgesto the Julia side.Currently this commit simply moves the implementation without adding any new features.