Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(112)

Issue 3003613002: improve DDC's type checks (Closed)

Created:
3 years, 4 months ago by Jennifer Messerly
Modified:
3 years, 3 months ago
Reviewers:
Leaf
CC:
dev-compiler+reviews_dartlang.org, Jacob
Target Ref:
refs/heads/master
Visibility:
Public.

Description

improve DDC's type checks fixes #30495 - optimize subtype cache fixes #30464 - optimize `is List` Additional changes: - optimize double type checks - optimize Object type checks - optimize FutureOr<T> checks - optimize interface checks for some dart:core and dart:async types (List, Map, Iterable, Future, Stream, and StreamSubscription ) - optimize checks for generic class default instantiations and exact instantiation (e.g. `C<dynamic>` on any instance `C<T>`, and `C<T> on an instance of C<T>` - optimize checks for native types - optimize getReifiedType - optimize dcall/dsend/dput checks to use faster Type._check pattern - optimize instanceOf/cast functions by moving special cases to appropriate types - optimize Object.runtimeType - optimize JS interop type checks - optimize all function type checks There's more we can do (especially for class/interface types) but this should cover a lot of cases. R=leafp@google.com Committed: https://github.com/dart-lang/sdk/commit/45592ee4bb83ef7b72009b10a6311a58f0618950

Patch Set 1 #

Patch Set 2 : format #

Total comments: 14
Unified diffs Side-by-side diffs Delta from patch set Stats (+481 lines, -473 lines) Patch
M pkg/dev_compiler/lib/sdk/ddc_sdk.sum View 1 Binary file 0 comments Download
M pkg/dev_compiler/lib/src/compiler/code_generator.dart View 1 9 chunks +214 lines, -186 lines 6 comments Download
M pkg/dev_compiler/tool/input_sdk/patch/core_patch.dart View 1 1 chunk +0 lines, -3 lines 0 comments Download
M pkg/dev_compiler/tool/input_sdk/private/ddc_runtime/classes.dart View 1 2 chunks +65 lines, -1 line 0 comments Download
M pkg/dev_compiler/tool/input_sdk/private/ddc_runtime/errors.dart View 1 1 chunk +0 lines, -28 lines 0 comments Download
M pkg/dev_compiler/tool/input_sdk/private/ddc_runtime/generators.dart View 1 1 chunk +3 lines, -1 line 0 comments Download
M pkg/dev_compiler/tool/input_sdk/private/ddc_runtime/operations.dart View 1 8 chunks +58 lines, -118 lines 0 comments Download
M pkg/dev_compiler/tool/input_sdk/private/ddc_runtime/rtti.dart View 1 2 chunks +25 lines, -60 lines 4 comments Download
M pkg/dev_compiler/tool/input_sdk/private/ddc_runtime/types.dart View 1 18 chunks +106 lines, -49 lines 4 comments Download
M pkg/dev_compiler/tool/input_sdk/private/js_helper.dart View 1 2 chunks +10 lines, -27 lines 0 comments Download

Messages

Total messages: 12 (5 generated)
Jennifer Messerly
3 years, 4 months ago (2017-08-23 01:49:39 UTC) #5
Jennifer Messerly
https://codereview.chromium.org/3003613002/diff/10001/pkg/dev_compiler/tool/input_sdk/private/ddc_runtime/rtti.dart File pkg/dev_compiler/tool/input_sdk/private/ddc_runtime/rtti.dart (right): https://codereview.chromium.org/3003613002/diff/10001/pkg/dev_compiler/tool/input_sdk/private/ddc_runtime/rtti.dart#newcode97 pkg/dev_compiler/tool/input_sdk/private/ddc_runtime/rtti.dart:97: if (result == null) return JS('', '#', jsobject); FYI ...
3 years, 4 months ago (2017-08-23 23:36:01 UTC) #6
Leaf
Nice! LGTM! https://codereview.chromium.org/3003613002/diff/10001/pkg/dev_compiler/lib/src/compiler/code_generator.dart File pkg/dev_compiler/lib/src/compiler/code_generator.dart (right): https://codereview.chromium.org/3003613002/diff/10001/pkg/dev_compiler/lib/src/compiler/code_generator.dart#newcode996 pkg/dev_compiler/lib/src/compiler/code_generator.dart:996: if (classElem == objectClass) { Is there ...
3 years, 3 months ago (2017-08-24 17:19:40 UTC) #7
Jennifer Messerly
thanks Leaf! and great suggestions ... will def keep these in mind for a follow ...
3 years, 3 months ago (2017-08-24 18:26:37 UTC) #8
Jennifer Messerly
Committed patchset #2 (id:10001) manually as 45592ee4bb83ef7b72009b10a6311a58f0618950 (presubmit successful).
3 years, 3 months ago (2017-08-25 19:45:22 UTC) #10
Leaf
https://codereview.chromium.org/3003613002/diff/10001/pkg/dev_compiler/tool/input_sdk/private/ddc_runtime/types.dart File pkg/dev_compiler/tool/input_sdk/private/ddc_runtime/types.dart (right): https://codereview.chromium.org/3003613002/diff/10001/pkg/dev_compiler/tool/input_sdk/private/ddc_runtime/types.dart#newcode136 pkg/dev_compiler/tool/input_sdk/private/ddc_runtime/types.dart:136: check_T(obj) => I think this changes the behavior when ...
3 years, 3 months ago (2017-09-06 23:21:19 UTC) #11
Jennifer Messerly
3 years, 3 months ago (2017-09-06 23:40:58 UTC) #12
Message was sent while issue was closed.
https://codereview.chromium.org/3003613002/diff/10001/pkg/dev_compiler/tool/i...
File pkg/dev_compiler/tool/input_sdk/private/ddc_runtime/types.dart (right):

https://codereview.chromium.org/3003613002/diff/10001/pkg/dev_compiler/tool/i...
pkg/dev_compiler/tool/input_sdk/private/ddc_runtime/types.dart:136: check_T(obj)
=>
On 2017/09/06 23:21:19, Leaf wrote:
> I think this changes the behavior when you have an instance of a Dart class
that
> implements an anonymous JS class and are checking against that anonymous JS
> class: I believe that we would previously have fallen back to the check code,
> which I think would iterate through the implements clause of the Dart class,
> find the anonymous JS type there, and succeed.  Now I think we fail.  
> 
> Was the previous behavior wrong?  Or am I misunderstanding something?  Or do
we
> need to fix this up?

I have no idea whether it was intended behavior in the old code, though I
personally doubt it. 

These types are a bit of a hack--they're completely fake types with no runtime
reification and unsound casting rules. Having a Dart user code implement these
types is really scary, because they're not types we want to encourage use of.
(Also, even in JS, these types are legacy--they're slowly going away as
libraries move to JS classes)

Also, since the entire point of these types is that aren't reified, it's a weird
to allow Dart classes to implement them and reify them at runtime.

Anyways, if there's an important use case, then we can change it. It is an easy
change, just fall back to the as/_check methods. Before changing DDC though, it
would be good to check what subtyping rules dart2js uses for anonymous JS types,
to see what it supports. And I would be interested in seeing the use case.

Powered by Google App Engine
This is Rietveld 408576698