Use new faster application snapshots for "pub global activate"

[Scorpiion]

Hi,

I recently learned about and have been testing Dart's application snapshots. In my tests it speeds up execution quite a lot* and I was thinking it would be nice if pub global activate either used app snapshots by default or at least if it was possible to add a flag to support faster CLI apps.

What do you think?

* In the CLI app I tested the speed up was from ~200 ms to ~50 ms, it's a very noticeable speedup. Currently I use a custom bash install script for this but using pub would be much easier for distribution.

[natebosch]

One thing that might make this tricky is that these snapshots are tied to a particular SDK version - I'm not sure if anything pub is doing today has that same constraint. This would have consequences when upgrading an SDK and using previously global activate binaries, and would be doubly tricky if someone moves between different SDK versions frequently.

[jakemac53]

Additionally you have to train the snapshot on something but pub has no way of knowing a representative way to invoke the script in order to train it. I think you would have to add some sort of configuration or something to tell pub what to do?

[Scorpiion]

Regarding SDK versions, as far as I know the old/current snapshots is also tied to a specific version of Dart and pub already detects that and creates a new snapshot when that happens, so that should not be different. It also shows when a new version is available so there is some checks being done already.

Regarding training, I have not been doing any training at all and there is still a big speedup. I think you could maybe increase the speedup even more with training but even without training it's a lot faster it seems to me. Correct me if I'm wrong, but the training is not required, it's just there to make things even faster?

[jakemac53]

Regarding training, I have not been doing any training at all and there is still a big speedup. I think you could maybe increase the speedup even more with training but even without training it's a lot faster it seems to me. Correct me if I'm wrong, but the training is not required, it's just there to make things even faster?

I am curious how you are building the snapshot? Afaik if you are adding --snapshot-kind=app-jit then it is going to actually run the dart script. If the script happens to work with no arguments and actually do some work then you will get some amount of benefit from just adding that arg, but I don't know that it would be generally useful.

Also, I don't know to much about how it actually works but it seems like running a script arbitrarily with zero args could be a potential security risk, or just have adverse side effects.

[Scorpiion]

@jakemac53 yes I run the script without any arguments, and it prints out the help section when I do so. The app uses the args package and is very similar to pub in structure. It's faster also with arguments, at least showing the help info for various commands.

From the snapshot link above:

application snapshots, which include all the parsed classes and compiled code generated during a training run of a program.

Is it not so that most or all classes gets parsed in many cases with a CLI app?

For example:

• https://github.com/dart-lang/pub/blob/master/bin/pub.dart
  runs/loads PubCommandRunner
• https://github.com/dart-lang/pub/blob/master/lib/src/command_runner.dart
  PubCommandRunner imports all classes with CLI app logic

[Scorpiion]

If it is not possible to create the snapshot in a secure fashion then a flag could maybe do for all packages that don't do something bad when executed (I think that is most of them). It's worth noting that running any untrusted code from pub locally is a potential security risk and I think most CLI applications don't do anything without arguments. Another option could maybe be if it could be specified in some config file what snapshot type that should be generated.

[nex3]

We could potentially allow users to declare snapshot arguments in the executables section, like this:

executables:
  dartfmt:
    bin: format
    snapshot: --app-snapshot

We'd probably want to wait until after #69 to do this, though, so people don't start relying on it as a hacky way of faking that functionality.

[Scorpiion]

That would work for me. =)

[natebosch]

Instead I think what we should do is stop eagerly creating snapshots and instead wait for the first pub run for a given binary and create an app jit snapshot then.

This is becoming more pressing since we currently can't run with snapshots at all for Dart 2 mode and that makes things very slow.

@jakemac53

[nex3]

I'd really like to avoid the first run of an executable being super slow—that seems like a really bad user experience. I'd rather continue to generate script snapshots ahead-of-time, and then generate app-jit snapshots from those on the first run. This will still add some first-run overhead, but it'll be a lot faster than running app-jit compilation from source.

Also, I think we need to use script snapshots for Dart 2 for now—it looks like as of 2.0.0-dev.55, application snapshots still aren't supported in Dart 2 mode (dart-lang/sdk#33176).

[jakemac53]

I'd really like to avoid the first run of an executable being super slow—that seems like a really bad user experience. I'd rather continue to generate script snapshots ahead-of-time, and then generate app-jit snapshots from those on the first run. This will still add some first-run overhead, but it'll be a lot faster than running app-jit compilation from source.

Doing this ahead of time just moves the slowness to pub get/upgrade though right? I find it a lot more annoying when those commands are slow because they are pre-compiling snapshots I will never use, and presumably that will get a lot slower now.

[nex3]

When the slowness is in pub get, it's obvious that it's not caused by the executable itself. When the first run is very slow, it looks like the executable is busted and reflects poorly on package authors. I think we should optimize for having pub clearly take responsibility for that time rather than our users.

If packages are shipping unused executables, maybe that's a problem we should solve separately, either by encouraging authors to ship executables as separate packages or providing dependers a way to indicate that they don't care about a given package's executables.

[jakemac53]

I think we should optimize for having pub clearly take responsibility for that time rather than our users.

This doesn't mean it has to be eagerly done for all executables though. For instance pub run ... could log a message like creating snapshot for <executable>... so that its clear where the time is going.

If packages are shipping unused executables, maybe that's a problem we should solve separately, either by encouraging authors to ship executables as separate packages

I don't personally think this is the right approach - you should be able to ship with your package diagnostic tools and other utilities which most people won't use, but that you can direct them to use to solve or diagnose specific problems, and it shouldn't cause pub get/upgrade to be slower for everybody.

These types of tools often are going to use internal apis so you don't really want to ship them as separate packages either.

or providing dependers a way to indicate that they don't care about a given package's executables.

You could potentially resolve this to some degree by letting package authors specify that certain executables should be lazily compiled. I would still strongly prefer lazy to be the default though.

I would not want to push this on users, because it doesn't scale well (you just end up with the same code copied across every pubspec, and if the executable is renamed they all have to change).

[kevmoo]

Here's a +1 for lazy.

…

On Mon, May 21, 2018 at 7:23 AM Jacob MacDonald ***@***.***> wrote: I think we should optimize for having pub clearly take responsibility for that time rather than our users. This doesn't mean it has to be eagerly done for all executables though. For instance pub run ... could log a message like creating snapshot for <executable>... so that its clear where the time is going. If packages are shipping unused executables, maybe that's a problem we should solve separately, either by encouraging authors to ship executables as separate packages I don't personally think this is the right approach - you should be able to ship with your package diagnostic tools and other utilities which most people won't use, but that you can direct them to use to solve or diagnose specific problems, and it shouldn't cause pub get/upgrade to be slower for everybody. These types of tools often are going to use internal apis so you don't really want to ship them as separate packages either. or providing dependers a way to indicate that they don't care about a given package's executables. You could potentially resolve this to some degree by letting package *authors* specify that certain executables should be lazily compiled. I would still strongly prefer lazy to be the default though. I would not want to push this on users, because it doesn't scale well (you just end up with the same code copied across every pubspec, and if the executable is renamed they all have to change). — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#1683 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AABCirPFFa9vX1_bwcNioMPQJV02cYjgks5t0s3TgaJpZM4OyTfk> .

[nex3]

Any messages we might produce would conflict with an executable's ability to use stdout to emit data.

…

On Mon, May 21, 2018, 3:23 PM Jacob MacDonald ***@***.***> wrote: I think we should optimize for having pub clearly take responsibility for that time rather than our users. This doesn't mean it has to be eagerly done for all executables though. For instance pub run ... could log a message like creating snapshot for <executable>... so that its clear where the time is going. If packages are shipping unused executables, maybe that's a problem we should solve separately, either by encouraging authors to ship executables as separate packages I don't personally think this is the right approach - you should be able to ship with your package diagnostic tools and other utilities which most people won't use, but that you can direct them to use to solve or diagnose specific problems, and it shouldn't cause pub get/upgrade to be slower for everybody. These types of tools often are going to use internal apis so you don't really want to ship them as separate packages either. or providing dependers a way to indicate that they don't care about a given package's executables. You could potentially resolve this to some degree by letting package *authors* specify that certain executables should be lazily compiled. I would still strongly prefer lazy to be the default though. I would not want to push this on users, because it doesn't scale well (you just end up with the same code copied across every pubspec, and if the executable is renamed they all have to change). — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#1683 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAAAvOwct4ofo5KGYfPNzmcGbzj2I5HTks5t0s3RgaJpZM4OyTfk> .

[jakemac53]

Any messages we might produce would conflict with an executable's ability
to use stdout to emit data.

This could be resolved with a --silent flag or something, but that would be a breaking change. The use case here I think is fairly limited - typically one tool talking to another - in which case hard coding in a flag like that is pretty reasonable.

[natebosch]

I would be fine either with or without an opt-in by package authors or end users to eagerly parse binaries on pub get like pub get --precompile, but I don't think either is required or blocking. I think that the default should be lazy and it would be fine if it was the only option.

pub global activate could eagerly parse, because when I run that command I'm expressing my intent to run the binary and so optimizing for that makes sense. When I add a dependency to my pubspec and pub get I'm not necessarily expressing an intent to run the binary and so we might be optimizing for a situation that never occurs at the expense of a slowdown for everyone.

I think that pub run should never print on stdout or stderr. I don't think an extra slowness on first run is going to be confusing for our users or make package authors look bad. It's unlikely for a developer to be working in the Dart 2 ecosystem for very long before they discover how slow parsing is.

[matanlurey]

I think lazy by default makes sense to me. TBH the only place the majority of users will "feel" it is likely the first run of pub run test (and like you mentioned, the 5-10s is going to be the least of their worries), most other applications of pub run are very infrequent (and probably mostly via dart bin/foo.dart during development).

[jakemac53]

I think that pub run should never print on stdout or stderr.

What if instead of a flag it only did it when stdout/stderr were connected to a terminal? I don't think that would break any use cases.

[natebosch]

I don't think that would break any use cases.

I bet I could find one where it does 😜

I don't think that users are going to be confused enough to risk the cases where the run appears connected to a terminal but precise output is necessary.

[nex3]

I agree that if we do this lazily we should never print any output.

[nex3]

There's another issue here that we haven't considered yet. We run all applications in isolates in order to ensure that standard IO and signals and so on are all hooked up properly. But the isolate API doesn't have a way of compiling an application snapshot.

[jakemac53]

There's another issue here that we haven't considered yet. We run all applications in isolates in order to ensure that standard IO and signals and so on are all hooked up properly. But the isolate API doesn't have a way of compiling an application snapshot.

Good point - we could create non-optimized snapshots lazily still.

Another alternative could be using ProcessStartMode.inheritStdio but that doesn't solve the signal issues I don't believe.

[itsjoeconway]

Following this thread in from the issue on using --preview-dart-2 with pub, so some of this info may be irrelevant. I'm happy to create a new issue if needed.

1. I hope there isn't output from pub at all. We have commands that are intended to to spit out something machine readable (a JSON blob, for example) so that they can be piped into another tool.
2. How will the different snapshotting options work with a Docker image? Will you be able to precompile the Dart script such that Docker's layer caching will be able to take advantage of it?

On the isolate topic, FWIW, we have a lot of CLI tools that spawn more isolates. When testing those tools via pub run test with DART_VM_OPTIONS=--preview-dart-2, the tests are failing because the Dart VM process is leaking memory (the process got up to 10GB before it started failing), and this is slowing down the tests so much that they are timing out. Even with increased timeouts, running multiple test files grinds everything to a halt (such that what was once a 5 second test takes over 2 minutes).

[sigurdm]

We are now using "kernel/dill binaries" (since quite a while) they offer fast recompilation. Not sure if that is the best for global binaries...

[jakemac53]

For global binaries AOT probably would be better, but it would be breaking to switch because they don't support mirrors...

[sigurdm]

But are application snapshots faster than the current dill binaries?

[jakemac53]

But are application snapshots faster than the current dill binaries?

The terminology is a bit weird/inconsistent here so I am actually not entirely sure what you are talking about when you say "application snapshot". Would the be a trained VM snapshot or an AOT binary (dart compile exe)?

Both of those will start up faster, but I am not sure pub could reasonably create a trained VM snapshot without additional help from the package.

The AOT binary will have much more predictable performance regardless (and start up very fast), but it isn't necessarily faster in general. Depends on the program and how long it runs etc... and also this has been a rapidly evolving space so the answer now is probably different than it was last I tried to compare.

[sigurdm]

Yeah - I meant trained VM snapshot - what is mentioned in the subject of this issue.

We have been discussing several times moving to a model where we dart compile exe the files - that would also remove the dependency of keeping the right sdk version. But as you say, it would not support mirrors.

[jakemac53]

Yeah - I meant trained VM snapshot - what is mentioned in the subject of this issue.

Ah ok, fwiw I am surprised these are even still a thing, now that AOT is pretty mature, and solves the same problems without requiring a representative training command. I do not think application snapshots are a good target for pub compilation because of requiring a representative command for training in order to be better than just precompiled dills.