Limit code size generated for very large regexps

Limit code size generated for very large regexps

R=jkummerow@chromium.org, yangguo@chromium.org
BUG=

Committed: https://crrev.com/d287f225a3630e71a6d14009e7baf71f465be861
Cr-Commit-Position: refs/heads/master@{#25839}

[Erik Corry, 2014-12-15 16:54:10]

erik.corry@gmail.com changed reviewers:
+ jkummerow@chromium.org, yangguo@chromium.org
- kummerow@chromium.org, yang@chromium.org

[Jakob Kummerow, 2014-12-15 20:00:29]

Thanks for the fix! LGTM with minor comments.

https://codereview.chromium.org/799403003/diff/1/src/jsregexp.cc
File src/jsregexp.cc (right):

https://codereview.chromium.org/799403003/diff/1/src/jsregexp.cc#newcode6165
src/jsregexp.cc:6165: bool too_much = pattern->length() >
RegExpImpl::kRegExpTooLargeToOptimize;
I'd combine this into a single return statement:

  return pattern->length() > RegExpImpl::kRegExpTooLargeToOptimize ||
         (heap->total_regexp_code_generated() >
              RegExpImpl::kRegExpCompiledLimit &&
          heap->isolate()->memory_allocator()->SizeExecutable() >
              RegExpImpl::kRegExpExecutableMemoryLimit);

But I'm fine with whatever you find more readable.

https://codereview.chromium.org/799403003/diff/1/test/cctest/test-heap.cc
File test/cctest/test-heap.cc (right):

https://codereview.chromium.org/799403003/diff/1/test/cctest/test-heap.cc#new...
test/cctest/test-heap.cc:1619: "while (reg_exp_source.length < 10000) {"
The limit here mirrors kRegExpTooLargeToOptimize, right? Maybe add a
STATIC_ASSERT here with a comment that the test needs to be updated if the limit
is changed.

[Erik Corry, 2014-12-16 12:05:33]

https://codereview.chromium.org/799403003/diff/1/src/jsregexp.cc
File src/jsregexp.cc (right):

https://codereview.chromium.org/799403003/diff/1/src/jsregexp.cc#newcode6165
src/jsregexp.cc:6165: bool too_much = pattern->length() >
RegExpImpl::kRegExpTooLargeToOptimize;
On 2014/12/15 20:00:29, Jakob wrote:
> I'd combine this into a single return statement:
> 
>   return pattern->length() > RegExpImpl::kRegExpTooLargeToOptimize ||
>          (heap->total_regexp_code_generated() >
>               RegExpImpl::kRegExpCompiledLimit &&
>           heap->isolate()->memory_allocator()->SizeExecutable() >
>               RegExpImpl::kRegExpExecutableMemoryLimit);
> 
> But I'm fine with whatever you find more readable.

I find the existing version more readable.

https://codereview.chromium.org/799403003/diff/1/test/cctest/test-heap.cc
File test/cctest/test-heap.cc (right):

https://codereview.chromium.org/799403003/diff/1/test/cctest/test-heap.cc#new...
test/cctest/test-heap.cc:1619: "while (reg_exp_source.length < 10000) {"
On 2014/12/15 20:00:29, Jakob wrote:
> The limit here mirrors kRegExpTooLargeToOptimize, right? Maybe add a
> STATIC_ASSERT here with a comment that the test needs to be updated if the
limit
> is changed.

Done.

[Erik Corry, 2014-12-16 12:05:37]

The CQ bit was checked by erik.corry@gmail.com

[commit-bot: I haz the power, 2014-12-16 12:06:06]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/799403003/20001

[commit-bot: I haz the power, 2014-12-16 12:33:27]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2014-12-17 10:14:07]

Patchset 2 (id:??) landed as
https://crrev.com/d287f225a3630e71a6d14009e7baf71f465be861
Cr-Commit-Position: refs/heads/master@{#25839}