Repository navigation
Conversation
Flow moves trailing comments off nodes it has already built with an AST mapper class (Flow_ast_mapper, 336 methods) and two subclasses in Comment_attachment. OCaml creates classes when their module is initialized, so every compiler process built their method tables: about 206,000 of the compiler's ~275,000 startup words, although ReScript only parses %raw code with the Flow parser and never reads trailing comments (only the leading comment of a %raw literal, which the parser collects without the mapper). The removers now return the node unchanged; what they kept is the consumption of comments, which decides where later comments attach. Flow_ast_mapper and both classes go. The Flow parser's startup allocation, measured by linking it into an otherwise empty program, drops from 227,172 to 20,669 words. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Kbf3gLDo8mBJKpNE25WAwT Signed-off-by: Christoph Knittel <ck@cca.io>
Changelog💅 Polish
Semantic Diff1 commit · Full page Drop the Flow parser's comment-moving AST mapper classesdefd652 · cknitt + Claude · Nov 08, 2026, 18:03 UTC Flow moves trailing comments off nodes it has already built with an AST mapper class (Flow_ast_mapper, 336 methods) and two subclasses in Comment_attachment. OCaml creates classes when their module is initialized, so every compiler process built their method tables: about 206,000 of the compiler's ~275,000 startup words, although ReScript only parses %raw code with the Flow parser and never reads trailing comments (only the leading comment of a %raw literal, which the parser collects without the mapper). The removers now return the node unchanged; what they kept is the consumption of comments, which decides where later comments attach. Flow_ast_mapper and both classes go. The Flow parser's startup allocation, measured by linking it into an otherwise empty program, drops from 227,172 to 20,669 words. |
Member
Author
|
@codex review |
cknitt
marked this pull request as ready for review
November 8, 2026 18:14
Codex Review Summary: ✅ CompletedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codex Review: Didn't find any major issues. Breezy! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
🎉 Compiler benchmarksAllocated wordsIn total: 456,745,704 → 346,086,726 (-24.23%). 🎉 Improvement: 253 benchmarks allocate more than 1% less.
247 more changes
InstructionsIn total: 10,228,138,742 → 9,755,245,397 (-4.62%). 🎉 Improvement: 30 benchmarks execute more than 1% fewer instructions.
23 more changes
Pull request defd652 compared with base 98faffc in this run. |
rescript
@rescript/belt
@rescript/darwin-arm64
@rescript/linux-arm64
@rescript/linux-x64
@rescript/runtime
@rescript/win32-x64
commit: |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #9173. Makes the compiler start with much less work by dropping the Flow parser's comment-moving AST mapper classes.
Why
The Flow parser (
compiler/flow_parser) only parses%rawcode, but OCaml initializes all its modules when the compiler starts. ItsFlow_ast_mapper.mapperclass (336 methods) and the twoComment_attachmentclasses that inherit it are created then, method tables and a closure per method, in every compiler process.Measured by linking the Flow parser into an otherwise empty program:
bsc -vallocates in total.This PR's benchmark (see the comment below) shows the effect on whole compilations:
Primitive_list.resi: 568,603 → 122,397).Stdlib_Math.resi: 44.1M → 24.6M).The saving is a fixed amount per compiler process, so small files gain the most;
parse/Napkinscript.resgains 2–3%.What changes
Flow uses the mapper to move trailing comments off nodes it has already built. ReScript never reads trailing comments. It only reads the leading comment of a
%rawliteral, which the parser collects without the mapper, and whichtests/tests/src/raw_pure_test.rescovers.Comment_attachmentnow return the node unchanged.Flow_ast_mapperand both classes are removed, along with the~kindarguments the parsers passed for them.Without the mapper, most fields of the Flow AST are only written; #9208 handles that for
make dead-code.🤖 Generated with Claude Code
https://claude.ai/code/session_01Kbf3gLDo8mBJKpNE25WAwT