Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Failure in some Fuschia tests that use --v8-snapshot-profile-writer=... #45787

Closed
sstrickl opened this issue Apr 21, 2021 · 2 comments
Closed

Failure in some Fuschia tests that use --v8-snapshot-profile-writer=... #45787

sstrickl opened this issue Apr 21, 2021 · 2 comments
Assignees
@sstrickl
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. gardening

Comments

@sstrickl
Copy link
Contributor

sstrickl commented Apr 21, 2021 •
edited
Loading

The type of a v8 snapshot profile node is set when SetObjectTypeAndName is called for a given object id, which happens when the corresponding object is written to the snapshot. However, when adding references or roots to the profile, there may be forward references to nodes that have not been written yet.

Thus, in methods like AddRoot, AttributeReferenceTo, etc., a call to EnsureId is made. If the node doesn't already exist, EnsureId creates a preliminary version of the node which has no type, with the expectation that there will eventually be a call to SetObjectTypeAndName for that id.

While none of our CI runs have triggered a case where a node with unknown type (where SetObjectTypeAndName was never called for a given object id) is written to the snapshot, there are currently a couple of Fuchsia tests which do (example log).
@sstrickl sstrickl added area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. gardening labels Apr 21, 2021
@sstrickl sstrickl self-assigned this Apr 21, 2021
@sstrickl
Copy link
Contributor Author

Have a patch in CL 196322 that writes a special type for nodes with unknown types to the profile in non-DEBUG mode, as happened before recent refactorings, and keep the failure in DEBUG mode.

@sstrickl
Copy link
Contributor Author

sstrickl commented Apr 21, 2021 •
edited
Loading

I think I've found the issue that caused that to happen, though, so have a followup CL 196323 which adds back the strict checking and fixes a case where SetObjectTypeAndName was not called (only happened in non-precompiled mode). Will run it through the flutter trybots and put it up for review if it passes.

dart-bot pushed a commit that referenced this issue Apr 22, 2021
@sstrickl
[vm] Re-tighten requirements that the node type be set before writing.
927c591 
Fixes the case where the Image name and type was not written
to the v8 snapshot profile in non-precompiled mode.

Adds core-jit snapshot tests to the v8 snapshot profile writer test.

In core-jit and app-aot-elf modes, verify that all bytes in the
snapshot sections are accounted for in the profile.

Bug: #45787

TEST=vm/dart{,_2}/v8_snapshot_profile_writer_test

Cq-Include-Trybots: luci.dart.try:vm-kernel-precomp-linux-product-x64-try,vm-kernel-precomp-linux-release-x64-try,vm-kernel-precomp-linux-debug-x64-try
Change-Id: I88be309cdea6293d768e2fa458492387d70fde22
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/196323
Commit-Queue: Tess Strickland <sstrickl@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
Reviewed-by: Daco Harkes <dacoharkes@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@sstrickl sstrickl

Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. gardening
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@sstrickl