-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Support AOT splitting #41974
Comments
This bug does not mention this explicitly but as noted on the internal bug - I think it is worth sticking with the language and choose to implement deferred libraries support in the AOT compiler. If there is no strong arguments why we should choose any other implementation strategy, I suggest we go with that. Any opinion @rmacnak-google @a-siva @mkustermann? |
Thanks Slava. I wanted to dump some of my thoughts here:
Related to last point, we also need to come up with a recommended testing strategy. This is not necessary for the initial release but still critical. |
I agree let us first stick to the deferred library support. |
@chingjun FYI |
From testing last year, the android_dlopen_ext part seems to be in the 10-20ms ballpark. I think a simpler timing API like basing it off of Dart imports is probably sensible. |
…bers are used. Not loading is actually deferred. Bug: #41974 Change-Id: I62688007bd36dbcb2e8ffb4a1fd2dceb1775b1c8 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/149053 Commit-Queue: Ryan Macnak <rmacnak@google.com> Reviewed-by: Siva Annamalai <asiva@google.com>
…efix members are used." This reverts commit f049a41. Reason for revert: Broke reload bots. Original change's description: > [vm] Check prefix.loadLibrary is called and returns before prefix members are used. > > Not loading is actually deferred. > > Bug: #41974 > Change-Id: I62688007bd36dbcb2e8ffb4a1fd2dceb1775b1c8 > Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/149053 > Commit-Queue: Ryan Macnak <rmacnak@google.com> > Reviewed-by: Siva Annamalai <asiva@google.com> TBR=rmacnak@google.com,alexmarkov@google.com,asiva@google.com Change-Id: I923e339465fdf13199efc11a9cef4a842abebd67 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: #41974 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/149423 Reviewed-by: Aske Simon Christensen <askesc@google.com> Commit-Queue: Aske Simon Christensen <askesc@google.com>
…bers are used. Restore checks against reloading a library with deferred prefixes. No loading is actually deferred. Bug: #26878 Bug: #41974 Change-Id: Iec2662de117453d596cca28dd9481a9751091ce9 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/149613 Commit-Queue: Ryan Macnak <rmacnak@google.com> Reviewed-by: Alexander Markov <alexmarkov@google.com> Reviewed-by: Siva Annamalai <asiva@google.com>
…efix members are used." This reverts commit b0484ec. Reason for revert: timeouts on Flutter integration tests (#42350). Original change's description: > [vm] Check prefix.loadLibrary is called and returns before prefix members are used. > > Restore checks against reloading a library with deferred prefixes. > > No loading is actually deferred. > > Bug: #26878 > Bug: #41974 > Change-Id: Iec2662de117453d596cca28dd9481a9751091ce9 > Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/149613 > Commit-Queue: Ryan Macnak <rmacnak@google.com> > Reviewed-by: Alexander Markov <alexmarkov@google.com> > Reviewed-by: Siva Annamalai <asiva@google.com> TBR=rmacnak@google.com,alexmarkov@google.com,asiva@google.com # Not skipping CQ checks because original CL landed > 1 day ago. Bug: #26878, #41974 Change-Id: I78709650e91d206b84a8ddd9171ef66d6cf1b008 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/151169 Reviewed-by: Alexander Markov <alexmarkov@google.com> Commit-Queue: Alexander Markov <alexmarkov@google.com>
…efix members are used." This reverts commit 9a87cf9. Reason for revert: Broken test disabled Original change's description: > Revert "[vm] Check prefix.loadLibrary is called and returns before prefix members are used." > > This reverts commit b0484ec. > > Reason for revert: timeouts on Flutter integration tests > (#42350). > > Original change's description: > > [vm] Check prefix.loadLibrary is called and returns before prefix members are used. > > > > Restore checks against reloading a library with deferred prefixes. > > > > No loading is actually deferred. > > > > Bug: #26878 > > Bug: #41974 > > Change-Id: Iec2662de117453d596cca28dd9481a9751091ce9 > > Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/149613 > > Commit-Queue: Ryan Macnak <rmacnak@google.com> > > Reviewed-by: Alexander Markov <alexmarkov@google.com> > > Reviewed-by: Siva Annamalai <asiva@google.com> > > TBR=rmacnak@google.com,alexmarkov@google.com,asiva@google.com > > # Not skipping CQ checks because original CL landed > 1 day ago. > > Bug: #26878, #41974 > Change-Id: I78709650e91d206b84a8ddd9171ef66d6cf1b008 > Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/151169 > Reviewed-by: Alexander Markov <alexmarkov@google.com> > Commit-Queue: Alexander Markov <alexmarkov@google.com> TBR=rmacnak@google.com,alexmarkov@google.com,asiva@google.com # Not skipping CQ checks because this is a reland. Bug: #26878, #41974 Change-Id: Ife76bd51db65ca58e08655a9b8406c8ca483447f Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/151326 Reviewed-by: Ryan Macnak <rmacnak@google.com> Reviewed-by: Alexander Markov <alexmarkov@google.com> Commit-Queue: Ryan Macnak <rmacnak@google.com>
The partitioning has the property that for every reference from loading unit S to unit T, either - the reference is through a deferred prefix, or - T is guarenteed to be loaded before S The partitioning is constructed by cutting the dominator tree of the library import graph at deferred imports. It is suboptimal if there are deferred imports to multiple libraries in a connected component, but the expected use case involves loading units with a single deferred entry. Bug: #41974 Change-Id: I3c49044ae19112525f197b077b36e4ce874b81ac Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/151470 Commit-Queue: Ryan Macnak <rmacnak@google.com> Reviewed-by: Alexander Markov <alexmarkov@google.com>
…hich instructions to include. The ImageWriter is meant to include exactly those instructions passed to GetTextOffset, which isn't necessarily the current Instructions for every Code object. Bug: #41974 Change-Id: I77f09c70fd45f387f4802b30a86b155506529e29 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/153043 Reviewed-by: Martin Kustermann <kustermann@google.com> Commit-Queue: Ryan Macnak <rmacnak@google.com>
Loading unit metadata is expanded to create an object representing each loading unit, and each library to reference the loading unit it is a part of. To be used at compile-time to help assign objects to snapshots. To be used at runtime to track which units have been loading and the base objects for cross-snapshot references. Bug: #41974 Change-Id: Icd3fa95ffd5949bd8f7f84a6f750c4c515de0dba Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/151865 Commit-Queue: Ryan Macnak <rmacnak@google.com> Reviewed-by: Alexander Markov <alexmarkov@google.com>
Bug: #41974 Change-Id: Ib7918b75f123d7d7121b7bac11b4bd7dfc861025 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/154482 Reviewed-by: Liam Appelbe <liama@google.com> Commit-Queue: Ryan Macnak <rmacnak@google.com>
(Assignment of libraries to loading units is already done in the kernel generation step.) After compiling and before serializing, we walk the program and for each Code we assign its Instructions, CodeSourceMap and CompressedStackMap to the loading unit of that Code's defining library. Deduplication may cause Instructions, CodeSourceMaps and CompressedStackMaps to belong to more than one loading unit; in this case the objects are assigned to the root loading unit. Later they can be more precisely assigned to the dominating loading unit. All objects except some Instructions, CodeSourceMaps and CompressedStackMaps belong to the root loading unit's snapshot. This snapshot is written like an unsplit snapshot, except that when serializing Code, we will write a reference to a stub or null when the Code's Instructions, CodeSourceMap or CompressedStackMap belongs to a non-root loading unit. The snapshots of non-root loading units contain these deferred objects and references to the corresponding Code objects to patch. The types of objects we defer (Instructions, CodeSourceMaps and CompressedStackMaps) usually represent 70+% of the snapshot size. Bare instructions mode must be disabled when splitting because we cannot have PC-relative calls between loading units. Later we can re-enable this for calls within loading units. Broken: Compactor probably crashes we can now have an unbounded number of image pages and the compactor assumes a fixed number. Embedder's guide: At compile-time, gen_snapshot should be passed --loading_unit_manifest with a path, which will enable splitting and output a mapping from loading unit ids to snapshot output paths. At runtime, sometime during isolate startup, an embedder should call Dart_SetDeferredLoadHandler, probably near an existing call to Dart_SetLibraryTagHandler. The callback is given a loading unit id, and should eventually call Dart_DeferredLoadComplete[Error]. Bug: #41974 Change-Id: Ib597eb87c8cd634416d5ee1f00629c5550aebb00 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/152427 Commit-Queue: Ryan Macnak <rmacnak@google.com> Reviewed-by: Alexander Markov <alexmarkov@google.com>
Bug: #41974 Change-Id: I030629110026bbc4b1c339764ed805f65f753448 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/154827 Reviewed-by: Alexander Markov <alexmarkov@google.com> Commit-Queue: Ryan Macnak <rmacnak@google.com>
…s already loaded. c5a94db unintentionally changed the behavior of loadLibrary on a prefix that was already loaded from completing in a future turn of the message loop to completing in the same turn. Completing in the same turn is a legal behavior for loadLibrary, but consistently completing in a later turn helps to prevent code that works on accident. Bug: #41974 Change-Id: Ib18d16f6b5517c882248f39b486e58e2635f24a1 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/155101 Reviewed-by: Alexander Markov <alexmarkov@google.com> Commit-Queue: Ryan Macnak <rmacnak@google.com>
Bug: #41974 Change-Id: I23201f28e5d1e2ba298611206fc3eb0a9a989c2b Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/155241 Reviewed-by: Alexander Markov <alexmarkov@google.com> Commit-Queue: Ryan Macnak <rmacnak@google.com>
- Don't use PC-relative calls when calling between loading units. - Sort the contents of the code cluster by loading unit, then by text offset. - Handle binding PC-relatives calls and inserting trampolines per loading unit. - Create one code order table per loading unit to implement PC -> Code lookup. - Read code order tables directly, instead of copying into malloc'd memory. --use_table_dispatch still not yet supported. This slightly shrinks non-split binaries (~2% clustered part, 0.4% total snapshot) due to the new delta encoding when Code references Instructions. Bug: #41974 Change-Id: I51052ebc7b4968b554897d1710135a6c41821302 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/157820 Commit-Queue: Ryan Macnak <rmacnak@google.com> Reviewed-by: Alexander Markov <alexmarkov@google.com> Reviewed-by: Tess Strickland <sstrickl@google.com>
With --no-use-bare-instructions or with splitting, not all calls are pc-relative. We must replace the Function with its Code in the static call table before detaching the Function's code as part of removing Function objects. Also handle WeakSerializationReferences in AssignLoadingUnitsCodeVisitor. Bug: #41974 Change-Id: I29f9258c240f5a2b1b8dca52f74146dfe44d6401 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/158165 Reviewed-by: Tess Strickland <sstrickl@google.com> Commit-Queue: Ryan Macnak <rmacnak@google.com>
Bug: #41974 Change-Id: Ifb7de4698563b5c1210d388bd3d069986fc0f541 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/158725 Commit-Queue: Ryan Macnak <rmacnak@google.com> Reviewed-by: Alexander Markov <alexmarkov@google.com>
When reading a secondary snapshot, re-run the dispatch table initialization found in the root snapshot. Since each loading unit's contribution to the dispatch table is scattered, it would take more snapshot size to try to serialize their contributions separately. We'll revisit this when we can defer Class, Function and Code, which will prevent the root snapshot from referencing all Codes. Bug: #41974 Change-Id: Iefd2b98647b96ae59a7efe92897538f5cf8c2426 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/158923 Reviewed-by: Aske Simon Christensen <askesc@google.com> Commit-Queue: Ryan Macnak <rmacnak@google.com>
The input is already a URI, not a path. Bug: #41974 Change-Id: I6f2ba57867c75ddedff99c5723207bf5fe6d37d0 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/159180 Reviewed-by: Alexander Markov <alexmarkov@google.com> Commit-Queue: Ryan Macnak <rmacnak@google.com>
This field won't be used at runtime and is not accounted for during tree-shaking. Bug: #41974 Bug: #43245 Change-Id: Ie5a669608e896aa8cee74a3a4249ba99ec6f980a Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/161220 Reviewed-by: Alexander Markov <alexmarkov@google.com> Commit-Queue: Ryan Macnak <rmacnak@google.com>
Prevents snapshots from including canonical strings and types with no real uses and only referenced from the canonical tables. Prep for constants not all being in the root snapshot during snapshot splitting, which is why we fill the table during load instead of rebuilding tables during snapshot writing. Removes the size of the tables from the snapshot. flutter_gallery_app_so_size (Pixel 2) -1.618% 7967664 8098736 flutter_gallery_app_so_gzip_size (Pixel 2) -2.724% 2980344 3063788 flutter_gallery_app_so_brotli_size (Pixel 2) -3.530% 2561824 2655566 Bug: #41974 Bug: #43319 Change-Id: I4f3b62da1cea9ee17caa965e809f16856518c9a6 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/164102 Reviewed-by: Régis Crelier <regis@google.com> Commit-Queue: Ryan Macnak <rmacnak@google.com>
…omes earlier. Clusters should not read information produced by any other cluster until PostLoad. Bug: #41974 Change-Id: I39ec580d9011604f7041615cefaa907aa99a3ff8 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/164280 Reviewed-by: Régis Crelier <regis@google.com>
…d of within clusters. Ensure canonical clusters come first; prep for canonical objects in snapshot accounting for the possibility an existing canonical object may substitute for them. Bug: #41974 Change-Id: I8083f4130c359f0f86f73c25fa2869af672700e6 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/164250 Reviewed-by: Régis Crelier <regis@google.com> Commit-Queue: Ryan Macnak <rmacnak@google.com>
- Use null instead of empty array for the initial constants set, as fields dropped during snapshot are populated with null. - Remove assumption that canonicalization will always be bottom-up. Deserialization clusters will cause canonicalization to be ordered by class instead of by depth. Bug: #41974 Change-Id: Ia3ab68281c783299e682f6c60a744c320c19e981 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/165160 Commit-Queue: Ryan Macnak <rmacnak@google.com> Reviewed-by: Régis Crelier <regis@google.com>
…pshot instead of leaving associations from all previously written snapshots. For the main program snapshot, this has no effect. For secondary loading unit snapshots, this ensures they only reference their own objects or objects from parents and not objects from siblings that were written earlier. Bug: #41974 Change-Id: Ia7c5bec3b0c66564685ed2fd1b2bec52d2459024 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/163460 Reviewed-by: Régis Crelier <regis@google.com> Commit-Queue: Ryan Macnak <rmacnak@google.com>
… units but not present in the main unit. Bug: #41974 Change-Id: I29a00aa6139dc2a1f865f8c5f659169c84c8e45d Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/169790 Reviewed-by: Régis Crelier <regis@google.com> Commit-Queue: Ryan Macnak <rmacnak@google.com>
TEST=flutter Bug: #41974 Change-Id: Ia0e99c5e84cdff8736c44e4e82ce40b045ed0207 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/174563 Reviewed-by: Régis Crelier <regis@google.com> Commit-Queue: Ryan Macnak <rmacnak@google.com>
This is a step toward deferring literals, to effectively split binary size for libraries that are more data than code, such as localizations. Bare instructions mode still has all constants in the root snapshot because it has one global ObjectPool. dart2js+analyzer product X64 non-bare: app.so 10072728 -> 8479824 (-15.8%) app.so-2.part.so 14074512 -> 15016976 (6.69%) app.so-3.part.so 9225936 -> 9888984 (7.18%) TEST=ci Bug: #41974 Change-Id: If61f6156e3fc2dd539331335b3bf0458102b7c75 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/168991 Commit-Queue: Ryan Macnak <rmacnak@google.com> Reviewed-by: Régis Crelier <regis@google.com>
TEST=ci Bug: #41974 Change-Id: I37d2b83802a11b9cf5ffc2d7d4852daa18941006 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/178720 Reviewed-by: Alexander Aprelev <aam@google.com> Commit-Queue: Ryan Macnak <rmacnak@google.com>
… by a Code in bare instructions mode. Fix disassembly in bare instructions mode to query the global object pool. TEST=ci Bug: #41974 Change-Id: I3ad66a8cd7d9ee19c848b68ed584d58dca6bf583 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/179300 Reviewed-by: Régis Crelier <regis@google.com> Commit-Queue: Ryan Macnak <rmacnak@google.com>
dart2js+analyzer product X64 bare: app.so 9289320 -> 8491864 (-8.58%) app.so-2.part.so 12010936 -> 12315880 (2.53%) app.so-3.part.so 7579704 -> 7847768 (3.53%) TEST=ci Bug: #41974 Change-Id: I8e695966bc11c3912a01cd525ca69be6ef8e5ea2 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/171102 Commit-Queue: Ryan Macnak <rmacnak@google.com> Reviewed-by: Régis Crelier <regis@google.com>
…nt tables. TEST=ci Bug: #41974 Change-Id: Icc4a3ebf861dca5172a0cfa2cd2eea266e814d0c Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/181480 Commit-Queue: Ryan Macnak <rmacnak@google.com> Reviewed-by: Siva Annamalai <asiva@google.com> Reviewed-by: Alexander Markov <alexmarkov@google.com>
IMHO, grapheme clusters( |
@a-siva @rmacnak-google - is there still open work here? |
Yes there is still this CL to land https://dart-review.googlesource.com/c/sdk/+/183781 |
This defers literals, to effectively split binary size for libraries that are more data than code, such as localizations. TEST=ci Bug: #41974 Change-Id: I506722037cc0247b90756959e6a6f12bb5021b5c Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/183781 Reviewed-by: Alexander Markov <alexmarkov@google.com> Reviewed-by: Siva Annamalai <asiva@google.com> Commit-Queue: Ryan Macnak <rmacnak@google.com>
CLs have landed, closing issue. |
…oot" libraries when partitioning the program into loading units." Weaken assertion in gen_snapshot requiring all libraries to have a loading unit as there can still be unreachable libraries: - Google3 and Fuchsia will compile all the sources in a package to a single dill file, then present multiple input dill files to the AOT compilation. Since the set of libraries was derived from package membership instead of imports, many can be unreachable. - When the root library's main comes from an export, the frontend's representation will incorrectly report the library containing main as the root library and the true root library may be unreachable from it. Instead, assert only that surviving compiled code is assigned a loading unit. TEST=gallery Bug: flutter/gallery#545 Bug: #49325 Bug: #41974 Bug: b/237016312 Change-Id: Ia52563a6f517308d041368be11dcc85270f19acc Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/249724 Commit-Queue: Ryan Macnak <rmacnak@google.com> Reviewed-by: Alexander Markov <alexmarkov@google.com>
One of our biggest internal clients would like to split their AOT code per country. They plan to release to over 25 countries in the next year or so and some of them (such as NBU) are highly size sensitive. Their current calculations indicate that all countries put together will definitely exceed the minimum size requirements for the strictest countries. Their timeline for release is August/September 2020.
To avoid blocking that release, they would like to be able to ship smaller .so modules on Android and have Play Store deliver a combination of these modules to the user depending on their country. As a first step, these will be install-time only modules (i.e. no dynamically downloaded AOT code).
The request from VM team is two folds:
The text was updated successfully, but these errors were encountered: