Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 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
3 changes: 0 additions & 3 deletions working/333 - shared memory multithreading/proposal.md
Original file line number Diff line number Diff line change
Expand Up @@ -1111,9 +1111,6 @@ abstract interface class Coroutine {
external static Coroutine create(void Function() body);

/// Suspends the given currently running coroutine.
///
/// This makes `resume` return with
/// Expects resumer to pass back a value of type [R].
external static void suspend();

/// Resumes previously suspended coroutine.
Expand Down
111 changes: 97 additions & 14 deletions working/333 - shared memory multithreading/shared_native_memory.md
Original file line number Diff line number Diff line change
Expand Up @@ -173,14 +173,14 @@ All compile time constants are deeply immutable instances.
Unmodifiable lists (`List.unmodifiable`) which contain deeply immutable
instances are deeply immutable.

Closures which capture only `final` variables containing deeply immutable
instances are deeply immutable.
Closures which capture only `final`, non-`late` variables containing deeply
Comment thread
mraleph marked this conversation as resolved.
immutable instances are deeply immutable.

Finally, instances of classes annotated with `@pragma('vm:deeply-immutable')`
are deeply immutable. It is a compile error if classes annotated with this
pragma contain non-`final` fields. It is an compile time error if static
type of field within annotated class excludes deeply immutable instances.
If the static type of a field in a deeply immutable class is not
pragma contain non-`final` or `late final` fields. It is an compile time error
Comment thread
mraleph marked this conversation as resolved.
Outdated
if static type of field within annotated class excludes deeply immutable
Comment thread
mraleph marked this conversation as resolved.
Outdated
instances. If the static type of a field in a deeply immutable class is not
deeply immutable type - then compiler must insert checks in the constructor to
guarantee that this field is initialized to a deeply immutable value.

Expand All @@ -206,7 +206,8 @@ values which are deeply immutable objects.
* If static type of a field is a super-type for both deeply immutable and
non-deeply immutable objects then compiler will insert a runtime check
which ensures that values assigned to such field are deeply immutable.
Comment thread
mraleph marked this conversation as resolved.
Outdated
* A field or variable annotated with `@pragma('vm:shared')` must be `final`.
* A field or variable annotated with `@pragma('vm:shared')` must be `final` and
Comment thread
mraleph marked this conversation as resolved.
Outdated
non-`late`.

> [!NOTE]
>
Expand Down Expand Up @@ -306,21 +307,103 @@ class Isolate {
/// to acquire exclusive access to the isolate.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

All these thread-based members of isolate, would it make sense to put them somewhere else than in Isolate, like in dart:isolate_threads, where they could be extension members on Isolate if we really want them there.

Do we expect most people who use isolates to also want to use these thread-based functions, or will they clutter the API for the "normal" uses of Isolate and enocurage people to use them who really shoudn't?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yeah, these are fairly low-level. Maybe we need to introduce dart:threading?

///
/// Throws [StateError] if target isolate is owned by another thread and
/// thus can't be entered from a different thread.
/// thus can't be entered from a different thread. See
Comment thread
mraleph marked this conversation as resolved.
Outdated
/// [markOwnedByCurrentThread] and [isOwnedByCurrentThread].
///
/// Throws [ArgumentError] if the target isolate belongs to another
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

How can an isolate obtain an Isolate object from a different group?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

spawnUri gives you an Isolate object in a different group.

/// isolate group.
Comment thread
mraleph marked this conversation as resolved.
///
/// Throws [ArgumentError] if [f] is not deeply immutable.
///
/// Throws [StateError] if result returned by [f] is not deeply immutable.
Comment thread
mraleph marked this conversation as resolved.
Outdated
R runSync<R>(R Function() f, {Duration? timeout});
external R runSync<R>(R Function() f, {Duration? timeout});

/// Create a new isolate in the current isolate group.
///
/// Similar to `Dart_CreateIsolateInGroup` Dart VM C API.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is this line useful to a reader? (Should I go read the docs on that function. If so, link!)
If not, I don't see the relevance for a Dart user reading this API.

(If it actualy calls that function, it might be relevant information, at some abstraction level. If it's just sort-of similar to a function that I don't know and can't be bothered to look at, that's just noise.)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I wish I could link to these - but we don't publish documentation for these. So the best I could do is to link to sources, which is a bit wonky.

It is hard to say whether these are useful or not - if reader is familiar with C API layer they are certainly useful because most of APIs introduced here are exposing functionality which is available through C API.

I would prefer to keep these here for the purpose of spec review / but we can strip them later when landing actual code.

///
/// Created isolate is in runnable state, but its event loop is not running.
Comment thread
mraleph marked this conversation as resolved.
Outdated
///
/// To start processing isolate's messages:
///
/// * start isolate's event loop synchronously on the current thread
/// by calling [Isolate.runEventLoopSync]
/// * integrate isolate's event loop with other event loop by registering
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why is the first step not enough? (What does the second step do?)

Which "other event loop"?
(Phrasing-wise I expected one of "another event loop", "the other event loop" or "other event loops".)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I have renamed "other event loop" to "external event loop". This path exists for integrating Dart isolate with code which already contains some sort of an event loop (e.g. imagine main platform thread on Android which has its own event loop). In this situation you use Isolate.onMessage to post notifications about pending messages to this external event loop - and then when that external event loop turns around you can drain the message you have been notified about using handleMessage.

I welcome any further suggestions.

/// message callback ([Isolate.onMessage]) and draining pending messages
/// ([Isolate.handleMessage]).
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Who does that? (Which isolate should call those methods?)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

There is documentation on those methods on how to use them. I am not sure if it makes sense to copy it here?

In general I would expect that these are called from outside the isolate (i.e. from the code running inside the isolate group, but not in any concrete isolate).

external static Isolate fork({String? debugName});
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I find the name fork strange as a C fork() call actually gives you a copy of the state of the process, there's no clearing of state to some initial state happening.

Furthermore from the motivating example in #4620 (comment) it seems that the caller may not even be an isolate with state. So when a Thread (aka shared isolate) calls Isolate.fork() it gets an isolate with freshly initialized state.

Why not call it Isolate.create() instead?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Renamed to Isolate.create.


/// Shutdown target isolate.
Comment thread
mraleph marked this conversation as resolved.
Outdated
///
/// This function will block until it acquires exclusive access to the
/// target isolate. Isolate can only be entered for synchronous execution
/// between turns of its event loop, when no other thread is
Comment thread
mraleph marked this conversation as resolved.
/// executing code in the target isolate.
external void shutdown();
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

How does this differ from isolate.kill(Isolate.beforeNextEvent)?

Could it be called killSync()?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It's closer to Isolate.kill(Isolate.immediate) but completely synchrounous.

I would prefer to align terms with existing names from VM C API.


/// Set current OS thread as owner of the isolate.
///
/// Once an isolate is owned by some OS thread it can not be
Comment thread
mraleph marked this conversation as resolved.
Outdated
/// entered by any other OS thread. An attempt to acquire
/// exclusive access to it from another thread will fail with
/// an error.
///
/// Equivalent to `Dart_SetCurrentThreadOwnsIsolate` Dart VM C API.
Comment thread
mraleph marked this conversation as resolved.
///
/// Throws [ArgumentError] if `this` is not `Isolate.current`.
Comment thread
mraleph marked this conversation as resolved.
Outdated
///
/// Throws [StateError] if target isolate is already owned by another thread.
Comment thread
mraleph marked this conversation as resolved.
Outdated
external void markOwnedByCurrentThread();
Comment thread
mraleph marked this conversation as resolved.
Outdated
Comment thread
mraleph marked this conversation as resolved.
Outdated

/// Returns `true` if the isolate is owned by the current OS thread.
Comment thread
mraleph marked this conversation as resolved.
Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

When would you use this property?

Unlike markAsOwnedByCurrentThread, it doesn't require the isolate to be Isolate.current, so you can query other isolates for whether you own them, presumably before trying to call into them.
But you need to run code inside the isolate to get to own it, so not sure what the constraints are.

If you don't own the isolate, then you probably want to. In that case, you may need to know whether someone else owns it already. (So back to wanting a way to know that.)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

You can use this to decide whether you can just enter the isolate or you need to message it.

///
/// Equivalent to `Dart_GetCurrentThreadOwnsIsolate` Dart VM C API.
external bool get isOwnedByCurrentThread;

/// Run event loop for the target isolate synchronously on the current thread.
///
/// This function will block until it acquires exclusive access to the
/// target isolate. Isolate can only be entered for synchronous execution
/// between turns of its event loop, when no other thread is
/// executing code in the target isolate.
Comment thread
mraleph marked this conversation as resolved.
///
/// The isolate will be marked as owned by the current thread.
///
/// Similar to `Dart_RunLoop` Dart VM C API, but unlike `Dart_RunLoop` this
/// function executes isolate's event loop on the current thread instead
/// of delegating it into the thread-pool.
///
/// Throws [StateError] if target isolate is owned by another thread.
Comment thread
mraleph marked this conversation as resolved.
Outdated
external static void runEventLoopSync();
Comment thread
mraleph marked this conversation as resolved.
Outdated
Comment thread
mraleph marked this conversation as resolved.
Outdated

/// Set message notify callback for the isolate.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Drop the "Set". (Document properties with noun phrases.)
Or make it a function instead of a setter.

Styles do not agree on how to handle onFoos. Some uses streams. This is probably too low-level for that.
The StreamSubscription has methods, sub.onData((data) { .... }). I think it works slightly better than setters-without-getters.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Unless we have some specific rule prohibiting setter-without-getter I would prefer to use that.

We already have similar code in other places: https://api.dart.dev/dart-isolate/RawReceivePort/handler.html

///
/// Provided callback will be called once for every message added to the
Comment thread
mraleph marked this conversation as resolved.
Outdated
/// isolates message queue. Pending messages can be then later be drained
/// by calling [Isolate.handleMessage].
///
/// Provided [callback] must be deeply immutable and will be called
/// on an arbitrary thread and not necessarily within some isolate. See
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

"not necessarily within some isolate"

Is that "not necessarily within a specific isolate" or "not necessarily within any isolate"?
(Guessing latter. I don't like the concept of running "outside of any isolate". I guess Isolate.current will throw, along with a lot of other things, but it's still inside an isolate group, which is weird if it's not an isolate. Terminology is hard. How about running in a "raw isolate", which doesn't have access to any of the things that require an event loop, or even being properly initialized. Sound .... "cool"! 😎 ).

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yeah, I just don't see any good naming for this, hence the reference to isolateGroupBound. I think names like xyz isolate don't work - because it is not isolate at all. The most accurate description is "no-non-shared-statics-isolate", but alas it's way too long.

/// [NativeCallable.isolateGroupBound].
///
/// IMPORTANT: [Isolate.handleMessage] must *not* be called from the
Comment thread
mraleph marked this conversation as resolved.
Outdated
/// `callback`.
Comment thread
mraleph marked this conversation as resolved.
Outdated
///
/// Similar to `Dart_SetMessageNotifyCallback` Dart VM C API.
external void set onMessage(void Function(Isolate) callback);
Comment thread
mraleph marked this conversation as resolved.
Outdated

/// Handle a single pending message from isolate's message queue.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What if there aren't any?
(I hope nothing happens.)

What happens if the isolate is paused?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

  • Nothing happens.

  • Not all isolates can be paused (e.g. pauseCapability can be null). I think currently you can't pause isolates which use custom event loops.

///
/// This function will block until it acquires exclusive access to the
/// target isolate. Isolate can only be entered for synchronous execution
/// between turns of its event loop, when no other thread is
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What is a "turn"? 😉

/// executing code in the target isolate.
Comment thread
mraleph marked this conversation as resolved.
///
/// Similar to `Dart_HandleMessage` Dart VM C API.
external void handleMessage();
}
```

**TODO**: Furthermore we might want to facilitate integration with third-party
event-loops: e.g. allow to create isolate without scheduling its event loop on
our own thread pool and provide equivalents of `Dart_SetMessageNotifyCallback`
and `Dart_HandleMessage`. Though maybe we should not bundle this all together
into one update.

### Scoped thread local values

```dart
Expand Down