[JSC] ExpressionInfo: start a chapter every 256 words. BytecodeRewriter: keep running sums

JSTests/BUN-TEST-DIFFERENCES.md

@@ -234,6 +234,7 @@ Not upstream's: a sync neither brings nor changes them. Most are named `stress/b
 | `stress/inline-cache-stub-freed-during-call-to-custom-setter.js` | an inline cache stub that has called native code stays until the call returns, also when the callee signed the return address that it saved (`JITStubRoutineSet::mark`). The setter of `$vm`'s `customAccessor` is compiled with `pac-ret` on ARM64 Linux in every build. On a processor without pointer authentication nothing is signed and the test passes whatever the code does |
 | `stress/bun-return-address-of-function-is-signed.js` | with `ENABLE(RETURN_ADDRESS_SIGNING)` a function signs the return address that it saves in LLInt, Baseline, the DFG and the FTL alike: `$vm.returnAddressOfCallerIsSigned()` signs the address again and compares. Elsewhere, and on a processor without pointer authentication (`$vm.functionsSignReturnAddress()`), the test ends at once |
 | `stress/bun-return-address-with-wrong-signature-is-not-returned-to.js` | with `ENABLE(RETURN_ADDRESS_SIGNING)` a function does not return once a bit of the signature of its saved return address was flipped, in each of the four tiers: the run must crash. Where nothing is signed the test crashes by itself |
+| `stress/expression-info-chapters.js` | expression info starts a chapter every `--expressionInfoWordsBetweenChapters` words (256; upstream has the constant 10,000 and no option). With 256, 8 and 1 words, every throw of a function, a generator, an async function and an async generator reports its own line, column and text, also from a bytecode cache, and no lookup of a position steps over more than one chapter (`$vm.expressionInfoWordsDecoded()`) |
 | the fr-CA block of `stress/intl-date-pattern-includes-literal-text.js` (the file is upstream's) | keeps the file's coverage on CLDR 49, where fr's pattern no longer has literal text: an `h` inside literal text is not mistaken for the hour field |
 | the last block of `stress/regexp-test-sticky-first-char-filter.js` (the file is upstream's; nothing of upstream's text changes) | the first-character filter of a sticky `test()` comes from the first sets of the alternatives (`yarr/YarrFirstFollowSets.*`): behind a leading lookbehind, forward reference or lookahead, and in groups nested more than eight deep, a byte that passes the filter still reaches the matcher |
 

JSTests/stress/expression-info-chapters.js

@@ -0,0 +1,159 @@
+//@ runDefault
+//@ runDefault("--useBytecodeOptimizer=1", "--expressionInfoWordsBetweenChapters=8")
+//@ runDefault("--forceDebuggerBytecodeGeneration=1", "--expressionInfoWordsBetweenChapters=1")
+//@ runBytecodeCache("--useBytecodeOptimizer=1")
+//@ runBytecodeCache("--diskCachePayloadIsPersistentForTesting=1", "--expressionInfoWordsBetweenChapters=8")
+
+// Expression info starts a chapter every 256 words, and a lookup of a source position decodes from the start of one
+// chapter. So every throw of a function reports its own line, column and expression, and no lookup steps over more
+// words than a chapter has. A generator and an async function have their bytecode rewritten after the expression info
+// is recorded, which moves the instruction that each chapter starts at. With the option a chapter is short, and a
+// function of a few cases has many.
+
+function shouldBe(actual, expected, message) {
+    if (actual !== expected)
+        throw new Error(`${message}: expected ${expected} but got ${actual}`);
+}
+
+function shouldBeTrue(condition, message) {
+    if (!condition)
+        throw new Error(message);
+}
+
+function thrownBy(run) {
+    try {
+        run();
+    } catch (error) {
+        return error;
+    }
+    throw new Error("did not throw");
+}
+
+function rejectionOf(promise) {
+    let reason;
+    promise.then(() => { throw new Error("did not reject"); }, error => { reason = error; });
+    drainMicrotasks();
+    return reason;
+}
+
+function resultOf(promise) {
+    let result;
+    promise.then(value => { result = value; });
+    drainMicrotasks();
+    return result;
+}
+
+const wordsInAChapter = jscOptions().expressionInfoWordsBetweenChapters;
+// 256 without the option.
+shouldBeTrue([256, 8, 1].includes(wordsInAChapter), `${wordsInAChapter} words in a chapter`);
+const wordsInAnEntry = 8; // at most: a chapter ends with the entry that fills it
+
+// `error` is what case i throws, and `codeBlock` what case -1 returns: the code block with the cases, which for a
+// generator or an async function is not the one of the function itself.
+const kinds = {
+    "function": {
+        head: "function f(i, o)",
+        selector: "i",
+        receiver: "o",
+        pause: "",
+        error: (f, i) => thrownBy(() => f(i, {})),
+        codeBlock: f => f(-1, {}),
+    },
+    "generator": {
+        head: "function* f(i, o)",
+        selector: "i",
+        receiver: "o",
+        pause: "yield 0;",
+        error: (f, i) => thrownBy(() => { let iterator = f(i, {}); iterator.next(); iterator.next(); }),
+        codeBlock: f => f(-1, {}).next().value,
+    },
+    // Behind the `yield` of a case nothing is in a register, so the rewrite saves nothing there and leaves less
+    // bytecode than it found: what a chapter behind it starts at moves down.
+    "generator that saves nothing": {
+        head: "function* f()",
+        selector: "yield",
+        receiver: "this",
+        pause: "yield 0;",
+        error: (f, i) => thrownBy(() => { let iterator = f.call({}); iterator.next(); iterator.next(i); iterator.next(); }),
+        codeBlock: f => { let iterator = f.call({}); iterator.next(); return iterator.next(-1).value; },
+    },
+    "async function": {
+        head: "async function f(i, o)",
+        selector: "i",
+        receiver: "o",
+        pause: "await 0;",
+        error: (f, i) => rejectionOf(f(i, {})),
+        codeBlock: f => resultOf(f(-1, {})),
+    },
+    "async generator": {
+        head: "async function* f(i, o)",
+        selector: "i",
+        receiver: "o",
+        pause: "yield 0; await 0;",
+        error: (f, i) => { let iterator = f(i, {}); iterator.next(); return rejectionOf(iterator.next()); },
+        codeBlock: f => resultOf(f(-1, {}).next()).value,
+    },
+};
+
+// Case i throws a TypeError: an even case where it reads an index of undefined, an odd case where it calls
+// undefined. `expected[i]` is its line, column and message: the column of an index is its `[`, of a call its `(`.
+let functionCount = 0;
+function makeFunction(kind, count) {
+    let { head, selector, receiver, pause } = kinds[kind];
+    // A name of its own: functions with the same text share their code, and with it the positions asked for before.
+    let lines = [`(${head.replace("f(", `f${functionCount++}(`)} {`, `switch (${selector}) {`, "case -1: return $vm.codeBlockForFrame(0);"];
+    let expected = [];
+    for (let i = 0; i < count; i++) {
+        let id = String(i).padStart(5, "0");
+        let [site, divot, message] = i % 2
+            ? [`${receiver}["n${id}"](1)`, "(1)", `${receiver}["n${id}"] is not a function. (In '${receiver}["n${id}"](1)', '${receiver}["n${id}"]' is undefined)`]
+            : [`${receiver}.x[${i}]`, `[${i}]`, `undefined is not an object (evaluating '${receiver}.x[${i}]')`];
+        let line = `case ${i}: ${pause} ${receiver}.a; ${receiver}.b; return ${site};`;
+        lines.push(line);
+        expected.push(`${lines.length}:${line.indexOf(divot) + 1} ${message}`);
+    }
+    lines.push("}", "})");
+    // Not eval: a bytecode cache does not hold the code of an eval.
+    return { f: loadString(lines.join("\n")), expected };
+}
+
+function check(kind, count) {
+    let message = `${kind} with ${count} cases`;
+    let { f, expected } = makeFunction(kind, count);
+    let codeBlock = kinds[kind].codeBlock(f);
+
+    // A chapter starts behind wordsInAChapter words or more of the one before it. The first chapter has no start.
+    let { words, chapters } = $vm.expressionInfoStats(codeBlock);
+    shouldBeTrue(words >= chapters * wordsInAChapter, `${message}: ${chapters} chapter starts in ${words} words`);
+    shouldBeTrue(words < (chapters + 1) * (wordsInAChapter + wordsInAnEntry), `${message}: ${chapters} chapter starts in ${words} words`);
+
+    for (let i = 0; i < count; i++) {
+        let error = kinds[kind].error(f, i);
+        shouldBe(`${error.line}:${error.column} ${error.message}`, expected[i], `${message}, case ${i}`);
+    }
+
+    // Every offset of the bytecode, not only the instructions that throw. Where that is a lot of decoding, every
+    // eighth.
+    let step = chapters && wordsInAChapter > 8 ? 8 : 1;
+    let most = 0;
+    for (let offset = 0; ; offset += step) {
+        let decoded = $vm.expressionInfoWordsDecoded(codeBlock, offset);
+        if (decoded === undefined)
+            break;
+        shouldBeTrue(decoded < wordsInAChapter + wordsInAnEntry, `${message}: the lookup of offset ${offset} decodes ${decoded} words`);
+        most = Math.max(most, decoded);
+    }
+    // The lookup of the last entry of a chapter decodes all of that chapter, or all but that entry.
+    if (step === 1)
+        shouldBeTrue(most >= words / (chapters + 1) - wordsInAnEntry, `${message}: no lookup decodes more than ${most} of ${words} words in ${chapters + 1} chapters`);
+
+    return { words, chapters };
+}
+
+for (let kind in kinds) {
+    let { words, chapters } = check(kind, 24);
+    // Without the option that is about a chapter of words: as many cases as make three.
+    if (chapters < 2)
+        ({ chapters } = check(kind, Math.ceil(3 * wordsInAChapter * 24 / words)));
+    shouldBeTrue(chapters >= 2, `${kind}: ${chapters} chapter starts`);
+}

Source/JavaScriptCore/bytecode/BytecodeRewriter.cpp

@@ -59,6 +59,16 @@ void BytecodeRewriter::execute()
         return lhs.index < rhs.index;
     });
 
+    // Every jump, exception handler and chapter of expression info asks calculateDifference() about a range of these.
+    int difference = 0;
+    for (auto& insertion : m_insertions) {
+        if (insertion.type == Insertion::Type::Remove)
+            difference -= insertion.length();
+        else
+            difference += insertion.length();
+        insertion.differenceThrough = difference;
+    }
+
     m_codeBlock->applyModification(*this);
 }
 
@@ -83,6 +93,7 @@ void BytecodeRewriter::insertImpl(InsertionPoint insertionPoint, IncludeBranch i
         Insertion::Type::Insert,
         includeBranch,
         0,
+        0,
         WTF::move(writer)
     });
 }

Source/JavaScriptCore/bytecode/BytecodeRewriter.h

@@ -130,6 +130,8 @@ WTF_MAKE_NONCOPYABLE(BytecodeRewriter);
         InsertionPoint index;
         Type type;
         IncludeBranch includeBranch;
+        // What this Insertion and every one in front of it add to the bytecode, less what they remove. execute() sets it.
+        int differenceThrough;
         size_t removeLength;
         JSInstructionStreamWriter instructions;
     };
@@ -193,7 +195,7 @@ WTF_MAKE_NONCOPYABLE(BytecodeRewriter);
     template<class Function>
     void replaceBytecodeWithFragment(const JSInstructionStream::Ref& instruction, Function function)
     {
-        m_insertions.append(Insertion { InsertionPoint(instruction.offset(), Position::OriginalBytecodePoint), Insertion::Type::Remove, IncludeBranch::No, instruction->size(), { } });
+        m_insertions.append(Insertion { InsertionPoint(instruction.offset(), Position::OriginalBytecodePoint), Insertion::Type::Remove, IncludeBranch::No, 0, instruction->size(), { } });
         insertFragmentAfter(instruction, function);
     }
 
@@ -236,14 +238,11 @@ WTF_MAKE_NONCOPYABLE(BytecodeRewriter);
 template<typename Iterator>
 inline int BytecodeRewriter::calculateDifference(Iterator begin, Iterator end)
 {
-    int result = 0;
-    for (; begin != end; ++begin) {
-        if (begin->type == Insertion::Type::Remove)
-            result -= begin->length();
-        else
-            result += begin->length();
-    }
-    return result;
+    ASSERT(begin <= end);
+    if (begin == end)
+        return 0;
+    int differenceBeforeBegin = begin == m_insertions.begin() ? 0 : (begin - 1)->differenceThrough;
+    return (end - 1)->differenceThrough - differenceBeforeBegin;
 }
 
 template<typename Func>

Source/JavaScriptCore/bytecode/ExpressionInfo.cpp

@@ -26,6 +26,7 @@
 #include "config.h"
 #include "ExpressionInfo.h"
 
+#include "Options.h"
 #include "SourceProvider.h"
 #include "VM.h"
 #include <numeric>
@@ -151,7 +152,7 @@ namespace JSC {
     EncodedInfo vector is large.
 
     To mitigate against this, we break the EncodedInfo regions into Chapters every
-    numberOfWordsBetweenChapters (currently 10000) words or so. A Chapter is always started
+    Options::expressionInfoWordsBetweenChapters() (256) words or so. A Chapter is always started
     with an AbsInstPC (Absolute Instruction PC) word. Unlike other EncodedInfo words with defines
     delta values for fields to add, AbsInstPC defines an absolute value (currently 26 bits) for
     the InstPC to mark the start of the Chapter. If 26-bits isn't enough, we can always add
@@ -161,7 +162,11 @@ namespace JSC {
     clears to 0 the cummulative delta for all the other fields. As a result, we can always jump
     to the start of a Chapter and start decoding from there (instead of starting from the start
     of the EncodedInfo vector). It limits the number of words that we need to decode to around
-    numberOfWordsBetweenChapters.
+    that many.
+
+    A Chapter is 16 bytes in most functions: its Chapter info, its AbsInstPC word, and a Wide
+    word for the Divot, whose delta starts over from 0. Every ExpressionInfo with more words
+    than one Chapter holds pays that, looked up or not, which is why a Chapter is not shorter.
 
     Now, all we need to know is which Chapter a given InstPC falls into.
 
@@ -172,7 +177,7 @@ namespace JSC {
 
     The first Chapter always starts at InstPC 0 and EncodedInfo index 0. Hence, we do not emit
     a Chapter entry for the 1st Chapter. Hence, for most small functions that do not have
-    more than numberOfWordsBetweenChapters EncodedInfo, there is no Chapter info in the
+    more than expressionInfoWordsBetweenChapters EncodedInfo, there is no Chapter info in the
     ExpressionInfo.
 
     Additional Compression Details (of Basic word fields)
@@ -491,7 +496,7 @@ void ExpressionInfo::Encoder::encode(InstPC instPC, unsigned divot, unsigned sta
 
     unsigned currentEncodedInfoIndex = m_expressionInfoEncodedInfo.size();
     unsigned chapterSize = currentEncodedInfoIndex - m_currentChapterStartIndex;
-    if (chapterSize >= numberOfWordsBetweenChapters) {
+    if (chapterSize >= Options::expressionInfoWordsBetweenChapters()) {
         m_expressionInfoChapters.append({ instPC, currentEncodedInfoIndex });
         m_currentChapterStartIndex = currentEncodedInfoIndex;
         unsigned absInstPC = std::min(instPC, maxSingleValue);
@@ -908,6 +913,17 @@ auto ExpressionInfo::decodeEntryForInstPC(InstPC instPC) -> Entry
     return decoder.entry();
 }
 
+unsigned ExpressionInfo::numberOfEncodedInfoDecodedForInstPC(InstPC instPC)
+{
+    Decoder decoder(*this);
+
+    auto* chapterStart = findChapterEncodedInfoJustBelow(instPC);
+    decoder.setNextInfo(chapterStart);
+    while (decoder.decode(instPC) != IterationStatus::Done) { }
+    // Done leaves currentInfo() at the entry that decode() did not keep, or at the end.
+    return decoder.currentInfo() - chapterStart;
+}
+
 LineColumn ExpressionInfo::lineColumnInTextForInstPC(InstPC instPC, SourceProvider& provider, unsigned sourceOffset)
 {
     return m_cachedLineColumns.ensure(instPC, [&] {

Source/JavaScriptCore/bytecode/ExpressionInfo.h

@@ -191,6 +191,9 @@ class ExpressionInfo {
     Entry entryForInstPC(const ConcurrentJSLocker&, InstPC);
     // Decodes without touching the cache: takes no lock and does not allocate (UnlinkedCodeBlock::expressionInfoIfDecoded).
     Entry LEGACY_NODELETE decodeEntryForInstPC(InstPC);
+    // For tests: how many EncodedInfo words decodeEntryForInstPC() steps over, from the start of the chapter to the
+    // entry behind the one that it returns.
+    unsigned LEGACY_NODELETE numberOfEncodedInfoDecodedForInstPC(InstPC);
 
     // The zero-based line and column of the instruction's divot in the text of its source, where this code starts at
     // sourceOffset. Decoding an entry starts from the start of its chapter, and stack traces ask for the
@@ -202,6 +205,8 @@ class ExpressionInfo {
     LineColumn lineColumnInTextForInstPC(InstPC, SourceProvider&, unsigned sourceOffset);
 
     bool isEmpty() const { return !m_numberOfEncodedInfo; };
+    unsigned numberOfEncodedInfo() const { return m_numberOfEncodedInfo; }
+    unsigned numberOfChapters() const { return m_numberOfChapters; }
     size_t LEGACY_NODELETE byteSize() const; // owned by this object
     size_t LEGACY_NODELETE byteSizeForGCPacing() const; // what a generated (non-borrowed) one this size would own
 
@@ -328,8 +333,6 @@ class ExpressionInfo {
     static constexpr unsigned multiSizeShift = firstFieldIDShift - multiSizeBits;
     static constexpr unsigned multiFirstFieldShift = multiSizeShift - fieldIDBits;
 
-    static constexpr unsigned numberOfWordsBetweenChapters = 10000;
-
     using LineColumnMap = UncheckedKeyHashMap<InstPC, LineColumn, WTF::IntHash<InstPC>, WTF::UnsignedWithZeroKeyHashTraits<InstPC>>;
 
     UncheckedKeyHashMap<InstPC, Entry, WTF::IntHash<InstPC>, WTF::UnsignedWithZeroKeyHashTraits<InstPC>> m_cachedEntries;

Source/JavaScriptCore/runtime/OptionsList.h

@@ -183,6 +183,7 @@ bool hasCapacityToUseLargeGigacage();
     v(Bool, validateBytecodeOptimizerStaticScopes, false, Normal, "check statically resolved scopes against JSScope::abstractResolve when linking") \
     v(Bool, dumpBytecodeOptimizer, false, Normal, "dump the bytecode optimizer IR") \
     v(Bool, reportBytecodeOptimizer, false, Normal, "report per-code-block bytecode optimizer statistics") \
+    v(Unsigned, expressionInfoWordsBetweenChapters, 256, Restricted, "How many words of expression info a chapter has before the next one starts. A lookup of a source position decodes from the start of a chapter. Tests lower it so that a small function has chapters: every lookup gives the same answer."_s) \
     v(Bool, dumpBytecodeLivenessResults, false, Normal, nullptr) \
     v(Bool, validateBytecode, false, Normal, nullptr) \
     v(Bool, forceDebuggerBytecodeGeneration, false, Normal, nullptr) \