Bug 1112537 - Optimize String#split('foo').join('bar') pattern; r=nbp

js/src/jit-test/tests/ion/bug977966.js

@@ -72,6 +72,40 @@ function split_join_4(i) {
     return i;
 }
 
+function split_join_5(i) {
+    var s = "abca";
+    assertEq(s.split("a").join("") + i, "bc" + i);
+}
+
+function split_join_two_byte_char(i) {
+    var s1 = "ab";
+    assertEq(s1.split("").join("\u03c0"), "a\u03c0b");
+    var s2 = i + "\u03c0" + i;
+    assertEq(s2.split("\u03c0").join("-"), i + "-" + i);
+}
+
+if (getBuildConfiguration()['asan']) {
+    function split_join_overflow() {}
+} else {
+    function split_join_overflow()
+    {
+        try {
+            var s = "  ";
+            for (var i = 1; i < 10; i++)
+                s = s.split("").join(s); // 2^(2^i)
+        } catch (exn) {
+            if (exn != "out of memory")
+                assertEq(exn instanceof InternalError, true);
+        };
+    }
+}
+
+function split_join_underflow(i)
+{
+    var s = "";
+    assertEq(s.split("").join("x" + i), "");
+}
+
 // Check that we do not consider the string argument of join as a replacement
 // pattern, as the string replace primitive is supposed to do.
 function split_join_pattern(i) {
@@ -104,6 +138,12 @@ for (var i = 0; i < 100; ++i) {
     split_join_2(i);
     split_join_3(i);
     split_join_4(i);
+    split_join_5(i);
     split_join_pattern(i);
     split_join_multiple(i);
+    split_join_two_byte_char(i);
+    split_join_underflow(i);
 }
+
+for (var i = 0; i < 5; i++)
+   split_join_overflow();

js/src/jit/CodeGenerator.cpp

@@ -1634,6 +1634,7 @@ CodeGenerator::visitRegExpReplace(LRegExpReplace *lir)
 }
 
 typedef JSString *(*StringReplaceFn)(JSContext *, HandleString, HandleString, HandleString);
+static const VMFunction StringFlatReplaceInfo = FunctionInfo<StringReplaceFn>(js::str_flat_replace_string);
 static const VMFunction StringReplaceInfo = FunctionInfo<StringReplaceFn>(StringReplace);
 
 void
@@ -1654,6 +1655,9 @@ CodeGenerator::visitStringReplace(LStringReplace *lir)
     else
         pushArg(ToRegister(lir->string()));
 
+    if (lir->mir()->isFlatReplacement())
+        return callVM(StringFlatReplaceInfo, lir);
+
     callVM(StringReplaceInfo, lir);
 }
 
@@ -5901,7 +5905,6 @@ CodeGenerator::visitStringSplit(LStringSplit *lir)
     pushArg(ToRegister(lir->separator()));
     pushArg(ToRegister(lir->string()));
     pushArg(ImmGCPtr(lir->mir()->typeObject()));
-
     callVM(StringSplitInfo, lir);
 }
 

js/src/jit/MIR.cpp

@@ -4064,18 +4064,16 @@ MTableSwitch::foldsTo(TempAllocator &alloc)
 }
 
 MDefinition *
-MArrayJoin::foldsTo(TempAllocator &alloc) {
-    // :TODO: Enable this optimization after fixing Bug 977966 test cases.
-    return this;
-
+MArrayJoin::foldsTo(TempAllocator &alloc)
+{
     MDefinition *arr = array();
 
     if (!arr->isStringSplit())
         return this;
 
-    this->setRecoveredOnBailout();
+    setRecoveredOnBailout();
     if (arr->hasLiveDefUses()) {
-        this->setNotRecoveredOnBailout();
+        setNotRecoveredOnBailout();
         return this;
     }
 
@@ -4089,7 +4087,9 @@ MArrayJoin::foldsTo(TempAllocator &alloc) {
     MDefinition *replacement = sep();
 
     setNotRecoveredOnBailout();
-    return MStringReplace::New(alloc, string, pattern, replacement);
+    MStringReplace *substr = MStringReplace::New(alloc, string, pattern, replacement);
+    substr->setFlatReplacement();
+    return substr;
 }
 
 bool

js/src/jit/MIR.h

@@ -6999,8 +6999,10 @@ class MStringReplace
 {
   private:
 
+    bool isFlatReplacement_;
+
     MStringReplace(MDefinition *string, MDefinition *pattern, MDefinition *replacement)
-      : MStrReplace< StringPolicy<1> >(string, pattern, replacement)
+      : MStrReplace< StringPolicy<1> >(string, pattern, replacement), isFlatReplacement_(false)
     {
     }
 
@@ -7011,7 +7013,20 @@ class MStringReplace
         return new(alloc) MStringReplace(string, pattern, replacement);
     }
 
+    void setFlatReplacement() {
+        MOZ_ASSERT(!isFlatReplacement_);
+        isFlatReplacement_ = true;
+    }
+
+    bool isFlatReplacement() const {
+        return isFlatReplacement_;
+    }
+
     bool congruentTo(const MDefinition *ins) const MOZ_OVERRIDE {
+        if (!ins->isStringReplace())
+            return false;
+        if (isFlatReplacement_ != ins->toStringReplace()->isFlatReplacement_)
+            return false;
         return congruentIfOperandsEqual(ins);
     }
 
@@ -7021,6 +7036,8 @@ class MStringReplace
 
     bool writeRecoverData(CompactBufferWriter &writer) const MOZ_OVERRIDE;
     bool canRecoverOnBailout() const MOZ_OVERRIDE {
+        if (isFlatReplacement_)
+            return false;
         if (pattern()->isRegExp())
             return !pattern()->toRegExp()->source()->global();
         return false;

js/src/jsstr.cpp

@@ -3326,9 +3326,122 @@ StrReplaceString(JSContext *cx, ReplaceData &rdata, const FlatMatch &fm, Mutable
 
 static const uint32_t ReplaceOptArg = 2;
 
+template <typename StrChar, typename RepChar>
+static bool
+StrFlatReplaceGlobal(JSContext *cx, JSLinearString *str, JSLinearString *pat, JSLinearString *rep,
+                     StringBuffer &sb)
+{
+    MOZ_ASSERT(str->length() > 0);
+
+    AutoCheckCannotGC nogc;
+    const StrChar *strChars = str->chars<StrChar>(nogc);
+    const RepChar *repChars = rep->chars<RepChar>(nogc);
+
+    // The pattern is empty, so we interleave the replacement string in-between
+    // each character.
+    if (!pat->length()) {
+        CheckedInt<uint32_t> strLength(str->length());
+        CheckedInt<uint32_t> repLength(rep->length());
+        CheckedInt<uint32_t> length = repLength * (strLength - 1) + strLength;
+        if (!length.isValid()) {
+            js_ReportAllocationOverflow(cx);
+            return false;
+        }
+
+        if (!sb.reserve(length.value()))
+            return false;
+
+        for (unsigned i = 0; i < str->length() - 1; ++i, ++strChars) {
+            sb.infallibleAppend(*strChars);
+            sb.infallibleAppend(repChars, rep->length());
+        }
+        sb.infallibleAppend(*strChars);
+        return true;
+    }
+
+    // If it's true, we are sure that the result's length is, at least, the same
+    // length as |str->length()|.
+    if (rep->length() >= pat->length()) {
+        if (!sb.reserve(str->length()))
+            return false;
+    }
+
+    uint32_t start = 0;
+    for (;;) {
+        int match = StringMatch(str, pat, start);
+        if (match < 0)
+            break;
+        if (!sb.append(strChars + start, match - start))
+            return false;
+        if (!sb.append(repChars, rep->length()))
+            return false;
+        start = match + pat->length();
+    }
+
+    if (!sb.append(strChars + start, str->length() - start))
+        return false;
+
+    return true;
+}
+
+// This is identical to "str.split(pattern).join(replacement)" except that we
+// do some deforestation optimization in Ion.
+JSString *
+js::str_flat_replace_string(JSContext *cx, HandleString string, HandleString pattern,
+                            HandleString replacement)
+{
+    MOZ_ASSERT(string);
+    MOZ_ASSERT(pattern);
+    MOZ_ASSERT(replacement);
+
+    if (!string->length())
+        return string;
+
+    RootedLinearString linearRepl(cx, replacement->ensureLinear(cx));
+    if (!linearRepl)
+        return nullptr;
+
+    RootedLinearString linearPat(cx, pattern->ensureLinear(cx));
+    if (!linearPat)
+        return nullptr;
+
+    RootedLinearString linearStr(cx, string->ensureLinear(cx));
+    if (!linearStr)
+        return nullptr;
+
+    StringBuffer sb(cx);
+    if (linearStr->hasTwoByteChars()) {
+        if (!sb.ensureTwoByteChars())
+            return nullptr;
+        if (linearRepl->hasTwoByteChars()) {
+            if (!StrFlatReplaceGlobal<char16_t, char16_t>(cx, linearStr, linearPat, linearRepl, sb))
+                return nullptr;
+        } else {
+            if (!StrFlatReplaceGlobal<char16_t, Latin1Char>(cx, linearStr, linearPat, linearRepl, sb))
+                return nullptr;
+        }
+    } else {
+        if (linearRepl->hasTwoByteChars()) {
+            if (!sb.ensureTwoByteChars())
+                return nullptr;
+            if (!StrFlatReplaceGlobal<Latin1Char, char16_t>(cx, linearStr, linearPat, linearRepl, sb))
+                return nullptr;
+        } else {
+            if (!StrFlatReplaceGlobal<Latin1Char, Latin1Char>(cx, linearStr, linearPat, linearRepl, sb))
+                return nullptr;
+        }
+    }
+
+    JSString *str = sb.finishString();
+    if (!str)
+        return nullptr;
+
+    return str;
+}
+
 bool
 js::str_replace_string_raw(JSContext *cx, HandleString string, HandleString pattern,
-                          HandleString replacement, MutableHandleValue rval)
+                           HandleString replacement, MutableHandleValue rval)
 {
     ReplaceData rdata(cx);
 

js/src/jsstr.h

@@ -425,6 +425,10 @@ bool
 str_replace_regexp_raw(JSContext *cx, HandleString string, HandleObject regexp,
                        HandleString replacement, MutableHandleValue rval);
 
+JSString *
+str_flat_replace_string(JSContext *cx, HandleString string, HandleString pattern,
+                        HandleString replacement);
+
 bool
 str_replace_string_raw(JSContext *cx, HandleString string, HandleString pattern,
                        HandleString replacement, MutableHandleValue rval);