Reviewers: Mads Ager, Erik Corry, Message: This "fixes" our 10% regression on a string benchmark. I was only able to reproduce the regression on a P4 system. I'd like to know better what was going on, but there was too many things that changed. I diffed the generated code, and it looks like it comes down to either 1) stack size increase, but I don't think this was it, 2) register allocation, there were a few spots where the compiler used the stack instead of a register, 3) code size. I would say it's mostly 3, which is strange. It didn't look like any of the branching or anything else changed. I can dig into it further if needed.
I think moving this code out makes sense anyway, and we get better numbers that before on this test, which is a bit unexpected. In general, I'd like to understand the P4 performance issues better, if anyone has insight, I'd love to know. Description: Move the pattern.length == 0 check from the runtime to JS wrapper code. We already have the length and index in the wrapper, so this makes sense, and will avoid the runtime in this case. Cleaned up the naming to be consistent between the runtime and JS code. Please review this at http://codereview.chromium.org/3181 Affected files: M src/runtime.cc M src/string.js Index: src/runtime.cc diff --git a/src/runtime.cc b/src/runtime.cc index 44bfb3bfcf633424146ffb65df5129bb051cec47..73fe260793784d1d13a0d924289c8b14b8889f3e 100644 --- a/src/runtime.cc +++ b/src/runtime.cc @@ -909,7 +909,8 @@ static Object* Runtime_StringIndexOf(Arguments args) { CONVERT_CHECKED(String, sub, args[0]); CONVERT_CHECKED(String, pat, args[1]); - Object* index = args[2]; + uint32_t start_index; + if (!Array::IndexFromObject(args[2], &start_index)) return Smi::FromInt(-1); sub->TryFlatten(); pat->TryFlatten(); @@ -917,10 +918,6 @@ static Object* Runtime_StringIndexOf(Arguments args) { int subject_length = sub->length(); int pattern_length = pat->length(); - uint32_t start_index; - if (!Array::IndexFromObject(index, &start_index)) return Smi::FromInt(-1); - if (pattern_length == 0) return Smi::FromInt(start_index); - // Searching for one specific character is common. For one // character patterns the KMP algorithm is guaranteed to slow down // the search, so we just run through the subject string. Index: src/string.js diff --git a/src/string.js b/src/string.js index d8105fdfc0b88ffaa02a0e8ec110559a4c3b79a4..272eef0b85c1055b1c527b47948a33c9868a308e 100644 --- a/src/string.js +++ b/src/string.js @@ -331,18 +331,20 @@ function ApplyReplacementFunction(replace, captures, subject) { // ECMA-262 section 15.5.4.7 %AddProperty($String.prototype, "indexOf", function(searchString /* position */) { // length == 1 - var str = ToString(this); - var str_len = str.length; - var searchStr = ToString(searchString); + var subject_str = ToString(this); + var pattern_str = ToString(searchString); + var subject_str_len = subject_str.length; + var pattern_str_len = pattern_str.length; var index = 0; if (%_ArgumentsLength() > 1) { var arg1 = %_Arguments(1); // position index = TO_INTEGER(arg1); } if (index < 0) index = 0; - if (index > str_len) index = str_len; - if (searchStr.length + index > str_len) return -1; - return %StringIndexOf(str, searchStr, index); + if (index > subject_str_len) index = subject_str_len; + if (pattern_str_len == 0) return index; + if (pattern_str_len + index > subject_str_len) return -1; + return %StringIndexOf(subject_str, pattern_str, index); }, DONT_ENUM); --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
