Reviewers: Yang, Benedikt Meurer,

Message:
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~

Description:
Refactored code a bit to improve StringReplace performance

1. Inline ToInteger manually to mitigate the side effect emulation overhead
2. Refactor Zone::DeleteAll() to merge two loops together

Please review this at https://codereview.chromium.org/18057004/

SVN Base: http://v8.googlecode.com/svn/branches/bleeding_edge/

Affected files:
  M     src/string.js
  M     src/zone.cc


Index: src/string.js
===================================================================
--- src/string.js       (revision 15496)
+++ src/string.js       (working copy)
@@ -184,8 +184,10 @@
   var subject = TO_STRING_INLINE(this);
   if (IS_REGEXP(regexp)) {
     // Emulate RegExp.prototype.exec's side effect in step 5, even though
-    // value is discarded.
-    ToInteger(regexp.lastIndex);
+    // value is discarded. Inline ToInteger() manually.
+    if (! %_IsSmi(regexp.lastIndex)) {
+      ToNumber(regexp.lastIndex);
+    }
     if (!regexp.global) return RegExpExecNoTests(regexp, subject, 0);
     %_Log('regexp', 'regexp-match,%0S,%1r', [subject, regexp]);
     // lastMatchInfo is defined in regexp.js.
@@ -235,8 +237,10 @@

   if (IS_REGEXP(search)) {
     // Emulate RegExp.prototype.exec's side effect in step 5, even if
-    // value is discarded.
-    ToInteger(search.lastIndex);
+    // value is discarded. Inline ToInteger() manually.
+    if (! %_IsSmi(search.lastIndex)) {
+      ToNumber(search.lastIndex);
+    }
     %_Log('regexp', 'regexp-replace,%0r,%1S', [search, subject]);

     if (!IS_SPEC_FUNCTION(replace)) {
Index: src/zone.cc
===================================================================
--- src/zone.cc (revision 15496)
+++ src/zone.cc (working copy)
@@ -92,18 +92,15 @@
 #endif

   // Find a segment with a suitable size to keep around.
-  Segment* keep = segment_head_;
-  while (keep != NULL && keep->size() > kMaximumKeptSegmentSize) {
-    keep = keep->next();
-  }
-
+  Segment* keep = NULL;
   // Traverse the chained list of segments, zapping (in debug mode)
   // and freeing every segment except the one we wish to keep.
   for (Segment* current = segment_head_; current != NULL; ) {
     Segment* next = current->next();
-    if (current == keep) {
+    if (!keep && current->size() <= kMaximumKeptSegmentSize) {
       // Unlink the segment we wish to keep from the list.
-      current->clear_next();
+      keep = current;
+      keep->clear_next();
     } else {
       int size = current->size();
 #ifdef DEBUG


--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
--- You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.


Reply via email to