From b87311a7cd5bbfc9779ffd42ea5fbf5db7a56adb Mon Sep 17 00:00:00 2001 From: fisker Cheung Date: Tue, 4 Nov 2025 03:54:35 +0800 Subject: [PATCH] Move cursor tracking out of the doc print buffer (#18202) Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> --- benchmarks/print-doc-to-string.js | 33 +++++ benchmarks/string-buffer-compress.js | 51 +++++++ benchmarks/string-concatenation.js | 30 ++++ benchmarks/trim-indentation.js | 25 ++++ benchmarks/utilities.js | 63 ++++++++ eslint.config.js | 6 +- knip.config.js | 3 +- src/document/printer/printer.js | 176 ++++++++++------------- src/document/printer/trim-indentation.js | 41 ++++++ tests/unit/doc-builders.js | 3 + 10 files changed, 325 insertions(+), 106 deletions(-) create mode 100644 benchmarks/print-doc-to-string.js create mode 100644 benchmarks/string-buffer-compress.js create mode 100644 benchmarks/string-concatenation.js create mode 100644 benchmarks/trim-indentation.js create mode 100644 benchmarks/utilities.js create mode 100644 src/document/printer/trim-indentation.js diff --git a/benchmarks/print-doc-to-string.js b/benchmarks/print-doc-to-string.js new file mode 100644 index 000000000..0d7f1dbb3 --- /dev/null +++ b/benchmarks/print-doc-to-string.js @@ -0,0 +1,33 @@ +import assert from "node:assert/strict"; +import fs from "node:fs/promises"; +import * as prettierProduction from "../node_modules/prettier/index.mjs"; +import * as prettierDevelopment from "../src/index.js"; +import { runBenchmark } from "./utilities.js"; + +assert.notEqual(prettierProduction.version, prettierDevelopment.version); + +const text = await fs.readFile(new URL(import.meta.url), "utf8"); +// @ts-expect-error -- No types +const doc = await prettierDevelopment.__debug.printToDoc(text, { + parser: "babel", + cursorOffset: text.indexOf("cursorOffset"), +}); + +const run = (prettier) => + prettier.doc.printer.printDocToString(doc, { printWidth: 80, tabWidth: 2 }); + +const expected = await run(prettierProduction); + +await runBenchmark( + { + name: "printDocToString", + assert: (result) => assert.deepEqual(result, expected), + }, + [ + { name: "Development", prettier: prettierProduction }, + { name: "Production", prettier: prettierProduction }, + ].map(({ name, prettier }) => ({ + name, + implementation: () => run(prettier), + })), +); diff --git a/benchmarks/string-buffer-compress.js b/benchmarks/string-buffer-compress.js new file mode 100644 index 000000000..d0e7d610a --- /dev/null +++ b/benchmarks/string-buffer-compress.js @@ -0,0 +1,51 @@ +import assert from "node:assert/strict"; +import { runBenchmark } from "./utilities.js"; + +for (const size of [1e1, 1e2, 1e3, 1e4, 1e5]) { + const maxLength = 10; + const seperator = "|"; + const strings = Array.from({ length: size }, () => "_"); + const expected = { + length: strings.join(seperator).length, + times: Math.floor(size / maxLength) - 1, + }; + + await runBenchmark( + { + name: `String buffer compress (${size} strings)`, + assert: (result) => assert.deepEqual(result, expected), + }, + { + "Array#splice()"() { + const result = []; + let times = 0; + + for (let index = 0; index < strings.length; index++) { + result.push(strings[index]); + if (result.length > maxLength) { + result.splice(0, Number.POSITIVE_INFINITY, result.join(seperator)); + assert.equal(result.length, 1); + times++; + } + } + + return { length: result.join(seperator).length, times }; + }, + "let reassign"() { + let result = []; + let times = 0; + + for (let index = 0; index < strings.length; index++) { + result.push(strings[index]); + if (result.length > maxLength) { + result = [result.join(seperator)]; + assert.equal(result.length, 1); + times++; + } + } + + return { length: result.join(seperator).length, times }; + }, + }, + ); +} diff --git a/benchmarks/string-concatenation.js b/benchmarks/string-concatenation.js new file mode 100644 index 000000000..619556f78 --- /dev/null +++ b/benchmarks/string-concatenation.js @@ -0,0 +1,30 @@ +import assert from "node:assert/strict"; +import { runBenchmark } from "./utilities.js"; + +for (const size of [1e1, 1e2, 1e3, 1e4, 1e5]) { + const strings = Array.from({ length: size }, () => "_"); + const expected = strings.join(""); + + await runBenchmark( + { + name: `String concatentation (${size} strings)`, + assert: (result) => assert.equal(result, expected), + }, + { + "Array#join()"() { + const result = []; + for (const string of strings) { + result.push(string); + } + return result.join(""); + }, + "string +="() { + let result = ""; + for (const string of strings) { + result += string; + } + return result; + }, + }, + ); +} diff --git a/benchmarks/trim-indentation.js b/benchmarks/trim-indentation.js new file mode 100644 index 000000000..c1a042c0c --- /dev/null +++ b/benchmarks/trim-indentation.js @@ -0,0 +1,25 @@ +import assert from "node:assert/strict"; +import { trimTrailingIndentation } from "../src/document/printer/trim-indentation.js"; +import { runBenchmark } from "./utilities.js"; + +for (const size of [1, 1e1, 1e2, 1e3]) { + const trimmed = "foo".repeat(size / 2); + const text = trimmed + " \t".repeat(size / 2); + + await runBenchmark( + { + name: `Trim indentation (${size} spaces)`, + assert: (result) => assert.equal(result, trimmed), + }, + { + loop: () => trimTrailingIndentation(text), + "RegExp (inline)": () => text.replace(/[ \t]*$/u, ""), + "RegExp (stored)": ( + (regexp) => () => + text.replace(regexp, "") + )(/[ \t]*$/gu), + // eslint-disable-next-line require-unicode-regexp + "RegExp (no `u` flag)": () => text.replace(/[ \t]*$/, ""), + }, + ); +} diff --git a/benchmarks/utilities.js b/benchmarks/utilities.js new file mode 100644 index 000000000..e54f20687 --- /dev/null +++ b/benchmarks/utilities.js @@ -0,0 +1,63 @@ +import { Bench } from "tinybench"; + +/** +@import {BenchOptions} from "tinybench"; +@typedef { + | { + name: string, + implementation: () => any + }[] + | Record any> +} Implementations +*/ + +/** +@param { + | string + | { + name: string, + assert?: (result: any) => void, + } & BenchOptions +} options +@param {Implementations} implementations +*/ +async function runBenchmark(options, implementations) { + if (typeof options === "string") { + options = { name: options }; + } + + const { assert, ...benchOptions } = options; + + const bench = new Bench(benchOptions); + + let error; + + for (const { name, implementation } of Array.isArray(implementations) + ? implementations + : Object.entries(implementations).map(([name, implementation]) => ({ + name, + implementation, + }))) { + bench.add(name, () => { + try { + const result = implementation(); + assert?.(result); + } catch (assertionError) { + error ??= Object.assign(assertionError, { + case: `${benchOptions.name} >> ${name}`, + }); + } + }); + } + + await bench.run(); + + console.log(bench.name); + console.table(bench.table()); + + if (error) { + throw error; + } +} + +export { runBenchmark }; diff --git a/eslint.config.js b/eslint.config.js index 235a79340..4cadfb36e 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -308,7 +308,11 @@ const configs = [ }, }, { - files: ["scripts/**/*", "tests/config/install-prettier.js"], + files: [ + "scripts/**/*", + "benchmarks/**", + "tests/config/install-prettier.js", + ], rules: { "no-console": "off", }, diff --git a/knip.config.js b/knip.config.js index ee96b4c8b..4b75ae489 100644 --- a/knip.config.js +++ b/knip.config.js @@ -15,8 +15,9 @@ const config = { "src/experimental-cli/index.js", "packages/plugin-oxc/index.js", "packages/plugin-hermes/index.js", + "benchmarks/**", ], - project: ["src/**", "scripts/**"], + project: ["src/**", "scripts/**", "benchmark/**"], ignore: [ "scripts/build/config.js", "scripts/build/build-javascript-module.js", diff --git a/src/document/printer/printer.js b/src/document/printer/printer.js index 16e9b3e0d..33e265c7d 100644 --- a/src/document/printer/printer.js +++ b/src/document/printer/printer.js @@ -22,6 +22,7 @@ import { import { getDocType, propagateBreaks } from "../utilities/index.js"; import InvalidDocError from "../utilities/invalid-doc-error.js"; import { makeAlign, makeIndent, ROOT_INDENT } from "./indent.js"; +import { trimIndentation } from "./trim-indentation.js"; /** @import {EndOfLineOption} from "../../common/end-of-line.js"; @@ -37,53 +38,8 @@ const MODE_BREAK = Symbol("MODE_BREAK"); /** @type {unique symbol} */ const MODE_FLAT = Symbol("MODE_FLAT"); -const CURSOR_PLACEHOLDER = Symbol("cursor"); - const DOC_FILL_PRINTED_LENGTH = Symbol("DOC_FILL_PRINTED_LENGTH"); -// Trim `Tab(U+0009)` and `Space(U+0020)` at the end of line -function trim(out) { - let trimCount = 0; - let cursorCount = 0; - let outIndex = out.length; - - outer: while (outIndex--) { - const last = out[outIndex]; - - if (last === CURSOR_PLACEHOLDER) { - cursorCount++; - continue; - } - - /* c8 ignore next 3 */ - if (process.env.NODE_ENV !== "production" && typeof last !== "string") { - throw new Error(`Unexpected value in trim: '${typeof last}'`); - } - - // Not using a regexp here because regexps for trimming off trailing - // characters are known to have performance issues. - for (let charIndex = last.length - 1; charIndex >= 0; charIndex--) { - const char = last[charIndex]; - if (char === " " || char === "\t") { - trimCount++; - } else { - out[outIndex] = last.slice(0, charIndex + 1); - break outer; - } - } - } - - if (trimCount > 0 || cursorCount > 0) { - out.length = outIndex + 1; - - while (cursorCount-- > 0) { - out.push(CURSOR_PLACEHOLDER); - } - } - - return trimCount; -} - /** * @param {Command} next * @param {Command[]} restCommands @@ -149,7 +105,7 @@ function fits( break; case DOC_TYPE_TRIM: - width += trim(out); + width += trimIndentation(out).count; break; case DOC_TYPE_GROUP: { @@ -222,11 +178,12 @@ function printDocToString(doc, options) { // commands to the array instead of recursively calling `print`. /** @type Command[] */ const commands = [{ indent: ROOT_INDENT, mode: MODE_BREAK, doc }]; - const out = []; + /** @type string[] */ + let out = []; let shouldRemeasure = false; /** @type Command[] */ const lineSuffix = []; - let printedCursorCount = 0; + const cursorPositions = []; propagateBreaks(doc); @@ -252,13 +209,15 @@ function printDocToString(doc, options) { } break; - case DOC_TYPE_CURSOR: - if (printedCursorCount >= 2) { + case DOC_TYPE_CURSOR: { + if (cursorPositions.length >= 2) { throw new Error("There are too many 'cursor' in doc."); } - out.push(CURSOR_PLACEHOLDER); - printedCursorCount++; + const text = out.join(""); + out = [text]; + cursorPositions.push(text.length - 1); break; + } case DOC_TYPE_INDENT: commands.push({ @@ -277,7 +236,7 @@ function printDocToString(doc, options) { break; case DOC_TYPE_TRIM: - position -= trim(out); + trim(); break; case DOC_TYPE_GROUP: @@ -563,7 +522,7 @@ function printDocToString(doc, options) { position = indent.root.length; } } else { - position -= trim(out); + trim(); out.push(newLine + indent.value); position = indent.length; } @@ -591,58 +550,67 @@ function printDocToString(doc, options) { } } - const cursorPlaceholderIndex = out.indexOf(CURSOR_PLACEHOLDER); - if (cursorPlaceholderIndex !== -1) { - const otherCursorPlaceholderIndex = out.indexOf( - CURSOR_PLACEHOLDER, - cursorPlaceholderIndex + 1, - ); - - if (otherCursorPlaceholderIndex === -1) { - // If we got here, the doc must have contained ONE cursor command, - // instead of the expected zero or two. If the doc being printed was - // returned by printAstToDoc, then the only ways this can have happened - // are if: - // 1. a plugin added a cursor command itself, or - // 2. one (but not both) of options.nodeAfterCursor and - // options.nodeAfterCursor pointed to a node within a subtree of the - // AST that the printer plugin used in printAstToDoc simply omits from - // the doc, or that it prints without recursively calling mainPrint, - // with the consequence that the logic for adding a cursor command in - // callPluginPrintFunction was never called for that node. - // These are both weird scenarios that should be considered a bug if they - // ever occur with one of Prettier's built-in plugins. If a third-party - // plugin was used when printing the AST to a doc, the possibility of - // reaching this scenario MIGHT be reasonable to consider a bug in the - // plugin. However, we try to at least not crash if this ever happens; - // instead we simply give up on returning a cursorNodeStart or - // cursorNodeText. - // - // coreFormat has logic specifically to handle this scenario - where it - // is supposed to preserve the cursor position but the printer gives it - // no information about where the nodes around the cursor ended up - - // although that logic is unavoidably slower (and has more potential to - // return a perverse result) than the happy path where we help out - // coreFormat by returning a cursorNodeStart and cursorNodeText here. - return { - formatted: out.filter((char) => char !== CURSOR_PLACEHOLDER).join(""), - }; - } - - const beforeCursor = out.slice(0, cursorPlaceholderIndex).join(""); - const aroundCursor = out - .slice(cursorPlaceholderIndex + 1, otherCursorPlaceholderIndex) - .join(""); - const afterCursor = out.slice(otherCursorPlaceholderIndex + 1).join(""); - - return { - formatted: beforeCursor + aroundCursor + afterCursor, - cursorNodeStart: beforeCursor.length, - cursorNodeText: aroundCursor, - }; + const formatted = out.join(""); + if (cursorPositions.length !== 2) { + // If the doc contained ONE cursor command, + // instead of the expected zero or two. If the doc being printed was + // returned by printAstToDoc, then the only ways this can have happened + // are if: + // 1. a plugin added a cursor command itself, or + // 2. one (but not both) of options.nodeAfterCursor and + // options.nodeAfterCursor pointed to a node within a subtree of the + // AST that the printer plugin used in printAstToDoc simply omits from + // the doc, or that it prints without recursively calling mainPrint, + // with the consequence that the logic for adding a cursor command in + // callPluginPrintFunction was never called for that node. + // These are both weird scenarios that should be considered a bug if they + // ever occur with one of Prettier's built-in plugins. If a third-party + // plugin was used when printing the AST to a doc, the possibility of + // reaching this scenario MIGHT be reasonable to consider a bug in the + // plugin. However, we try to at least not crash if this ever happens; + // instead we simply give up on returning a cursorNodeStart or + // cursorNodeText. + // + // coreFormat has logic specifically to handle this scenario - where it + // is supposed to preserve the cursor position but the printer gives it + // no information about where the nodes around the cursor ended up - + // although that logic is unavoidably slower (and has more potential to + // return a perverse result) than the happy path where we help out + // coreFormat by returning a cursorNodeStart and cursorNodeText here. + return { formatted }; } - return { formatted: out.join("") }; + const cursorNodeStart = cursorPositions[0] + 1; + return { + formatted, + cursorNodeStart, + cursorNodeText: formatted.slice( + cursorNodeStart, + cursorPositions.at(-1) + 1, + ), + }; + + function trim() { + const { text, count } = trimIndentation(out); + out = [text]; + + if (count === 0) { + return; + } + + position -= count; + + if (cursorPositions.length === 0) { + return; + } + + for (let index = 0; index < cursorPositions.length; index++) { + cursorPositions[index] = Math.min( + cursorPositions[index], + text.length - 1, + ); + } + } } export { printDocToString }; diff --git a/src/document/printer/trim-indentation.js b/src/document/printer/trim-indentation.js new file mode 100644 index 000000000..6c70aadf1 --- /dev/null +++ b/src/document/printer/trim-indentation.js @@ -0,0 +1,41 @@ +// Not using a regexp here because regexps for trimming off trailing +// characters are known to have performance issues. + +/** +Trim trailing `Tab(U+0009)` and `Space(U+0020)` from text + +@param {string} text +@returns +*/ +function trimTrailingIndentation(text) { + let end = text.length - 1; + + while (end >= 0) { + const character = text[end]; + if (character !== " " && character !== "\t") { + break; + } + end--; + } + + return text.slice(0, end + 1); +} + +/** +Trim `Tab(U+0009)` and `Space(U+0020)` at the end of line + +@param {string[]} buffer +@returns {{text: string, count: number}} +*/ +function trimIndentation(buffer) { + const text = buffer.join(""); + const trimmed = trimTrailingIndentation(text); + + return { text: trimmed, count: text.length - trimmed.length }; +} + +export { + trimIndentation, + // Exposed for benchmark test + trimTrailingIndentation, +}; diff --git a/tests/unit/doc-builders.js b/tests/unit/doc-builders.js index 801bd1cac..74c5d5d3e 100644 --- a/tests/unit/doc-builders.js +++ b/tests/unit/doc-builders.js @@ -15,6 +15,7 @@ import { line, lineSuffix, markAsRoot, + trim, } from "../../src/document/index.js"; const invalidDoc = { type: "invalid-type" }; @@ -91,6 +92,8 @@ describe("doc builders", () => { ifBreak(validDoc, undefined), align("any string", validDoc), + + trim, ]; describe("Invalid usage", () => {