Commit graph

27 commits

Author SHA1 Message Date
Devon Carew a3256e6b2f [pkg/vm_snapshot_analysis] switch to using package:lints
Change-Id: Ia36b1bf1127e0b78e0294422cff78338ef19ec26
TEST=this CL updates static analysis settings and address associated warnings
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/204640
Commit-Queue: Devon Carew <devoncarew@google.com>
Reviewed-by: Tess Strickland <sstrickl@google.com>
Reviewed-by: Slava Egorov <vegorov@google.com>
2021-06-24 17:07:48 +00:00
Vyacheslav Egorov 458be23fa7 [vm_snapshot_analysis] Migrate to null-safety
Fixes https://github.com/dart-lang/sdk/issues/45683

TEST=pkg-*-try bots

Cq-Include-Trybots: luci.dart.try:pkg-linux-release-try,pkg-mac-release-try,pkg-win-release-try
Change-Id: Ie10f313da9778d001f9c4fb618997e3b3c781dd0
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/195263
Commit-Queue: Vyacheslav Egorov <vegorov@google.com>
Reviewed-by: Tess Strickland <sstrickl@google.com>
2021-04-15 12:03:47 +00:00
Tess Strickland dc88076767 [vm] Handle WSRs more generally in v8 snapshot profile writing.
Instead of special-casing the current fields that may have
WeakSerializationReferences, handle WSRs appearing as elements
or properties of objects more generally. This removes existing
special casing and avoids the need for it in case of new
future uses of WSRs.

For artificial nodes being added for dropped
WeakSerializationReference targets, add them as kArtificial nodes
(not kSnapshot) that has the original offset (element) or
name (property). The replacement is added as a kSnapshot node
that has a negative offset with the same magnitude as the
artificial node (element) or ":real_<property name>" (property).
This simplifies the work done in pkg/vm_snapshot_analysis to
use the artificial nodes instead of replacement ones for
reassembling hierarchies and the like.

This CL also cleans up the old SerializerWritingObjectScope
class, both moving it to Serializer::WritingObjectScope and
allowing nesting of WritingObjectScopes with the correct
semantics.

Thanks to this, not only can we recur when under a WritingObjectScope
instead of lifting recursion outside of those scopes, but we can
also create artificial nodes for non-empty per-code object pools and
static call target tables instead of attributing their contents
as supposed elements of the Code object being written.

TEST=pkg/vm_snapshot_analysis/test/instruction_sizes_test

Cq-Include-Trybots: luci.dart.try:pkg-linux-release-try,pkg-mac-release-try,pkg-win-release-try
Change-Id: Ib945c5afcd89b1458b8be3559b6eae24048aba2f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/194243
Commit-Queue: Tess Strickland <sstrickl@google.com>
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Reviewed-by: Vyacheslav Egorov <vegorov@google.com>
2021-04-14 08:44:58 +00:00
Tess Strickland bc924667f4 [vm/compiler] Add artificial nodes for dropped parent functions.
Fixes https://github.com/dart-lang/sdk/issues/45483

TEST=pkg/vm_snapshot_analysis/test/instruction_sizes_test

Cq-Include-Trybots: luci.dart.try:pkg-linux-release-try,pkg-mac-release-try,pkg-win-release-try
Change-Id: I9e347bd38ec339b22d2ecefcce1b276d8ce3e3e7
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/193303
Reviewed-by: Vyacheslav Egorov <vegorov@google.com>
Commit-Queue: Tess Strickland <sstrickl@google.com>
2021-04-07 04:48:46 +00:00
Vyacheslav Egorov d499f68df5 [vm] Fix vm_snapshot_analysis logic and tests
* When looking for Code node owner simply always check for synthetic
  owner (referenced through ':owner_' property) first. If it is present
  then it means 'owner_' property is invalid anyway. 25fd0200ef changed
  how WSRs are written into snapshot (instead of writing WSRs we are
  simply writing SMI CIDs) which broke previous logic.
* When building ProgramInfoNode for a Class node accommodate for the
  existence of classes which don't have 'library_' property (e.g. base
  objects like void and dynamic). Additionally, adjust how these base
  objects are written into the snapshot profile so that we preserve
  their names.
* Tweak tests diff and diff-collapse tests to pass, they were failing
  because prologue removal compiler change made some of the diffs so
  tiny that they are ignored by threshold filtering.
* Add two more tests: one that verifies that tracings flags don't have
  any impact on the snapshot size and one that checks we correct
  attribute instructions size to the function node from which these
  instructions originated. This last test is added in anticipation of
  Code object removal to make sure that vm_snapshot_analysis continues
  to work correctly.
* Adjust sorting rules for Code cluster to make it more stable between
  slightly different programs to stabilize output of
  --print-instructions-sizes-to (depends on the sorting algorithm used
  in libc, the test was failing on Mac but not on Linux).

These fixes should unblock relanding of b6dc4dad4d

Fixes #44507
Fixes flutter/flutter#76313

TEST=pkg/vm_snapshot_analysis/test/instruction_sizes_test.dart

Cq-Include-Trybots: luci.dart.try:pkg-linux-release-try,pkg-win-release-try,pkg-mac-release-try
Change-Id: Iee0221dd3a507cb4f36d530404693ebfab3b7bf2
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/185826
Commit-Queue: Vyacheslav Egorov <vegorov@google.com>
Reviewed-by: Alexander Markov <alexmarkov@google.com>
2021-02-21 15:48:27 +00:00
Lasse R.H. Nielsen 6e29700e16 Update List constructor documentation, deprecate constructor.
Emphasize that the operation is going away,
and mark constructor as deprecated.

TEST= Refactoring+deprecation only, covered by existing tests.

Change-Id: I82aa044cd2cf7bf347b624371399f44bda8f4a07
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/173261
Commit-Queue: Lasse R.H. Nielsen <lrn@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Nate Bosch <nbosch@google.com>
2020-12-07 16:20:28 +00:00
Vyacheslav Egorov 4d182203ca [vm] Include static call references into V8 profile
Code objects might be connected by static calls and these references
were not included into the profile. This lead to some objects being
unreachable from the root.

This CL also adds a test which computes dominators for all nodes
in the snapshot (this in turn verifies that there are no
unreachable nodes).

Cq-Include-Trybots: luci.dart.try:pkg-linux-release-try,pkg-win-release-try,pkg-mac-release-try,vm-kernel-precomp-linux-release-x64-try,vm-kernel-precomp-linux-product-x64-try,vm-kernel-precomp-linux-debug-x64-try
Change-Id: I2af4107fdb7f875624192e892ce1cec78cbf0dd0
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/161709
Commit-Queue: Vyacheslav Egorov <vegorov@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
2020-09-08 12:11:34 +00:00
Vyacheslav Egorov 80504c8ffe [vm_snapshot_analysis] Fix issues with artificial nodes
This CL addresses two problems, which only reveal themselves when
analysing snapshots produced when function dropping is enabled.
When tests for snapshot analysis were written functions were dropped
in PRODUCT build by default - by currently this functionality needs
to be enabled separately or is automatically enabled when DWARF stack
traces are enabled. This CL changes test suite to enable DWARF stack
traces.

* When constructing ProgramInfo from V8 snapshot profile we should
not assume that Function nodes have any fields outside of owner_,
because Function node might be artificial and thus have no real
fields except for owner link.
* When writing V8 snapshot profile we should take care not to call
CreateArtificalNodeIfNeeded inside TraceStartWritingObject scope,
because CreateArtificalNodeIfNeeded might itself call
TraceStartWritingObject and these calls don't nest, the second one
overwrites the first one.

R=sstrickl@google.com

Bug: b/167601345
Change-Id: I40879ee087c38992388776af58b81bbefd147631
Cq-Include-Trybots: luci.dart.try:pkg-linux-release-try,pkg-win-release-try,pkg-mac-release-try
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/161703
Reviewed-by: Tess Strickland <sstrickl@google.com>
Commit-Queue: Vyacheslav Egorov <vegorov@google.com>
2020-09-03 13:31:14 +00:00
Kenzie Schmoll 42dfb9d304 Add generateCallGraphWithDominators method to generate a CallGraph from a precompiler trace.
Exposing this functionality makes it possible to use this logic in Dart DevTools.

This CL also includes some renames that make the code more readable. When there are both `ProgramInfoNode`s and `CallGraphNode`s in scope, the name "node" is ambiguous.

Bug: https://github.com/dart-lang/sdk/issues/43169
Change-Id: Ic8ef04e10c48db011cd28e1786bee34223766e47
Cq-Include-Trybots: luci.dart.try:pkg-linux-release-try,pkg-win-release-try,pkg-mac-release-try
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/160342
Commit-Queue: Kenzie Schmoll <kenzieschmoll@google.com>
Reviewed-by: Vyacheslav Egorov <vegorov@google.com>
2020-09-02 16:24:42 +00:00
Vyacheslav Egorov 3471490bb2 [vm_snapshot_analysis] Fix CallGraph.collapse by package
Take into account that not all libraries are grouped into packages
avoid collapsing such libraries into the root node.

Cq-Include-Trybots: luci.dart.try:pkg-linux-release-try,pkg-win-release-try,pkg-mac-release-try
Change-Id: I73a14a2498c0399073be432d1e22badf99eca3eb
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/161167
Commit-Queue: Kenzie Schmoll <kenzieschmoll@google.com>
Reviewed-by: Kenzie Schmoll <kenzieschmoll@google.com>
2020-09-01 21:35:21 +00:00
Kenzie Schmoll dabc54edb8 [vm_snapshot_analysis] modify -d flag and add -s flag for summary command
Change-Id: Ibaa017ffc80d674f439af1f2097729df50755fe7
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/158368
Commit-Queue: Kenzie Schmoll <kenzieschmoll@google.com>
Reviewed-by: Vyacheslav Egorov <vegorov@google.com>
2020-08-13 15:32:59 +00:00
Vyacheslav Egorov 1f27788f41 [vm_snapshot_analysis] Avoid name clash
dart:core contains a class Type and it can also contain objects which
are instances of this class. Previously we would try to put size
information for both under dart:core/Type bucket which caused issues.

To prevent the clash we now wrap snapshot node types in '<...>' before using
them as path components, so that the first case would remain dart:core/Type
but size of Type instances would be written into dart:core/<Type> bucket.

Fixes https://github.com/dart-lang/sdk/issues/42969

Fixed: 42969
Cq-Include-Trybots: luci.dart.try:pkg-linux-release-try,pkg-win-release-try,pkg-mac-release-try
Change-Id: Ie6c3400e90511507ff37b7ca037bd385dea1b11c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/157493
Commit-Queue: Vyacheslav Egorov <vegorov@google.com>
Reviewed-by: Kenzie Schmoll <kenzieschmoll@google.com>
2020-08-07 07:06:14 +00:00
Peter Lee 9b185c9dd3 [vm_snapshot_analysis] Add compareProgramInfo helper method
Exposing this helper method will allow comparison between program info without using buildComparisonTreemap


Change-Id: Ib771801530dd34515ccecc4ec01e39cd4546e916
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/157080
Commit-Queue: Kenzie Schmoll <kenzieschmoll@google.com>
Reviewed-by: Kenzie Schmoll <kenzieschmoll@google.com>
2020-08-06 15:49:49 +00:00
Kenzie Schmoll 99a233505f [vm_snapshot_analysis] Add partsForPath util to split google3 paths
Support splitting paths that look like: `package:foo.bar.baz/src/foobar.dart` as well
as "normal" paths (`package:foo/bar/baz/src/foobar.dart`).

Change-Id: I7816b7d6fd34d786a735bf74e3ea2a2606c27af5
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/157023
Commit-Queue: Kenzie Schmoll <kenzieschmoll@google.com>
Reviewed-by: Vyacheslav Egorov <vegorov@google.com>
2020-08-04 22:30:50 +00:00
Kenzie Schmoll cb10c778a0 [vm_snapshot_analysis] add missing await to fix type error
We were getting the type error "type 'Future<Object>' is not a subtype of type 'Map<String, dynamic>'" because we weren't awaiting this future.

Change-Id: I1247151d5a7bcf688eb90f0188c783cbffc432fb
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/156918
Reviewed-by: Vyacheslav Egorov <vegorov@google.com>
Commit-Queue: Kenzie Schmoll <kenzieschmoll@google.com>
2020-08-04 22:27:10 +00:00
Vyacheslav Egorov a99edb8ac5 [vm_snapshot_analysis] Fix treemap construction
Do not include package nodes names into path when building treemap from
ProgramInfo. Library names already contain full package name.

Fixes #42907

Fixed: 42907

Cq-Include-Trybots: luci.dart.try:pkg-linux-release-try,pkg-win-release-try,pkg-mac-release-try
Change-Id: I91c4fc73edb3b345dfcc485e418d50b2ec5f4fe7
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/156910
Commit-Queue: Vyacheslav Egorov <vegorov@google.com>
Reviewed-by: Kenzie Schmoll <kenzieschmoll@google.com>
2020-08-04 20:39:38 +00:00
Kenzie Schmoll d9860c8b4f [vm_snapshot_analysis] remove unused import
Change-Id: I42f5144e4f657d39a34cf2b1a4906af745372153
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/155220
Reviewed-by: David Morgan <davidmorgan@google.com>
Commit-Queue: Kenzie Schmoll <kenzieschmoll@google.com>
2020-07-21 15:44:47 +00:00
Kenzie Schmoll 6607655382 [vm_snapshot_analysis] move command/* lib files out of bin
The files in `command/*` were moved as part of https://dart-review.googlesource.com/c/sdk/+/154360.

This CL responds to Slava's comment "FWIW having this files in bin violates pub layout guidelines and looks pretty different from all other packages we have: bin/ is a folder for scripts. Libraries should live in lib."

I verified in DevTools that with this change, we can still import files from package:vm_snapshot_analysis and run on the web (as long as we do not import files that depend on dart:io - lib/commands/utils.dart)

Change-Id: I8c4f2a55c9cf346260fa5b80116c21993ccfc249
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/154481
Reviewed-by: Vyacheslav Egorov <vegorov@google.com>
Commit-Queue: Kenzie Schmoll <kenzieschmoll@google.com>
2020-07-15 20:06:47 +00:00
Martin Kustermann 06183779d7 [vm/compiler] Make the compiler use dyn:* selectors when calling getters
We already make the callsites use dyn:* selectors for normal method
calls as well as setters, this is doing the same thing for getters.

Once callsites use dyn:get:* various pieces in runtime need to be
adjusted to accomodate for that (e.g. NSM handling, etc).

A follow-up CL will then start actually generating dyn:* getters in
certain situations.

Issue https://github.com/dart-lang/sdk/issues/40876

Change-Id: If219603bc0b8eb119edd08b211a8897d21ec0fb6
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/154320
Commit-Queue: Martin Kustermann <kustermann@google.com>
Reviewed-by: Tess Strickland <sstrickl@google.com>
2020-07-15 12:38:04 +00:00
Kenzie Schmoll 4f55ba2639 [vm_snapshot_analysis] rearrange dart:io dep and move commands to bin
Change-Id: Ib22ae8d1ec3c4032c646915bad94361efef48a2c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/154360
Commit-Queue: Kenzie Schmoll <kenzieschmoll@google.com>
Reviewed-by: Jacob Richman <jacobr@google.com>
2020-07-14 23:09:18 +00:00
Kenzie Schmoll 70bbc663e2 Remove dart:io dep from utils.dart and add conditional imports for File
Change-Id: I92c00dc0a44fffda81aaec13a4598815b4148d32
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/154283
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Commit-Queue: Kenzie Schmoll <kenzieschmoll@google.com>
2020-07-13 23:19:19 +00:00
Vyacheslav Egorov 5d47c5f278 [vm_snapshot_analysis] Improve compare command
Add breakdown by node type when this information is available.

Change-Id: If2d477d0720aff69e3a168b9520fc3c2ccd153c1
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/153767
Commit-Queue: Vyacheslav Egorov <vegorov@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
2020-07-09 09:43:32 +00:00
Vyacheslav Egorov 69ec1a9965 [vm/aot] Add machine readable precompiler trace
This CL adds --trace-precompiler-to option which generates a machine readable
precompiler trace (list of all compiled functions and their dependencies).

It also expands package:vm_snapshot_analysis with tools for reading and
analysing this trace.

For example snapshot_analysis explain dynamic-calls command allows
to list all dynamic calls sorted by their impact on the size of the AOT
snapshot.

Issue https://github.com/dart-lang/sdk/issues/41249

Cq-Include-Trybots: luci.dart.try:pkg-linux-debug-try,pkg-linux-release-try,pkg-win-release-try,pkg-mac-release-try
Change-Id: Ie49143f4da375067991991e2ad20a41ec67bb1c3
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/152851
Commit-Queue: Vyacheslav Egorov <vegorov@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
2020-07-03 09:29:10 +00:00
Vyacheslav Egorov 28a3cd2039 [vm_snapshot_analysis] Add buildComparisonTreemap
This helper method compares two size profiles and returns result
as a treemap.

Cq-Include-Trybots: luci.dart.try:pkg-linux-debug-try,pkg-linux-release-try,pkg-win-release-try,pkg-mac-release-try
Change-Id: I82e5617bc367a2b89d3685ce7f7babc01492b531
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/152908
Reviewed-by: Vyacheslav Egorov <vegorov@google.com>
Commit-Queue: Vyacheslav Egorov <vegorov@google.com>
2020-07-02 11:35:49 +00:00
Vyacheslav Egorov 4079c373c0 [vm/snapshot_analysis] Extract treemap from snapshot
This also includes few other changes:

- Add ability to collapse leaf nodes in a treemap created from V8 snapshot
profile. This behavior is programmatically controlled by `TreemapFormat format`
parameter and from CLI via `--format` flag. The following options are available
    - `collapsed` essentially renders `ProgramInfo` as a treemap, individual
    snapshot nodes are ignored.
    - `data-and-code` collapses snapshot nodes based on whether they represent
    data or executable code.
    - `object-type` collapses snapshot nodes based on their type only.
- When computing `ProgramInfo` from a V8 snapshot profile no longer create
`ProgramInfoNode` for `Code` nodes which are owned by a function - instead
directly attribute the `Code` node itself and all retained nodes into
`ProgramInfoNode` for the function itself. For stubs (including allocation
stubs) create an artificial `functionNode` instead of using `NodeType.other`.
The only remaining use of `NodeType.other` is for fields.

Cq-Include-Trybots: luci.dart.try:pkg-linux-debug-try,pkg-linux-release-try,pkg-win-release-try,pkg-mac-release-try
Change-Id: I18e5437a10dd9141adaed5dfd6f0a3545740e2a2
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/152320
Commit-Queue: Vyacheslav Egorov <vegorov@google.com>
Reviewed-by: Kenzie Schmoll <kenzieschmoll@google.com>
Reviewed-by: Tess Strickland <sstrickl@google.com>
2020-06-25 12:58:28 +00:00
Vyacheslav Egorov e148167747 [vm/snapshot_analysis] Fix a minor bug in attribution of node sizes.
Previously we given the info node and its corresponding snapshot node we
would attribute the size of snapshot node to the parent of the info node,
which is incorrect.

This CL also includes few other minor changes (all described in
CHANGELOG.md):

-  `--help` output of the command,
the method for detecting executable name was not working correctly
under `pub global activate`: it printed snapshot name, instead of
`snapshot_analysis`;
- rename `README.dart` to `README.md` because `.dart` extension
confuses `pub` analysis.
- add documentation to `ProgramInfoNode.size`.
- add documentation on passing flags to AOT compiler


Cq-Include-Trybots: luci.dart.try:pkg-linux-release-try,pkg-win-release-try,pkg-mac-release-try
Change-Id: I8e8f2e9b7b9bfb3fe688caa16c0490eb8ec60e7c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/152148
Commit-Queue: Vyacheslav Egorov <vegorov@google.com>
Reviewed-by: Tess Strickland <sstrickl@google.com>
2020-06-24 11:03:55 +00:00
Vyacheslav Egorov 117df0e98b [vm/tools] Move snapshot analysis tool to its own package.
This code will be published on Pub so that Dev Tools can consume it.

Cq-Include-Trybots: luci.dart.try:pkg-linux-debug-try,pkg-linux-release-try,pkg-win-release-try,pkg-mac-release-try
Change-Id: Iddfbd3f0976af218d29ac20b452fbb139983a484
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/152008
Reviewed-by: Tess Strickland <sstrickl@google.com>
Commit-Queue: Vyacheslav Egorov <vegorov@google.com>
2020-06-23 13:14:41 +00:00