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

Issue 12262035: Simplify the selector map and get rid of the partial type tree. (Closed)

Created:
7 years, 10 months ago by kasperl
Modified:
7 years, 10 months ago
Reviewers:
ngeoffray
CC:
reviews_dartlang.org
Visibility:
Public.

Description

Simplify the selector map and get rid of the partial type tree. This new approach is a lot faster than the old approach, but we do not use selector maps enough for this to really matter for the overall compilation speed (yet). There are definitely cases where having the cache has a small positive impact and I expect that the significance of it will increase as we start using selector maps in more places. R=ngeoffray@google.com BUG= Committed: https://code.google.com/p/dart/source/detail?r=18512

Patch Set 1 #

Patch Set 2 : Avoid creating iterable on cache hit. #

Patch Set 3 : iterateMatching -> computeMatching. #

Total comments: 5
Unified diffs Side-by-side diffs Delta from patch set Stats (+98 lines, -235 lines) Patch
M sdk/lib/_internal/compiler/implementation/universe/function_set.dart View 2 chunks +3 lines, -3 lines 1 comment Download
D sdk/lib/_internal/compiler/implementation/universe/partial_type_tree.dart View 1 chunk +0 lines, -141 lines 0 comments Download
M sdk/lib/_internal/compiler/implementation/universe/selector_map.dart View 1 2 2 chunks +95 lines, -90 lines 4 comments Download
M sdk/lib/_internal/compiler/implementation/universe/universe.dart View 1 chunk +0 lines, -1 line 0 comments Download

Messages

Total messages: 2 (0 generated)
kasperl
7 years, 10 months ago (2013-02-14 07:40:03 UTC) #1
ngeoffray
7 years, 10 months ago (2013-02-14 10:14:33 UTC) #2
LGTM

https://codereview.chromium.org/12262035/diff/5001/sdk/lib/_internal/compiler...
File sdk/lib/_internal/compiler/implementation/universe/function_set.dart
(right):

https://codereview.chromium.org/12262035/diff/5001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/universe/function_set.dart:92: if
(!cache.isEmpty) cache.clear();
Why adding this?

https://codereview.chromium.org/12262035/diff/5001/sdk/lib/_internal/compiler...
File sdk/lib/_internal/compiler/implementation/universe/selector_map.dart
(right):

https://codereview.chromium.org/12262035/diff/5001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/universe/selector_map.dart:18: : null;
fits in one line?

https://codereview.chromium.org/12262035/diff/5001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/universe/selector_map.dart:89: } else
if (!cache.isEmpty) {
Remove this check?

https://codereview.chromium.org/12262035/diff/5001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/universe/selector_map.dart:102: if
(cache != null && !cache.isEmpty) cache.clear();
Remove this check?

https://codereview.chromium.org/12262035/diff/5001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/universe/selector_map.dart:117: //
matching iterable to cut down on the overhead for cache hits.
Not sure I understand the last sentence. Looks like you're explaining why we
have a cache.

Powered by Google App Engine
This is Rietveld 408576698