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

Issue 28311: Flattened the representation of compiled RegExp data. (Closed)

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

Description

All RegExp data are set on a single FixedArray instead of nesting them three deep.

Patch Set 1 #

Patch Set 2 : ... and it lints. #

Total comments: 7
Unified diffs Side-by-side diffs Delta from patch set Stats (+368 lines, -254 lines) Patch
M src/factory.h View 1 chunk +14 lines, -6 lines 0 comments Download
M src/factory.cc View 1 2 chunks +26 lines, -6 lines 0 comments Download
M src/jsregexp.h View 1 5 chunks +35 lines, -20 lines 0 comments Download
M src/jsregexp.cc View 1 14 chunks +239 lines, -215 lines 7 comments Download
M src/objects.h View 3 chunks +35 lines, -5 lines 0 comments Download
M src/objects-debug.cc View 1 chunk +12 lines, -2 lines 0 comments Download
M src/objects-inl.h View 1 chunk +7 lines, -0 lines 0 comments Download

Messages

Total messages: 3 (0 generated)
Lasse Reichstein
Review, please.
11 years, 9 months ago (2009-03-02 11:07:34 UTC) #1
Erik Corry
Does it lint and compile on ARM? LGTM. http://codereview.chromium.org/28311/diff/1001/1004 File src/jsregexp.cc (right): http://codereview.chromium.org/28311/diff/1001/1004#newcode389 Line 389: ...
11 years, 9 months ago (2009-03-02 12:01:48 UTC) #2
Lasse Reichstein
11 years, 9 months ago (2009-03-02 13:54:48 UTC) #3
Lint: yes.
Compile on ARM: Yes, now. I had actually broken the bytecode matching.

http://codereview.chromium.org/28311/diff/1001/1004
File src/jsregexp.cc (right):

http://codereview.chromium.org/28311/diff/1001/1004#newcode389
Line 389: // Ensures that the regexp object containst a compiled version of the
On 2009/03/02 12:01:48, Erik Corry wrote:
> spolling!

Done.

http://codereview.chromium.org/28311/diff/1001/1004#newcode482
Line 482: int RegExpImpl::IrregexpNumberOfRegisters(Handle<FixedArray> re) {
It is. Or it was, not it's gone.

http://codereview.chromium.org/28311/diff/1001/1004#newcode759
Line 759: // TODO(lrn): How do we know the array is long enough.
Done.

Powered by Google App Engine
This is Rietveld 408576698