Improve Wasm Debugging. #41054

eyebrowsoffire · 2023-04-10T23:25:06Z

This PR makes some improvements to the debugging experience for built wasm modules (CanvasKit and Skwasm):

We now make sure to copy over the .map files to the artifacts directory for the host.dart.js files as well as any .wasm outputs, if they are produced.
We serve source files so that devtools can find them
We also support DWARF instead of sourcemaps if --dwarf is passed to felt on the command line.
- This requires installing a Chrome extension. I have set up a flow for this, as well as a caching scheme so that the develper only need to install the extension once.
  This requires some buildroot changes as well, PR for that is here: Add some options to make wasm debugging work better. buildroot#710

One thing to note is that the sourcemap strategy doesn't currently work for CanvasKit. This is because the CanvasKit target specifies the --source-map-base to be the location inside node_modules for npm clients. I will follow up with the Skia team to try to change this behavior so that the sourcemap strategy works. However, DWARF debugging works for both CanvasKit and Skwasm.

flutter-dashboard · 2023-04-10T23:25:10Z

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 (don't just cc him here, he won't see it! He's on Discord!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

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

yjbanov · 2023-04-11T01:28:41Z

lib/web_ui/dev/build.dart

    );
+    argParser.addFlag(
+      'dwarf',
+      help: 'Embed dwarf debugging info into the output wasm modules. This is'


nit: s/dwarf/DWARF/ (but keep the small case flag name)

yjbanov · 2023-04-11T01:29:47Z

lib/web_ui/dev/build.dart

    );
+    argParser.addFlag(
+      'dwarf',
+      help: 'Embed dwarf debugging info into the output wasm modules. This is'


nit2: add one space after "is" so it's not glued to "only" on the next line.

yjbanov · 2023-04-11T01:30:18Z

lib/web_ui/dev/build.dart

  FutureOr<bool> run() async {
+    final bool embedDwarf = boolArg('dwarf');
+    if (embedDwarf && runtimeMode != RuntimeMode.debug) {
+      throw ToolExit('Embedding dwarf data requires debug runtime mode.');


yjbanov · 2023-04-11T01:40:01Z

lib/web_ui/dev/test_platform.dart

        //  * /test/alarm_clock_test.dart
        //  * /lib/src/engine/alarm_clock.dart
        .add(createStaticHandler(env.environment.webUiRootDir.path))
+        .add(_createSourceHandler())


Both _createSourceHandler and the previous handler attempt to handle .dart sources. Can't tell which one wins when.

Turns out the first one is subsumed by the new one, so I removed the old one.

Actually, we still need that static handler, because it serves specific files that the tests use, but it isn't for sources anymore, so I updated the comment.

yjbanov · 2023-04-11T01:41:23Z

lib/web_ui/dev/test_runner.dart

+      )
+      ..addFlag(
+        'dwarf',
+        help: 'Debug wasm modules using embedded dwarf data.'


yjbanov · 2023-04-11T01:45:06Z

tools/gn

+      '--wasm-use-dwarf',
+      action='store_true',
+      default=False,
+      help='Embed dwarf debugging info in the output module instead of using '


mdebbar · 2023-04-11T15:21:14Z

DEPS

+  # TODO(jacksongardner): This targets a commit that is not on the master branch.
+  # This needs to be removed before this PR lands, but this will allow us to verify
+  # these changes in CI.
+  'src': 'https://github.com/flutter/buildroot.git' + '@' + '5adf0d1951587debc612831a285d765d8603dfbb',


Just a reminder

mdebbar

Thanks for improving the debugging experience!

lib/web_ui/dev/build.dart

mdebbar · 2023-04-11T19:42:16Z

third_party/canvaskit/icu_bidi/BUILD.gn

    "//third_party/icu/source/common/ustrtrns.cpp",
    "//third_party/icu/source/common/utf_impl.cpp",
    "//third_party/icu/source/common/utrie2.cpp",
+    "//third_party/icu/source/common/utypes.cpp",


Mind explaining what these changes mean? I think I understand adding utypes.cpp. It's used in debug mode to show error messages.

What about static_library => source_set?

utypes.cpp was because of a compile error I was getting when debugging, for the u_errorName_skia symbol. Changing from static_library to source_set was just something that is a holdover from when I was troubleshooting this issue. I think I can change it back.

Co-authored-by: Mouad Debbar <mouad.debbar@gmail.com>

auto-submit · 2023-04-12T00:13:03Z

auto label is removed for flutter/engine, pr: 41054, due to - This commit is not mergeable and has conflicts. Please rebase your PR and fix all the conflicts.

auto-submit · 2023-04-12T00:14:57Z

auto label is removed for flutter/engine, pr: 41054, due to - This commit is not mergeable and has conflicts. Please rebase your PR and fix all the conflicts.

Improve Wasm Debugging.

eyebrowsoffire added 3 commits April 10, 2023 16:05

Improve wasm debugging.

0ceea5f

Temporarily target a buildroot that is a working branch.

e3f63e9

Fix formatting issue.

b5623e4

flutter-dashboard bot added the platform-web Code specifically for the web engine label Apr 10, 2023

flutter-dashboard bot added the needs tests label Apr 10, 2023

eyebrowsoffire added 2 commits April 10, 2023 16:39

Update buildroot again.

091499d

Merge branch 'main' into wasm_debugging

1efbf3d

eyebrowsoffire requested a review from mdebbar April 11, 2023 00:35

yjbanov approved these changes Apr 11, 2023

View reviewed changes

mdebbar suggested changes Apr 11, 2023

View reviewed changes

mdebbar reviewed Apr 11, 2023

View reviewed changes

eyebrowsoffire and others added 4 commits April 11, 2023 13:28

Update lib/web_ui/dev/build.dart

2e43452

Co-authored-by: Mouad Debbar <mouad.debbar@gmail.com>

Addressed Yegor and Mouad's comments.

c00325f

Remove unused import.

800bbd9

Add back static file handler.

0ba6426

mdebbar approved these changes Apr 11, 2023

View reviewed changes

Roll the buildroot to the real landed hash.

63797be

eyebrowsoffire added the autosubmit Merge PR when tree becomes green via auto submit App label Apr 12, 2023

Merge branch 'main' into wasm_debugging

d5a711b

auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Apr 12, 2023

eyebrowsoffire added the autosubmit Merge PR when tree becomes green via auto submit App label Apr 12, 2023

auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Apr 12, 2023

eyebrowsoffire added the autosubmit Merge PR when tree becomes green via auto submit App label Apr 12, 2023

auto-submit bot merged commit 11d3300 into flutter:main Apr 12, 2023

engine-flutter-autoroll mentioned this pull request Apr 12, 2023

Roll Flutter Engine from 2123f462f463 to 11d33002546c (2 revisions) flutter/flutter#124645

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Apr 12, 2023

11d3300 Improve Wasm Debugging. (flutter/engine#41054)

50aee23

zhongwuzw pushed a commit to zhongwuzw/engine that referenced this pull request Apr 14, 2023

Improve Wasm Debugging. (flutter#41054)

cef29c5

Improve Wasm Debugging.

Improve Wasm Debugging. #41054

Improve Wasm Debugging. #41054

Uh oh!

Conversation

eyebrowsoffire commented Apr 10, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

flutter-dashboard bot commented Apr 10, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mdebbar left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

auto-submit bot commented Apr 12, 2023

Uh oh!

auto-submit bot commented Apr 12, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

eyebrowsoffire commented Apr 10, 2023 •

edited

Loading