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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 2494093002: Refactor enqueuers (Closed)

Created:
4 years, 1 month ago by Johnni Winther
Modified:
4 years, 1 month ago
Reviewers:
Harry Terkelsen
CC:
reviews_dartlang.org
Target Ref:
refs/heads/master
Visibility:
Public.

Description

Refactor enqueuers - limit use of globalDepencies - remove mirrorDependencies - compute native class through WorldImpact - decouple reflectable elements analysis from enqueuers - prepare for creating multiple enqueuers for closed world tests R=het@google.com Committed: https://github.com/dart-lang/sdk/commit/99cd985a40cdfeb06fd106ebc40f20074f7639b4

Patch Set 1 #

Patch Set 2 : Refactor enqueuers #

Total comments: 12

Patch Set 3 : Updated cf. comments. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+628 lines, -741 lines) Patch
M pkg/compiler/lib/src/common/backend_api.dart View 1 7 chunks +12 lines, -17 lines 0 comments Download
M pkg/compiler/lib/src/compiler.dart View 1 2 7 chunks +41 lines, -51 lines 0 comments Download
M pkg/compiler/lib/src/deferred_load.dart View 2 chunks +3 lines, -1 line 0 comments Download
M pkg/compiler/lib/src/enqueue.dart View 1 2 14 chunks +39 lines, -213 lines 0 comments Download
M pkg/compiler/lib/src/js_backend/backend.dart View 1 27 chunks +125 lines, -144 lines 0 comments Download
M pkg/compiler/lib/src/js_backend/enqueuer.dart View 1 2 10 chunks +25 lines, -190 lines 0 comments Download
A pkg/compiler/lib/src/js_backend/mirrors_analysis.dart View 1 chunk +243 lines, -0 lines 0 comments Download
M pkg/compiler/lib/src/js_backend/type_variable_handler.dart View 2 chunks +4 lines, -6 lines 0 comments Download
M pkg/compiler/lib/src/native/enqueue.dart View 1 2 9 chunks +98 lines, -95 lines 0 comments Download
M pkg/compiler/lib/src/ssa/builder.dart View 1 1 chunk +1 line, -1 line 0 comments Download
M pkg/compiler/lib/src/ssa/codegen.dart View 1 chunk +2 lines, -1 line 0 comments Download
M pkg/compiler/lib/src/universe/use.dart View 2 chunks +12 lines, -0 lines 0 comments Download
M pkg/compiler/tool/perf.dart View 1 chunk +1 line, -1 line 0 comments Download
M pkg/dart2js_incremental/lib/caching_compiler.dart View 1 3 chunks +6 lines, -5 lines 0 comments Download
M tests/compiler/dart2js/compiler_helper.dart View 1 1 chunk +1 line, -2 lines 0 comments Download
M tests/compiler/dart2js/deferred_load_graph_segmentation_test.dart View 1 chunk +0 lines, -1 line 0 comments Download
M tests/compiler/dart2js/mirror_tree_shaking_test.dart View 1 chunk +8 lines, -5 lines 0 comments Download
M tests/compiler/dart2js/type_combination_test.dart View 1 1 chunk +7 lines, -8 lines 0 comments Download

Messages

Total messages: 7 (3 generated)
Johnni Winther
4 years, 1 month ago (2016-11-11 13:35:11 UTC) #3
Harry Terkelsen
lgtm https://codereview.chromium.org/2494093002/diff/20001/pkg/compiler/lib/src/compiler.dart File pkg/compiler/lib/src/compiler.dart (right): https://codereview.chromium.org/2494093002/diff/20001/pkg/compiler/lib/src/compiler.dart#newcode2141 pkg/compiler/lib/src/compiler.dart:2141: .transformResolutionImpact(compiler.enqueuer.resolution, resolutionImpact); dartfmt https://codereview.chromium.org/2494093002/diff/20001/pkg/compiler/lib/src/enqueue.dart File pkg/compiler/lib/src/enqueue.dart (right): https://codereview.chromium.org/2494093002/diff/20001/pkg/compiler/lib/src/enqueue.dart#newcode193 ...
4 years, 1 month ago (2016-11-11 21:24:28 UTC) #4
Johnni Winther
Committed patchset #3 (id:40001) manually as 99cd985a40cdfeb06fd106ebc40f20074f7639b4 (presubmit successful).
4 years, 1 month ago (2016-11-14 09:08:18 UTC) #6
Johnni Winther
4 years, 1 month ago (2016-11-14 09:08:36 UTC) #7
Message was sent while issue was closed. 
https://codereview.chromium.org/2494093002/diff/20001/pkg/compiler/lib/src/co...
File pkg/compiler/lib/src/compiler.dart (right):

https://codereview.chromium.org/2494093002/diff/20001/pkg/compiler/lib/src/co...
pkg/compiler/lib/src/compiler.dart:2141:
.transformResolutionImpact(compiler.enqueuer.resolution, resolutionImpact);
On 2016/11/11 21:24:27, Harry Terkelsen wrote:
> dartfmt

Done.

https://codereview.chromium.org/2494093002/diff/20001/pkg/compiler/lib/src/en...
File pkg/compiler/lib/src/enqueue.dart (right):

https://codereview.chromium.org/2494093002/diff/20001/pkg/compiler/lib/src/en...
pkg/compiler/lib/src/enqueue.dart:193: byMirrors: mirrorUsage, onImplemented:
(ClassElement cls) {
On 2016/11/11 21:24:27, Harry Terkelsen wrote:
> dartfmt?

Done.

https://codereview.chromium.org/2494093002/diff/20001/pkg/compiler/lib/src/js...
File pkg/compiler/lib/src/js_backend/mirrors_analysis.dart (right):

https://codereview.chromium.org/2494093002/diff/20001/pkg/compiler/lib/src/js...
pkg/compiler/lib/src/js_backend/mirrors_analysis.dart:46: class MirrorsHandler {
On 2016/11/11 21:24:27, Harry Terkelsen wrote:
> fyi didn't look at this thoroughly... just assuming it is copied over from the
> enqueuers

It is!

https://codereview.chromium.org/2494093002/diff/20001/pkg/compiler/lib/src/na...
File pkg/compiler/lib/src/native/enqueue.dart (right):

https://codereview.chromium.org/2494093002/diff/20001/pkg/compiler/lib/src/na...
pkg/compiler/lib/src/native/enqueue.dart:346: bool maybeFirstTime =
registeredClasses.isEmpty;
On 2016/11/11 21:24:27, Harry Terkelsen wrote:
> is this used anywhere?

No.

https://codereview.chromium.org/2494093002/diff/20001/pkg/compiler/lib/src/na...
pkg/compiler/lib/src/native/enqueue.dart:468:
instantiated(coreTypes.doubleType);
On 2016/11/11 21:24:27, Harry Terkelsen wrote:
> doubleType -> intType

Good catch!

https://codereview.chromium.org/2494093002/diff/20001/pkg/compiler/lib/src/na...
pkg/compiler/lib/src/native/enqueue.dart:647: // Add the information that this
class is a subtype of its supertypes.  The
On 2016/11/11 21:24:27, Harry Terkelsen wrote:
> long line

Done.

Powered by Google App Engine
This is Rietveld 408576698