Introduce code flushing of RegExp code.

Introduce code flushing of RegExp code.

Due to issues relating mostly to chrome extensions we have lately been
running into OOMs that are caused by our executable space running
out. This change introduces flushing of code from regexps if we have
not used the code for 5 mark sweeps.

The approach is different from the normal function code flusing. Here
we make a copy of the code inside the data array, and exchange the
original code with a smi determined by the sweep_generation (a new
heap variable increased everytime we do mark sweep/compact). If we
encounter a smi in EnsureCompiled we simply reinstate the code
object. If, in the marking phase of mark sweep, we find a regexp that
already have a smi in the code field, and this is more than 5
generations old we flush the code from the saved index.
Committed: http://code.google.com/p/v8/source/detail?r=8532

[Rico, 2011-06-30 13:34:48]

On 2011/06/30 12:05:45, Rico wrote:
Added marking of the in object properties of the RegExp

[Erik Corry, 2011-06-30 18:48:24]

LGTM

http://codereview.chromium.org/7282026/diff/7018/src/arm/code-stubs-arm.cc
File src/arm/code-stubs-arm.cc (right):

http://codereview.chromium.org/7282026/diff/7018/src/arm/code-stubs-arm.cc#ne...
src/arm/code-stubs-arm.cc:4425: __ b(ne, &runtime);
This b is superfluous

http://codereview.chromium.org/7282026/diff/7018/src/ia32/code-stubs-ia32.cc
File src/ia32/code-stubs-ia32.cc (right):

http://codereview.chromium.org/7282026/diff/7018/src/ia32/code-stubs-ia32.cc#...
src/ia32/code-stubs-ia32.cc:3385: __ j(not_equal, &runtime);
This j is superfluous.

http://codereview.chromium.org/7282026/diff/7018/src/jsregexp.cc
File src/jsregexp.cc (right):

http://codereview.chromium.org/7282026/diff/7018/src/jsregexp.cc#newcode298
src/jsregexp.cc:298: // We could potentially have marked this as flushable, but
have keept
keept -> kept

http://codereview.chromium.org/7282026/diff/7018/src/mark-compact.cc
File src/mark-compact.cc (right):

http://codereview.chromium.org/7282026/diff/7018/src/mark-compact.cc#newcode733
src/mark-compact.cc:733: if (value == (heap->sweep_generation() -
kRegExpCodeThreshold) % 256) {
If the lhs is negative then according to the standard the result of the %
operator can be either positive or negative.  Therefore I would prefer using &
0xff which can yield only a positive answer.  The same applies above.

http://codereview.chromium.org/7282026/diff/7018/src/objects-debug.cc
File src/objects-debug.cc (right):

http://codereview.chromium.org/7282026/diff/7018/src/objects-debug.cc#newcode472
src/objects-debug.cc:472: ASSERT(ascii_data->IsSmi() || ascii_data->IsJSObject()
||
I can see that it was already there before, but it is not clear to me how this
can be a JSObject?

http://codereview.chromium.org/7282026/diff/7018/src/objects-debug.cc#newcode478
src/objects-debug.cc:478: Object* ascii_saved =
arr->get(JSRegExp::kIrregexpUC16CodeSavedIndex);
Should be the ASCII index here.

http://codereview.chromium.org/7282026/diff/7018/src/objects-inl.h
File src/objects-inl.h (right):

http://codereview.chromium.org/7282026/diff/7018/src/objects-inl.h#newcode3914
src/objects-inl.h:3914: fa->set_unchecked(heap, index, value,
UPDATE_WRITE_BARRIER);
I think this is OK, but I would like to ask Slava.

http://codereview.chromium.org/7282026/diff/7018/src/objects-visiting.h
File src/objects-visiting.h (right):

http://codereview.chromium.org/7282026/diff/7018/src/objects-visiting.h#newco...
src/objects-visiting.h:304: table_.Register(kVisitJSRegExp,
Fits on one line.

http://codereview.chromium.org/7282026/diff/7018/src/objects.h
File src/objects.h (right):

http://codereview.chromium.org/7282026/diff/7018/src/objects.h#newcode5297
src/objects.h:5297: // used for tracking the last usage (used for code
flushing)..
Missing description of the 'saved' fields.

http://codereview.chromium.org/7282026/diff/7018/src/x64/code-stubs-x64.cc
File src/x64/code-stubs-x64.cc (right):

http://codereview.chromium.org/7282026/diff/7018/src/x64/code-stubs-x64.cc#ne...
src/x64/code-stubs-x64.cc:2460: __ j(not_equal, &runtime);
Hat trick!

[Rico, 2011-07-01 05:58:09]

All comments addressed, will check with Slava if we can skip the write barrier
entirely.

http://codereview.chromium.org/7282026/diff/7018/src/arm/code-stubs-arm.cc
File src/arm/code-stubs-arm.cc (right):

http://codereview.chromium.org/7282026/diff/7018/src/arm/code-stubs-arm.cc#ne...
src/arm/code-stubs-arm.cc:4425: __ b(ne, &runtime);
On 2011/06/30 18:48:25, Erik Corry wrote:
> This b is superfluous

Done.

http://codereview.chromium.org/7282026/diff/7018/src/ia32/code-stubs-ia32.cc
File src/ia32/code-stubs-ia32.cc (right):

http://codereview.chromium.org/7282026/diff/7018/src/ia32/code-stubs-ia32.cc#...
src/ia32/code-stubs-ia32.cc:3385: __ j(not_equal, &runtime);
On 2011/06/30 18:48:25, Erik Corry wrote:
> This j is superfluous.

Done.

http://codereview.chromium.org/7282026/diff/7018/src/jsregexp.cc
File src/jsregexp.cc (right):

http://codereview.chromium.org/7282026/diff/7018/src/jsregexp.cc#newcode298
src/jsregexp.cc:298: // We could potentially have marked this as flushable, but
have keept
On 2011/06/30 18:48:25, Erik Corry wrote:
> keept -> kept

Done.

http://codereview.chromium.org/7282026/diff/7018/src/mark-compact.cc
File src/mark-compact.cc (right):

http://codereview.chromium.org/7282026/diff/7018/src/mark-compact.cc#newcode733
src/mark-compact.cc:733: if (value == (heap->sweep_generation() -
kRegExpCodeThreshold) % 256) {
On 2011/06/30 18:48:25, Erik Corry wrote:
> If the lhs is negative then according to the standard the result of the %
> operator can be either positive or negative.  Therefore I would prefer using &
> 0xff which can yield only a positive answer.  The same applies above.

Done.

http://codereview.chromium.org/7282026/diff/7018/src/objects-debug.cc
File src/objects-debug.cc (right):

http://codereview.chromium.org/7282026/diff/7018/src/objects-debug.cc#newcode472
src/objects-debug.cc:472: ASSERT(ascii_data->IsSmi() || ascii_data->IsJSObject()
||
On 2011/06/30 18:48:25, Erik Corry wrote:
> I can see that it was already there before, but it is not clear to me how this
> can be a JSObject?
Compilation error, see comment two lines above?

http://codereview.chromium.org/7282026/diff/7018/src/objects-debug.cc#newcode478
src/objects-debug.cc:478: Object* ascii_saved =
arr->get(JSRegExp::kIrregexpUC16CodeSavedIndex);
On 2011/06/30 18:48:25, Erik Corry wrote:
> Should be the ASCII index here.
Ups, nice spot

http://codereview.chromium.org/7282026/diff/7018/src/objects-inl.h
File src/objects-inl.h (right):

http://codereview.chromium.org/7282026/diff/7018/src/objects-inl.h#newcode3914
src/objects-inl.h:3914: fa->set_unchecked(heap, index, value,
UPDATE_WRITE_BARRIER);
On 2011/06/30 18:48:25, Erik Corry wrote:
> I think this is OK, but I would like to ask Slava.

Done.

http://codereview.chromium.org/7282026/diff/7018/src/objects-visiting.h
File src/objects-visiting.h (right):

http://codereview.chromium.org/7282026/diff/7018/src/objects-visiting.h#newco...
src/objects-visiting.h:304: table_.Register(kVisitJSRegExp,
On 2011/06/30 18:48:25, Erik Corry wrote:
> Fits on one line.

Done.

http://codereview.chromium.org/7282026/diff/7018/src/x64/code-stubs-x64.cc
File src/x64/code-stubs-x64.cc (right):

http://codereview.chromium.org/7282026/diff/7018/src/x64/code-stubs-x64.cc#ne...
src/x64/code-stubs-x64.cc:2460: __ j(not_equal, &runtime);
On 2011/06/30 18:48:25, Erik Corry wrote:
> Hat trick!

Done.

[Rico, 2011-07-01 08:49:06]

Please have another look. As discussed offline I changed the logic that puts the
error object into the code place to instead write a specific smi
(kCompilationError) and put the error object at the saved index instead. In
addition, I put the mask into the RegExp class and assert that we always have a
valid smi in the RegExp compiler.

Also, unrelated, but removed some trailing whitespaces in two js files.


On 2011/07/01 05:58:09, Rico wrote:
> All comments addressed, will check with Slava if we can skip the write barrier
> entirely.
> 
> http://codereview.chromium.org/7282026/diff/7018/src/arm/code-stubs-arm.cc
> File src/arm/code-stubs-arm.cc (right):
> 
>
http://codereview.chromium.org/7282026/diff/7018/src/arm/code-stubs-arm.cc#ne...
> src/arm/code-stubs-arm.cc:4425: __ b(ne, &runtime);
> On 2011/06/30 18:48:25, Erik Corry wrote:
> > This b is superfluous
> 
> Done.
> 
> http://codereview.chromium.org/7282026/diff/7018/src/ia32/code-stubs-ia32.cc
> File src/ia32/code-stubs-ia32.cc (right):
> 
>
http://codereview.chromium.org/7282026/diff/7018/src/ia32/code-stubs-ia32.cc#...
> src/ia32/code-stubs-ia32.cc:3385: __ j(not_equal, &runtime);
> On 2011/06/30 18:48:25, Erik Corry wrote:
> > This j is superfluous.
> 
> Done.
> 
> http://codereview.chromium.org/7282026/diff/7018/src/jsregexp.cc
> File src/jsregexp.cc (right):
> 
> http://codereview.chromium.org/7282026/diff/7018/src/jsregexp.cc#newcode298
> src/jsregexp.cc:298: // We could potentially have marked this as flushable,
but
> have keept
> On 2011/06/30 18:48:25, Erik Corry wrote:
> > keept -> kept
> 
> Done.
> 
> http://codereview.chromium.org/7282026/diff/7018/src/mark-compact.cc
> File src/mark-compact.cc (right):
> 
>
http://codereview.chromium.org/7282026/diff/7018/src/mark-compact.cc#newcode733
> src/mark-compact.cc:733: if (value == (heap->sweep_generation() -
> kRegExpCodeThreshold) % 256) {
> On 2011/06/30 18:48:25, Erik Corry wrote:
> > If the lhs is negative then according to the standard the result of the %
> > operator can be either positive or negative.  Therefore I would prefer using
&
> > 0xff which can yield only a positive answer.  The same applies above.
> 
> Done.
> 
> http://codereview.chromium.org/7282026/diff/7018/src/objects-debug.cc
> File src/objects-debug.cc (right):
> 
>
http://codereview.chromium.org/7282026/diff/7018/src/objects-debug.cc#newcode472
> src/objects-debug.cc:472: ASSERT(ascii_data->IsSmi() ||
ascii_data->IsJSObject()
> ||
> On 2011/06/30 18:48:25, Erik Corry wrote:
> > I can see that it was already there before, but it is not clear to me how
this
> > can be a JSObject?
> Compilation error, see comment two lines above?
> 
>
http://codereview.chromium.org/7282026/diff/7018/src/objects-debug.cc#newcode478
> src/objects-debug.cc:478: Object* ascii_saved =
> arr->get(JSRegExp::kIrregexpUC16CodeSavedIndex);
> On 2011/06/30 18:48:25, Erik Corry wrote:
> > Should be the ASCII index here.
> Ups, nice spot
> 
> http://codereview.chromium.org/7282026/diff/7018/src/objects-inl.h
> File src/objects-inl.h (right):
> 
> http://codereview.chromium.org/7282026/diff/7018/src/objects-inl.h#newcode3914
> src/objects-inl.h:3914: fa->set_unchecked(heap, index, value,
> UPDATE_WRITE_BARRIER);
> On 2011/06/30 18:48:25, Erik Corry wrote:
> > I think this is OK, but I would like to ask Slava.
> 
> Done.
> 
> http://codereview.chromium.org/7282026/diff/7018/src/objects-visiting.h
> File src/objects-visiting.h (right):
> 
>
http://codereview.chromium.org/7282026/diff/7018/src/objects-visiting.h#newco...
> src/objects-visiting.h:304: table_.Register(kVisitJSRegExp,
> On 2011/06/30 18:48:25, Erik Corry wrote:
> > Fits on one line.
> 
> Done.
> 
> http://codereview.chromium.org/7282026/diff/7018/src/x64/code-stubs-x64.cc
> File src/x64/code-stubs-x64.cc (right):
> 
>
http://codereview.chromium.org/7282026/diff/7018/src/x64/code-stubs-x64.cc#ne...
> src/x64/code-stubs-x64.cc:2460: __ j(not_equal, &runtime);
> On 2011/06/30 18:48:25, Erik Corry wrote:
> > Hat trick!
> 
> Done.

[Erik Corry, 2011-07-01 09:05:23]

LGTM

http://codereview.chromium.org/7282026/diff/14007/src/heap.cc
File src/heap.cc (right):

http://codereview.chromium.org/7282026/diff/14007/src/heap.cc#newcode1312
src/heap.cc:1312: 
There is an extra blank line here.

http://codereview.chromium.org/7282026/diff/14007/src/jsregexp.cc
File src/jsregexp.cc (right):

http://codereview.chromium.org/7282026/diff/14007/src/jsregexp.cc#newcode377
src/jsregexp.cc:377: re->SetDataAt(JSRegExp::saved_code_index(is_ascii),
*regexp_err);
Shouldn't you be setting the kCompilationErrorValue here?

[Rico, 2011-07-01 09:38:26]

Added better error message handling in jsregexp

http://codereview.chromium.org/7282026/diff/14007/src/heap.cc
File src/heap.cc (right):

http://codereview.chromium.org/7282026/diff/14007/src/heap.cc#newcode1312
src/heap.cc:1312: 
On 2011/07/01 09:05:23, Erik Corry wrote:
> There is an extra blank line here.

Done.

http://codereview.chromium.org/7282026/diff/14007/src/jsregexp.cc
File src/jsregexp.cc (right):

http://codereview.chromium.org/7282026/diff/14007/src/jsregexp.cc#newcode377
src/jsregexp.cc:377: re->SetDataAt(JSRegExp::saved_code_index(is_ascii),
*regexp_err);
On 2011/07/01 09:05:23, Erik Corry wrote:
> Shouldn't you be setting the kCompilationErrorValue here?

Done.

[Erik Corry, 2011-07-01 10:26:38]

LGTM

http://codereview.chromium.org/7282026/diff/7047/src/objects-debug.cc
File src/objects-debug.cc (right):

http://codereview.chromium.org/7282026/diff/7047/src/objects-debug.cc#newcode479
src/objects-debug.cc:479: ASSERT(ascii_saved->IsSmi() ||
ascii_saved->IsJSObject() ||
Should be IsString!