Skip to content

[Distributed] Fix _executeDistributedFunction resume and result buffer #40814

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jan 12, 2022

Conversation

xedin
Copy link
Contributor

@xedin xedin commented Jan 12, 2022

  • Setup correct context so that swift_distributed_execute_target resume back to the correct fame.

Since swift_distributed_execute_target is not a regular async function, it needs to do some extra
bookkeeping before accessor could be called.

  • Drop optionality from result buffer, works fine with Void and makes it simpler to implement the accessor.

  • Add some more tests to make sure that different result types are handled correctly.

xedin added 3 commits January 11, 2022 15:48
Since this is a special function, `calleeContext` doesn't point to
a direct parent but instead both parent context (uninitialized)
and resume function are passed as last arguments which means that
`callContext` has to act as an intermediate context in call to accessor.
…utedTarget`

`RawPointer?` is lowered into a two arguments since it's a struct,
to make it easy let's just allocate an empty pointer for `Void` result.
@xedin xedin added the distributed Feature → concurrency: distributed actor label Jan 12, 2022
@xedin xedin requested a review from ktoso January 12, 2022 02:03
@xedin
Copy link
Contributor Author

xedin commented Jan 12, 2022

@swift-ci please test

@@ -277,7 +274,7 @@ func _executeDistributedTarget(
on actor: AnyObject, // DistributedActor
_ targetName: UnsafePointer<UInt8>, _ targetNameLength: UInt,
argumentBuffer: Builtin.RawPointer, // HeterogeneousBuffer of arguments
resultBuffer: Builtin.RawPointer?
resultBuffer: Builtin.RawPointer
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gotcha, noted 👍

callContext->Parent = callerContext;
callContext->ResumeParent = resumeFunc;

calleeContext->Parent = callContext;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's tricky... thanks a lot for digging through!

@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - 0767e20

@xedin
Copy link
Contributor Author

xedin commented Jan 12, 2022

@swift-ci please test macOS platform

@ktoso
Copy link
Contributor

ktoso commented Jan 12, 2022

Looking good, let's merge 👍

I'll send a mini PR for the parameters :)

@ktoso ktoso merged commit 680698e into swiftlang:main Jan 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
distributed Feature → concurrency: distributed actor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants