Skip to content
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

Add note regarding synchronous communication between too many isolates #6182

Merged
merged 12 commits into from
Nov 8, 2024

Conversation

aam
Copy link
Contributor

@aam aam commented Oct 30, 2024

@dart-github-bot
Copy link
Collaborator

dart-github-bot commented Oct 30, 2024

Visit the preview URL for this PR (updated for commit 473bf61):

https://dart-dev--pr6182-comment-on-limit-concurrency-4azolmdd.web.app


- The limit is not hardcoded to a particular number, it is calculated based on the Dart VM heap size available to the Dart application, can be considered to be between 8 and 32 depending on the platform.
- This limit doesn't affect asynchronous communction between isolates via messages - you can have hundreds of isolates running and making progress. The isolates are scheduled on the CPU in round-robin fashion and yield to each other often.
- Attempts to do *synchronous* communication between isolates over the limit though may result in a deadlock.
Copy link
Member

Choose a reason for hiding this comment

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

... unless special care is taken (the C code that does synchronous communication would need to leave the current isolate before it blocks and re-enter it before returning to dart, see ...)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the comment, I added this. PTAL.

@@ -39,6 +39,12 @@ but here are some more situations where they can be useful:
- Performing I/O, such as communicating with a database.
- Handling a large volume of network requests.

Also note a caveat due to a limit to the number of concurrently running isolates:
Copy link
Member

Choose a reason for hiding this comment

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

I'd rephrase this as

"""
Synchronous blocking communication between isolates.

If isolates want to communicate synchronously they have to leave the safety of Dart and resort to using C code via FFI to do so. Special care must be taken in this circumstance due to the Dart VM imposing a limit on the number of isolates that can run in parallel. If the number of isolates that synchronously block in FFI calls exceeds that limit no other isolate can run. If those blocking isolates wait on another unscheduled isolate to act, then deadlock can occur. To avoid this situation (preventing other isolates from running & possibly deadlock scenarios) such blocking C code must take care of leaving the current isolate before performing the blocking operation and re-entering the isolate before returning from the FFI call (see ...)
"""

(especially concurrently -> in parallel - we can run thousands of isolates concurrently but not in parallel, i.e. at the same time)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This text sounds good, but did you mean for it to replace my version or you thought about adding it?

@aam aam requested a review from ericwindmill November 4, 2024 17:36
@aam
Copy link
Contributor Author

aam commented Nov 4, 2024

not able to merge the PR, let's see if having Eric approve it allows merging. Or whether Eric can merge it. Adding Andrew since Eric seems to be ooo.

Copy link
Contributor

@MaryaBelanger MaryaBelanger left a comment

Choose a reason for hiding this comment

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

I think it makes more sense to include this topic in the "Limitations of isolates" section on the Concurrency > Overview page.

The page titled "Isolates" that you have it on now is a walkthrough / example, whereas the Overview page is all the conceptual / background stuff. You can give your new content its own section under "Limitations of isolates", maybe titled "Synchronous blocking communication between isolates" like @mkustermann said in his comment.

Sorry about the confusion! I can see now that it's not clear what the purpose of each page is, and that they don't really point to each other like they should. I'll fix that.

I haven't reviewed the actual content yet, but I'll come back if you agree with this change or want to discuss it more, thanks!

Edit: I can take over the PR to make this change too, if that's easier for you! Let me know :)

@aam
Copy link
Contributor Author

aam commented Nov 4, 2024

Edit: I can take over the PR to make this change too, if that's easier for you! Let me know :)

Didn't see the edit, so made the changes. Feel free to take over, thanks Marya!

@MaryaBelanger MaryaBelanger self-assigned this Nov 4, 2024

- The limit is not hardcoded to a particular number, it is calculated based on the Dart VM heap size available to the Dart application, can be considered to be between 8 and 32 depending on the platform.
- This limit doesn't affect asynchronous communction between isolates via messages - you can have hundreds of isolates running and making progress. The isolates are scheduled on the CPU in round-robin fashion and yield to each other often.
- Attempts to do *synchronous* communication between isolates over the limit though may result in a deadlock unless special care is taken (the C code that does synchronous communication would need to leave the current isolate before it blocks and re-enter it before returning to dart, see [`Dart_EnterIsolate`]({{site.repo.dart.sdk}}/blob/c9a8bbd8d6024e419b5e5f26b5131285eb19cc93/runtime/include/dart_api.h#L1254) and [`Dart_ExitIsolate`]({{site.repo.dart.sdk}}/blob/c9a8bbd8d6024e419b5e5f26b5131285eb19cc93/runtime/include/dart_api.h#L1455).
Copy link
Contributor

Choose a reason for hiding this comment

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

FWIW I would expand on this last bullet a bit. It's full of advanced concepts that aren't discussed in these docs at all, and they're concepts that many Dart developers probably have never thought about in their life.

Given the recent user survey that found folks want more advanced isolate samples, we might even want to add example code of attempting synchronous communication between isolates. That would probably constitute a new issue and separate PR. I'll leave that up to you, @MaryaBelanger, and let me know if you want to discuss.

@MaryaBelanger
Copy link
Contributor

MaryaBelanger commented Nov 7, 2024

@parlough Can you checkout the links on Dart_EnterIsolate and Dart_ExitIsolate in the staged view? When I follow the links normally they work but in staging and serving the site locally they give 404/cannot be reached, respectively. Just wasn't refreshed :)

@aam Are there api docs pages for Dart_EnterIsolate and Dart_ExitIsolate on https://api.dart.dev/? I looked but couldn't find anything. Even if they're in dev and not stable, that works fine.

Also, I combined a little of Martin's comment with your work to provide more context in the entry. I don't think you can review your own PR, but let me know if you have any suggestions! Thanks

@aam
Copy link
Contributor Author

aam commented Nov 7, 2024

@aam Are there api docs pages for Dart_EnterIsolate and Dart_ExitIsolate on https://api.dart.dev/? I looked but couldn't find anything. Even if they're in dev and not stable, that works fine.

No, those API calls are not on api.dart.dev. They are part of "Dart Embedding API Reference" with dart_api.h C++ header being documentation itself.

@MaryaBelanger
Copy link
Contributor

Thanks for the feedback @aam. I'll merge once the builds done

@MaryaBelanger MaryaBelanger merged commit cdedf03 into dart-lang:main Nov 8, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove the limitation on the number of concurrently running mutators
5 participants