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.

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

Minimize performance hit from strong mode checks on microbenchmarks and flutter benchmarks #31798

Open
mraleph opened this issue Jan 8, 2018 · 2 comments
Assignees
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends.

Comments

@mraleph
Copy link
Member

mraleph commented Jan 8, 2018

This is a metabug to track design and implementation of necessary optimizations in the compilation pipeline.

@mraleph mraleph added the area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. label Jan 8, 2018
@mraleph mraleph added this to the 2.0-alpha milestone Jan 8, 2018
@mraleph mraleph added the P0 A serious issue requiring immediate resolution label Jan 8, 2018
whesse pushed a commit that referenced this issue Jan 10, 2018
This allows us to convert UnboxInt64Instr's to UnboxedConstantIntrs if
the input is a constant.

Issue #31798

Change-Id: Ieefd28503059302817baa6f1a463a6e6a9a61398
Reviewed-on: https://dart-review.googlesource.com/33462
Commit-Queue: Martin Kustermann <[email protected]>
Reviewed-by: Vyacheslav Egorov <[email protected]>
whesse pushed a commit that referenced this issue Jan 10, 2018
…nstants as inputs

Issue #31798

Change-Id: I3a64fc9e6a038ae28e3a98149b0494ebe254fd73
Reviewed-on: https://dart-review.googlesource.com/33780
Commit-Queue: Martin Kustermann <[email protected]>
Reviewed-by: Vyacheslav Egorov <[email protected]>
whesse pushed a commit that referenced this issue Jan 10, 2018
…ticCallInstr is used

We have devirtualization logic which turns [InstanceCallInst]s to
[StaticCallInstr]s (which are subject to different optimizations).

This makes the inlining decisions done in dart-aot-v2 a little
bit closer to dart-aot.

Issue #31798

Change-Id: If7961ad8f05ac2544f04044d05541a86a8074984
Reviewed-on: https://dart-review.googlesource.com/34000
Reviewed-by: Vyacheslav Egorov <[email protected]>
Commit-Queue: Martin Kustermann <[email protected]>
whesse pushed a commit that referenced this issue Jan 12, 2018
…ng mode

Since implicit field getters have the receiver as the only argument
(on which don't have to perform any type checks), we can allow
intrinsification of implicit getters even in checked/strong mode.

Issue #31798

Change-Id: If08c6ee33818ab513a9dbf1457fede0eeb8c4404
Reviewed-on: https://dart-review.googlesource.com/34142
Reviewed-by: Vyacheslav Egorov <[email protected]>
Commit-Queue: Vyacheslav Egorov <[email protected]>
whesse pushed a commit that referenced this issue Jan 12, 2018
…if a StaticCallInstr is used"

This also changes the implementation of Function::LookupImplicitGetterSetterField to **not** use
token positions for finding the right field, but rather the name.

Issue #31798

Change-Id: I418c89a1426c33b2bfa8adc00534511657af51f1
Reviewed-on: https://dart-review.googlesource.com/34141
Commit-Queue: Martin Kustermann <[email protected]>
Reviewed-by: Vyacheslav Egorov <[email protected]>
whesse pushed a commit that referenced this issue Jan 17, 2018
…ignableInstr]

Use a register calling-convention between "inlined code" and the stub. Also
avoid unnecessary push/pop/load-from-stack instructions.

Issue #31798

Change-Id: I0c07f0bb7ee524b9f5bd6d989b0203f3a53a3625
Reviewed-on: https://dart-review.googlesource.com/34641
Commit-Queue: Martin Kustermann <[email protected]>
Reviewed-by: Erik Corry <[email protected]>
@mkustermann
Copy link
Member

@kmillikin Just fyi: We are working now on eliminating the cost of the checks in the VM and have already some very promising prototypes.

whesse pushed a commit that referenced this issue Jan 25, 2018
…n-generic, instantiated types

When the type to test against is instantiated and has no type arguments
there is a high probability that we receive instances of that class or
subclasses at runtime.

This CL therefore extends the fast-path of AssertAssignable/InstanceOf
by checking whether the instance class id is within the cid ranges that
directly/indirectly implement/extend the type to test against.

Currently we have an almost depth-first preorder numbering of class ids
in AOT, but there are exceptions.  So each class can have a number of
cid-ranges as subclasses / classes which implement it's interface.

This seems to improve performance of dart-aot-v2
  * flutter stock build by 15+%
  * DeltaBlueClosures by 10+%

and reduces code size on
  * flutter gallery by -3%

Issue #31798

Change-Id: I07dd91589cc3fcd8c5952bdba339e2e2a459e08e
Reviewed-on: https://dart-review.googlesource.com/35620
Commit-Queue: Martin Kustermann <[email protected]>
Reviewed-by: Régis Crelier <[email protected]>
Reviewed-by: Vyacheslav Egorov <[email protected]>
@a-siva a-siva modified the milestones: 2.0-alpha1, 2.0-alpha2 Jan 31, 2018
dart-bot pushed a commit that referenced this issue Feb 14, 2018
…cit setters

This applies optimization from d117760 to implicit setters.

(This CL also reformats kernel_binary_flowgraph.cc because some previous CL
was pushed without proper formatting)

Bug: #31798
Change-Id: I6f1590bfd40e36b972b857c49d6ce8435bb25187
Reviewed-on: https://dart-review.googlesource.com/41300
Commit-Queue: Vyacheslav Egorov <[email protected]>
Reviewed-by: Martin Kustermann <[email protected]>
@JekCharlsonYu JekCharlsonYu modified the milestones: 2.0-alpha2, I/O Beta 2 Feb 23, 2018
@dgrove dgrove modified the milestones: I/O Beta 2, I/O Beta 3 Feb 27, 2018
dart-bot pushed a commit that referenced this issue Mar 1, 2018
With the [_table] field being `dynamic` we were unable to eliminate
checks for the [_isModifiedSince] method, since there is a dynamic call
site.

This CL adds a type to [_table] so [_isModifiedSince] does not need to
perform any checks (via package:vm/transf.../no_dynamic_invocations_annotator.dart)

Issue #31798

Change-Id: If773c4b63fab62d1ccdd2e783aa16c19f780000a
Reviewed-on: https://dart-review.googlesource.com/43423
Commit-Queue: Martin Kustermann <[email protected]>
Reviewed-by: Vyacheslav Egorov <[email protected]>
dart-bot pushed a commit that referenced this issue Mar 1, 2018
Clear out generic-covariant-{impl,interface} on parameters which are
statically checked on call site.  If all call sites are this-dispatches then
we are guaranteed to not need the checks.

Issue #31798

Change-Id: I5452a1c9eb3c3e36c1dfc978327bfdcb256cc003
Reviewed-on: https://dart-review.googlesource.com/43421
Commit-Queue: Martin Kustermann <[email protected]>
Reviewed-by: Vyacheslav Egorov <[email protected]>
dart-bot pushed a commit that referenced this issue Mar 1, 2018
The VM uses normally strong mode types for LoadField instructions.
Though for certain fields we have known class-ids which we attach to the
Field instructions.

Before we had a case where the strong mode type was dynamic, but we had
a very specific cid for the Field.  Though when using the [CompileType]
afterwards via [CompileType()->ToAbstractType()] it was returning
`dynamic`.

We should use CompileType::ComputeRefineType() to get the most specific
one of those two.

Issue #31798

Change-Id: Ib0b7a596449cba0bc53e118ee603b2039aa312b3
Reviewed-on: https://dart-review.googlesource.com/43422
Commit-Queue: Martin Kustermann <[email protected]>
Reviewed-by: Vyacheslav Egorov <[email protected]>
dart-bot pushed a commit that referenced this issue Mar 2, 2018
…ad,OffsetFromThread}

Not only does it speed the two methods a bit up, it will also allow us to use the assembler
before all of the stubs are initialized.

Issue #31798

Change-Id: Ic14743ecd9d11ca4cbc5208cacad30beee0982ef
Reviewed-on: https://dart-review.googlesource.com/44500
Commit-Queue: Martin Kustermann <[email protected]>
Reviewed-by: Vyacheslav Egorov <[email protected]>
dart-bot pushed a commit that referenced this issue Mar 2, 2018
Issue #31798

Change-Id: Ifb08ec17b29a5e8d1f2a9b8b53a32cb810a8a3da
Reviewed-on: https://dart-review.googlesource.com/44520
Reviewed-by: Vyacheslav Egorov <[email protected]>
Commit-Queue: Martin Kustermann <[email protected]>
dart-bot pushed a commit that referenced this issue Mar 2, 2018
…fore usable from stubs

Issue #31798

Change-Id: I375cc01bf59d848a203dbcdbd59377d55e9aafe4
Reviewed-on: https://dart-review.googlesource.com/44540
Commit-Queue: Martin Kustermann <[email protected]>
Reviewed-by: Vyacheslav Egorov <[email protected]>
dart-bot pushed a commit that referenced this issue Mar 2, 2018
…s of subtypes but also subclasses

Issue #31798

Change-Id: I1df4b9238500a1f78b5237ee6a6be4154e7f41d2
Reviewed-on: https://dart-review.googlesource.com/44543
Commit-Queue: Martin Kustermann <[email protected]>
Reviewed-by: Vyacheslav Egorov <[email protected]>
@dgrove dgrove added P1 A high priority bug; for example, a single project is unusable or has many test failures and removed P0 A serious issue requiring immediate resolution labels Mar 15, 2018
@dgrove dgrove removed this from the I/O Beta 3 milestone Mar 15, 2018
dart-bot pushed a commit that referenced this issue Apr 6, 2018
This CL:

  * Adds a field to [RawAbstractType] which will always hold a pointer
    to the entrypoint of a type testing stub

  * Makes this new field be initialized to a default stub whenever a
    instances are created (e.g. via Type::New(), snapshot reader, ...)

  * Makes the clustered snapshotter write a reference to the
    corresponding [RawInstructions] object when writing the field and do
    the reverse when reading it.

  * Makes us call the type testing stub for performing assert-assignable
    checks.

To reduce unnecessary loads on callsites, we store the entrypoint of the
type testing stubs directly in the type objects.  This means that the
caller of type testing stubs can simply branch there without populating
a code object first.  This also means that the type testing stubs
themselves have no access to a pool and we therefore also don't hold on
to the [Code] object, only the [Instruction] object is necessary.

The type testing stubs do not setup a frame themselves and also have no
safepoint.  In the case when the type testing stubs could not determine
a positive answer they will tail-call a general-purpose stub.

The general-purpose stub sets up a stub frame, tries to consult a
[SubtypeTestCache] and bails out to runtime if this was unsuccessful.

This CL is just the the first, for ease of reviewing.  The actual
type-specialized type testing stubs will be generated in later CLs.

Issue #31798

Change-Id: I174a11b3b812799f399a60af799144c2ba3c26ec
Reviewed-on: https://dart-review.googlesource.com/44787
Reviewed-by: Vyacheslav Egorov <[email protected]>
Reviewed-by: Régis Crelier <[email protected]>
dart-bot pushed a commit that referenced this issue Apr 6, 2018
This CL starts building type testing stubs specialzed for [Type] objects
we test against.

More specifically, it adds support for:

  * Handling obvious fast cases on the call sites (while still having a
    call to stub for negative case)

  * Handling type tests against type parameters, by loading the value
    of the type parameter on the call sites and invoking it's type testing stub.

  * Specialzed type testing stubs for instantiated types where we can
    do [CidRange]-based subtype-checks.

    ==> e.g. String/List<dynamic>

  * Specialzed type testing stubs for instantiated types where we can
    do [CidRange]-based subclass-checks for the class and
    [CidRange]-based subtype-checks for the type arguments.

    ==> e.g. Widget<State>, where we know [Widget] is only extended and not
	     implemented.

  * Specialzed type testing stubs for certain non-instantiated types where we
    can do [CidRange]-based subclass-checks for the class and
    [CidRange]-based subtype-checks for the instantiated type arguments and
    cid based comparisons for type parameters.  (Note that this fast-case migth
    result in some false-negatives!)

    ==> e.g. _HashMapEntry<K, V>, where we know [_HashMapEntry] is only
	     extended and not implemented.

   This optimizes cases where the caller uses `new HashMap<A, B>()` and only
   uses `A` and `B` as key/values (and not subclasses of it).  The false-negative
   can occur when subtypes of A or B are used.  In such cases we fall back to the
   [SubtypeTestCache]-based imlementation.

Issue #31798

Change-Id: Ic1853977bf55d815755b0d652ec8e20e51efb4cf
Reviewed-on: https://dart-review.googlesource.com/44788
Reviewed-by: Vyacheslav Egorov <[email protected]>
Reviewed-by: Régis Crelier <[email protected]>
dart-bot pushed a commit that referenced this issue Apr 6, 2018
The changes include:

  * Make AssertAssignableInstr no longer have a call-summary, which
    helps methods with several parameter checks by not having to
    re-load/re-initialize type arguments registers

  * Lazily create SubtypeTestCaches: We already go to runtime to warm up
    the caches, so we now also create the caches on the first runtime
    call and patch the pool entries.

  * No longer load the destination name into a register: We only need
    the name when we throw an exception, so it is not on the hot path.
    Instead we let the runtime look at the call site, decoding a pool
    index from the instructions stream.  The destination name will be
    available in the pool, at a consecutive index to the subtype cache.

  * Remove the fall-through to N=1 case for probing subtypeing tests,
    since those will always be handled by the optimized stubs.

  * Do not generate optimized stubs for FutureOr<T> (so far it just
    falled-through to TTS).  We can make optimzed version of that later,
    but it requires special subtyping rules.

  * Local code quality improvement in the type-testing-stubs: Avoid
    extra jump at last case of cid-class-range checks.

There are still a number of optimization opportunities we can do in
future changes.

Issue #31798

Change-Id: I4dc5a8a49f939178fe74d44736ef69e4b9088e46
Reviewed-on: https://dart-review.googlesource.com/46984
Reviewed-by: Vyacheslav Egorov <[email protected]>
Reviewed-by: Régis Crelier <[email protected]>
dart-bot pushed a commit that referenced this issue Jun 1, 2018
For now we are limiting this to type checks against type parameter types.

# Performance improvements

In Dart 1 mode Dart2JS compiles itself in 28s when running from source
and in 23s when running from ideal app-jit snapshot (trained on the
same workload).

Before this change in Dart 2 mode numbers were 51s and 57s respectively.

After this change in Dart 2 mode numbers are 38s and 32s. Meaning
that regression is reduced by 50%.

Issue #31798
Issue #33257

Change-Id: I34bf5385a5cc3c7702dc281c6dfa89da85d3dde1
Reviewed-on: https://dart-review.googlesource.com/57601
Reviewed-by: Régis Crelier <[email protected]>
Commit-Queue: Vyacheslav Egorov <[email protected]>
dart-bot pushed a commit that referenced this issue Jun 1, 2018
… types."

This reverts commit 4be50d6.

Reason for revert: Failures on SIMDBC64 and Analyzer bots.

Original change's description:
> [vm] Enable type stubs based type checks in JIT mode for some types.
> 
> For now we are limiting this to type checks against type parameter types.
> 
> # Performance improvements
> 
> In Dart 1 mode Dart2JS compiles itself in 28s when running from source
> and in 23s when running from ideal app-jit snapshot (trained on the
> same workload).
> 
> Before this change in Dart 2 mode numbers were 51s and 57s respectively.
> 
> After this change in Dart 2 mode numbers are 38s and 32s. Meaning
> that regression is reduced by 50%.
> 
> Issue #31798
> Issue #33257
> 
> Change-Id: I34bf5385a5cc3c7702dc281c6dfa89da85d3dde1
> Reviewed-on: https://dart-review.googlesource.com/57601
> Reviewed-by: Régis Crelier <[email protected]>
> Commit-Queue: Vyacheslav Egorov <[email protected]>

[email protected],[email protected],[email protected]

Change-Id: I85a30c962b0cd556310e19193f5993ab76ecf2e7
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://dart-review.googlesource.com/57840
Reviewed-by: Vyacheslav Egorov <[email protected]>
Commit-Queue: Vyacheslav Egorov <[email protected]>
dart-bot pushed a commit that referenced this issue Jun 1, 2018
Relanding 4be50d6 with fixes to DBC
and location summaries: AssertAssignable must save FPU registers.

For now we are limiting this to type checks against type parameter types.


In Dart 1 mode Dart2JS compiles itself in 28s when running from source
and in 23s when running from ideal app-jit snapshot (trained on the
same workload).

Before this change in Dart 2 mode numbers were 51s and 57s respectively.

After this change in Dart 2 mode numbers are 38s and 32s. Meaning
that regression is reduced by 50%.

Issue #31798
Issue #33257

Change-Id: Ifb55f86453bfdf36a2e03bcd7f3197cfde257103
Reviewed-on: https://dart-review.googlesource.com/57980
Commit-Queue: Vyacheslav Egorov <[email protected]>
Reviewed-by: Régis Crelier <[email protected]>
dart-bot pushed a commit that referenced this issue Aug 16, 2018
…iver type.

Currently we only annotate those call-sites that would result
in generic covariant checks performed on the callee side.

Bug: #31798
Change-Id: Ifcf60032036575f615015d716276484a7c1236b3
Reviewed-on: https://dart-review.googlesource.com/69580
Commit-Queue: Vyacheslav Egorov <[email protected]>
Reviewed-by: Kevin Millikin <[email protected]>
Reviewed-by: Martin Kustermann <[email protected]>
dart-bot pushed a commit that referenced this issue Aug 17, 2018
We say that field's static type G<T0, ..., Tn> is exact if for
any value that can be loaded from this field, its runtime type
T is such that T at G has type arguments exactly equal to
<T0, ..., Tn>.

Know if field's static type is exact allows us to apply optimizations
that require knowing type arguments e.g.

- we can fold o.f.:type_arguments
to a constant value if we know that o.f is trivially exact;
- for method invocations o.f.m(...) we can skip argument type checks
on the callee if we know that o.f is invariant (this optimization will
be enabled once multiple entry points CLs will land).

Bug: #31798

Change-Id: Id565046d45a842625d41feb002b65db48451034c
Reviewed-on: https://dart-review.googlesource.com/69969
Commit-Queue: Vyacheslav Egorov <[email protected]>
Reviewed-by: Alexander Markov <[email protected]>
dart-bot pushed a commit that referenced this issue Aug 20, 2018
This function can be used to avoid checked down casts in the parts
of the code where we know by construction that the type of the
value matches expectations.

Use it to avoid down-cast in the _LinkedHashMap.operator[].

Bug: #31798
Change-Id: I80949bfb84dde7716fc23aeba311931970ce16aa
Reviewed-on: https://dart-review.googlesource.com/70511
Commit-Queue: Vyacheslav Egorov <[email protected]>
Reviewed-by: Alexander Markov <[email protected]>
dart-bot pushed a commit that referenced this issue Aug 20, 2018
We will still type check returns from native methods but everything
that is explicitly handled by graph builder would be trusted.

Bug: #31798
Change-Id: Ia2f5dfba63349bef7578ca4774686f47b5e484b1
Reviewed-on: https://dart-review.googlesource.com/70644
Reviewed-by: Martin Kustermann <[email protected]>
Commit-Queue: Vyacheslav Egorov <[email protected]>
dart-bot pushed a commit that referenced this issue Aug 20, 2018
…gainst "this".

Test Plan:

Behavioral correctness should be ensured by existing tests. Tests in vm/dart/entrypoints
ensure that the unchecked entrypoint is used in cases where the optimization should trigger.

Bug: #31798

Change-Id: I5b880b2dfa6343b4bb0a96ad23562facff73e41f
Cq-Include-Trybots: luci.dart.try:vm-kernel-win-release-x64-try,vm-kernel-optcounter-threshold-linux-release-x64-try,vm-kernel-precomp-linux-debug-x64-try,vm-kernel-precomp-linux-release-simarm-try,vm-kernel-precomp-linux-release-simarm64-try,vm-kernel-precomp-linux-release-x64-try,vm-kernel-precomp-win-release-x64-try
Reviewed-on: https://dart-review.googlesource.com/69741
Commit-Queue: Samir Jindel <[email protected]>
Reviewed-by: Vyacheslav Egorov <[email protected]>
dart-bot pushed a commit that referenced this issue Aug 21, 2018
…lly-typed closure calls.

Test Plan:

Behavioral correctness should be ensured by existing tests. Tests in vm/dart/entrypoints
ensure that the unchecked entrypoint is used in cases where the optimization should trigger.

Bug: #31798

Change-Id: Id25ecba86e20c22f0678c12986ad620db312ddaa
Cq-Include-Trybots: luci.dart.try:vm-kernel-win-release-x64-try,vm-kernel-optcounter-threshold-linux-release-x64-try,vm-kernel-precomp-linux-debug-x64-try,vm-kernel-precomp-linux-release-simarm-try,vm-kernel-precomp-linux-release-simarm64-try,vm-kernel-precomp-linux-release-x64-try,vm-kernel-precomp-win-release-x64-try
Reviewed-on: https://dart-review.googlesource.com/69743
Commit-Queue: Samir Jindel <[email protected]>
Reviewed-by: Vyacheslav Egorov <[email protected]>
dart-bot pushed a commit that referenced this issue Aug 21, 2018
… statically-typed closure calls."

This reverts commit 19126e8.

Reason for revert: Breaks several bots.

Original change's description:
> [vm] Use multiple entrypoints to remove unnecessary checks on statically-typed closure calls.
> 
> Test Plan:
> 
> Behavioral correctness should be ensured by existing tests. Tests in vm/dart/entrypoints
> ensure that the unchecked entrypoint is used in cases where the optimization should trigger.
> 
> Bug: #31798
> 
> Change-Id: Id25ecba86e20c22f0678c12986ad620db312ddaa
> Cq-Include-Trybots: luci.dart.try:vm-kernel-win-release-x64-try,vm-kernel-optcounter-threshold-linux-release-x64-try,vm-kernel-precomp-linux-debug-x64-try,vm-kernel-precomp-linux-release-simarm-try,vm-kernel-precomp-linux-release-simarm64-try,vm-kernel-precomp-linux-release-x64-try,vm-kernel-precomp-win-release-x64-try
> Reviewed-on: https://dart-review.googlesource.com/69743
> Commit-Queue: Samir Jindel <[email protected]>
> Reviewed-by: Vyacheslav Egorov <[email protected]>

[email protected],[email protected]

Change-Id: Ia33e9d141827d3c990c65d839333443c224dc85d
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: #31798
Cq-Include-Trybots: luci.dart.try:vm-kernel-win-release-x64-try, vm-kernel-optcounter-threshold-linux-release-x64-try, vm-kernel-precomp-linux-debug-x64-try, vm-kernel-precomp-linux-release-simarm-try, vm-kernel-precomp-linux-release-simarm64-try, vm-kernel-precomp-linux-release-x64-try, vm-kernel-precomp-win-release-x64-try
Reviewed-on: https://dart-review.googlesource.com/70920
Reviewed-by: Samir Jindel <[email protected]>
Commit-Queue: Samir Jindel <[email protected]>
dart-bot pushed a commit that referenced this issue Aug 29, 2018
…non-generic.

This guarantees that type checks are skipped in the code like this:

class X<T> {
  void method(X<T> other) {
  }
}

class Y extends X<String> {
}

void foo(Y y, Y z) {
  y.method(z);  // No need to check on the callee side. No variance.
}

Additionally ammend IL printing to print user visible type names instead
of internal type names for brevity.

Bug: #31798
Change-Id: I4fe16d5dc7de01bb0a8ba834569d90ee5ce7ac74
Reviewed-on: https://dart-review.googlesource.com/72001
Reviewed-by: Samir Jindel <[email protected]>
Commit-Queue: Vyacheslav Egorov <[email protected]>
dart-bot pushed a commit that referenced this issue Aug 30, 2018
…d static call

JitCallSpecializer::VisitInstanceCall has two places where it converts
instance call into a static call. Previously we only made use of
exactness information in one of those places.

On dart2js compiling dart2js this change reduces number of times
we enter Map.putIfAbsent through checked entry point from
1107368 to 8640.

Bug: #31798
Change-Id: Id1665b998ecdf4c4d46cb7605a11bdde2e8c9782
Reviewed-on: https://dart-review.googlesource.com/72220
Reviewed-by: Alexander Markov <[email protected]>
Commit-Queue: Vyacheslav Egorov <[email protected]>
@mkustermann
Copy link
Member

To give some update on this CL:

Multiple entrypoints were finally finished: ia32 in 8a39cb8, arm64 in f134164, unoptimized code in 4d95ec1

Exactness tracking is only implemented on x64. For completeness we might want to also port it to other architectures as well.

Right now there's no other work planned for this issue so I'm going to remove the P1 priority here.

@mkustermann mkustermann removed the P1 A high priority bug; for example, a single project is unusable or has many test failures label Mar 17, 2020
@mkustermann mkustermann removed their assignment Sep 25, 2020
tekknolagi pushed a commit to tekknolagi/dart-assembler that referenced this issue Nov 3, 2020
…n-generic, instantiated types

When the type to test against is instantiated and has no type arguments
there is a high probability that we receive instances of that class or
subclasses at runtime.

This CL therefore extends the fast-path of AssertAssignable/InstanceOf
by checking whether the instance class id is within the cid ranges that
directly/indirectly implement/extend the type to test against.

Currently we have an almost depth-first preorder numbering of class ids
in AOT, but there are exceptions.  So each class can have a number of
cid-ranges as subclasses / classes which implement it's interface.

This seems to improve performance of dart-aot-v2
  * flutter stock build by 15+%
  * DeltaBlueClosures by 10+%

and reduces code size on
  * flutter gallery by -3%

Issue dart-lang#31798

Change-Id: I07dd91589cc3fcd8c5952bdba339e2e2a459e08e
Reviewed-on: https://dart-review.googlesource.com/35620
Commit-Queue: Martin Kustermann <[email protected]>
Reviewed-by: Régis Crelier <[email protected]>
Reviewed-by: Vyacheslav Egorov <[email protected]>
tekknolagi pushed a commit to tekknolagi/dart-assembler that referenced this issue Nov 3, 2020
This CL:

  * Adds a field to [RawAbstractType] which will always hold a pointer
    to the entrypoint of a type testing stub

  * Makes this new field be initialized to a default stub whenever a
    instances are created (e.g. via Type::New(), snapshot reader, ...)

  * Makes the clustered snapshotter write a reference to the
    corresponding [RawInstructions] object when writing the field and do
    the reverse when reading it.

  * Makes us call the type testing stub for performing assert-assignable
    checks.

To reduce unnecessary loads on callsites, we store the entrypoint of the
type testing stubs directly in the type objects.  This means that the
caller of type testing stubs can simply branch there without populating
a code object first.  This also means that the type testing stubs
themselves have no access to a pool and we therefore also don't hold on
to the [Code] object, only the [Instruction] object is necessary.

The type testing stubs do not setup a frame themselves and also have no
safepoint.  In the case when the type testing stubs could not determine
a positive answer they will tail-call a general-purpose stub.

The general-purpose stub sets up a stub frame, tries to consult a
[SubtypeTestCache] and bails out to runtime if this was unsuccessful.

This CL is just the the first, for ease of reviewing.  The actual
type-specialized type testing stubs will be generated in later CLs.

Issue dart-lang#31798

Change-Id: I174a11b3b812799f399a60af799144c2ba3c26ec
Reviewed-on: https://dart-review.googlesource.com/44787
Reviewed-by: Vyacheslav Egorov <[email protected]>
Reviewed-by: Régis Crelier <[email protected]>
tekknolagi pushed a commit to tekknolagi/dart-assembler that referenced this issue Nov 3, 2020
This CL starts building type testing stubs specialzed for [Type] objects
we test against.

More specifically, it adds support for:

  * Handling obvious fast cases on the call sites (while still having a
    call to stub for negative case)

  * Handling type tests against type parameters, by loading the value
    of the type parameter on the call sites and invoking it's type testing stub.

  * Specialzed type testing stubs for instantiated types where we can
    do [CidRange]-based subtype-checks.

    ==> e.g. String/List<dynamic>

  * Specialzed type testing stubs for instantiated types where we can
    do [CidRange]-based subclass-checks for the class and
    [CidRange]-based subtype-checks for the type arguments.

    ==> e.g. Widget<State>, where we know [Widget] is only extended and not
	     implemented.

  * Specialzed type testing stubs for certain non-instantiated types where we
    can do [CidRange]-based subclass-checks for the class and
    [CidRange]-based subtype-checks for the instantiated type arguments and
    cid based comparisons for type parameters.  (Note that this fast-case migth
    result in some false-negatives!)

    ==> e.g. _HashMapEntry<K, V>, where we know [_HashMapEntry] is only
	     extended and not implemented.

   This optimizes cases where the caller uses `new HashMap<A, B>()` and only
   uses `A` and `B` as key/values (and not subclasses of it).  The false-negative
   can occur when subtypes of A or B are used.  In such cases we fall back to the
   [SubtypeTestCache]-based imlementation.

Issue dart-lang#31798

Change-Id: Ic1853977bf55d815755b0d652ec8e20e51efb4cf
Reviewed-on: https://dart-review.googlesource.com/44788
Reviewed-by: Vyacheslav Egorov <[email protected]>
Reviewed-by: Régis Crelier <[email protected]>
tekknolagi pushed a commit to tekknolagi/dart-assembler that referenced this issue Nov 3, 2020
…gainst "this".

Test Plan:

Behavioral correctness should be ensured by existing tests. Tests in vm/dart/entrypoints
ensure that the unchecked entrypoint is used in cases where the optimization should trigger.

Bug: dart-lang#31798

Change-Id: I5b880b2dfa6343b4bb0a96ad23562facff73e41f
Cq-Include-Trybots: luci.dart.try:vm-kernel-win-release-x64-try,vm-kernel-optcounter-threshold-linux-release-x64-try,vm-kernel-precomp-linux-debug-x64-try,vm-kernel-precomp-linux-release-simarm-try,vm-kernel-precomp-linux-release-simarm64-try,vm-kernel-precomp-linux-release-x64-try,vm-kernel-precomp-win-release-x64-try
Reviewed-on: https://dart-review.googlesource.com/69741
Commit-Queue: Samir Jindel <[email protected]>
Reviewed-by: Vyacheslav Egorov <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends.
Projects
None yet
Development

No branches or pull requests

5 participants