Skip to content

Remove --disable-dart-dev across org: flutter #154268

Closed
Closed
Remove --disable-dart-dev across org: flutter#154268
Assignees
matanlurey
Labels
P2Important issues not at the top of the work listc: contributor-productivityTeam-specific productivity, code health, technical debt.c: tech-debtTechnical debt, code quality, testing, etc.dependency: dartDart team may need to help usdependency:dart-triagedTriaged by Dart team

Description

@matanlurey
matanlurey
opened on Aug 28, 2024

This issue is to track removing a flag that used to be important-ish, but no longer is, and is used across the Flutter codebase.

@bkonyi:

There used to be a slight performance benefit running with --disable-dart-dev when attempting to debug since we used to launch DDS from dartdev, so providing that flag would effectively just tell the VM to skip starting the dartdev isolate and DDS. Now that dart:_vmservice is responsible for starting DDS, this isn't really the case anymore.

Activity

  1. matanlurey
    added
    c: contributor-productivityTeam-specific productivity, code health, technical debt.
    P2Important issues not at the top of the work list
    c: tech-debtTechnical debt, code quality, testing, etc.
    on Aug 28, 2024
  2. matanlurey
    self-assigned this
    on Aug 28, 2024
  3. auto-submit
    added a commit that references this issue on Aug 29, 2024

    Remove `--disable-dart-dev` across `flutter/engine`. (#54845)

    1198996

  4. aam commented on Aug 29, 2024

    @aam
    aam
    on Aug 29, 2024
    Member

    @bkonyi:

    There used to be a slight performance benefit running with --disable-dart-dev when attempting to
    debug since we used to launch DDS from dartdev, so providing that flag would effectively just tell
    the VM to skip starting the dartdev isolate and DDS. Now that dart:_vmservice is responsible for
    starting DDS, this isn't really the case anymore.

    Looking at https://github.com/dart-lang/sdk/blob/a0411ae24485526470b7f99148132f29fe01edcb/runtime/bin/main_impl.cc#L1419 it seems that --disable-dart-dev still bypasses dartdev isolate from launching, which might still be relevant for flutter to avoid.

  5. bkonyi commented on Aug 29, 2024

    @bkonyi
    bkonyi
    on Aug 29, 2024
    Contributor

    @bkonyi:

    There used to be a slight performance benefit running with --disable-dart-dev when attempting to
    debug since we used to launch DDS from dartdev, so providing that flag would effectively just tell
    the VM to skip starting the dartdev isolate and DDS. Now that dart:_vmservice is responsible for
    starting DDS, this isn't really the case anymore.

    Looking at https://github.com/dart-lang/sdk/blob/a0411ae24485526470b7f99148132f29fe01edcb/runtime/bin/main_impl.cc#L1419 it seems that --disable-dart-dev still bypasses dartdev isolate from launching, which might still be relevant for flutter to avoid.

    The DartDev isolate will only launch if a command is provided (e.g., dart run foo.dart instead of dart foo.dart) so there really won't be much performance benefit.

  6. aam commented on Aug 29, 2024

    @aam
    aam
    on Aug 29, 2024
    Member

    I see, thanks Ben, but conceptually if flutter doesn't use dartdev, passing the flag still makes sense.

  7. matanlurey commented on Aug 29, 2024

    @matanlurey
    matanlurey
    on Aug 29, 2024
    ContributorAuthor

    I see, thanks Ben, but conceptually if flutter doesn't use dartdev, passing the flag still makes sense.

    Alex / Ben - My understanding from talking to Ben (but also Jonah, etc), is this flag was historically used when there was a transition from dart being strictly the VM entrypoint to being a unified CLI tool, and during that transition period there being significant performance and stability benefits.

    If we still think those are true (either or/both), let's revert this change - otherwise if we don't think there is any real motivation to do so, I'd prefer not having flags that "used to be useful" in the codebase as they tend to get copied without understanding.

  8. bkonyi commented on Aug 29, 2024

    @bkonyi
    bkonyi
    on Aug 29, 2024
    Contributor

    I don't think there's any significant performance or stability benefits of passing this flag anymore. Also, there's a good chance this flag goes away as Siva migrates dart to be an AOT executable of DartDev that calls into dartvm to run code from source, so it's better to remove it.

  9. matanlurey commented on Aug 29, 2024

    @matanlurey
    matanlurey
    on Aug 29, 2024
    ContributorAuthor

    I ran into one difficult case where dev/devicelab/lib/framework/runner.dart seems load-bearing on --disable-dart-dev. I debugged it a bit with @jonahwilliams. We can reproduce the error locally by running the following (substitute your own paths):

    BOT=true LANG=en_US.UTF-8 /Users/matanl/Developer/flutter/bin/cache/dart-sdk/bin/dart --enable-vm-service --no-pause-isolates-on-exit bin/tasks/smoke_test_setup_failure.dart 
Could not start the VM service:
DartDevelopmentServiceException: Failed to start Dart Development Service
vm-service: Error: Unhandled exception:
HttpException: HttpServer is not bound to a socket
#0      _HttpServer.address (dart:_http/http_impl.dart:3327:17)
#1      Server.serverAddress (dart:vmservice_io/vmservice_server.dart:259:25)
#2      Server.startup (dart:vmservice_io/vmservice_server.dart:616:24)
<asynchronous suspension>
#3      _toggleWebServer (dart:vmservice_io:244:5)
<asynchronous suspension>

    I'll work on addressing this in the next PR.

  10. auto-submit
    added a commit that references this issue on Aug 29, 2024

    Remove `--disable-dart-dev` across `flutter/flutter`. (#154280)

    712cf65

  11. matanlurey commented on Aug 29, 2024

    @matanlurey
    matanlurey
    on Aug 29, 2024
    ContributorAuthor

    Found the root cause, blocked on dart-lang/sdk#56606.

  12. matanlurey
    added
    dependency: dartDart team may need to help us
    blockedIssue is blocked by another issue
    on Aug 29, 2024
  13. Buchimi
    added a commit that references this issue on Sep 2, 2024

    Remove `--disable-dart-dev` across `flutter/flutter`. (flutter#154280)

    0972a03
  14. auto-submit
    added a commit that references this issue on Sep 3, 2024

    Get reasonable output when a stream test fails. (#154377)

    f070ffc
  15. a-siva
    added
    dependency:dart-triagedTriaged by Dart team
    on Sep 4, 2024
  16. matanlurey
    removed
    blockedIssue is blocked by another issue
    on Sep 10, 2024

  17. matanlurey commented on Sep 10, 2024

    @matanlurey
    matanlurey
    on Sep 10, 2024
    ContributorAuthor

    This is now unblocked and the last 2 PRs are in review.

  18. auto-submit
    added 2 commits that reference this issue on Sep 10, 2024

    Remove last `--disable-dart-dev` in `flutter/flutter`. (#154948)

    4b5cf85

    Remove the last `--disable-dart-dev` in the engine. (#55086)

    77c2053

  19. matanlurey commented on Sep 10, 2024

    @matanlurey
    matanlurey
    on Sep 10, 2024
    ContributorAuthor

    This is now complete: https://github.com/search?q=org%3Aflutter%20%22--disable-dart-dev%22&type=code

  20. matanlurey
    closed this as completedon Sep 10, 2024
  21. jesswrd
    added a commit that references this issue on Sep 11, 2024

    Remove the last `--disable-dart-dev` in the engine. (flutter-team-arc…

    68611a0

  22. github-actions commented on Sep 24, 2024

    @github-actions
    github-actionsbot
    on Sep 24, 2024 – with GitHub Actions

    This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of flutter doctor -v and a minimal reproduction of the issue.

  23. github-actions
    locked as resolved and limited conversation to collaborators on Sep 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

P2Important issues not at the top of the work listc: contributor-productivityTeam-specific productivity, code health, technical debt.c: tech-debtTechnical debt, code quality, testing, etc.dependency: dartDart team may need to help usdependency:dart-triagedTriaged by Dart team

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions