Open Bug 1388034 Opened 2 years ago Updated 2 years ago

Avoid extra string copies in RegExpGlobalReplaceOpt by returning the input string when no match was found

Categories

(Core :: JavaScript: Standard Library, defect, P3)

defect

Tracking

()

Tracking Status
firefox57 --- affected

People

(Reporter: anba, Unassigned)

References

(Blocks 1 open bug)

Details

(Keywords: perf, Whiteboard: [qf:p3])

I've added the following lines to RegExpGlobalReplaceOpt.h.js
---
     // Step 15.
     if (nextSourcePosition >= lengthS)
         return accumulatedResult;
 
+    if (nextSourcePosition === 0)
+        intrinsic_ReplaceWithoutAction();
+    else
+        intrinsic_ReplaceWithAction();
+
     // Step 16.
     return accumulatedResult + Substring(S, nextSourcePosition, lengthS - nextSourcePosition);
---

to count how often RegExp#[Symbol.replace] is called in Speedometer (single run of each framework from InteractiveRunner.html) when no match is actually found. Turns out this happens quite often:

(gdb) info br
Num     Type           Disp Enb Address            What
1       breakpoint     keep y   0x00007fffe9cbb580 in intrinsic_ReplaceWithoutAction(JSContext*, unsigned int, JS::Value*) at /home/andre/hg/mozilla-inbound/js/src/vm/SelfHosting.cpp:2146
        breakpoint already hit 63679 times
        ignore next 9936321 hits
2       breakpoint     keep y   0x00007fffe9cbb550 in intrinsic_ReplaceWithAction(JSContext*, unsigned int, JS::Value*) at /home/andre/hg/mozilla-inbound/js/src/vm/SelfHosting.cpp:2154
        breakpoint already hit 155 times
        ignore next 9999845 hits


That means when we inline the Substring(Kernel) call [1], we always create new strings in [2], even though we could simply return the input string instead.

[1] http://searchfox.org/mozilla-central/rev/b52285fffc13f36eca6b47de735d4e4403b3859e/js/src/builtin/RegExpGlobalReplaceOpt.h.js#129
[2] http://searchfox.org/mozilla-central/rev/b52285fffc13f36eca6b47de735d4e4403b3859e/js/src/jit/CodeGenerator.cpp#7718,7760





When measuring all calls to intrinsic_SubstringKernel (after disabling its inlining) in Speedometer, it seems like most calls to Substring(Kernel) which could be optimized by returning the input string come from RegExp#[Symbol.replace]:

Num     Type           Disp Enb Address            What
1       breakpoint     keep y   0x00007fffe9cb87f0 in IncreaseSameLength() at /home/andre/hg/mozilla-inbound/js/src/vm/SelfHosting.cpp:258
        breakpoint already hit 69913 times
        ignore next 99930087 hits
2       breakpoint     keep y   0x00007fffe9cb8800 in IncreaseDifferentLength() at /home/andre/hg/mozilla-inbound/js/src/vm/SelfHosting.cpp:261
        breakpoint already hit 22631 times
        ignore next 99977369 hits


Patch snippets to count calls to SubstringKernel:
---
+static size_t sameLength = 0;
+static size_t differentLength = 0;
+
+static MOZ_NEVER_INLINE void
+IncreaseSameLength() { sameLength++; }
+
+static MOZ_NEVER_INLINE void
+IncreaseDifferentLength() { differentLength++; }
+
 static bool
 intrinsic_SubstringKernel(JSContext* cx, unsigned argc, Value* vp)
 {
     CallArgs args = CallArgsFromVp(argc, vp);
     MOZ_ASSERT(args[0].isString());
     MOZ_ASSERT(args[1].isInt32());
     MOZ_ASSERT(args[2].isInt32());
 
     RootedString str(cx, args[0].toString());
     int32_t begin = args[1].toInt32();
     int32_t length = args[2].toInt32();
 
+    if (length - begin == str->length())
+        IncreaseSameLength();
+    else
+        IncreaseDifferentLength();

....


-    JS_INLINABLE_FN("SubstringKernel", intrinsic_SubstringKernel,       3,0,
-                    IntrinsicSubstringKernel),
+    JS_FN("SubstringKernel", intrinsic_SubstringKernel,       3,0),
---
Priority: -- → P3
Whiteboard: [qf:p3]
Keywords: perf
You need to log in before you can comment on or make changes to this bug.