Increase threshold for usage of compute for utf8 decoding on large strings to 50 KB

[jonahwilliams]

Description

#36552

On a local benchmark, removing compute is faster than compute in both debug and release mode on a Pixel 4. Looking for more confirmation on other older phone types.

Debug:
314 ms load/parse asset manifest with compute
91 ms load/parse without compute

Release:
14 ms load/parse with compute
2 ms load/parse without compute

I measured perf by inserting a stopwatch/print:

diff --git a/packages/flutter/lib/src/services/asset_bundle.dart b/packages/flutter/lib/src/services/asset_bundle.dart
index 0e8bb02874..f0028a0103 100644
--- a/packages/flutter/lib/src/services/asset_bundle.dart
+++ b/packages/flutter/lib/src/services/asset_bundle.dart
@@ -178,6 +178,7 @@ abstract class CachingAssetBundle extends AssetBundle {
       return _structuredDataCache[key] as Future<T>;
     Completer<T>? completer;
     Future<T>? result;
+    var sw = Stopwatch()..start();
     loadString(key, cache: false).then<T>(parser).then<void>((T value) {
       result = SynchronousFuture<T>(value);
       _structuredDataCache[key] = result!;
@@ -197,6 +198,9 @@ abstract class CachingAssetBundle extends AssetBundle {
     // completer for it to use when it does run.
     completer = Completer<T>();
     _structuredDataCache[key] = completer.future;
+    completer.future.whenComplete(() {
+      print('LoadStructuredDataTook: ${sw.elapsedMilliseconds}');
+    });
     return completer.future;
   }

[flutter-dashboard]

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

[jonahwilliams]

That stopwatch is actually measuring UTF8 decode and JSON parse, but only UTF8 decode ran on an isolate. Locally it takes about 100 microseconds on 18 KB (the size of the flutter gallery asset manifest)

[aam]

perhaps add a comment regarding "50 * 1024" number?

[jonahwilliams]

Done

[chingjun]

While this change LGTM, I wouldn't say that it fixes #36552, it only delayed the issue until AssetManifest.json is larger than 50KB.

Since AssetManifest is only loaded once in the app lifetime, what if we could load AssetManifest without using compute regardless of size?

[jonahwilliams]

Yeah, I agree this doesn't completely fix the issue. I'm not sure about always skipping compute because I could imagine the asset manifest causing a significant amount of jank if it were truly that big - but mostly due to JSON parsing.

Another approach would be to revisit running code-generation to create a const map of the asset manifest