Re-working async-main #38604

etcwilde · 2021-07-23T16:50:55Z

I've been working on changing the behavior of the async-main function a little bit, as well as getting the bits implemented in the compiler rather than in the standard library. This will allow us to change things later without needing to update the standard library dramatically.

The issues with the current implementation of the async-main is in the interaction between Swift and ObjC. The current implementation has an implicit suspension point before main ever gets to run. If folks have enqueued things in a static constructor in ObjC, those will get enqueued before the main task does, so their tasks will run before main. This is not desirable.

This doesn't quite work yet. If it were written in Swift, the current implementation would look something like

struct Main {
  static func main() async throws { }
}

public func baseMain() -> Never {
  let flags = _taskCreateFlags(...)
  let (task, _) = Builtin.CreateAsynTask(flags, Main.main())
  _swiftJobRun(task)
  _asyncMainDrainQueue()
}

Unfortunately, unless you put an exit and an _exitInMain in your main function, this results in a program that never finishes since we get stuck draining the main queue.

What we really want is something more like

struct Main {
  @MainActor
  static func main() async throws { }
}

@MainActor
public func baseMain() -> Never {
  let wrappedMain = { @MainActor () async -> () in
    do {
       try Main.main()
    catch {
        _errorInMain(error)
    }
  }

  let flags = _taskCreateFlags(...)
  let (task, _) = Builtin.CreateAsynTask(flags, wrappedMain)
  _swiftJobRun(task)
  _asyncMainDrainQueue()
}

I still need to figure out how to create a separate thunk to wrap the call to the actual main function in.
I'm just putting this up to start getting eyes on it.

rdar://80027250

etcwilde · 2021-07-23T17:49:11Z

lib/SILGen/SILGenFunction.cpp

+      // Emit the CreateAsyncTask builtin
+      // We can't use emitBuiltinCreateAsyncTask because it doesn't pass the
+      // main metatype into the thunk correctly. :(


Ignore this comment, it's wrong. Fixed it with the partial apply directly above.

etcwilde · 2021-07-27T18:17:35Z

lib/SILGen/SILGenFunction.cpp

+          JobType, {}, {task});
+
+      // Run first part synchronously
+      FuncDecl *swiftJobRunFuncDecl = SGM.getSwiftJobRun();


This is the moral equivalent of a bit-cast.

etcwilde · 2021-08-02T19:24:52Z

Hanging in Concurrency/Runtime/Output/async_task_handle_cancellation.swift.
Trying to figure out why.

etcwilde · 2021-08-05T17:53:59Z

@swift-ci please test

swift-ci · 2021-08-05T20:16:00Z

Build failed
Swift Test OS X Platform
Git Sha - 676dde589e5a0c3613691d76e58eba013d926113

etcwilde · 2021-08-05T23:36:28Z

@swift-ci please test

DougGregor · 2021-08-25T19:35:31Z

lib/SIL/IR/SILLocation.cpp

@@ -42,7 +42,7 @@ SourceLoc SILLocation::getSourceLoc() const {

  // Don't crash if the location is a FilenameAndLocation.
  // TODO: this is a workaround until rdar://problem/25225083 is implemented.
-  if (isFilenameAndLocation())
+  if (getStorageKind() == FilenameAndLocationKind)


Is this needed?

Yes, isFilenameAndLocation verifies that both the storage kind is a FilenameAndLocationKind and that it actually stores a location. While the source location is a FilenameAndLocationKind, the location is null, so it still returns false.
Then we crash in getPrimaryASTNode() because SourceLocations don't have a corresponding ASTNode.

DougGregor · 2021-08-25T19:36:07Z

lib/SILGen/SILGen.cpp

+
+FuncDecl *SILGenModule::getTaskCreateFlags() {
+  return lookupConcurrencyIntrinsic(getASTContext(), TaskCreateFlags,
+                                    "taskCreateFlags");


We shouldn't need this, because we can form task creation flags directly.

DougGregor · 2021-08-25T19:37:56Z

lib/SILGen/SILGen.cpp

@@ -1046,7 +1066,13 @@ void SILGenModule::emitFunctionDefinition(SILDeclRef constant, SILFunction *f) {
    auto *decl = constant.getDecl();
    auto *dc = decl->getDeclContext();
    PrettyStackTraceSILFunction X("silgen emitArtificialTopLevel", f);
-    SILGenFunction(*this, *f, dc).emitArtificialTopLevel(decl);
+    if (constant.kind == SILDeclRef::Kind::EntryPoint && isa<FuncDecl>(decl) &&


It's worth a comment here to describe the intent, e.g., that @main is initiating the asynchronous task on the main actor, and @async_Main is actually running the user's async main

DougGregor · 2021-08-25T19:38:20Z

lib/SILGen/SILGen.h

@@ -131,6 +131,11 @@ class LLVM_LIBRARY_VISIBILITY SILGenModule : public ASTVisitor<SILGenModule> {
  Optional<FuncDecl*> ResumeUnsafeThrowingContinuationWithError;
  Optional<FuncDecl*> CheckExpectedExecutor;

+  Optional<FuncDecl *> AsyncMainDrainQueue;
+  Optional<FuncDecl *> TaskCreateFlags;


Shouldn't need this now

DougGregor · 2021-08-25T19:38:28Z

lib/SILGen/SILGen.h

+  /// Retrieve the _Concurrency._asyncMainDrainQueue intrinsic.
+  FuncDecl *getAsyncMainDrainQueue();
+  /// Retrieve the _Concurrency.taskCreateFlags intrinsic.
+  FuncDecl *getTaskCreateFlags();


Shouldn't need this one now

DougGregor · 2021-08-25T19:39:43Z

lib/SILGen/SILGenFunction.cpp

+      FuncDecl *exitFuncDecl = nullptr;
+      {
+        SmallVector<ValueDecl *, 1> decls;
+        concShimsModule->lookupQualified(


Hmm. Could we factor out the "call exit with this value" code into a separate function? emitArtificialTopLevel is pretty big.

DougGregor · 2021-08-25T19:42:33Z

lib/SILGen/SILGenFunction.cpp

+    // If the types are the same, we don't need to do anything!
+    if (apiSILType == originalValue->getType())
+      return originalValue;
+    return this->B.createUncheckedReinterpretCast(moduleLoc, originalValue,


I think you can do this with a struct instruction rather than a reinterpret cast.

DougGregor · 2021-08-25T19:43:45Z

lib/SILGen/SILGenFunction.cpp

+
+  // Emit the CreateAsyncTask builtin
+  TaskCreateFlags taskCreationFlagMask(0);
+  taskCreationFlagMask.setPriority(JobPriority::Default);


I think you want to set Task_InheritContext and leave the priority unset, so it inherits the priority of the running (main) thread?

DougGregor · 2021-08-25T19:45:24Z

stdlib/public/Concurrency/Task.swift

@@ -753,7 +753,11 @@ func _enqueueJobGlobalWithDelay(_ delay: UInt64, _ task: Builtin.Job)

 @available(SwiftStdlib 5.5, *)
 @_silgen_name("swift_task_asyncMainDrainQueue")
-public func _asyncMainDrainQueue() -> Never
+internal func _asyncMainDrainQueue() -> Never


Doesn't this need to be public for SILGen to reference it?

DougGregor · 2021-08-25T19:45:32Z

stdlib/public/Concurrency/Task.swift

+
+@available(SwiftStdlib 5.5, *)
+@_silgen_name("swift_task_getMainExecutor")
+internal func _getMainExecutor() -> Builtin.Executor


Same question here.

etcwilde · 2021-09-28T17:22:12Z

@swift-ci please test

swift-ci · 2021-09-28T20:16:13Z

Build failed
Swift Test OS X Platform
Git Sha - af9298dabf40a8e79898067afd63a89c67e0f173

etcwilde · 2021-09-29T06:18:08Z

@swift-ci please test

The lookupConcurrencyIntrinsic function only looked in the concurrency module. It is useful to look in other modules for intrinsics too.

The AsyncEntryPoint represents the thunk that is wrapped in a task. This thunk is used to ensure that the main function explicitly calls "exit", and to properly unwrap and report any unhandled errors returned from the user-written main. The function takes on the name `@async_main` in the emitted SIL.

@main

This patch updates the asynchronous main function to run the first thunk of the function synchronously through a call to `swift_job_run`. The runloop is killed by exiting or aborting the task that it is running on. As such, we need to ensure that the task contains an async function that either calls exit explicitly or aborts. The AsyncEntryPoint, that contains this code, was added in the previous patch. This patch adds the pieces for the actual implementation of this behaviour as well as adding the necessary code to start the runloop. There are now four layers of main functions before hitting the "real" code. @main: This is the actual main entrypoint of the program. This constructs the task containing @async_main, grabs the main executor, runs swift_job_run to run the first part synchronously, and finally kicks off the runloop with a call to _asyncMainDrainQueue. This is generated in the call to `emitAsyncMainThreadStart`. @async_main: This thunk exists to ensure that the main function calls `exit` at some point so that the runloop stops. It also handles emitting an error if the user-written main function throws. e.g: ``` func async_main() async -> () { do { try await Main.$main() exit(0) } catch { _errorInMain(error) } } ``` Main.$main(): This still has the same behaviour as with the synchronous case. It just calls `try await Main.main()` and exists to simplify typechecking. Main.main(): This is the actual user-specified main. It serves the same purpose as in the synchronous, allowing the programmer to write code, but it's async! The control flow in `emitFunctionDefinition` is a little confusing (to me anyway), so here it is spelled out: If the main function is synchronous, the `constant.kind` will be a `SILDeclRef::Kind::EntryPoint`, but the `decl` won't be async, so it drops down to `emitArtificalTopLevel` anyway. If the main function is async and we're generating `@main`, the `constant.kind` will be `SILDeclRef::Kind::AsyncEntryPoint`, so we also call `emitArtificalTopLevel`. `emitArtificalTopLevel` is responsible for detecting whether the decl is async and deciding whether to emit code to extract the argc/argv variables that get passed into the actual main entrypoint to the program. If we're generating the `@async_main` body, the kind will be `SILDeclRef::Kind::EntryPoint` and the `decl` will be async, so we grab the mainEntryPoint decl and call `emitAsyncMainThreadStart` to generate the wrapping code. Note; there is a curious change in `SILLocation::getSourceLoc()` where instead of simply checking `isFilenameAndLocation()`, I change it to `getStorageKind() == FilenameAndLocationKind`. This is because the SILLocation returned is to a FilenameAndLocationKind, but the actual storage returns true for the call to `isNull()` inside of the `isFilenameAndLocation()` call. This results in us incorrectly falling through to the `getASTNode()` call below that, which asserts when asked to get the AST node of a location. I also did a little bit of refactoring in the SILGenModule for grabbing intrinsics. Previously, there was only a `getConcurrencyIntrinsic` function, which would only load FuncDecls out of the concurrency module. The `exit` function is in the concurrency shims module, so I refactored the load code to take a ModuleDecl to search from. The emitBuiltinCreateAsyncTask function symbol is exposed from SILGenBuiltin so that it is available from SILGenFunction. There is a fair bit of work involved going from what is available at the SGF to what is needed for actually calling the CreateAsyncTask builtin, so in order to avoid additional maintenance, it's good to re-use that.

This patch changes the main task to inherit the context of the main thread. This should assign the appropriate priority based on how the program was invoked. I've also updated the tests to reflect these changes.

etcwilde · 2021-10-02T23:53:33Z

@swift-ci please test

Priorities keep shifting since I first wrote this and are apparently different on different OS's and different versions of the same OS. Since the test is checking that entering and exiting tasks doesn't effect the outer-scoped priority, I've set a variable based on the main thread's priories, which should be inherited by the child.

etcwilde · 2021-10-04T16:36:00Z

@swift-ci please test

A single-element struct is structurally the same as the member type itself.

etcwilde · 2021-10-04T16:55:10Z

@swift-ci please test

etcwilde added the concurrency Feature: umbrella label for concurrency language features label Jul 23, 2021

etcwilde requested a review from DougGregor July 23, 2021 16:50

etcwilde marked this pull request as draft July 23, 2021 16:51

etcwilde commented Jul 23, 2021

View reviewed changes

etcwilde commented Jul 27, 2021

View reviewed changes

etcwilde force-pushed the ewilde/rewrite-main branch from e43029e to 2298e41 Compare August 5, 2021 01:49

DougGregor reviewed Aug 25, 2021

View reviewed changes

etcwilde mentioned this pull request Sep 7, 2021

Run part of async main synchronously swiftlang/swift-evolution#1437

Merged

etcwilde force-pushed the ewilde/rewrite-main branch 2 times, most recently from 8bb6cbf to af9298d Compare September 25, 2021 07:53

etcwilde marked this pull request as ready for review September 28, 2021 17:22

etcwilde changed the title ~~WIP: Re-working async-main~~ Re-working async-main Sep 28, 2021

etcwilde force-pushed the ewilde/rewrite-main branch from b8cbaa3 to 708a8a7 Compare September 29, 2021 06:17

etcwilde added 4 commits October 2, 2021 16:53

Paramterize module in lookupIntrinsic

b10c2c8

The lookupConcurrencyIntrinsic function only looked in the concurrency module. It is useful to look in other modules for intrinsics too.

Inherit main thread priority/context

493eca3

This patch changes the main task to inherit the context of the main thread. This should assign the appropriate priority based on how the program was invoked. I've also updated the tests to reflect these changes.

etcwilde force-pushed the ewilde/rewrite-main branch from 708a8a7 to e5cacc7 Compare October 2, 2021 23:53

etcwilde force-pushed the ewilde/rewrite-main branch from e5cacc7 to d2ad13e Compare October 4, 2021 16:35

Replace reinterpret cast with struct

bb80f9f

A single-element struct is structurally the same as the member type itself.

etcwilde force-pushed the ewilde/rewrite-main branch from d2ad13e to bb80f9f Compare October 4, 2021 16:53

etcwilde merged commit 704b370 into swiftlang:main Oct 4, 2021

etcwilde mentioned this pull request Oct 8, 2021

Implementing 0323 async main #39607

Merged

Re-working async-main #38604

Re-working async-main #38604

Uh oh!

Conversation

etcwilde commented Jul 23, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

etcwilde commented Aug 2, 2021

Uh oh!

etcwilde commented Aug 5, 2021

Uh oh!

swift-ci commented Aug 5, 2021

Uh oh!

etcwilde commented Aug 5, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

etcwilde commented Sep 28, 2021

Uh oh!

swift-ci commented Sep 28, 2021

Uh oh!

etcwilde commented Sep 29, 2021

Uh oh!

etcwilde commented Oct 2, 2021

Uh oh!

etcwilde commented Oct 4, 2021

Uh oh!

etcwilde commented Oct 4, 2021

Uh oh!

Uh oh!

etcwilde commented Jul 23, 2021 •

edited

Loading