Refactored code a bit to improve StringReplace performance

Refactored code a bit to improve StringReplace performance

1. Use inline macro to mitigate the side effect emulation overhead
2. Refactor Zone::DeleteAll() to merge two loops together

R=bmeurer@chromium.org, yangguo@chromium.org

Committed: https://code.google.com/p/v8/source/detail?r=15522

[weiliang.lin2, 2013-07-05 03:12:47]

Hi Yang && bmeurer, 
Please have a review. 
1. Inline ToInteger manually to mitigate the side effect emulation overhead
ToInteger() is a builtin function which is not inlinable. Acutally on normal
case, regexp.lastIndex is alawys SMI. So inlining here is mostly to eliminate
the function call overhead but still follows the spec

2. Refactor Zone::DeleteAll() to merge two loops together. 
This change seems to be like "finding quarrel in a straw". Actually it is
although I find the tick number of Zone::DeleteAll is reduced by 28% in the
profiling result. 

Overall, this commit could improve StringReplace by more than 6% and StringMatch
by more than 3% on my x86 PC. 

Thanks for review~

[Benedikt Meurer, 2013-07-05 05:50:56]

LGTM with comment.

https://codereview.chromium.org/18057004/diff/1/src/zone.cc
File src/zone.cc (right):

https://codereview.chromium.org/18057004/diff/1/src/zone.cc#newcode100
src/zone.cc:100: if (!keep && current->size() <= kMaximumKeptSegmentSize) {
Test for NULL using keep == NULL.

[weiliang.lin2, 2013-07-05 06:04:18]

On 2013/07/05 05:50:56, Benedikt Meurer wrote:
> LGTM with comment.
> 
> https://codereview.chromium.org/18057004/diff/1/src/zone.cc
> File src/zone.cc (right):
> 
> https://codereview.chromium.org/18057004/diff/1/src/zone.cc#newcode100
> src/zone.cc:100: if (!keep && current->size() <= kMaximumKeptSegmentSize) {
> Test for NULL using keep == NULL.

Thanks for review, addressed comment

[Yang, 2013-07-05 08:10:57]

LGTM with a comment. I'll land this once you address the comment.

https://codereview.chromium.org/18057004/diff/5001/src/string.js
File src/string.js (right):

https://codereview.chromium.org/18057004/diff/5001/src/string.js#newcode242
src/string.js:242: ToNumber(search.lastIndex);
I suggest adding a macro to src/macros.py for this, something like

macro TO_INTEGER_FOR_SIDE_EFFECT(arg) = (%_IsSmi(%IS_VAR(arg)) ? arg :
ToNumber(arg));

and use in the places you changed.

I suspect that the performance gain is mostly because you are omitting the
unnecessary call to %NumberToInteger.

[weiliang.lin2, 2013-07-05 12:44:33]

On 2013/07/05 08:10:57, Yang wrote:
> LGTM with a comment. I'll land this once you address the comment.
> 
> https://codereview.chromium.org/18057004/diff/5001/src/string.js
> File src/string.js (right):
> 
> https://codereview.chromium.org/18057004/diff/5001/src/string.js#newcode242
> src/string.js:242: ToNumber(search.lastIndex);
> I suggest adding a macro to src/macros.py for this, something like
> 
> macro TO_INTEGER_FOR_SIDE_EFFECT(arg) = (%_IsSmi(%IS_VAR(arg)) ? arg :
> ToNumber(arg));
> 
> and use in the places you changed.
> 
> I suspect that the performance gain is mostly because you are omitting the
> unnecessary call to %NumberToInteger.

Address the comment.
Actually on normal cases, TO_INTEGER_FOR_SIDE_EFFECT always hits the true
branch, so no matter what the false branch is.

Thanks for the review~

[Yang, 2013-07-05 12:57:44]

Committed patchset #3 manually as r15522 (presubmit successful).