diff --git a/__tests__/keep-lint-item.spec.ts b/__tests__/keep-lint-item.spec.ts index 07341c8..cc584f6 100644 --- a/__tests__/keep-lint-item.spec.ts +++ b/__tests__/keep-lint-item.spec.ts @@ -118,4 +118,36 @@ describe("keepLintItem", () => { expect(keepLintItem(baseItem({ executionErrors: [] }))).toBe(false); expect(keepLintItem(baseItem({ executionErrors: undefined }))).toBe(false); }); + + test("drops compact fixedResult items (no notAppliedFixes)", () => { + expect( + keepLintItem({ + path: "clean.md", + diagnostics: [], + summary: { + errorCount: 0, + warningCount: 0, + fixableErrorCount: 0, + fixableWarningCount: 0, + }, + fixedResult: { convergence: "stable" }, + }) + ).toBe(false); + }); + + test("keeps compact fixedResult items with cycle convergence", () => { + expect( + keepLintItem({ + path: "cycle.md", + diagnostics: [], + summary: { + errorCount: 0, + warningCount: 0, + fixableErrorCount: 0, + fixableWarningCount: 0, + }, + fixedResult: { convergence: "cycle" }, + }) + ).toBe(true); + }); }); diff --git a/__tests__/lint-worker.spec.ts b/__tests__/lint-worker.spec.ts index 7bd64fd..dbd1647 100644 --- a/__tests__/lint-worker.spec.ts +++ b/__tests__/lint-worker.spec.ts @@ -3,6 +3,11 @@ import { jest } from "@jest/globals"; jest.mock("@lint-md/core", () => ({ fixMarkdown: jest.fn(), lintMarkdown: jest.fn(), + FixConvergence: { + STABLE: "stable", + CYCLE_DETECTED: "cycle", + MAX_ROUNDS: "max", + }, })); import { fixMarkdown, lintMarkdown } from "@lint-md/core"; @@ -118,4 +123,152 @@ describe("lintWorker executionErrors passthrough", () => { ); expect(mockedLintMarkdown).not.toHaveBeenCalled(); }); + + test("returns compact fixedResult for clean fix items", async () => { + const file = path.join(tmpDir, "clean-fix.md"); + await writeFile(file, "# Clean\n", "utf8"); + + mockedFixMarkdown.mockReturnValue({ + lintResult: [], + diagnostics: [], + summary: { + errorCount: 0, + warningCount: 0, + fixableErrorCount: 0, + fixableWarningCount: 0, + }, + fixedResult: { + result: "# Clean\n", + notAppliedFixes: [], + convergence: "stable", + metrics: { rounds: 1, wallTime: 0.5, perRound: [0.5] }, + }, + executionErrors: [], + } as any); + + const result = await lintWorker({ + filePath: file, + rules: {}, + isFixMode: true, + }); + + // Compact form: convergence and metrics preserved, result and notAppliedFixes dropped + expect(result.fixedResult).toEqual({ + convergence: "stable", + metrics: { rounds: 1, wallTime: 0.5, perRound: [0.5] }, + }); + expect(result.fixedResult).not.toHaveProperty("result"); + expect(result.fixedResult).not.toHaveProperty("notAppliedFixes"); + }); + + test("returns full fixedResult for actionable fix items (diagnostics)", async () => { + const file = path.join(tmpDir, "actionable-fix.md"); + await writeFile(file, "1. hello\n2.\n", "utf8"); + + mockedFixMarkdown.mockReturnValue({ + lintResult: [], + diagnostics: [ + { + ruleId: "no-empty-list", + message: "empty list item", + line: 2, + column: 1, + severity: 2, + }, + ], + summary: { + errorCount: 1, + warningCount: 0, + fixableErrorCount: 0, + fixableWarningCount: 0, + }, + fixedResult: { + result: "1. hello\n2. item\n", + notAppliedFixes: [], + convergence: "stable", + }, + executionErrors: [], + } as any); + + const result = await lintWorker({ + filePath: file, + rules: {}, + isFixMode: true, + }); + + // Full form preserved because item has diagnostics + expect(result.fixedResult).toHaveProperty("result", "1. hello\n2. item\n"); + expect(result.fixedResult).toHaveProperty("notAppliedFixes"); + }); + + test("returns full fixedResult when convergence is cycle", async () => { + const file = path.join(tmpDir, "cycle-fix.md"); + await writeFile(file, "# Title\n", "utf8"); + + mockedFixMarkdown.mockReturnValue({ + lintResult: [], + diagnostics: [], + summary: { + errorCount: 0, + warningCount: 0, + fixableErrorCount: 0, + fixableWarningCount: 0, + }, + fixedResult: { + result: "# Title\n", + notAppliedFixes: [], + convergence: "cycle", + metrics: { + rounds: 5, + wallTime: 1.0, + perRound: [0.2, 0.2, 0.2, 0.2, 0.2], + }, + }, + executionErrors: [], + } as any); + + const result = await lintWorker({ + filePath: file, + rules: {}, + isFixMode: true, + }); + + // Full form preserved because isIncompleteFix(item) is true + expect(result.fixedResult).toHaveProperty("result", "# Title\n"); + expect(result.fixedResult).toHaveProperty("notAppliedFixes"); + }); + + test("returns full fixedResult when notAppliedFixes is non-empty", async () => { + const file = path.join(tmpDir, "unapplied-fix.md"); + await writeFile(file, "# Title\n", "utf8"); + + mockedFixMarkdown.mockReturnValue({ + lintResult: [], + diagnostics: [], + summary: { + errorCount: 0, + warningCount: 0, + fixableErrorCount: 0, + fixableWarningCount: 0, + }, + fixedResult: { + result: "# Title\n", + notAppliedFixes: [ + { targetRule: "r", range: [0, 1], text: "x", reason: "overlap" }, + ], + convergence: "stable", + }, + executionErrors: [], + } as any); + + const result = await lintWorker({ + filePath: file, + rules: {}, + isFixMode: true, + }); + + // Full form preserved because notAppliedFixes is non-empty + expect(result.fixedResult).toHaveProperty("result", "# Title\n"); + expect(result.fixedResult).toHaveProperty("notAppliedFixes"); + }); }); diff --git a/__tests__/report-incomplete-fixes.spec.ts b/__tests__/report-incomplete-fixes.spec.ts index fe0f691..e105bdc 100644 --- a/__tests__/report-incomplete-fixes.spec.ts +++ b/__tests__/report-incomplete-fixes.spec.ts @@ -67,6 +67,38 @@ describe("report-incomplete-fixes", () => { test("returns false when fixedResult is null", () => { expect(isIncompleteFix(makeItem({ fixedResult: null }))).toBe(false); }); + + test("works with compact fixedResult (convergence only)", () => { + expect( + isIncompleteFix({ + path: "compact.md", + diagnostics: [], + summary: { + errorCount: 0, + warningCount: 0, + fixableErrorCount: 0, + fixableWarningCount: 0, + }, + fixedResult: { convergence: FixConvergence.CYCLE_DETECTED }, + }) + ).toBe(true); + }); + + test("returns false for compact fixedResult with stable convergence", () => { + expect( + isIncompleteFix({ + path: "compact-stable.md", + diagnostics: [], + summary: { + errorCount: 0, + warningCount: 0, + fixableErrorCount: 0, + fixableWarningCount: 0, + }, + fixedResult: { convergence: FixConvergence.STABLE }, + }) + ).toBe(false); + }); }); describe("getIncompleteFixWarnings", () => { diff --git a/scripts/benchmark-compact-fix.mjs b/scripts/benchmark-compact-fix.mjs new file mode 100644 index 0000000..98c31dc --- /dev/null +++ b/scripts/benchmark-compact-fix.mjs @@ -0,0 +1,196 @@ +#!/usr/bin/env node + +/** + * RSS comparison benchmark for compact fix results. + * + * Runs the existing benchmark-memory.mjs on both the current branch + * (compact results) and the baseline (full results), then prints + * a side-by-side comparison. + * + * Linux only: requires GNU /usr/bin/time -v. + * + * Usage: node scripts/benchmark-compact-fix.mjs [options] + * + * Options: + * --files Number of generated Markdown files (default: 1000) + * --bytes-per-file Approximate bytes per file (default: 65536) + * --threads Worker thread count (default: 4) + * --runs Benchmark repetitions (default: 3) + * -h, --help Show this help + */ + +import { spawnSync } from 'child_process'; +import { existsSync, mkdtempSync, rmSync, writeFileSync } from 'fs'; +import { tmpdir } from 'os'; +import path from 'path'; +import { fileURLToPath } from 'url'; + +const rootDir = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..'); +const benchmarkScript = path.join(rootDir, 'scripts/benchmark-memory.mjs'); +const timeCommand = '/usr/bin/time'; + +const usage = `Usage: node scripts/benchmark-compact-fix.mjs [options] + +Linux only: requires GNU /usr/bin/time -v. + +Runs the benchmark on the current branch (compact) and baseline (full), +then prints a side-by-side comparison. + +Options: + --files Number of generated Markdown files (default: 1000) + --bytes-per-file Approximate bytes per file (default: 65536) + --threads Worker thread count (default: 4) + --runs Benchmark repetitions (default: 3) + -h, --help Show this help +`; + +const parsePositiveInteger = (value, option) => { + if (!/^[1-9]\d*$/.test(value)) { + throw new Error(`${option} must be a positive integer`); + } + return Number(value); +}; + +const parseArgs = (args) => { + const options = { files: 1000, bytesPerFile: 65536, threads: 4, runs: 3 }; + for (let i = 0; i < args.length; i += 1) { + const arg = args[i]; + if (arg === '-h' || arg === '--help') { console.log(usage); process.exit(0); } + const val = args[i + 1]; + if (val === undefined) throw new Error(`Missing value for ${arg}`); + if (arg === '--files') options.files = parsePositiveInteger(val, arg); + else if (arg === '--bytes-per-file') options.bytesPerFile = parsePositiveInteger(val, arg); + else if (arg === '--threads') options.threads = val === 'auto' ? 'auto' : parsePositiveInteger(val, arg); + else if (arg === '--runs') options.runs = parsePositiveInteger(val, arg); + else throw new Error(`Unknown option: ${arg}`); + i += 1; + } + return options; +}; + +const runBenchmark = (label) => { + const fixtureDir = mkdtempSync(path.join(tmpdir(), 'lint-md-compact-')); + try { + const prefix = '# Title\n\n'; + const bodyLen = Math.max(options.bytesPerFile - prefix.length - 1, 0); + const body = 'word '.repeat(Math.ceil(bodyLen / 5)).slice(0, bodyLen); + const content = `${prefix}${body}\n`; + const filePaths = []; + + for (let i = 0; i < options.files; i += 1) { + const fp = path.join(fixtureDir, `fixture-${i}.md`); + writeFileSync(fp, content); + filePaths.push(fp); + } + + const cliArgs = [ + '-v', process.execPath, + path.join(rootDir, 'node_modules/tsx/dist/cli.mjs'), + path.join(rootDir, 'src/lint-md.ts'), + '--fix', + '--threads', String(options.threads), + ...filePaths, + ]; + + const measurements = []; + for (let run = 1; run <= options.runs; run += 1) { + const result = spawnSync(timeCommand, cliArgs, { + cwd: rootDir, + encoding: 'utf8', + env: { ...process.env, LC_ALL: 'C' }, + maxBuffer: 10 * 1024 * 1024, + }); + + if (result.error) throw result.error; + if (result.status !== 0) { + process.stderr.write(result.stderr); + throw new Error(`${label} run ${run} exited with code ${result.status}`); + } + + const rssMatch = result.stderr.match(/Maximum resident set size \(kbytes\): (\d+)/); + const elapsedMatch = result.stderr.match(/Elapsed \(wall clock\) time.*?: ([\d:.]+)/); + measurements.push({ + run, + maxRssKiB: rssMatch ? Number(rssMatch[1]) : null, + elapsed: elapsedMatch?.[1] ?? null, + }); + } + + return { label, config: options, measurements }; + } finally { + rmSync(fixtureDir, { recursive: true, force: true }); + } +}; + +if (process.platform !== 'linux') { + throw new Error(`Unsupported platform: ${process.platform}. Requires GNU time on Linux.`); +} +if (!existsSync(timeCommand)) { + throw new Error('GNU /usr/bin/time is required for this benchmark'); +} + +const options = parseArgs(process.argv.slice(2)); + +// Get current branch name +const currentBranch = spawnSync('git', ['rev-parse', '--abbrev-ref', 'HEAD'], { + cwd: rootDir, encoding: 'utf8', +}).stdout.trim(); + +// Get baseline commit (last commit on master before this branch) +const baseRef = spawnSync('git', ['merge-base', 'master', currentBranch], { + cwd: rootDir, encoding: 'utf8', +}).stdout.trim(); + +console.log(`\n=== RSS Benchmark: compact fix results ===`); +console.log(`Current: ${currentBranch}`); +console.log(`Baseline: ${baseRef.slice(0, 8)}\n`); + +// Run on current branch (compact) +console.log(`>>> Running on current branch (${currentBranch})...`); +const compact = runBenchmark('compact'); + +// Stash, checkout baseline, run, restore +console.log(`\n>>> Stashing changes and checking out baseline...`); +spawnSync('git', ['stash'], { cwd: rootDir, encoding: 'utf8' }); +spawnSync('git', ['checkout', baseRef], { cwd: rootDir, encoding: 'utf8' }); + +// Rebuild for baseline +console.log(`>>> Building baseline...`); +spawnSync('npm', ['run', 'build'], { cwd: rootDir, encoding: 'utf8', stdio: 'inherit' }); + +console.log(`>>> Running on baseline (${baseRef.slice(0, 8)})...`); +const baseline = runBenchmark('baseline'); + +// Restore +console.log(`\n>>> Restoring current branch...`); +spawnSync('git', ['checkout', currentBranch], { cwd: rootDir, encoding: 'utf8' }); +spawnSync('git', ['stash', 'pop'], { cwd: rootDir, encoding: 'utf8' }); + +// Rebuild for current +console.log(`>>> Rebuilding current branch...`); +spawnSync('npm', ['run', 'build'], { cwd: rootDir, encoding: 'utf8', stdio: 'inherit' }); + +// Print comparison +const avg = (arr) => arr.reduce((s, v) => s + v, 0) / arr.length; +const compactRss = compact.measurements.map((m) => m.maxRssKiB).filter(Boolean); +const baselineRss = baseline.measurements.map((m) => m.maxRssKiB).filter(Boolean); +const compactAvg = avg(compactRss); +const baselineAvg = avg(baselineRss); +const delta = baselineAvg - compactAvg; +const pct = baselineAvg > 0 ? ((delta / baselineAvg) * 100).toFixed(1) : '0.0'; + +console.log(`\n=== Results ===`); +console.log(`Config: ${options.files} files × ${options.bytesPerFile} bytes, --fix --threads ${options.threads}, ${options.runs} runs`); +console.log(`\n baseline (full) compact delta`); +console.log(`Peak RSS (avg) ${(baselineAvg / 1024).toFixed(1)} MiB ${(compactAvg / 1024).toFixed(1)} MiB ${(delta > 0 ? '-' : '+')}${(Math.abs(delta) / 1024).toFixed(1)} MiB (${pct}%)`); +console.log(`\nPer-run detail:`); +console.log(` baseline: ${baselineRss.map((v) => `${(v / 1024).toFixed(1)}M`).join(', ')}`); +console.log(` compact: ${compactRss.map((v) => `${(v / 1024).toFixed(1)}M`).join(', ')}`); + +console.log(JSON.stringify({ + type: 'comparison', + baseline: { measurements: baseline.measurements, avgRssKiB: baselineAvg }, + compact: { measurements: compact.measurements, avgRssKiB: compactAvg }, + deltaRssKiB: delta, + deltaPercent: Number(pct), +})); diff --git a/src/cli/run-lint.ts b/src/cli/run-lint.ts index 73e8633..8682ad5 100644 --- a/src/cli/run-lint.ts +++ b/src/cli/run-lint.ts @@ -2,6 +2,7 @@ import * as process from "process"; import { fixMarkdown, lintMarkdown } from "@lint-md/core"; import type { LintMdRulesConfig } from "@lint-md/core"; import type { ThreadCount } from "../types"; +import { isFullFixedResult } from "../types"; import { safeWriteFile } from "../utils/safe-write-file"; import { resolveAdaptiveConcurrency } from "../utils/adaptive-concurrency"; import { batchLint } from "../utils/batch-lint"; @@ -223,16 +224,22 @@ export const runFileLint = async ({ return FAILURE_EXIT; } } else { - await runTasksWithLimit( - actionableResults - .filter(({ fixedResult }) => fixedResult) - .map( - ({ path, fixedResult }) => - () => - safeWriteFile(path, fixedResult!.result) - ), - effectiveThreads - ); + const writeTasks = actionableResults + .filter( + ({ fixedResult }) => + fixedResult != null && isFullFixedResult(fixedResult) + ) + .map( + ({ path, fixedResult }) => + () => + safeWriteFile( + path, + (fixedResult as Extract) + .result + ) + ); + + await runTasksWithLimit(writeTasks, effectiveThreads); for (const warning of getIncompleteFixWarnings(actionableResults)) { console.error(warning); diff --git a/src/types.ts b/src/types.ts index 22d1f86..89c930b 100644 --- a/src/types.ts +++ b/src/types.ts @@ -21,12 +21,25 @@ export interface LintWorkerOptions { isFixMode: boolean; } +/** Compact fixed-result for clean files (no diagnostics, no unapplied fixes). + * Only carries the fields getFixDevMetrics needs. The full FixedResult.result + * (repair Markdown text) is intentionally absent to avoid structured-cloning + * large strings across the worker thread boundary. */ +export type CompactFixedResult = Pick; + +/** Type guard: true when fixedResult carries the full Markdown text + * (result + notAppliedFixes). Compact results from the worker have + * these fields absent at runtime. */ +export const isFullFixedResult = ( + fixedResult: FixedResult | CompactFixedResult +): fixedResult is FixedResult => "result" in fixedResult; + /** batchLint 单个文件的 lint 结果 */ export interface BatchLintItem { path: string; diagnostics: LintDiagnostic[]; summary: LintSummary; - fixedResult?: FixedResult | null; + fixedResult?: FixedResult | CompactFixedResult | null; // Per-round, per-phase rule execution errors from @lint-md/core 2.1.5 // (core #185). CLI surfaces these as stderr warnings and exits 1 // regardless of --suppress-warnings. diff --git a/src/utils/batch-lint.ts b/src/utils/batch-lint.ts index edd09d4..6c72305 100644 --- a/src/utils/batch-lint.ts +++ b/src/utils/batch-lint.ts @@ -3,6 +3,7 @@ import { existsSync } from "fs"; import { Piscina } from "piscina"; import type { LintMdRulesConfig } from "@lint-md/core"; import type { BatchLintItem, LintWorkerOptions } from "../types"; +import { isFullFixedResult } from "../types"; import { isIncompleteFix } from "./report-incomplete-fixes"; import { runTasksWithLimit } from "./run-tasks-with-limit"; @@ -24,7 +25,9 @@ const resolveWorkerFilename = (): string => { // that predate these fields leave them undefined and are filtered as before. export const keepLintItem = (item: BatchLintItem): boolean => item.diagnostics.length > 0 || - Boolean(item.fixedResult?.notAppliedFixes?.length) || + (item.fixedResult != null && + isFullFixedResult(item.fixedResult) && + item.fixedResult.notAppliedFixes.length > 0) || isIncompleteFix(item) || (item.executionErrors?.length ?? 0) > 0; diff --git a/src/utils/lint-worker.ts b/src/utils/lint-worker.ts index d5fc3ae..f560664 100644 --- a/src/utils/lint-worker.ts +++ b/src/utils/lint-worker.ts @@ -1,7 +1,25 @@ import { readFile } from "fs/promises"; import { fixMarkdown, lintMarkdown } from "@lint-md/core"; -import type { LintWorkerOptions } from "../types"; +import type { + BatchLintItem, + CompactFixedResult, + LintWorkerOptions, +} from "../types"; +import { isFullFixedResult } from "../types"; import { toBatchLintItem } from "./to-batch-lint-item"; +import { isIncompleteFix } from "./report-incomplete-fixes"; + +// Mirrors keepLintItem conditions. An item is clean when ALL are false: +// diagnostics, notAppliedFixes, incomplete convergence, executionErrors. +const isCleanFixItem = (item: BatchLintItem): boolean => + item.diagnostics.length === 0 && + item.fixedResult != null && + !( + isFullFixedResult(item.fixedResult) && + item.fixedResult.notAppliedFixes?.length > 0 + ) && + !isIncompleteFix(item) && + (item.executionErrors?.length ?? 0) === 0; const lintWorker = async (options: LintWorkerOptions) => { const { filePath, rules, isFixMode } = options; @@ -11,7 +29,19 @@ const lintWorker = async (options: LintWorkerOptions) => { ? fixMarkdown(content, { rules }) : lintMarkdown(content, rules, false); - return toBatchLintItem(filePath, result); + const item = toBatchLintItem(filePath, result); + + // For clean fix items, project to compact form to avoid + // structured-cloning the full Markdown text across threads. + if (isFixMode && isCleanFixItem(item)) { + const { convergence, metrics } = item.fixedResult!; + const compact: CompactFixedResult = {}; + if (convergence !== undefined) compact.convergence = convergence; + if (metrics !== undefined) compact.metrics = metrics; + item.fixedResult = compact; + } + + return item; }; export default lintWorker; diff --git a/src/utils/report-unapplied-fixes.ts b/src/utils/report-unapplied-fixes.ts index 1632b52..5ee248d 100644 --- a/src/utils/report-unapplied-fixes.ts +++ b/src/utils/report-unapplied-fixes.ts @@ -1,4 +1,5 @@ import type { BatchLintItem } from "../types"; +import { isFullFixedResult } from "../types"; import { sanitizeTerminalText } from "./sanitize-terminal"; // Returns stderr warning lines for files whose --fix pass left fixes @@ -12,7 +13,10 @@ export const getUnappliedFixesWarnings = ( const warnings: string[] = []; for (const item of lintResult) { - const count = item.fixedResult?.notAppliedFixes?.length ?? 0; + const count = + item.fixedResult != null && isFullFixedResult(item.fixedResult) + ? item.fixedResult.notAppliedFixes.length + : 0; if (count > 0) { warnings.push( `[lint-md] ${sanitizeTerminalText(