Merge pull request #81134 from ktoso/wip-adjust-isIsolatingCurrentContext-impl

Author: ktoso

include/swift/ABI/Executor.h

@@ -17,10 +17,10 @@
 #ifndef SWIFT_ABI_EXECUTOR_H
 #define SWIFT_ABI_EXECUTOR_H
 
-#include <inttypes.h>
 #include "swift/ABI/Actor.h"
 #include "swift/ABI/HeapObject.h"
 #include "swift/Runtime/Casting.h"
+#include <inttypes.h>
 
 namespace swift {
 class AsyncContext;
@@ -413,6 +413,23 @@ class AsyncFunctionPointer {
   uint32_t ExpectedContextSize;
 };
 
+/// Type-safe wrapper around the return value of `isIsolatingCurrentContext`.
+enum class IsIsolatingCurrentContextDecision : int8_t {
+  // The function call could not determine if the current context is isolated
+  // by this executor or not. Default value for executors which do not implement
+  // `isIsolatingCurrentContext`.
+  Unknown = -1,
+  // The current context is definitely not isolated by this executor.
+  NotIsolated = 0,
+  // The current context is definitely isolated by this executor.
+  Isolated = 1,
+};
+
+IsIsolatingCurrentContextDecision
+getIsIsolatingCurrentContextDecisionFromInt(int8_t value);
+
+StringRef getIsIsolatingCurrentContextDecisionNameStr(IsIsolatingCurrentContextDecision decision);
+
 }
 
 #endif

include/swift/ABI/Metadata.h

@@ -2984,14 +2984,6 @@ struct TargetProtocolConformanceDescriptor final
     return Demangle::makeSymbolicMangledNameStringRef(this->template getTrailingObjects<TargetGlobalActorReference<Runtime>>()->type);
   }
 
-  /// True if this is a conformance to 'SerialExecutor' which has a non-default
-  /// (i.e. not the stdlib's default implementation) witness. This means that
-  /// the developer has implemented this method explicitly and we should prefer
-  /// calling it.
-  bool hasNonDefaultSerialExecutorIsIsolatingCurrentContext() const {
-    return Flags.hasNonDefaultSerialExecutorIsIsolatingCurrentContext();
-  }
-
   /// Retrieve the protocol conformance of the global actor type to the
   /// GlobalActor protocol.
   const TargetProtocolConformanceDescriptor<Runtime> *

include/swift/ABI/MetadataValues.h

@@ -754,14 +754,6 @@ class ConformanceFlags {
     IsConformanceOfProtocolMask = 0x01u << 18,
     HasGlobalActorIsolation = 0x01u << 19,
 
-    // Used to detect if this is a conformance to SerialExecutor that has
-    // an user defined implementation of 'isIsolatingCurrentContext'. This
-    // requirement is special in the sense that if a non-default impl is present
-    // we will avoid calling the `checkIsolated` method which would lead to a
-    // crash. In other words, this API "soft replaces" 'checkIsolated' so we
-    // must at runtime the presence of a non-default implementation.
-    HasNonDefaultSerialExecutorIsIsolatingCurrentContext = 0x01u << 20,
-
     NumConditionalPackDescriptorsMask = 0xFFu << 24,
     NumConditionalPackDescriptorsShift = 24
   };
@@ -828,15 +820,7 @@ class ConformanceFlags {
                                  : 0));
   }
 
-  ConformanceFlags withHasNonDefaultSerialExecutorIsIsolatingCurrentContext(
-                                           bool hasNonDefaultSerialExecutorIsIsolatingCurrentContext) const {
-    return ConformanceFlags((Value & ~HasNonDefaultSerialExecutorIsIsolatingCurrentContext)
-                            | (hasNonDefaultSerialExecutorIsIsolatingCurrentContext
-                                 ? HasNonDefaultSerialExecutorIsIsolatingCurrentContext
-                                 : 0));
-  }
-
-  /// Retrieve the type reference kind kind.
+  /// Retrieve the type reference kind.
   TypeReferenceKind getTypeReferenceKind() const {
     return TypeReferenceKind(
                       (Value & TypeMetadataKindMask) >> TypeMetadataKindShift);
@@ -880,10 +864,6 @@ class ConformanceFlags {
     return Value & HasGlobalActorIsolation;
   }
 
-  bool hasNonDefaultSerialExecutorIsIsolatingCurrentContext() const {
-    return Value & HasNonDefaultSerialExecutorIsIsolatingCurrentContext;
-  }
-
   /// Retrieve the # of conditional requirements.
   unsigned getNumConditionalRequirements() const {
     return (Value & NumConditionalRequirementsMask)

include/swift/Runtime/Concurrency.h

@@ -777,12 +777,12 @@ bool swift_task_invokeSwiftCheckIsolated(SerialExecutorRef executor);
 
 /// Invoke an executor's `isIsolatingCurrentContext` implementation;
 SWIFT_EXPORT_FROM(swift_Concurrency) SWIFT_CC(swift)
-bool swift_task_isIsolatingCurrentContext(SerialExecutorRef executor);
+int8_t swift_task_isIsolatingCurrentContext(SerialExecutorRef executor);
 
 /// Invoke a Swift executor's `isIsolatingCurrentContext` implementation; returns
 /// `true` if it invoked the Swift implementation, `false` otherwise.
 SWIFT_EXPORT_FROM(swift_Concurrency) SWIFT_CC(swift)
-bool swift_task_invokeSwiftIsIsolatingCurrentContext(SerialExecutorRef executor);
+int8_t swift_task_invokeSwiftIsIsolatingCurrentContext(SerialExecutorRef executor);
 
 /// A count in nanoseconds.
 using JobDelay = unsigned long long;
@@ -1041,12 +1041,6 @@ enum swift_task_is_current_executor_flag : uint64_t {
   /// The routine MUST NOT assert on failure.
   /// Even at the cost of not calling 'checkIsolated' if it is available.
   MustNotAssert = 0x10,
-
-  /// The routine should use 'isIsolatingCurrentContext' function on the
-  /// 'expected' executor instead of `checkIsolated`.
-  ///
-  /// This is a variant of `MustNotAssert`
-  UseIsIsolatingCurrentContext = 0x20,
 };
 
 SWIFT_EXPORT_FROM(swift_Concurrency)

include/swift/Runtime/ConcurrencyHooks.def

@@ -47,7 +47,7 @@ SWIFT_CONCURRENCY_HOOK(void, swift_task_enqueueGlobalWithDeadline,
 SWIFT_CONCURRENCY_HOOK(void, swift_task_checkIsolated,
     SerialExecutorRef executor);
 
-SWIFT_CONCURRENCY_HOOK(bool, swift_task_isIsolatingCurrentContext,
+SWIFT_CONCURRENCY_HOOK(int8_t, swift_task_isIsolatingCurrentContext,
     SerialExecutorRef executor);
 
 SWIFT_CONCURRENCY_HOOK(bool, swift_task_isOnExecutor,

lib/IRGen/GenProto.cpp

@@ -2225,7 +2225,6 @@ namespace {
         Flags = Flags.withIsSynthesizedNonUnique(conf->isSynthesizedNonUnique());
         Flags = Flags.withIsConformanceOfProtocol(conf->isConformanceOfProtocol());
         Flags = Flags.withHasGlobalActorIsolation(isolation.isGlobalActor());
-        Flags = withSerialExecutorCheckingModeFlags(Flags, conf);
       } else {
         Flags = Flags.withIsRetroactive(false)
                      .withIsSynthesizedNonUnique(false);
@@ -2442,31 +2441,6 @@ namespace {
       B.addRelativeAddress(globalActorConformanceDescriptor);
     }
 
-    static ConformanceFlags
-    withSerialExecutorCheckingModeFlags(ConformanceFlags Flags, const NormalProtocolConformance *conf) {
-      ProtocolDecl *proto = conf->getProtocol();
-      auto &C = proto->getASTContext();
-
-      ConformanceFlags UpdatedFlags = Flags;
-      if (proto->isSpecificProtocol(swift::KnownProtocolKind::SerialExecutor)) {
-        conf->forEachValueWitness([&](const ValueDecl *req,
-                                      Witness witness) {
-          bool nameMatch = witness.getDecl()->getBaseIdentifier() == C.Id_isIsolatingCurrentContext;
-          if (nameMatch) {
-            if (DeclContext *NominalOrExtension = witness.getDecl()->getDeclContext()) {
-              // If the witness is NOT the default implementation in the _Concurrency library,
-              // we should record that this is an user provided implementation and we should call it.
-              bool hasNonDefaultIsIsolatingCurrentContext =
-                  !NominalOrExtension->getParentModule()->isConcurrencyModule();
-              UpdatedFlags = UpdatedFlags.withHasNonDefaultSerialExecutorIsIsolatingCurrentContext(
-                            hasNonDefaultIsIsolatingCurrentContext);
-            }
-          }
-        });
-      }
-
-      return UpdatedFlags;
-    }
   };
 }
 

stdlib/public/Concurrency/Actor.cpp

@@ -410,9 +410,6 @@ static void _swift_task_debug_dumpIsCurrentExecutorFlags(
   if (options.contains(swift_task_is_current_executor_flag::Assert))
     SWIFT_TASK_DEBUG_LOG("%s swift_task_is_current_executor_flag::%s",
                          hint, "Assert");
-  if (options.contains(swift_task_is_current_executor_flag::UseIsIsolatingCurrentContext))
-    SWIFT_TASK_DEBUG_LOG("%s swift_task_is_current_executor_flag::%s",
-                         hint, "UseIsIsolatingCurrentContext");
 }
 
 // Shimming call to Swift runtime because Swift Embedded does not have
@@ -470,13 +467,6 @@ swift_task_is_current_executor_flag swift_bincompat_selectDefaultIsCurrentExecut
       // Remove the assert option which is what would cause the "crash" mode
       options = swift_task_is_current_executor_flag(
         options & ~swift_task_is_current_executor_flag::Assert);
-    } else if (strcmp(modeStr, "isIsolatingCurrentContext") == 0) {
-      options = swift_task_is_current_executor_flag(
-        options | swift_task_is_current_executor_flag::UseIsIsolatingCurrentContext);
-      // When we're using the isIsolatingCurrentContext we don't want to use crashing APIs,
-      // so disable it explicitly.
-      options = swift_task_is_current_executor_flag(
-        options & ~swift_task_is_current_executor_flag::Assert);
     } else if (strcmp(modeStr, "crash") == 0 ||
                strcmp(modeStr, "swift6") == 0) {
       options = swift_task_is_current_executor_flag(
@@ -501,43 +491,11 @@ extern "C" SWIFT_CC(swift) void _swift_task_enqueueOnExecutor(
     Job *job, HeapObject *executor, const Metadata *executorType,
     const SerialExecutorWitnessTable *wtable);
 
-/// Check the executor's witness table for specific information about e.g.
-/// being able ignore `checkIsolated` and only call `isIsolatingCurrentContext`.
-static swift_task_is_current_executor_flag
-_getIsolationCheckingOptionsFromExecutorWitnessTable(const SerialExecutorWitnessTable *_wtable) {
-  const WitnessTable* wtable = reinterpret_cast<const WitnessTable*>(_wtable);
-#if SWIFT_STDLIB_USE_RELATIVE_PROTOCOL_WITNESS_TABLES
-  auto description = lookThroughOptionalConditionalWitnessTable(
-    reinterpret_cast<const RelativeWitnessTable*>(wtable))
-    ->getDescription();
-#else
-  auto description = wtable->getDescription();
-#endif
-  if (!description) {
-    return swift_task_is_current_executor_flag::None;
-  }
-
-  if (description->hasNonDefaultSerialExecutorIsIsolatingCurrentContext()) {
-    // The specific executor has implemented `isIsolatingCurrentContext` and
-    // we do not have to call `checkIsolated`.
-    return swift_task_is_current_executor_flag::UseIsIsolatingCurrentContext;
-  }
-
-  // No changes to the checking mode.
-  return swift_task_is_current_executor_flag::None;
-}
-
 SWIFT_CC(swift)
 static bool swift_task_isCurrentExecutorWithFlagsImpl(
     SerialExecutorRef expectedExecutor,
     swift_task_is_current_executor_flag flags) {
   auto current = ExecutorTrackingInfo::current();
-  if (expectedExecutor.getIdentity() && expectedExecutor.hasSerialExecutorWitnessTable()) {
-    if (auto *wtable = expectedExecutor.getSerialExecutorWitnessTable()) {
-      auto executorSpecificMode = _getIsolationCheckingOptionsFromExecutorWitnessTable(wtable);
-      flags = swift_task_is_current_executor_flag(flags | executorSpecificMode);
-    }
-  }
 
   auto options = SwiftTaskIsCurrentExecutorOptions(flags);
   _swift_task_debug_dumpIsCurrentExecutorFlags(__FUNCTION__, flags);
@@ -557,16 +515,28 @@ static bool swift_task_isCurrentExecutorWithFlagsImpl(
     // We cannot use 'complexEquality' as it requires two executor instances,
     // and we do not have a 'current' executor here.
 
-    if (options.contains(swift_task_is_current_executor_flag::UseIsIsolatingCurrentContext)) {
-      SWIFT_TASK_DEBUG_LOG("executor checking mode option: UseIsIsolatingCurrentContext; invoke (%p).isIsolatingCurrentContext",
-                           expectedExecutor.getIdentity());
-      // The executor has the most recent 'isIsolatingCurrentContext' API
-      // so available so we prefer calling that to 'checkIsolated'.
-      auto result = swift_task_isIsolatingCurrentContext(expectedExecutor);
-
-      SWIFT_TASK_DEBUG_LOG("executor checking mode option: UseIsIsolatingCurrentContext; invoke (%p).isIsolatingCurrentContext => %s",
-                     expectedExecutor.getIdentity(), result ? "pass" : "fail");
-      return result;
+    // Invoke the 'isIsolatingCurrentContext', if "undecided" (i.e. nil), we need to make further calls
+    SWIFT_TASK_DEBUG_LOG("executor checking, invoke (%p).isIsolatingCurrentContext",
+                         expectedExecutor.getIdentity());
+    // The executor has the most recent 'isIsolatingCurrentContext' API
+    // so available so we prefer calling that to 'checkIsolated'.
+    auto isIsolatingCurrentContextDecision =
+        getIsIsolatingCurrentContextDecisionFromInt(
+            swift_task_isIsolatingCurrentContext(expectedExecutor));
+
+    SWIFT_TASK_DEBUG_LOG("executor checking mode option: UseIsIsolatingCurrentContext; invoke (%p).isIsolatingCurrentContext => %s",
+                   expectedExecutor.getIdentity(), getIsIsolatingCurrentContextDecisionNameStr(isIsolatingCurrentContextDecision));
+    switch (isIsolatingCurrentContextDecision) {
+      case IsIsolatingCurrentContextDecision::Isolated:
+        // We know for sure that this serial executor is isolating this context, return the decision.
+        return true;
+      case IsIsolatingCurrentContextDecision::NotIsolated:
+        // We know for sure that this serial executor is NOT isolating this context, return this decision.
+        return false;
+      case IsIsolatingCurrentContextDecision::Unknown:
+        // We don't know, so we have to continue trying to check using other methods.
+        // This most frequently would happen if a serial executor did not implement isIsolatingCurrentContext.
+        break;
     }
 
     // Otherwise, as last resort, let the expected executor check using
@@ -675,18 +645,21 @@ static bool swift_task_isCurrentExecutorWithFlagsImpl(
 
   // Invoke the 'isIsolatingCurrentContext' function if we can; If so, we can
   // avoid calling the `checkIsolated` because their result will be the same.
-  if (options.contains(swift_task_is_current_executor_flag::UseIsIsolatingCurrentContext)) {
-    SWIFT_TASK_DEBUG_LOG("executor checking: can call (%p).isIsolatingCurrentContext",
-      expectedExecutor.getIdentity());
+  SWIFT_TASK_DEBUG_LOG("executor checking: call (%p).isIsolatingCurrentContext",
+    expectedExecutor.getIdentity());
 
-    bool checkResult = swift_task_isIsolatingCurrentContext(expectedExecutor);
+  const auto isIsolatingCurrentContextDecision =
+    getIsIsolatingCurrentContextDecisionFromInt(swift_task_isIsolatingCurrentContext(expectedExecutor));
 
-    SWIFT_TASK_DEBUG_LOG("executor checking: can call (%p).isIsolatingCurrentContext => %p",
-      expectedExecutor.getIdentity(), checkResult ? "pass" : "fail");
-    return checkResult;
-  } else {
-    SWIFT_TASK_DEBUG_LOG("executor checking: can NOT call (%p).isIsolatingCurrentContext",
-                         expectedExecutor.getIdentity());
+  SWIFT_TASK_DEBUG_LOG("executor checking: can call (%p).isIsolatingCurrentContext => %p",
+    expectedExecutor.getIdentity(), getIsIsolatingCurrentContextDecisionNameStr(isIsolatingCurrentContextDecision));
+  switch (isIsolatingCurrentContextDecision) {
+  case IsIsolatingCurrentContextDecision::Isolated:
+    return true;
+  case IsIsolatingCurrentContextDecision::NotIsolated:
+    return false;
+  case IsIsolatingCurrentContextDecision::Unknown:
+    break;
   }
 
   // This provides a last-resort check by giving the expected SerialExecutor the

stdlib/public/Concurrency/ConcurrencyHooks.cpp

@@ -111,18 +111,20 @@ swift::swift_task_checkIsolated(SerialExecutorRef executor) {
     swift_task_checkIsolatedOrig(executor);
 }
 
-SWIFT_CC(swift) static bool
+SWIFT_CC(swift) static int8_t
 swift_task_isIsolatingCurrentContextOrig(SerialExecutorRef executor) {
   return swift_task_isIsolatingCurrentContextImpl(
       *reinterpret_cast<SwiftExecutorRef *>(&executor));
 }
 
-bool
+int8_t
 swift::swift_task_isIsolatingCurrentContext(SerialExecutorRef executor) {
-  if (SWIFT_UNLIKELY(swift_task_isIsolatingCurrentContext_hook))
-    return swift_task_isIsolatingCurrentContext_hook(executor, swift_task_isIsolatingCurrentContextOrig);
-  else
+  if (SWIFT_UNLIKELY(swift_task_isIsolatingCurrentContext_hook)) {
+    return swift_task_isIsolatingCurrentContext_hook(
+        executor, swift_task_isIsolatingCurrentContextOrig);
+  } else {
     return swift_task_isIsolatingCurrentContextOrig(executor);
+  }
 }
 
 // Implemented in Swift because we need to obtain the user-defined flags on the executor ref.

stdlib/public/Concurrency/CooperativeGlobalExecutor.cpp

@@ -155,7 +155,7 @@ void swift_task_checkIsolatedImpl(SwiftExecutorRef executor) {
 }
 
 SWIFT_CC(swift)
-bool swift_task_isIsolatingCurrentContextImpl(SwiftExecutorRef executor) {
+int8_t swift_task_isIsolatingCurrentContextImpl(SwiftExecutorRef executor) {
   return swift_executor_invokeSwiftIsIsolatingCurrentContext(executor);
 }