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

Issue 1087973002: Rewrite for-in loop as indexing loop for JavaScript indexable arrays (Closed)

Created:
5 years, 8 months ago by sra1
Modified:
5 years, 8 months ago
Reviewers:
floitsch
CC:
reviews_dartlang.org, Lasse Reichstein Nielsen
Target Ref:
refs/remotes/git-svn
Visibility:
Public.

Description

Rewrite for-in loop on JavaScript indexable arrays as indexing loop for (E variable in a) <body> is generated as: _end = a.length; for (_i = 0; _i < a.length; a.length == _end || (0, H.throwConcurrentModificatiionError)(a), ++_i) { variable = a[_i]; <body>; } or, when the list is known to be fixed length: for (_i = 0; _i < a.length; ++_i) { variable = a[_i]; <body>; } R=floitsch@google.com Committed: https://code.google.com/p/dart/source/detail?r=45270 Reverted: https://code.google.com/p/dart/source/detail?r=45271 Committed: https://code.google.com/p/dart/source/detail?r=45309

Patch Set 1 #

Patch Set 2 : #

Patch Set 3 : #

Total comments: 4

Patch Set 4 : #

Total comments: 8

Patch Set 5 : #

Patch Set 6 : fix loop variable assignment #

Patch Set 7 : revert #

Patch Set 8 : retry #

Unified diffs Side-by-side diffs Delta from patch set Stats (+285 lines, -38 lines) Patch
M pkg/compiler/lib/src/compiler.dart View 1 2 3 7 1 chunk +4 lines, -0 lines 0 comments Download
M pkg/compiler/lib/src/js_backend/backend.dart View 1 2 3 4 7 3 chunks +20 lines, -0 lines 0 comments Download
M pkg/compiler/lib/src/resolution/members.dart View 1 2 3 4 5 6 7 1 chunk +1 line, -0 lines 0 comments Download
M pkg/compiler/lib/src/resolution/registry.dart View 2 3 7 1 chunk +4 lines, -0 lines 0 comments Download
M pkg/compiler/lib/src/ssa/builder.dart View 1 2 3 4 5 7 2 chunks +149 lines, -20 lines 0 comments Download
M pkg/compiler/lib/src/ssa/codegen.dart View 1 2 3 4 7 1 chunk +30 lines, -4 lines 0 comments Download
M pkg/compiler/lib/src/ssa/codegen_helpers.dart View 2 3 7 1 chunk +3 lines, -1 line 0 comments Download
M pkg/compiler/lib/src/ssa/nodes.dart View 1 2 7 1 chunk +1 line, -1 line 0 comments Download
M pkg/compiler/lib/src/ssa/optimize.dart View 1 2 3 7 4 chunks +45 lines, -12 lines 0 comments Download
M sdk/lib/_internal/compiler/js_lib/js_helper.dart View 1 2 3 4 7 1 chunk +24 lines, -0 lines 0 comments Download
M tests/compiler/dart2js/mock_libraries.dart View 1 2 3 7 1 chunk +4 lines, -0 lines 0 comments Download

Messages

Total messages: 11 (3 generated)
floitsch
https://codereview.chromium.org/1087973002/diff/40001/sdk/lib/_internal/compiler/js_lib/js_helper.dart File sdk/lib/_internal/compiler/js_lib/js_helper.dart (right): https://codereview.chromium.org/1087973002/diff/40001/sdk/lib/_internal/compiler/js_lib/js_helper.dart#newcode1447 sdk/lib/_internal/compiler/js_lib/js_helper.dart:1447: throwConcurrentModificationError(sameLength, [collection]) { I would prefer a separate function ...
5 years, 8 months ago (2015-04-16 19:22:53 UTC) #2
sra1
https://codereview.chromium.org/1087973002/diff/40001/pkg/compiler/lib/src/ssa/builder.dart File pkg/compiler/lib/src/ssa/builder.dart (right): https://codereview.chromium.org/1087973002/diff/40001/pkg/compiler/lib/src/ssa/builder.dart#newcode5662 pkg/compiler/lib/src/ssa/builder.dart:5662: //if (!isFixed) { This will be changed back, was ...
5 years, 8 months ago (2015-04-16 19:27:09 UTC) #3
sra1
This is ready for review. On V8, DeltaBlueIterators is with 1% of DeltaBlue (indexed). On ...
5 years, 8 months ago (2015-04-16 23:37:24 UTC) #6
floitsch
LGTM. https://codereview.chromium.org/1087973002/diff/100001/pkg/compiler/lib/src/ssa/builder.dart File pkg/compiler/lib/src/ssa/builder.dart (right): https://codereview.chromium.org/1087973002/diff/100001/pkg/compiler/lib/src/ssa/builder.dart#newcode5529 pkg/compiler/lib/src/ssa/builder.dart:5529: // If the receiver supports JavaScript indexing we ...
5 years, 8 months ago (2015-04-17 12:40:34 UTC) #7
sra1
https://codereview.chromium.org/1087973002/diff/100001/pkg/compiler/lib/src/ssa/builder.dart File pkg/compiler/lib/src/ssa/builder.dart (right): https://codereview.chromium.org/1087973002/diff/100001/pkg/compiler/lib/src/ssa/builder.dart#newcode5529 pkg/compiler/lib/src/ssa/builder.dart:5529: // If the receiver supports JavaScript indexing we should ...
5 years, 8 months ago (2015-04-17 18:16:56 UTC) #8
sra1
Committed patchset #6 (id:140001) manually as 45270 (presubmit successful).
5 years, 8 months ago (2015-04-20 02:08:27 UTC) #9
sra1
Committed patchset #7 (id:160001) manually as 45271 (presubmit successful).
5 years, 8 months ago (2015-04-20 02:44:21 UTC) #10
sra1
5 years, 8 months ago (2015-04-21 03:13:53 UTC) #11
Message was sent while issue was closed.
Committed patchset #8 (id:180001) manually as 45309 (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698