Expand number-to-string cache to retain interleaved results

js/src/devtools/number-to-string-bench.js

@@ -0,0 +1,61 @@
+// Run with: js number-to-string-bench.js
+// Also supports: xpcshell -f number-to-string-bench.js
+// Compare identical optimized builds, alternating baseline and patched runs.
+// Reports milliseconds (lower is better); this is not a browser-suite score.
+
+(function() {
+    "use strict";
+    var iterations = 500000;
+    var samples = 7;
+    var checksum = 0;
+
+    function convert(values, radix, count) {
+        var total = 0;
+        for (var i = 0; i < count; i++)
+            total += values[i % values.length].toString(radix).length;
+        return total;
+    }
+
+    function unique(count) {
+        var total = 0;
+        for (var i = 0; i < count; i++)
+            total += (123456.125 + i).toString().length;
+        return total;
+    }
+
+    function measure(name, run) {
+        checksum += run(20000);
+        var times = [];
+        var expected = run(iterations);
+        for (var sample = 0; sample < samples; sample++) {
+            if (typeof gc === "function")
+                gc();
+            var start = Date.now();
+            var result = run(iterations);
+            times.push(Date.now() - start);
+            if (result !== expected)
+                throw new Error("inconsistent conversion: " + name);
+            checksum += result;
+        }
+        times.sort(function(a, b) { return a - b; });
+        print(name + ": median=" + times[3] + " ms; samples=" + times.join(","));
+    }
+
+    for (var size of [1, 2, 4, 5, 16]) {
+        var values = [];
+        for (var i = 0; i < size; i++)
+            values.push(123456.125 + i);
+        measure("decimal working set " + size, function(count) {
+            return convert(values, 10, count);
+        });
+    }
+    measure("unique decimals", unique);
+    var integers = [123456, 654321, 123457, 654322];
+    measure("integer working set 4", function(count) {
+        return convert(integers, 10, count);
+    });
+    measure("hexadecimal working set 4", function(count) {
+        return convert(integers, 16, count);
+    });
+    print("checksum=" + checksum);
+})();

js/src/jsapi-tests/testIndexToString.cpp

@@ -66,6 +66,56 @@ BEGIN_TEST(testIndexToString)
 }
 END_TEST(testIndexToString)
 
+BEGIN_TEST(testDtoaCacheInterleaved)
+{
+    JS::RootedString first(cx, js::NumberToString<js::CanGC>(cx, 1234.5));
+    CHECK(first);
+    JS::RootedString second(cx, js::NumberToString<js::CanGC>(cx, 6789.5));
+    CHECK(second);
+    JS::RootedString third(cx, js::IndexToString(cx, 123456));
+    CHECK(third);
+    JS::RootedString fourth(cx, js::IndexToString(cx, 654321));
+    CHECK(fourth);
+
+    for (size_t i = 0; i < 10; i++) {
+        CHECK(js::NumberToString<js::CanGC>(cx, 1234.5) == first);
+        CHECK(js::NumberToString<js::CanGC>(cx, 6789.5) == second);
+        CHECK(js::IndexToString(cx, 123456) == third);
+        CHECK(js::IndexToString(cx, 654321) == fourth);
+    }
+
+    // Every raw string pointer must be invalidated, not just the latest one.
+    JS_GC(cx);
+    CHECK(!cx->compartment()->dtoaCache.lookup(10, 1234.5));
+    CHECK(!cx->compartment()->dtoaCache.lookup(10, 6789.5));
+    CHECK(!cx->compartment()->dtoaCache.lookup(10, 123456));
+    CHECK(!cx->compartment()->dtoaCache.lookup(10, 654321));
+
+    // The radix is part of the key. Signed zero can share its string.
+    js::DtoaCache cache;
+    cache.cache(10, 0.0, &first->asFlat());
+    cache.cache(16, 0.0, &second->asFlat());
+    CHECK(cache.lookup(10, -0.0) == first);
+    CHECK(cache.lookup(16, -0.0) == second);
+    CHECK(!cache.lookup(2, 0.0));
+    cache.purge();
+    CHECK(!cache.lookup(10, 0.0));
+    CHECK(!cache.lookup(16, 0.0));
+
+    for (size_t i = 0; i < 12; i++)
+        cache.cache(10, double(i), &first->asFlat());
+    for (size_t i = 0; i < 8; i++)
+        CHECK(!cache.lookup(10, double(i)));
+    for (size_t i = 8; i < 12; i++)
+        CHECK(cache.lookup(10, double(i)) == first);
+
+    // A failed string allocation must never turn into a cache hit.
+    cache.cache(10, 12.0, nullptr);
+    CHECK(!cache.lookup(10, 12.0));
+    return true;
+}
+END_TEST(testDtoaCacheInterleaved)
+
 BEGIN_TEST(testStringIsIndex)
 {
     for (size_t i = 0, sz = ArrayLength(tests); i < sz; i++) {

js/src/jscompartment.cpp

@@ -214,7 +214,9 @@ JSCompartment::ensureJitCompartmentExists(JSContext* cx)
 void
 js::DtoaCache::checkCacheAfterMovingGC()
 {
-    MOZ_ASSERT(!s || !IsForwarded(s));
+    MOZ_ASSERT(!recent.s || !IsForwarded(recent.s));
+    for (const auto& entry : previous)
+        MOZ_ASSERT(!entry.s || !IsForwarded(entry.s));
 }
 
 namespace {

js/src/jscompartment.h

@@ -38,29 +38,47 @@ class ScriptSourceObject;
 struct NativeIterator;
 
 /*
- * A single-entry cache for some base-10 double-to-string conversions. This
- * helps date-format-xparb.js.  It also avoids skewing the results for
- * v8-splay.js when measured by the SunSpider harness, where the splay tree
- * initialization (which includes many repeated double-to-string conversions)
- * is erroneously included in the measurement; see bug 562553.
+ * A small cache for number-to-string conversions, keyed by number and radix.
+ * Keep several results so interleaved conversions do not evict each other.
+ * These strings are not traced: every entry must be cleared before GC.
  */
 class DtoaCache {
-    double       d;
-    int          base;
-    JSFlatString* s;      // if s==nullptr, d and base are not valid
+    struct Entry {
+        double d;
+        int base;
+        JSFlatString* s;  // if s == nullptr, d and base are not valid
+    };
+    static const size_t NumPrevious = 3;
+    Entry recent;
+    Entry previous[NumPrevious];
+    size_t next;
 
   public:
-    DtoaCache() : s(nullptr) {}
-    void purge() { s = nullptr; }
+    DtoaCache() { purge(); }
+    void purge() {
+        recent.s = nullptr;
+        for (auto& entry : previous)
+            entry.s = nullptr;
+        next = 0;
+    }
 
     JSFlatString* lookup(int base, double d) {
-        return this->s && base == this->base && d == this->d ? this->s : nullptr;
+        // Preserve the cheap path for consecutive conversions of one value.
+        if (recent.s && base == recent.base && d == recent.d)
+            return recent.s;
+        for (const auto& entry : previous) {
+            if (entry.s && base == entry.base && d == entry.d)
+                return entry.s;
+        }
+        return nullptr;
     }
 
     void cache(int base, double d, JSFlatString* s) {
-        this->base = base;
-        this->d = d;
-        this->s = s;
+        if (recent.s) {
+            previous[next] = recent;
+            next = (next + 1) % NumPrevious;
+        }
+        recent = { d, base, s };
     }
 
 #ifdef JSGC_HASH_TABLE_CHECKS