Skip to content

[Concurrency] repair cooperative executor and its tests #39365

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 6 commits into from
Sep 27, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions cmake/modules/AddSwiftUnittests.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,13 @@ function(add_swift_unittest test_dirname)
_ENABLE_EXTENDED_ALIGNED_STORAGE)
endif()

# some headers switch their inline implementations based on
# SWIFT_STDLIB_SINGLE_THREADED_RUNTIME definition
if(SWIFT_STDLIB_SINGLE_THREADED_RUNTIME)
target_compile_definitions("${test_dirname}" PRIVATE
SWIFT_STDLIB_SINGLE_THREADED_RUNTIME)
endif()

if(NOT SWIFT_COMPILER_IS_MSVC_LIKE)
if(SWIFT_USE_LINKER)
target_link_options(${test_dirname} PRIVATE
Expand Down
10 changes: 3 additions & 7 deletions stdlib/public/Concurrency/GlobalExecutor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ static DelayedJob *DelayedJobQueue = nullptr;

/// Get the next-in-queue storage slot.
static Job *&nextInQueue(Job *cur) {
return reinterpret_cast<Job*&>(&cur->SchedulerPrivate[NextWaitingTaskIndex]);
return reinterpret_cast<Job*&>(cur->SchedulerPrivate[Job::NextWaitingTaskIndex]);
}

/// Insert a job into the cooperative global queue.
Expand Down Expand Up @@ -448,13 +448,9 @@ void swift::swift_task_enqueueOnDispatchQueue(Job *job,
}
#endif

#if SWIFT_CONCURRENCY_COOPERATIVE_GLOBAL_EXECUTOR
static HeapObject _swift_mainExecutorIdentity;
#endif

ExecutorRef swift::swift_task_getMainExecutor() {
#if SWIFT_CONCURRENCY_COOPERATIVE_GLOBAL_EXECUTOR
return ExecutorRef::forOrdinary(&_swift_mainExecutorIdentity, nullptr);
return ExecutorRef::generic();
Copy link
Member

Choose a reason for hiding this comment

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

This is interesting. In the cooperative case, does@MainActor not really have its own executor, because all executors are the same? Or is it that we should have a main-actor actor and dispatching to it goes in through the same run loop? I think I agree that this is the right approach.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point. I respected the former from my understanding, but I'm not confident here.

#else
return ExecutorRef::forOrdinary(
reinterpret_cast<HeapObject*>(&_dispatch_main_q),
Expand All @@ -464,7 +460,7 @@ ExecutorRef swift::swift_task_getMainExecutor() {

bool ExecutorRef::isMainExecutor() const {
#if SWIFT_CONCURRENCY_COOPERATIVE_GLOBAL_EXECUTOR
return Identity == &_swift_mainExecutorIdentity;
return isGeneric();
#else
return Identity == reinterpret_cast<HeapObject*>(&_dispatch_main_q);
#endif
Expand Down
3 changes: 3 additions & 0 deletions stdlib/public/Concurrency/Mutex.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,3 +19,6 @@

#include "../runtime/MutexPThread.cpp"
#include "../runtime/MutexWin32.cpp"
#ifdef SWIFT_STDLIB_SINGLE_THREADED_RUNTIME
#include "swift/Runtime/MutexSingleThreaded.h"
#endif
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
// rdar://76038845
// REQUIRES: concurrency_runtime
// UNSUPPORTED: back_deployment_runtime
// Disable on cooperative executor because it can't dispatch jobs before the end of main function
// UNSUPPORTED: single_threaded_runtime

import Dispatch

Expand Down
1 change: 1 addition & 0 deletions test/Concurrency/Runtime/cancellation_handler.swift
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
// rdar://76038845
// REQUIRES: concurrency_runtime
// UNSUPPORTED: back_deployment_runtime
// UNSUPPORTED: single_threaded_runtime

// for sleep
#if canImport(Darwin)
Expand Down
5 changes: 3 additions & 2 deletions test/Concurrency/Runtime/data_race_detection.swift
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
// rdar://76038845
// REQUIRES: concurrency_runtime
// UNSUPPORTED: back_deployment_runtime
// UNSUPPORTED: single_threaded_runtime

import _Concurrency
import Dispatch
Expand Down Expand Up @@ -58,14 +59,14 @@ actor MyActor {
struct Runner {
static func main() async {
print("Launching a main-actor task")
// CHECK: warning: data race detected: @MainActor function at main/data_race_detection.swift:22 was not called on the main thread
// CHECK: warning: data race detected: @MainActor function at main/data_race_detection.swift:23 was not called on the main thread
launchFromMainThread()
sleep(1)

let actor = MyActor()
let actorFn = await actor.getTaskOnMyActor()
print("Launching an actor-instance task")
// CHECK: warning: data race detected: actor-isolated function at main/data_race_detection.swift:51 was not called on the same actor
// CHECK: warning: data race detected: actor-isolated function at main/data_race_detection.swift:52 was not called on the same actor
launchTask(actorFn)

sleep(1)
Expand Down
1 change: 1 addition & 0 deletions test/Concurrency/Runtime/mainactor.swift
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
// rdar://76038845
// REQUIRES: concurrency_runtime
// UNSUPPORTED: back_deployment_runtime
// UNSUPPORTED: single_threaded_runtime

import Dispatch

Expand Down
1 change: 1 addition & 0 deletions test/Interpreter/enforce_exclusive_access.swift
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
// RUN: %target-codesign %t/a.out
// RUN: %target-run %t/a.out
// REQUIRES: executable_test
// UNSUPPORTED: single_threaded_runtime

// Tests for traps at run time when enforcing exclusive access.

Expand Down
8 changes: 6 additions & 2 deletions unittests/runtime/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,10 @@ if(("${SWIFT_HOST_VARIANT_SDK}" STREQUAL "${SWIFT_PRIMARY_VARIANT_SDK}") AND
endif()
endif()

if(NOT SWIFT_STDLIB_SINGLE_THREADED_RUNTIME)
list(APPEND PLATFORM_SOURCES Mutex.cpp)
endif()

if(SWIFT_ENABLE_EXPERIMENTAL_DISTRIBUTED)
# list(APPEND PLATFORM_SOURCES
# DistributedActor.cpp
Expand All @@ -98,15 +102,15 @@ if(("${SWIFT_HOST_VARIANT_SDK}" STREQUAL "${SWIFT_PRIMARY_VARIANT_SDK}") AND
weak.mm
Refcounting.mm
Actor.cpp
TaskStatus.cpp)
TaskStatus.cpp
Mutex.cpp)

add_swift_unittest(SwiftRuntimeTests
Array.cpp
CompatibilityOverrideRuntime.cpp
CompatibilityOverrideConcurrency.cpp
Concurrent.cpp
Metadata.cpp
Mutex.cpp
Enum.cpp
Refcounting.cpp
Stdlib.cpp
Expand Down
3 changes: 3 additions & 0 deletions unittests/runtime/Concurrent.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ TEST(ConcurrentReadableArrayTest, SingleThreaded) {
check();
}

#ifndef SWIFT_STDLIB_SINGLE_THREADED_RUNTIME

TEST(ConcurrentReadableArrayTest, MultiThreaded) {
const int insertCount = 100000;

Expand Down Expand Up @@ -542,3 +544,4 @@ TEST(ConcurrentReadableHashMapTest, MultiThreaded4) {
runTest(16, 1);
runTest(16, 8);
}
#endif // !SWIFT_STDLIB_SINGLE_THREADED_RUNTIME
1 change: 1 addition & 0 deletions validation-test/Runtime/ConcurrentMetadata.swift
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
// RUN: %target-run-simple-swift
// REQUIRES: executable_test
// UNSUPPORTED: single_threaded_runtime

// Exercise the metadata cache from multiple threads to shake out any
// concurrency bugs.
Expand Down