[regexp] Port split

[regexp] Port split

This CL ports RegExp.prototype[@@split] to C++. Performance
regressions are expected due to:

* Slow RegExpImpl::Exec implementation instead of RegExpExec
  stub. We should be able to improve this by straight-lining
  RegExpImpl::Exec.
* Slow Factory::NewSubString instead of SubStringStub.
* Slow elements access to lastMatchInfo.

These points will be addressed in a follow-up CL.

BUG=v8:5339
Committed: https://crrev.com/05a55992340593e406fe79df1f26a4a503f06c87
Cr-Commit-Position: refs/heads/master@{#40161}

[jgruber, 2016-10-06 14:24:40]

The CQ bit was checked by jgruber@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-06 14:24:44]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[jgruber, 2016-10-06 14:31:41]

jgruber@chromium.org changed reviewers:
+ yangguo@chromium.org

[jgruber, 2016-10-06 14:31:42]

This, unfortunately, is where we start hurting octane/regexp.

Before: Mean: 4850.2 +- 382.451
After : Mean: 4612 +- 351.277

The most obvious reasons are:
* Slow RegExpImpl::Exec implementation instead of RegExpExec stub. We should be
able to improve this by straight-lining RegExpImpl::Exec.
* Slow Factory::NewSubString instead of SubStringStub.
* Slow elements access to lastMatchInfo. I'm planning on converting
lastMatchInfo to a simple FixedArray once all of regexp.js has been ported.

So there's still quite a bit of work to do here in order to reach pre-port
levels (and I doubt we will ever reach them if we stick with C++ only). But it
probably makes sense to land so we can do follow-up cleanups.

WDYT?

[jgruber, 2016-10-06 14:46:56]

On 2016/10/06 14:31:42, jgruber wrote:
> This, unfortunately, is where we start hurting octane/regexp.
> 
> Before: Mean: 4850.2 +- 382.451
> After : Mean: 4612 +- 351.277
> 
> The most obvious reasons are:
> * Slow RegExpImpl::Exec implementation instead of RegExpExec stub. We should
be
> able to improve this by straight-lining RegExpImpl::Exec.
> * Slow Factory::NewSubString instead of SubStringStub.
> * Slow elements access to lastMatchInfo. I'm planning on converting
> lastMatchInfo to a simple FixedArray once all of regexp.js has been ported.
> 
> So there's still quite a bit of work to do here in order to reach pre-port
> levels (and I doubt we will ever reach them if we stick with C++ only). But it
> probably makes sense to land so we can do follow-up cleanups.
> 
> WDYT?

The good news is that we recover most of what we lose here with the upcoming
RegExp.prototype.replace fast-path.

[commit-bot: I haz the power, 2016-10-06 14:53:23]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-06 14:53:24]

Dry run: This issue passed the CQ dry run.

[Yang, 2016-10-07 13:37:23]

LGTM. Using a simple FixedArray for LastMatchInfo is definitely a good idea.

https://codereview.chromium.org/2401643002/diff/1/src/builtins/builtins-regex...
File src/builtins/builtins-regexp.cc (right):

https://codereview.chromium.org/2401643002/diff/1/src/builtins/builtins-regex...
src/builtins/builtins-regexp.cc:1389: out:
Not a fan of goto...

I suggest simply duplicating that code after the out-label at the goto site.
Same below.

Then again I don't feel too strongly about this.

[jgruber, 2016-10-10 13:26:31]

The CQ bit was checked by jgruber@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-10 13:26:40]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[jgruber, 2016-10-10 13:29:13]

https://codereview.chromium.org/2401643002/diff/1/src/builtins/builtins-regex...
File src/builtins/builtins-regexp.cc (right):

https://codereview.chromium.org/2401643002/diff/1/src/builtins/builtins-regex...
src/builtins/builtins-regexp.cc:1389: out:
On 2016/10/07 13:37:23, Yang wrote:
> Not a fan of goto...
> 
> I suggest simply duplicating that code after the out-label at the goto site.
> Same below.
> 
> Then again I don't feel too strongly about this.

Personally, I don't mind this goto pattern - I think it can be more readable
than other alternatives in many cases. 

In this case, it also worked nicely to just extract the code to shrink a fixed
array and create a JSArray into a function. :)

Done.

[jgruber, 2016-10-10 13:39:18]

Description was changed from

==========
[regexp] Port split

BUG=v8:5339
==========

to

==========
[regexp] Port split

This CL ports RegExp.prototype[@@split] to C++. Performance
regressions are expected due to:

* Slow RegExpImpl::Exec implementation instead of RegExpExec
  stub. We should be able to improve this by straight-lining
  RegExpImpl::Exec.
* Slow Factory::NewSubString instead of SubStringStub.
* Slow elements access to lastMatchInfo.

These points will be addressed in a follow-up CL.

BUG=v8:5339
==========

[commit-bot: I haz the power, 2016-10-10 13:57:00]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-10 13:57:00]

Dry run: This issue passed the CQ dry run.

[jgruber, 2016-10-11 10:35:12]

The CQ bit was checked by jgruber@chromium.org

[jgruber, 2016-10-11 10:35:13]

The patchset sent to the CQ was uploaded after l-g-t-m from yangguo@chromium.org
Link to the patchset: https://codereview.chromium.org/2401643002/#ps20001
(title: "Address comments")

[commit-bot: I haz the power, 2016-10-11 10:35:24]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-11 10:38:05]

Description was changed from

==========
[regexp] Port split

This CL ports RegExp.prototype[@@split] to C++. Performance
regressions are expected due to:

* Slow RegExpImpl::Exec implementation instead of RegExpExec
  stub. We should be able to improve this by straight-lining
  RegExpImpl::Exec.
* Slow Factory::NewSubString instead of SubStringStub.
* Slow elements access to lastMatchInfo.

These points will be addressed in a follow-up CL.

BUG=v8:5339
==========

to

==========
[regexp] Port split

This CL ports RegExp.prototype[@@split] to C++. Performance
regressions are expected due to:

* Slow RegExpImpl::Exec implementation instead of RegExpExec
  stub. We should be able to improve this by straight-lining
  RegExpImpl::Exec.
* Slow Factory::NewSubString instead of SubStringStub.
* Slow elements access to lastMatchInfo.

These points will be addressed in a follow-up CL.

BUG=v8:5339
==========

[commit-bot: I haz the power, 2016-10-11 10:38:06]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-10-11 10:38:23]

Description was changed from

==========
[regexp] Port split

This CL ports RegExp.prototype[@@split] to C++. Performance
regressions are expected due to:

* Slow RegExpImpl::Exec implementation instead of RegExpExec
  stub. We should be able to improve this by straight-lining
  RegExpImpl::Exec.
* Slow Factory::NewSubString instead of SubStringStub.
* Slow elements access to lastMatchInfo.

These points will be addressed in a follow-up CL.

BUG=v8:5339
==========

to

==========
[regexp] Port split

This CL ports RegExp.prototype[@@split] to C++. Performance
regressions are expected due to:

* Slow RegExpImpl::Exec implementation instead of RegExpExec
  stub. We should be able to improve this by straight-lining
  RegExpImpl::Exec.
* Slow Factory::NewSubString instead of SubStringStub.
* Slow elements access to lastMatchInfo.

These points will be addressed in a follow-up CL.

BUG=v8:5339

Committed: https://crrev.com/05a55992340593e406fe79df1f26a4a503f06c87
Cr-Commit-Position: refs/heads/master@{#40161}
==========

[commit-bot: I haz the power, 2016-10-11 10:38:25]

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