Skip to content

perf: pre-declare position on table factories#9

Open
ChristianMurphy wants to merge 2 commits intosyntax-tree:mainfrom
ChristianMurphy:perf/stable-node-shape
Open

perf: pre-declare position on table factories#9
ChristianMurphy wants to merge 2 commits intosyntax-tree:mainfrom
ChristianMurphy:perf/stable-node-shape

Conversation

@ChristianMurphy
Copy link
Copy Markdown
Member

Initial checklist

  • I read the support docs
  • I read the contributing guide
  • I agree to follow the code of conduct
  • I searched issues and discussions and couldn’t find anything or linked relevant results below
  • I made sure the docs are up to date
  • I included tests (or that’s not needed)

Description of changes

Problem: the table handlers build table, tableRow, and tableCell
nodes as fresh literals, then mdast-util-from-markdown's enter() sets
node.position afterwards. V8 treats the late assignment as a shape
change, so each node walks through two hidden classes instead of one.
Core nodes already avoid this by declaring position up front;
extension nodes leave the tree with mixed layouts.

Goal: declare position on all three factory literals at construction
so V8 keeps a single hidden class per node type.

Changes:

  • lib/index.js: trailing position: undefined on the table literal in
    enterTable.
  • lib/index.js: trailing position: undefined on the tableRow literal
    in enterRow.
  • lib/index.js: trailing position: undefined on the tableCell literal
    in enterCell.

Notes: pairs with the matching core change in
syntax-tree/mdast-util-from-markdown#53;
merged on its own this branch is a no-op. Validated with npm test
(build, format, 100% coverage, 32 of 32 tests in dev and prod).

The branch also includes a small chore commit at lib/index.js:233
through 237 that drops three @ts-expect-error directives that no
longer match the current markdown-table types and that fail tsc on
upstream main as written. Without that cleanup the package's own test
script does not pass, so the chore commit is required to land before
this one. Squash or split per maintainer preference.

Bench (local harness, 3-run median-of-medians, GFM tokenizer and
mdast extensions stacked). Numbers below are the parse time delta
versus an unmodified upstream main; positive numbers mean slower,
negative numbers mean faster.

Inputs: a 4-column 1000-row table and a 20-column 100-row table.
Tables sit at low mdast-layer ratios on the baseline (1.08 and 1.09
respectively); the bottleneck for tables is in
micromark-extension-gfm-table, not in this util. Numbers should be
read as "no regression" rather than "credible win."

Effect of pairing this branch with PR #53:

  • 4 cols x 1000 rows:
    PR #53 alone: 4.7 percent slower.
    PR #53 with this branch alongside: 6.2 percent slower.
  • 20 cols x 100 rows:
    PR #53 alone: 5.0 percent slower.
    PR #53 with this branch alongside: 7.2 percent slower.

Both rows sit inside the machine's drift band of 4 to 12 percent
(main vs main with no code change in the same window). Pairing
neither helps nor hurts table parses meaningfully, which matches what
the baseline ratio predicts. The branch is included for shape
consistency across the GFM stack rather than for a measurable
per-table win.

Problem: tsc --build fails on upstream/main with three TS2578 "Unused
'@ts-expect-error' directive" errors at lib/index.js:233/235/237.
markdown-table's types now accept null for the align-delimiters,
padding, and string-length options, so the suppressions are obsolete
and the type checker now flags them.

Goal: restore tsc --build to a clean pass so the package's own test
suite runs end-to-end without an unrelated build failure.

Changes:
- lib/index.js: remove three @ts-expect-error comments around the
  alignDelimiters / padding / stringLength fields passed to
  markdownTable in serializeData.

Notes: drift from a markdown-table type update; verified npm test now
passes (build + format + 100% coverage + 32 of 32 tests dev and prod).
Problem: the table handlers build table, tableRow, and tableCell
nodes as fresh literals, then mdast-util-from-markdown's enter() sets
node.position afterwards. V8 treats the late assignment as a shape
change, so each node walks through two hidden classes instead of one.
Core nodes already avoid this by declaring position up front;
extension nodes leave the tree with mixed layouts.

Goal: declare position on all three factory literals at construction
so V8 keeps a single hidden class per node type.

Changes:
- lib/index.js: trailing position: undefined on the table literal in
  enterTable.
- lib/index.js: trailing position: undefined on the tableRow literal
  in enterRow.
- lib/index.js: trailing position: undefined on the tableCell literal
  in enterCell.

Notes: pairs with the matching core change in
syntax-tree/mdast-util-from-markdown#53;
merged on its own this branch is a no-op. Validated with npm test
(build, format, 100% coverage, 32 of 32 tests in dev and prod).

The branch also includes a small chore commit at lib/index.js:233
through 237 that drops three @ts-expect-error directives that no
longer match the current markdown-table types and that fail tsc on
upstream main as written. Without that cleanup the package's own test
script does not pass, so the chore commit is required to land before
this one. Squash or split per maintainer preference.

Bench (local harness, 3-run median-of-medians, GFM tokenizer and
mdast extensions stacked). Numbers below are the parse time delta
versus an unmodified upstream main; positive numbers mean slower,
negative numbers mean faster.

Inputs: a 4-column 1000-row table and a 20-column 100-row table.
Tables sit at low mdast-layer ratios on the baseline (1.08 and 1.09
respectively); the bottleneck for tables is in
micromark-extension-gfm-table, not in this util. Numbers should be
read as "no regression" rather than "credible win."

Effect of pairing this branch with PR #53:

- 4 cols x 1000 rows:
  PR #53 alone: 4.7 percent slower.
  PR #53 with this branch alongside: 6.2 percent slower.
- 20 cols x 100 rows:
  PR #53 alone: 5.0 percent slower.
  PR #53 with this branch alongside: 7.2 percent slower.

Both rows sit inside the machine's drift band of 4 to 12 percent
(main vs main with no code change in the same window). Pairing
neither helps nor hurts table parses meaningfully, which matches what
the baseline ratio predicts. The branch is included for shape
consistency across the GFM stack rather than for a measurable
per-table win.
@github-actions github-actions Bot added 👋 phase/new Post is being triaged automatically 🤞 phase/open Post is being triaged manually and removed 👋 phase/new Post is being triaged automatically labels May 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🤞 phase/open Post is being triaged manually

Development

Successfully merging this pull request may close these issues.

1 participant