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 ability to name isolates for debugging #34059

Closed
Hixie opened this issue Aug 1, 2018 · 3 comments
Closed

Add ability to name isolates for debugging #34059

Hixie opened this issue Aug 1, 2018 · 3 comments
Assignees
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends.

Comments

@Hixie
Copy link
Contributor

Hixie commented Aug 1, 2018

In flutter/flutter#19490 @rmacnak-google said "the framework's use of wrapper functions makes it impossible to tell just from the isolate's name". It would be great if we could provide information to Isolate.spawn to make future debugging easier, since we actually have name information at hand when we make that call (we use it in Timeline calls). (I'm not sure where the Isolate's name appears, but presumably in debugging dumps or some such?)

@a-siva a-siva added the area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. label Aug 14, 2018
@bkonyi
Copy link
Contributor

bkonyi commented Aug 20, 2018

Would we consider adding an optional debugName parameter to Isolate.spawn and Isolate.spawnUri a breaking change? I highly doubt anyone has used Isolate as an interface, so I can't see that being an issue, but maybe someone else can think of a situation where this might be breaking?

@natebosch
Copy link
Member

I highly doubt anyone has used Isolate as an interface, so I can't see that being an issue, but maybe someone else can think of a situation where this might be breaking?

If we only add it to the static methods Isolate.spawn they aren't part of the interface so I think we should be fine. If we need to add an instance field on Isolate then it might be breaking, but I would be very surprised to see a class implements Isolate.

@natebosch
Copy link
Member

Another suggestion from a while ago is to be able to set it from the Service class: #28207

dart-bot pushed a commit that referenced this issue Mar 14, 2019
Fixes issue #34059

Change-Id: I315498b02edc184e9e408c93eddb78aa1a5a8a1d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/90341
Commit-Queue: Ben Konyi <bkonyi@google.com>
Reviewed-by: Sigmund Cherem <sigmund@google.com>
Reviewed-by: Ryan Macnak <rmacnak@google.com>
@bkonyi bkonyi closed this as completed Mar 14, 2019
dart-bot pushed a commit that referenced this issue Mar 15, 2019
…isolates."

This reverts commit 5952526.

Reason for revert: causes Flutter test observatory and protocol to deadlock.
Issue: #36232

Original change's description:
> [ VM / dart:isolate ] Added ability to set names for spawned isolates.
> 
> Fixes issue #34059
> 
> Change-Id: I315498b02edc184e9e408c93eddb78aa1a5a8a1d
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/90341
> Commit-Queue: Ben Konyi <bkonyi@google.com>
> Reviewed-by: Sigmund Cherem <sigmund@google.com>
> Reviewed-by: Ryan Macnak <rmacnak@google.com>

TBR=bkonyi@google.com,rmacnak@google.com,asiva@google.com,sigmund@google.com

Change-Id: I5f2115a2ac394a8d4c7c175bc97f2b88b65fcb49
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/97107
Reviewed-by: Daco Harkes <dacoharkes@google.com>
Commit-Queue: Daco Harkes <dacoharkes@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends.
Projects
None yet
Development

No branches or pull requests

4 participants