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

Issue 1350003: Pre-create properties on JSRegExp objects (Closed)

Created:
10 years, 9 months ago by Lasse Reichstein
Modified:
9 years, 6 months ago
Reviewers:
Erik Corry
CC:
v8-dev
Visibility:
Public.

Description

Pre-create properties on JSRegExp objects Initialize properties in single runtime call.

Patch Set 1 #

Total comments: 6
Unified diffs Side-by-side diffs Delta from patch set Stats (+134 lines, -27 lines) Patch
M src/bootstrapper.cc View 1 chunk +60 lines, -1 line 2 comments Download
M src/factory.h View 1 chunk +1 line, -0 lines 2 comments Download
M src/heap.h View 1 chunk +5 lines, -0 lines 0 comments Download
M src/objects.h View 1 chunk +7 lines, -0 lines 0 comments Download
M src/regexp.js View 1 chunk +2 lines, -24 lines 0 comments Download
src/runtime.h View 1 chunk +1 line, -0 lines 0 comments Download
M src/runtime.cc View 1 chunk +56 lines, -0 lines 2 comments Download
M test/mjsunit/mirror-regexp.js View 1 chunk +2 lines, -2 lines 0 comments Download

Messages

Total messages: 3 (0 generated)
Lasse Reichstein
Medium size review.
10 years, 9 months ago (2010-03-26 13:18:58 UTC) #1
Erik Corry
LGTM. We could do the same for the strange array-with-two-extra-properties that is returned by RegExp.prototype.exec. ...
10 years, 9 months ago (2010-03-26 13:51:34 UTC) #2
Lasse Reichstein
10 years, 9 months ago (2010-03-26 14:19:09 UTC) #3
http://codereview.chromium.org/1350003/diff/1/2
File src/bootstrapper.cc (right):

http://codereview.chromium.org/1350003/diff/1/2#newcode765
src/bootstrapper.cc:765: 3);
Actually, not. This "3" is the enumeration index, not the field index. They just
happen to be the same value.

However, I'm not sure whether I need an index at all, since the value is not
enumerable.
I'll turn it into a counter. Easier to read that way, and the compiler should
constant fold it anyway.

http://codereview.chromium.org/1350003/diff/1/3
File src/factory.h (right):

http://codereview.chromium.org/1350003/diff/1/3#newcode44
src/factory.h:44: 
Yes, removed.

http://codereview.chromium.org/1350003/diff/1/7
File src/runtime.cc (right):

http://codereview.chromium.org/1350003/diff/1/7#newcode1263
src/runtime.cc:1263: // Map has changed, so use generic, but slower, method.
I guess the only way this can happen is when using RegExp.prototype.compile on a
regexp that has been fiddeled with. 
I'll add a test of this case.

Powered by Google App Engine
This is Rietveld 408576698