Allow for finer-grained kernel libraries for incremental hot-reload #34001
Comments
aam
added
area-vm
|
With https://dart-review.googlesource.com/c/sdk/+/77722 I'm seeing some improvements in size of kernel file, number of classes reloaded by VM for flutter gallery:
Peter, if you have a chance can you please take a look at the patch's changes. Does it align with what you had in mind for minimizing incremental kernel file? cc'ing @jensjoha @kmillikin since this was mentioned recently in context of flutter test improvements. |
|
It looks like vm would benefit from having information about libraries that transitively depend on changed library. Basic list of uris would be sufficient as vm simply need to know what compiled code it needs to drop since all of that compiled code would be invalidated by changed dependency. Oh, and assumption is that this dependents information is readily available in the compiler, it's just a matter of serializing and passing it out to vm via incremental kernel file. |
|
In https://dart-review.googlesource.com/c/sdk/+/77722 you update the VM to handle only sending over the single/few changed library/libraries in these cases. I'm working on allowing the incremental compiler do less work in some circumstances. This will naturally also enable us to serialize less. Doing so - so far - has seemed to work fine without any VM changes. Is this expected or have I just been "lucky" so far? |
|
Maybe a little context: I'm adding an experimental invalidation feature to the incremental compiler so - that if the structure/outline of the changed files doesn't change - we only re-compile those files (modulo mixins). whereas with this feature it gives a result along the lines of I have not modified the VM at it seems to work fine. (Note that the example above was picked as a 'bad case' for the 'transitively invalidate everything' standard mode of operating and as a working case for the new thing, so it's not always going to give a 10x speedup). |
|
Yes I believe you can get lucky and get smaller incremental kernel files loaded by vm without causing further problems in running program, but generally speaking we need to drop and recompile unoptimized code for indirectly-dependent libraries even in case when change to the library is limited to method's implementation. |
And how much work would it be to do something like that? (can the VM track that down itself? Or should it be included somehow?) By recompiling experimentally, and only outputting the single library I get reload times like this: But by recompiling experimentally, and outputting the same libraries as I normally would I get reload times like this: |
VM should be able to build closure over import-libraries itself - the way how I prototyped this on https://dart-review.googlesource.com/c/sdk/+/77722 seems to show that. Seems to be trivial to extract that out of that cl to pair with what you built. Interesting question would be to see where the savings are coming from by running flutter engine/dart with '--trace-reload', '--trace-reload-verbose'(added to kDartLanguageArgs in engine/src/flutter/runtime/dart_vm.cc). Is it in reduced size of the kernel file and reduced read/transfer/write time or is it in amount of recompilation that dart vm does or somewhere else. That would be awesome if we could manage this as a first step. Further it would be great to come up with a way to serialize/deserialize just a single method which body was updated rather than the whole library. Not sure how hard would be to use existing kernel file format for that(given that it has a rigid structure of library-class-method). |
|
I've got numbers... I've found that Original
From a verbose run I extracted this So that accounts for ~700 ms of the otherwise unaccounted for time. Better compilation, same serialization
A verbose run gave basically the same data as above, so would account for ~700 ms of the otherwise unaccounted for time. Better compilation, better serialization
From a verbose run I extracted this So that accounts for ~160 ms of the otherwise unaccounted for time.
From the compiler side that's probably not very likely to happen, we compile whole libraries. One could, I suppose, imagine that we could compile a whole library, find out which body had changed and then only send that, but I'm not sure that would be worth it... Also it would leave offsets in an inconsistent state for all "old" code in that library (technically only the code textually after the change, but still). Patches applied: and |
The idea behind this is that the incremental compiler can now (currently experimentally) recompile less if the outline didn't change. This also mean that it can output less (e.g. only the changed library). This change should make sure that the VM still invalidates everything it needs to. See also: #34001 https://dart-review.googlesource.com/c/sdk/+/77722 Change-Id: I8d04bb86d2a27dd2706ec83f53fa98453eb41ce1 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/144299 Commit-Queue: Jens Johansen <jensj@google.com> Reviewed-by: Alexander Aprelev <aam@google.com> Reviewed-by: Ryan Macnak <rmacnak@google.com>
This reverts commit 8e90d2b. Reason for revert: Causes test failures on reload/reload-rollback bots. Original change's description: > [VM] Mark all indirectly dependent code dirty on hot reload > > The idea behind this is that the incremental compiler can now > (currently experimentally) recompile less if the outline didn't change. > This also mean that it can output less (e.g. only the changed library). > This change should make sure that the VM still invalidates everything > it needs to. > > See also: > #34001 > https://dart-review.googlesource.com/c/sdk/+/77722 > > Change-Id: I8d04bb86d2a27dd2706ec83f53fa98453eb41ce1 > Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/144299 > Commit-Queue: Jens Johansen <jensj@google.com> > Reviewed-by: Alexander Aprelev <aam@google.com> > Reviewed-by: Ryan Macnak <rmacnak@google.com> TBR=kustermann@google.com,aam@google.com,rmacnak@google.com,jensj@google.com Change-Id: I5158447943c5dab18f4ed433f3709e6ee403606a No-Presubmit: true No-Tree-Checks: true No-Try: true Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/144822 Reviewed-by: Tess Strickland <sstrickl@google.com> Commit-Queue: Tess Strickland <sstrickl@google.com>
The idea behind this is that the incremental compiler can now (currently experimentally) recompile less if the outline didn't change. This also mean that it can output less (e.g. only the changed library). This change should make sure that the VM still invalidates everything it needs to. See also: #34001 https://dart-review.googlesource.com/c/sdk/+/77722 This reverts commit 4305061 (and fixed the issue). Change-Id: I0815635eff6df76edf95de686aefd1800e0a8a0a Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/144824 Commit-Queue: Jens Johansen <jensj@google.com> Reviewed-by: Alexander Aprelev <aam@google.com>
Currently as part of hot reload flow, even small incremental changes result in full kernel libraries being generated and sent from frontend to VM.
Further, even well-encapsulated changes to method bodies result in complete library chain from the main-method library down to the library with the change will get generated and set from frontend to VM.
This issue tracks improvements to address these deficiencies.
Initial plan to address this is to prototype diff-logic as part of frontend-server that is used as a intermediary between VM and compiler, evaluate performance and complexity, then based on evaluation results move that into compiler itself.
cc @peter-ahe-google @a-siva
The text was updated successfully, but these errors were encountered: