From dfe77607020d7ac41bd976a0b376fd3f03bfb6a8 Mon Sep 17 00:00:00 2001 From: wuggy Date: Mon, 7 Sep 2026 13:38:33 -0700 Subject: [PATCH] Expand number-to-string cache to retain interleaved results --- js/src/devtools/number-to-string-bench.js | 61 +++++++++++++++++++++++ js/src/jsapi-tests/testIndexToString.cpp | 50 +++++++++++++++++++ js/src/jscompartment.cpp | 4 +- js/src/jscompartment.h | 46 +++++++++++------ 4 files changed, 146 insertions(+), 15 deletions(-) create mode 100644 js/src/devtools/number-to-string-bench.js diff --git a/js/src/devtools/number-to-string-bench.js b/js/src/devtools/number-to-string-bench.js new file mode 100644 index 0000000000..957183d31d --- /dev/null +++ b/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); +})(); diff --git a/js/src/jsapi-tests/testIndexToString.cpp b/js/src/jsapi-tests/testIndexToString.cpp index 0c91d98873..58c854054b 100644 --- a/js/src/jsapi-tests/testIndexToString.cpp +++ b/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(cx, 1234.5)); + CHECK(first); + JS::RootedString second(cx, js::NumberToString(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(cx, 1234.5) == first); + CHECK(js::NumberToString(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++) { diff --git a/js/src/jscompartment.cpp b/js/src/jscompartment.cpp index 926c7e3c86..6778dd2ebe 100644 --- a/js/src/jscompartment.cpp +++ b/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 { diff --git a/js/src/jscompartment.h b/js/src/jscompartment.h index a02b39301d..dd356aa41a 100644 --- a/js/src/jscompartment.h +++ b/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