dart-lang / sdk

The Dart SDK, including the VM, JS and Wasm compilers, analysis, core libraries, and more.
https://dart.dev
BSD 3-Clause "New" or "Revised" License
10.09k stars 1.56k forks source link

IsolateMirror.loadUri appears to do much more work than it needs to #47656

Open jakemac53 opened 2 years ago

jakemac53 commented 2 years ago

Related to https://github.com/dart-lang/sdk/issues/47513, I discovered this when using the workaround so I could proceed with prototyping.

We may not want to address this right now, but filing for posterity sake.

Expected behavior

I expected IsolateMirror.loadUri to be quite fast - all the dependencies of the library I am loading are already loaded. So it should just have to compile the library that is passed in, which I would expect to take <1s (for comparison, a hot reload adding the same library to the program takes about 1s).

Actual behavior

It takes about 7 seconds, which is about how long the entire app takes to compile. Presumably it is recompiling the library and all transitive deps to kernel, and then only actually loading the one new library.

Repro case

You can repro this by importing any large package (like analyzer) in a library, and then loading that library with IsolateMirror.loadUri. It won't go any slower or faster based on whether the main isolate already imports all those same large libraries.

mraleph commented 2 years ago

By default when you start Dart application from source on the VM we use non-incremental compilation - that's done to optimize for memory usage: we don't know if user is planning to do reloads/recompilations and keeping around CFE state in memory causes large overhead. We also wanted to avoid creating any sort of temporary files in some known locations (which could be a caching strategy). So it is not surprising that loadUri takes significant time afterwards.

My suggestion would be to separate concerns here. VM does not know how developer plans to run the application (e.g. is it a server? is it an interactive app? is it a development session?). That's why VM's sole responsibility should be to run Kernel binaries. Developer on the other hand knows more about how they intend to run the application - e.g. in this case there is a main application and there are libraries which share dependencies with the main application. So developer should own compiling application (and libraries which they intend to load through loadUri) to kernel and feeding these binaries to the VM.

Splitting responsibilities like this would allow you to do fast incremental recompiles and stuff like that.

I think as a prototyping hack you could tell VM to perform compiles in incremental mode by supplying --observe - this should tell kernel-service to keep compilation state in memory.

jakemac53 commented 2 years ago

Thanks @mraleph I confirmed that passing --observe is much faster (<100ms).

Just to clarify a bit, are you saying that I would be able to pass a kernel file to IsolateMirror.loadUri? What would happen if that contained more than one library?

rmacnak-google commented 2 years ago

Yes, you can pass a kernel file to IsolateMirror.loadUri. It will load all libraries in the file, skipping any that are already loaded. I.e., IsolateMirror.loadUri does not cause reload. If the newly loaded libraries depend on a different version of libraries than are already loaded, terrible undefined things will happen.

jakemac53 commented 2 years ago

I was more wondering what would happen if the kernel file contained more than one new library? You only get a single LibraryMirror back, and that is the only way to really access the stuff loaded (although maybe you could just go through the currentMirrorSystem and look at all loaded libraries?).

Not really a big issue for me or anything, just wondering.

jakemac53 commented 2 years ago

Fwiw this approach is significantly less complex, and faster than the other approaches I have explored, because it enables me to use the lightweight isolates (Isolate.spawn).

The same functionality could be achieved without dart:mirrors if I had a similar way of loading a new library and then invoking a single function from that newly loaded library (could be main?). That would be a generally very useful, but also scary feature :). But no more scary than what we expose today haha.