Compiler: add Task call, invoke, and fetch optimizations and operations #59221
Conversation
This file contains hidden or 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
JeffBezanson
reviewed
Aug 6, 2025
| - `modifyglobal!(mod, var, op, x, order)` where `op(getval(), x)` is called | ||
| - `memoryrefmodify!(memref, op, x, order, boundscheck)` where `op(getval(), x)` is called | ||
| - `Intrinsics.atomic_pointermodify(ptr, op, x, order)` where `op(getval(), x)` is called | ||
| - `Core._task(f, size)` where `f()` will be called when the task runs |
Member
There was a problem hiding this comment.
Aren't task and finalizer different from the modify functions in that the modify functions eagerly call the argument at the call site, while task and finalizer are deferred? I thought that was important.
Member
Author
There was a problem hiding this comment.
It isn't important. All that matters is that there is some separate broker function (e.g. like https://llvm.org/doxygen/classllvm_1_1AbstractCallSite.html)
JeffBezanson
reviewed
Aug 6, 2025
| error("cannot serialize a running Task") | ||
| end | ||
| if isdefined(t, :invoked) | ||
| error("cannot serialize a Task constructed with invoke info") |
Member
There was a problem hiding this comment.
I'm not sure this is helpful, since if you hit this you can't really control it. We could just skip this field and hope it works out.
JeffBezanson
reviewed
Aug 6, 2025
JeffBezanson
reviewed
Aug 6, 2025
Member
|
Any benchmarks? |
aviatesk
reviewed
Aug 6, 2025
Member
I was unable to confirm optimization results even in the simplest case on my end unfortunately. julia> func2(i) = fetch(Threads.@spawn sin(i));
julia> using BenchmarkTools
julia> @benchmark func2(42)
|
Member
Author
|
It was entirely process_events and scheduling issues for me, which are separately addressable. The main point is inference and trim, not performance at this time. |
vtjnash
force-pushed
the
jn/Task-fetch-inference
branch
from
6c17db1 to
df3c99d
Compare
This commit implements a Task optimizations by adding a `invoked` field
to the Task definition, which optionally causes it to do an `invoke`
call internally instead of a regular `call`.
Key changes:
- Add `invoked` field to Task structure, supporting Type, Method, MethodInstance, and CodeInstance, just like invoke
- Implement `_task` builtin function to construct Task
- Create PartialTask lattice element for precise task result and error type tracking
- Unify several CallInfo types into IndirectCallInfo
- Optimized calls to `_task` constructor to inject CodeInstance.
Calling `fully_covers` isn't necessary, as the runtime will check that.
In the future we can make this field user-inaccessible (like scope) and
then be able to optimize based on the type returned from `fetch`.
Also added documentation to base/docs/basedocs.jl on how to easily add
more Builtin functions, following this as an example.
🤖 Generated with help from [Claude Code](https://claude.ai/code)
Very similar to return_type. Because of the design of Task reusing the `result` field to mean a lot of different things, it is hard to make this inference safe and sound now. Try anyways to add a type-assert so that `fetch` is inferred even if the design of Task make it unsafe.
aviatesk
reviewed
Sep 14, 2025
|
|
||
| # task_result_type effects modeling (should have !consistent effect) | ||
| let effects = Base.infer_effects(Core.task_result_type, (Task,)) | ||
| @test !Compiler.is_consistent(effects) # !consistent bit should be set |
Member
There was a problem hiding this comment.
Is this because of Task's mutability? I know this has been talked about a lot already, but can't we make Task immutable in its semantics?
Member
Author
There was a problem hiding this comment.
No, it is because inference becomes dangerously unsound if you accidentally ever infer the result of inference to be consistent
aviatesk
force-pushed
the
jn/Task-fetch-inference
branch
from
df3c99d to
c6a6187
Compare
Member
|
Rebased, added some higher-level tests with |
Co-authored-by: Shuhei Kadowaki <[email protected]>
aviatesk
approved these changes
Sep 18, 2025
Member
|
Looks like now we are hitting this assertion: |
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.
This commit implements a Task optimizations by adding a
invokedfield to the Task definition, which optionally causes it to do aninvokecall internally instead of a regularcall.Key changes:
invokedfield to Task structure, supporting Type, Method, MethodInstance, and CodeInstance, just like invoke_taskbuiltin function to construct Task_taskconstructor to inject CodeInstance. Callingfully_coversisn't necessary, as the runtime will check that.In the future we can consider making this field user-inaccessible (like scope) and then be able to optimize based on the type returned from
fetch.Also added documentation to base/docs/basedocs.jl on how to easily add more Builtin functions, following this as an example.
🤖 Generated with help from Claude Code