Improve array loading times for large snapshots #48910

dnfield · 2022-04-28T02:54:16Z

In a large snapshot for customer: money there's significant time spent on array deserialization in the snapshot. This area seems like it could be improved to either do less work or do its work a bit faster.

From discussion with @a-siva and @rmacnak-google:

can the array of class and function references in the dictionary of the library object be eliminated (except for those classes, functions that have the entry point pragma)
consider placing arrays that only have references to RO Data objects in a special cluster and use offsets instead of ids in the reference table
consider placing arrays that only have smis in a special cluster and use the actual smi values instead of ids in the reference table

dnfield · 2022-04-28T02:55:07Z

Some more assets are available at http://shortn/_09Yx1TBIBc (google only, sorry)

mraleph · 2022-04-28T10:36:27Z

can the array of class and function references in the dictionary of the library object be eliminated (except for those classes, functions that have the entry point pragma)

Here is the breakdown of where arrays are coming from in terms of snapshot sizes. This was computed from heapsnapshot generated by gen_snapshot:

total: 1499344
%shared: 13 0.00%
ArtificialRoot.canonical_type_arguments: 15 0.00%
ArtificialRoot.canonical_type_parameters: 15 0.00%
ArtificialRoot.canonical_function_types: 15 0.00%
ArtificialRoot.canonical_types: 15 0.00%
ArtificialRoot.symbol_table: 15 0.00%
Class.functions_hash_table_: 1428 0.10%
Function.data_: 2810 0.19%
ArtificialRoot.symbol-table: 2819 0.19%
TypeParameters.names_: 6057 0.40%
FunctionType.named_parameter_names_: 7220 0.48%
ExceptionHandlers.handled_types_data_: 21562 1.44%
Class.functions_: 28170 1.88%
Class.fields_: 36647 2.44%
Class.interfaces_: 55533 3.70%
ArtificialRoot.libraries_map: 61223 4.08%
FunctionType.parameter_types_: 82887 5.53%
GrowableObjectArray.data_: 84556 5.64%
Library.dictionary_: 105438 7.03%
Class.invocation_dispatcher_cache_: 129244 8.62%
Class.constants_: 179990 12.00%
const Array: 693672 46.27%

Some of this is fairly trivial to prune:

Library.dictionary_ 105438 7.03% drops non entry-points entirely
GrowableObjectArray.data_: 84556 5.64% seems to come from Library.used_scripts_: 51471 99.98%. This field should just be dropped entirely in product AOT snapshots AFAIK.
Class.constants_ I think this should be possible to drop as well (at least have a mode for that). I think it should only be needed for canonicalization of arriving messages sent between two different isolate groups. Maybe we can just have a special flag which disables multiple isolate groups in Flutter applications? (This is not properly supported by tooling anyway). /cc @mkustermann @aam

mraleph · 2022-04-28T11:47:48Z

Class.invocation_dispatcher_cache_: 129244 8.62%

This is caused by a bug which adds a lot of duplicated entries to these caches. I am going to fix this:

Function::GetDynamicInvocationForwarder creates duplicate entries in the dispatcher cache #48914

This field is only used only by vm-service (including hot reload and debugging), Kernel loading and mirrors. So drop it to prevent putting it into the AOT snapshot. Saves at 125Kb of snapshot size on large Flutter app. Related to #48910 TEST=ci Cq-Include-Trybots: luci.dart.try:vm-kernel-precomp-dwarf-linux-product-x64-try,vm-kernel-precomp-linux-product-x64-try Change-Id: Id7c47496e87482d462c2b318307f617d48758ad1 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/242864 Commit-Queue: Slava Egorov <[email protected]> Reviewed-by: Alexander Markov <[email protected]>

No code actually relies on it being there. This saves ~1kb of snapshot size on a large Flutter application. Related to #48910 TEST=ci Change-Id: I03cf8811f39fc8882f7c650a79f01d350b520e5d Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/242863 Commit-Queue: Slava Egorov <[email protected]> Reviewed-by: Martin Kustermann <[email protected]>

mkustermann · 2022-04-29T13:08:18Z

Class.constants_ I think this should be possible to drop as well (at least have a mode for that). I think it should only be needed for canonicalization of arriving messages sent between two different isolate groups. Maybe we can just have a special flag which disables multiple isolate groups in Flutter applications? (This is not properly supported by tooling anyway). /cc @mkustermann @aam

We do allow different isolate groups in the same AOT VM in general, so while adding a flag may work for normal flutter, it's maybe not ideal.

Since different isolate groups are based on different source code, the set of constants all IGs understand are limited to corelib-only-using constants.

Maybe it would be better to simply prune the constants: Exclude any constant based on user-defined class/function and any constant transitively referencing such? We can measure and see what % we could remove.
@aam Interested in looking at this?

mraleph · 2022-04-29T13:33:37Z

Maybe it would be better to simply prune the constants: Exclude any constant based on user-defined class/function and any constant transitively referencing such?

Ah, good point. I thought we match classes by name (like we used to do when sending static functions), but in reality we
just reject them unless they are in dart:core, dart:collection or dart:typed_data.

I have run some quick check on the heap snapshot and I think that would drop ~60% of constants. Would be nice to do that.

Another option I have considered is that we could recollect constants using full heap scan if we drop all caches - the situation of two or more isolate groups is very unlikely in a Flutter application so it seems strange to pay for this.

aam · 2022-04-29T16:00:25Z

Maybe it would be better to simply prune the constants: Exclude any constant based on user-defined class/function and any constant transitively referencing such? We can measure and see what % we could remove.
@aam Interested in looking at this?

Sure, created #48928 for that.

rmacnak-google · 2022-05-03T21:33:01Z

706e62a - [vm] Canonicalize empty Class::interfaces_.
72c153b - [vm] Keep snapshot cursor etc in registers during ReadFill.
5748add - [vm] Use a big-endian encoding for snapshot ref ids.

TEST=--print_array_optimization_candidates Bug: #48910 Change-Id: I6e5031ce9cca4128b8fb75dd5c9e43ad57e7013c Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/243527 Reviewed-by: Dan Field <[email protected]> Reviewed-by: Siva Annamalai <[email protected]> Commit-Queue: Ryan Macnak <[email protected]>

a-siva · 2022-05-10T00:48:33Z

Here is the data for the GPay AOT snapshot from using the flag print_array_optimization_candidates

INFO: From Flutter, aot, snapshotting ...:
Arrays
  total:  17416,  221765 elements
  smi-only:264,  16179 elements
  ro-only:1986 ,  18883 elements
  empty:160
Arrays
  total:  61449,  289799 elements
  smi-only:0,  0 elements
  ro-only:1247 ,  1879 elements
  empty:13288
VMIsolate(CodeSize): 3180
Isolate(CodeSize): 6029491
ReadOnlyData(CodeSize): 9108896
Instructions(CodeSize): 23826104
Total(CodeSize): 38967671
VMIsolate(HeapSize): 16684
Isolate(HeapSize): 14336996
Total(HeapSize): 14353680

a-siva · 2022-05-10T16:48:00Z

With 706e62a - [vm] Canonicalize empty Class::interfaces_
the empty array references are gone reducing the number of array references by 13k

INFO: From Flutter, aot, snapshotting //java/com/google/nbu/paisa/flutter/gpay/app:android_release_android_library_flutter_artifacts_android_aot:
Arrays
total: 17416, 221765 elements
smi-only:264, 16179 elements
ro-only:1986 , 18883 elements
empty:160
Arrays
total: 48161, 289799 elements
smi-only:0, 0 elements
ro-only:1247 , 1879 elements
empty:0
VMIsolate(CodeSize): 3180
Isolate(CodeSize): 5963051
ReadOnlyData(CodeSize): 9108896
Instructions(CodeSize): 23826104
Total(CodeSize): 38901231
VMIsolate(HeapSize): 16684
Isolate(HeapSize): 14124388
Total(HeapSize): 14141072

a-siva · 2022-05-10T16:49:40Z

Another related fix https://dart-review.googlesource.com/c/sdk/+/243709 (Trim user-defined entries from classes constants table)
This reduces the Arrays count as follows total: 45359, 243859 elements
VMIsolate(CodeSize): 3180
Isolate(CodeSize): 5852987
ReadOnlyData(CodeSize): 9108896
Instructions(CodeSize): 23826104
Total(CodeSize): 38791167
VMIsolate(HeapSize): 16684
Isolate(HeapSize): 13895796
Total(HeapSize): 13912480

a-siva · 2022-05-10T17:21:31Z

https://dart-review.googlesource.com/c/sdk/+/242864 (Removes used_scripts field from AOT product builds)
This reduces the Arrays count as follows total: 37990, 221028 elements
VMIsolate(CodeSize): 3180
Isolate(CodeSize): 5724746
ReadOnlyData(CodeSize): 9107792
Instructions(CodeSize): 23826104
Total(CodeSize): 38661822
VMIsolate(HeapSize): 16684
Isolate(HeapSize): 13597420
Total(HeapSize): 13614104

a-siva · 2022-05-10T17:43:58Z

https://dart-review.googlesource.com/c/sdk/+/242862 (Avoids duplicate entries in invocation dispatcher cache)
This reduces the Arrays count as follows total: 37395, 167934 elements
VMIsolate(CodeSize): 3180
Isolate(CodeSize): 5602004
ReadOnlyData(CodeSize): 9107464
Instructions(CodeSize): 23826104
Total(CodeSize): 38538752
VMIsolate(HeapSize): 16684
Isolate(HeapSize): 13376844
Total(HeapSize): 13393528

a-siva · 2022-05-13T18:57:36Z

https://dart-review.googlesource.com/c/sdk/+/244303 is the CL in progress to remove dictionary entries (The CL still has some failing tests that need investigation). A preliminary run of building the AOT snapshot including this CL shows the following stats
Arrays
total: 29632, 96654 elements

…runtime. flutter_gallery Isolate(CodeSize): 2116400 -> 1981238 (-6.28%) Total(CodeSize): 7217938 -> 7082600 (-1.87%) TEST=ci Bug: #48910 Change-Id: I8cd285ddab3a611cd7a2a91d50414be402f8543a Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/244303 Reviewed-by: Siva Annamalai <[email protected]> Commit-Queue: Ryan Macnak <[email protected]>

…eded at runtime." This reverts commit 6de162e. Reason for revert: Local testing of a large Flutter app with this change causes the app to crash on startup Original change's description: > [vm, compiler] Prune dictionaries to only contain elements needed at runtime. > > flutter_gallery > Isolate(CodeSize): 2116400 -> 1981238 (-6.28%) > Total(CodeSize): 7217938 -> 7082600 (-1.87%) > > TEST=ci > Bug: #48910 > Change-Id: I8cd285ddab3a611cd7a2a91d50414be402f8543a > Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/244303 > Reviewed-by: Siva Annamalai <[email protected]> > Commit-Queue: Ryan Macnak <[email protected]> # Not skipping CQ checks because original CL landed > 1 day ago. Bug: #48910 Change-Id: Ie51f004e84970907fa1233e8e7c3ed63e2da1c4c Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/246683 Reviewed-by: Siva Annamalai <[email protected]> Commit-Queue: Siva Annamalai <[email protected]> Reviewed-by: Ryan Macnak <[email protected]>

…eded at runtime." This is a reland of commit 6de162e Original change's description: > [vm, compiler] Prune dictionaries to only contain elements needed at runtime. > > flutter_gallery > Isolate(CodeSize): 2116400 -> 1981238 (-6.28%) > Total(CodeSize): 7217938 -> 7082600 (-1.87%) > > TEST=ci > Bug: #48910 > Change-Id: I8cd285ddab3a611cd7a2a91d50414be402f8543a > Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/244303 > Reviewed-by: Siva Annamalai <[email protected]> > Commit-Queue: Ryan Macnak <[email protected]> TEST=ci Bug: #48910 Change-Id: I3d3ecd04369585547963fb3efff1b3ff0723d8f8 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/246990 Commit-Queue: Ryan Macnak <[email protected]> Reviewed-by: Siva Annamalai <[email protected]>

a-siva assigned rmacnak-google Apr 28, 2022

aam mentioned this issue Apr 29, 2022

Reduce AOT snapshot size by removing unnecessary constants #48928

Closed

rmacnak-google closed this as completed May 25, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve array loading times for large snapshots #48910

Improve array loading times for large snapshots #48910

dnfield commented Apr 28, 2022

dnfield commented Apr 28, 2022

mraleph commented Apr 28, 2022

mraleph commented Apr 28, 2022 •

edited

Loading

mkustermann commented Apr 29, 2022 •

edited

Loading

mraleph commented Apr 29, 2022

aam commented Apr 29, 2022

rmacnak-google commented May 3, 2022

a-siva commented May 10, 2022 •

edited by mraleph

Loading

a-siva commented May 10, 2022

a-siva commented May 10, 2022 •

edited

Loading

a-siva commented May 10, 2022 •

edited

Loading

a-siva commented May 10, 2022 •

edited

Loading

a-siva commented May 13, 2022

Improve array loading times for large snapshots #48910

Improve array loading times for large snapshots #48910

Comments

dnfield commented Apr 28, 2022

dnfield commented Apr 28, 2022

mraleph commented Apr 28, 2022

mraleph commented Apr 28, 2022 • edited Loading

mkustermann commented Apr 29, 2022 • edited Loading

mraleph commented Apr 29, 2022

aam commented Apr 29, 2022

rmacnak-google commented May 3, 2022

a-siva commented May 10, 2022 • edited by mraleph Loading

a-siva commented May 10, 2022

a-siva commented May 10, 2022 • edited Loading

a-siva commented May 10, 2022 • edited Loading

a-siva commented May 10, 2022 • edited Loading

a-siva commented May 13, 2022

mraleph commented Apr 28, 2022 •

edited

Loading

mkustermann commented Apr 29, 2022 •

edited

Loading

a-siva commented May 10, 2022 •

edited by mraleph

Loading

a-siva commented May 10, 2022 •

edited

Loading

a-siva commented May 10, 2022 •

edited

Loading

a-siva commented May 10, 2022 •

edited

Loading