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.24k stars 1.57k forks source link

[dart2js] Field '' has not been initialized #52629

Open yjbanov opened 1 year ago

yjbanov commented 1 year ago

Unfortunately this did not reproduce for me locally, but a LUCI log for the failure can be found here.

I got the following seemingly non-actionable error when running a Flutter Web benchmark on LUCI:

LateInitializationError: Field '' has not been initialized.
    at Object.throwUnnamedLateFieldNI (http://localhost:9999/main.dart.js:8415:91)
    at http://localhost:9999/main.dart.js:175729:29
    at _wrapJsFunctionForAsync_closure.$protected (http://localhost:9999/main.dart.js:10318:15)
    at _wrapJsFunctionForAsync_closure.call$2 (http://localhost:9999/main.dart.js:67240:12)
    at Object._asyncStartSync (http://localhost:9999/main.dart.js:10282:20)
    at BenchBuildColorsGrid.run$0 (http://localhost:9999/main.dart.js:175765:16)
    at http://localhost:9999/main.dart.js:175313:39
    at _wrapJsFunctionForAsync_closure.$protected (http://localhost:9999/main.dart.js:10318:15)
    at _wrapJsFunctionForAsync_closure.call$2 (http://localhost:9999/main.dart.js:67240:12)
    at _awaitOnObject_closure.call$1 (http://localhost:9999/main.dart.js:67228:32)

The surprising part is that the field name in the error is empty. I'm not sure how this can be possible. Maybe there's a way to improve the message?

To reproduce

Theoretically this should be reproducible using this command:

# Starting from the root of the flutter/flutter repo
cd dev/devicelab
dart bin/test_runner.dart test -t web_benchmarks_html

However, this did not reproduce on my local MBP, only on LUCI.

To run the benchmark without the devicelab wrapper:

cd dev/benchmarks/macrobenchmarks
flutter run -d chrome lib/web_benchmarks.dart --profile --web-renderer=html

Then click on the "text_canvas_color_grid" button.

fishythefish commented 1 year ago

dart2js has a flag called --omit-late-names which is included in -O2 and higher. We have this to avoid bloating production builds with extra strings for late error messages.

There is --no-omit-late-names to negate this flag.

rakudrama commented 1 year ago

The sourcemap translated stack will show the locations of the late field and the access to the late field. Looking at these locations in the original source should be sufficient to identify the error. Like any other case of debugging code with inlining, one JavaScript stack frame can correspond to several Dart stack frames.

The code size cost of including the names in every error is quite high (several percent code size bloat, theoretically unbounded for really long names) and also leaks original source names into the compilation product.

sigmundch commented 1 year ago

btw, if you have a way to recreate that main.dart.js or extract it from Luci together with the source-map file, then you should be able to deobfuscate the stack using tools like what we have under pkg/dart2js_tools/bin/deobfuscate.dart in the SDK repo

rakudrama commented 1 year ago

@yjbanov If I understand, you find

LateInitializationError: Field '' has not been initialized.

to be confusing because the field appears to have no name.

When the compilation options direct the compiler to omit source code field names from the optimized JavaScript code, should we change it so that the empty name is presented without quotes,

LateInitializationError: Field has not been initialized.

or something else?

yjbanov commented 1 year ago

The motivation makes sense. Are there costs other than bundle size? We run benchmarks in profile mode only, which currently differs from release mode in that it passes --no-minify. If there's no penalty in execution speed, can we pass --no-omit-late-names to profile builds so we get all the names?

rakudrama commented 1 year ago

The motivation makes sense.

Are there costs other than bundle size? We run benchmarks in profile mode only, which currently differs from release mode in that it passes --no-minify. If there's no penalty in execution speed, can we pass --no-omit-late-names to profile builds so we get all the names?

It would be a bigger difference that --minify vs --no-minify.

We have tried with --minify/--no-minify to avoid changes other than JavaScript names. There are a couple of small exceptions, but generally the code is the same if you blur out the JavaScript identifiers. That means the parser sees the same number of tokens and function calls have the same number of arguments and the byte-code for the functions is the same. Except for some identifiers, you are running the same program.

With --no-omit-late-names there will be names in the code as string literals that are additional arguments, and/or calls to slightly different versions of functions. The code with names is slightly different and that can change optimizations like inlining and string-pooling. A late getter might no longer be inlined since the name-string makes it bigger. Simply passing extra arguments might increase run time, but I would expect the overhead to be unmeasurable due to being small and diffuse. However, you have started down the slippery slope of running a different program.

That said, I don't think it is unreasonable to use --no-omit-late-names as you suggest.

As has been mentioned above, you should not need the names. If you have the sourcemap to translate the JavaScript locations to Dart locations, one of those locations will be the access to the late variable.