Skip to content

Use the frontend server to compile pub executables #2968

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 11 commits into from
May 12, 2021

Conversation

jakemac53
Copy link
Contributor

Opening for discussion, I wanted to try this out to see if we can speed up the rebuilds of pub executables - it does appear to be much faster (basically immediate if there is no change). Surprisingly, it is almost twice as fast on the initial precompile as well for me. I think that probably means that the vm snapshot functionality is doing something additional - I don't know what the implications of using kernel instead might be.

Specifically this:

  • Creates a new dir .dart_tool/pub/incremental where it stores the previous dill outputs for each executable.
  • Uses the frontend server to compile to that directory (which will use those files automatically to initialize from).
  • Copies the output from there to the actual executable path if successful.

Previous snapshot behavior:
snapshot

New fe server behavior, first build:
kernel_first

New behavior, pub get and run again (no changes):
kernel_incremental

Note that in the rebuild case with no changes, it didn't actually log the time at all which I believe means it was <1 second.

cc @mit-mit

@jakemac53 jakemac53 requested review from jonasfj and sigurdm April 20, 2021 15:56
@google-cla google-cla bot added the cla: yes label Apr 20, 2021
@jakemac53
Copy link
Contributor Author

Possible resolution to #2962 as well - in the case of no changes the precompile step is now much faster.

@jakemac53
Copy link
Contributor Author

I will look more into the windows failures and do additional cleanup if we want to push forward with this change.

@jakemac53
Copy link
Contributor Author

cc @a-siva would you be able to speak to the implications of running from kernel versus snapshot?

@a-siva
Copy link

a-siva commented Apr 22, 2021

Could you clarify what kind of 'snapshot' you are referring to here?

Usage: dart compile <subcommand> [arguments]

Available subcommands:
  aot-snapshot   Compile Dart to an AOT snapshot.
  exe            Compile Dart to a self-contained executable.
  jit-snapshot   Compile Dart to a JIT snapshot.
  js             Compile Dart to JavaScript.
  kernel         Compile Dart to a kernel snapshot.

(aot-snapshot, jit-snapshot or just plain kernel which is also sometimes referred to as a snapshot).

@jakemac53
Copy link
Contributor Author

jakemac53 commented Apr 22, 2021

Could you clarify what kind of 'snapshot' you are referring to here?

It is just --snapshot - that must be an alias for some other listed flag?

'--snapshot=$snapshotPath',

@a-siva
Copy link

a-siva commented Apr 22, 2021

if you are just using the --snapshot option and not passing in the --snapshot-kind option then the default is kernel

--snapshot-kind=<snapshot_kind>
--snapshot=<file_name>
  These snapshot options are used to generate a snapshot of the loaded
  Dart script:
    <snapshot-kind> controls the kind of snapshot, it could be
                    kernel(default) or app-jit
    <file_name> specifies the file into which the snapshot is written

which means you are already only producing a kernel file in this step.

@jakemac53
Copy link
Contributor Author

Ok thanks @a-siva so there should be no difference in terms of the result then.

I do see some slight difference in output, the md5 hash is different and the new result is a bit smaller - (18016104 bytes versus 18633360 bytes).

@a-siva
Copy link

a-siva commented Apr 23, 2021

Is it possible that the version of CFE used in the two cases is different.

@jakemac53
Copy link
Contributor Author

@jonasfj @sigurdm anything you are waiting for me on here? If I can get a general affirmative I will spend some more time cleaning things up (get tests passing, delete the old snapshot function, etc)

@jonasfj
Copy link
Member

jonasfj commented Apr 26, 2021

@jakemac53 looks great to me.. I'm not familiar with FrontendServerClient, but I assume it incremental compilation infrastructure...

Only concern is how this will interact when users switch SDK versions forwards and backwards... But that might not be so bad.. also it might affect global packages too in the same way.

@jakemac53
Copy link
Contributor Author

jakemac53 commented Apr 26, 2021

@jakemac53 looks great to me.. I'm not familiar with FrontendServerClient, but I assume it incremental compilation infrastructure...

Yes, this is the same tool that flutter uses for incremental recompilation.

edit: just to clarify flutter does not use package:frontend_server_client, it has its own wrapper code. But they both talk to the same frontend_server binary which ships with the sdk and does the actual compilation.

Only concern is how this will interact when users switch SDK versions forwards and backwards... But that might not be so bad.. also it might affect global packages too in the same way.

The way I have implemented this it should all just work. I haven't changed the invalidation logic around the actual runnable executables, so they still get invalidated (deleted) in the same way as before.

When running from a cached dill file, the frontend server automatically throws it away if its kernel version doesn't match afaik. At least in some way it does handle this case and does a full recompile.

@jakemac53
Copy link
Contributor Author

@jonasfj the remaining test failure is here https://github.com/dart-lang/pub/pull/2968/checks?check_run_id=2442925564#step:7:415 (same failing test on the 3 bots).

SocketException: Failed host lookup: 'arg.com' (OS Error: Temporary failure in name resolution, errno = -3)

Do you have any more details/info on this test?

@jakemac53 jakemac53 changed the title WIP: use frontend server to compile pub executables Use the frontend server to compile pub executables Apr 28, 2021
@jakemac53
Copy link
Contributor Author

This is ready for review now @jonasfj @sigurdm

@jakemac53
Copy link
Contributor Author

@jonasfj This is ready to land from my perspective, but I will let you hit the button unless you want me to :D.

@jonasfj jonasfj merged commit e01e3a4 into dart-lang:master May 12, 2021
@jakemac53 jakemac53 deleted the fe-server branch May 12, 2021 19:34
jonasfj added a commit that referenced this pull request May 20, 2021
jonasfj added a commit that referenced this pull request May 20, 2021
jonasfj added a commit that referenced this pull request May 20, 2021
jonasfj added a commit that referenced this pull request May 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants