Skip to content

Keep only the written fields of a record literal in the typed tree - #9149

Merged
cknitt merged 2 commits into
masterfrom
sparse-record-fields
Oct 31, 2026
Merged

cknitt merged 2 commits into
masterfrom
sparse-record-fields

Conversation

@cknitt

@cknitt cknitt commented Oct 31, 2026

Copy link
Copy Markdown
Member

Closes #9148.

A record literal's typed tree used to hold one entry per field of the record type: Kept for a field copied from a spread, and a synthesized None expression for every optional field the literal left out. JSX DOM props (JsxDOM.domProps) have 612 optional fields, so every DOM element added about 610 expressions to the typed tree. Those expressions went into the .cmt and were also translated to Lambda.

Now Texp_record.fields holds only the fields the literal writes, ordered by lbl_pos. A field missing from it is copied from the spread record when there is one, and is otherwise an optional field set to None. The record's full label set is still available through lbl_all, so translcore rebuilds the positional view where it needs one: record blocks, and JSX props copied from a spread. record_label_definition (Kept / Overridden) is gone, as is the tuple's optional flag, which was always false.

Performance

Measured with a base bsc built from the same master commit, on generated files. Times are user time, minimum of 5 runs; memory is peak RSS. The JavaScript output is byte-identical in every case and across the test suites.

Workload .cmt before → after Time before → after Memory before → after
200 JSX DOM elements 6.3 MB → 0.23 MB 0.17 s → 0.02 s 121 MB → 26 MB
800 JSX DOM elements 25.3 MB → 0.84 MB 0.80 s → 0.08 s 422 MB → 50 MB
800 literals of a 5-field record (3 optional) 454 KB → 291 KB unchanged unchanged
800 record spreads {...base, a: i} 330 KB → 198 KB unchanged unchanged

Typing also got cheaper per literal. It used to search the written fields once for each label of the record type, and now it marks written labels in a boolean array indexed by position. The spread path still unifies once for each label of the type.

Editor fix

Each synthesized field carried an identifier located at the record literal, and the editor analysis recorded it as a reference to that label. So "find references" on an optional field listed every literal that omits it. The new case in tests/analysis_tests/tests/src/References.res covers this: the baseline analysis returns a fourth reference inside {name: "c"}, and now there are three.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Kbf3gKyiYMf54wPRxFdUWe

Why:
    A record literal stored one entry per field of its type: Kept for a
    field copied from a spread, and a synthesized `None` expression for
    every optional field it left out. JSX DOM props have 612 optional
    fields, so each DOM element added about 610 expressions to the typed
    tree, ~31 KB to the .cmt, and as many Lambda constants (#9148). With
    800 elements, compiling took 0.80 s and 422 MB; now 0.08 s and 50 MB,
    and the .cmt shrinks from 25 MB to 0.8 MB. Small records lose a third
    of their .cmt size. The generated JavaScript does not change.

Typedtree modified (record literals):
Example:
    type r = {a: int, b?: string}
    let x = {a: 1}
Before (typed):
    Texp_record
      fields =
        [
          "a"
            expression
              Texp_constant Const_int 1
          "b"
            expression
              Texp_construct "None"
              []
        ]
After (typed):
    Texp_record
      fields =
        [
          "a"
            expression
              Texp_constant Const_int 1
        ]

Analysis modified (find references):
Example:
    type labelled = {name: string, label?: string}
    let withLabel = {name: "a", label: "b"}
    let withoutLabel = {name: "c"}
    let _ = withLabel.label
Before (text): references of `label`
    26:31 declaration
    27:28 withLabel's field
    28:25 inside {name: "c"}, which has no label
    29:18 withLabel.label
After (text): references of `label`
    26:31 declaration
    27:28 withLabel's field
    29:18 withLabel.label

Review: Where do the left-out fields come from now? Translcore rebuilds
the positional view from lbl_all: a field missing from `fields` is read
from the spread record when there is one, and is `undefined` otherwise.
Review: What else went? The record_label_definition type, the unused
type of Kept fields, and the tuple's optional flag, which was always
false.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Kbf3gKyiYMf54wPRxFdUWe
Signed-off-by: Christoph Knittel <ck@cca.io>
@github-actions

github-actions Bot commented Oct 31, 2026 •

Copy link
Copy Markdown

Changelog

🐛 Bug fix

  • Editor "find references" on a record field no longer lists record literals that leave the optional field out.

💅 Polish

  • Stop storing a None expression for every optional field a record literal leaves out. Modules with many JSX DOM elements now compile up to 10× faster, use far less memory, and get .cmt files up to 30× smaller.

Semantic Diff

2 commits · Full page

Keep only the written fields of a record literal in the typed tree

ceaaf61 · cknitt + Claude · Oct 31, 2026, 07:01 UTC

Why

A record literal stored one entry per field of its type: Kept for a field copied from a spread, and a synthesized None expression for every optional field it left out. JSX DOM props have 612 optional fields, so each DOM element added about 610 expressions to the typed tree, ~31 KB to the .cmt, and as many Lambda constants (#9148). With 800 elements, compiling took 0.80 s and 422 MB; now 0.08 s and 50 MB, and the .cmt shrinks from 25 MB to 0.8 MB. Small records lose a third of their .cmt size. The generated JavaScript does not change.

Typedtree modified (record literals)

type r = {a: int, b?: string}
let x = {a: 1}

typed

 Texp_record
   fields =
       "a"
         expression
           Texp_constant Const_int 1
-      "b"
-        expression
-          Texp_construct "None"
-          []

Analysis modified (find references)

type labelled = {name: string, label?: string}
let withLabel = {name: "a", label: "b"}
let withoutLabel = {name: "c"}
let _ = withLabel.label

text · references of label

 26:31 declaration
 27:28 withLabel's field
-28:25 inside {name: "c"}, which has no label
 29:18 withLabel.label

Review

  • Where do the left-out fields come from now? Translcore rebuilds the positional view from lbl_all: a field missing from fields is read from the spread record when there is one, and is undefined otherwise.
  • What else went? The record_label_definition type, the unused type of Kept fields, and the tuple's optional flag, which was always false.
✨ Treat omitted mutable fields as allocated in the value restriction

9f7babe · cknitt + Claude · Oct 31, 2026, 07:49 UTC · new

Codex review: is_nonexpansive now saw only the written fields, so with type cell<'a> = {tag: int, mutable value?: 'a} the binding let cell = {tag: 0} generalized 'a, and cell.value could be written as option<int> and read as option<string>. Without a spread, every label of the record is allocated, so all must be immutable; with a spread, the check is unchanged. Master rejects the example as well; the new super_errors fixture covers it.

@codecov

codecov Bot commented Oct 31, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 93.33333% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.74%. Comparing base (b3f2c80) to head (9f7babe).

Files with missing lines Patch % Lines
compiler/ml/printtyped.ml 0.00% 2 Missing ⚠️
compiler/ml/rec_check.ml 50.00% 2 Missing ⚠️
analysis/reanalyze/src/arnold.ml 0.00% 1 Missing ⚠️
compiler/ml/typedtree_iter.ml 0.00% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           master    #9149   +/-   ##
=======================================
  Coverage   81.74%   81.74%           
=======================================
  Files         460      460           
  Lines       64949    64940    -9     
=======================================
- Hits        53090    53088    -2     
+ Misses      11859    11852    -7     
Files with missing lines Coverage Δ
analysis/reanalyze/src/dead_value.ml 85.89% <100.00%> (ø)
analysis/reanalyze/src/side_effects.ml 79.36% <100.00%> (+0.90%) ⬆️
analysis/src/process_extra.ml 87.34% <100.00%> (-0.11%) ⬇️
compiler/ml/tast_iterator.ml 86.82% <100.00%> (-0.07%) ⬇️
compiler/ml/tast_mapper.ml 99.17% <100.00%> (-0.01%) ⬇️
compiler/ml/translcore.ml 88.40% <100.00%> (+0.06%) ⬆️
compiler/ml/typecore.ml 89.21% <100.00%> (-0.01%) ⬇️
compiler/ml/typedtree.ml 91.22% <ø> (ø)
analysis/reanalyze/src/arnold.ml 84.54% <0.00%> (+0.39%) ⬆️
compiler/ml/typedtree_iter.ml 9.96% <0.00%> (+0.03%) ⬆️
... and 2 more

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@pkg-pr-new

pkg-pr-new Bot commented Oct 31, 2026 •

Copy link
Copy Markdown

Open in StackBlitz

rescript

npm i https://pkg.pr.new/rescript@9149

@rescript/belt

npm i https://pkg.pr.new/@rescript/belt@9149

@rescript/darwin-arm64

npm i https://pkg.pr.new/@rescript/darwin-arm64@9149

@rescript/linux-arm64

npm i https://pkg.pr.new/@rescript/linux-arm64@9149

@rescript/linux-x64

npm i https://pkg.pr.new/@rescript/linux-x64@9149

@rescript/runtime

npm i https://pkg.pr.new/@rescript/runtime@9149

@rescript/win32-x64

npm i https://pkg.pr.new/@rescript/win32-x64@9149

commit: 9f7babe

@github-actions

Copy link
Copy Markdown

@cknitt

cknitt commented Oct 31, 2026

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 31, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-31T07:51:38.648994Z 9f7babe Manual request
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ceaaf61f6a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

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".

Comment thread compiler/ml/typecore.ml Resolved
Codex review: is_nonexpansive now saw only the written fields, so with
`type cell<'a> = {tag: int, mutable value?: 'a}` the binding
`let cell = {tag: 0}` generalized `'a`, and `cell.value` could be
written as option<int> and read as option<string>. Without a spread,
every label of the record is allocated, so all must be immutable; with
a spread, the check is unchanged. Master rejects the example as well;
the new super_errors fixture covers it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Kbf3gKyiYMf54wPRxFdUWe
Signed-off-by: Christoph Knittel <ck@cca.io>
@cknitt

cknitt commented Oct 31, 2026

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. More of your lovely PRs please.

Reviewed commit: 9f7babec63

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

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".

@cknitt
cknitt merged commit 940b761 into master Oct 31, 2026
26 checks passed
@cknitt
cknitt deleted the sparse-record-fields branch October 31, 2026 08:29
@cristianoc

Copy link
Copy Markdown
Collaborator

The change is great.
The description of the semantic diff is terrible.
The visualization is good.

aspeddro pushed a commit to aspeddro/rescript-compiler that referenced this pull request Oct 31, 2026
…escript-lang#9149)

* Keep only the written fields of a record literal in the typed tree

Why:
    A record literal stored one entry per field of its type: Kept for a
    field copied from a spread, and a synthesized `None` expression for
    every optional field it left out. JSX DOM props have 612 optional
    fields, so each DOM element added about 610 expressions to the typed
    tree, ~31 KB to the .cmt, and as many Lambda constants (rescript-lang#9148). With
    800 elements, compiling took 0.80 s and 422 MB; now 0.08 s and 50 MB,
    and the .cmt shrinks from 25 MB to 0.8 MB. Small records lose a third
    of their .cmt size. The generated JavaScript does not change.

Typedtree modified (record literals):
Example:
    type r = {a: int, b?: string}
    let x = {a: 1}
Before (typed):
    Texp_record
      fields =
        [
          "a"
            expression
              Texp_constant Const_int 1
          "b"
            expression
              Texp_construct "None"
              []
        ]
After (typed):
    Texp_record
      fields =
        [
          "a"
            expression
              Texp_constant Const_int 1
        ]

Analysis modified (find references):
Example:
    type labelled = {name: string, label?: string}
    let withLabel = {name: "a", label: "b"}
    let withoutLabel = {name: "c"}
    let _ = withLabel.label
Before (text): references of `label`
    26:31 declaration
    27:28 withLabel's field
    28:25 inside {name: "c"}, which has no label
    29:18 withLabel.label
After (text): references of `label`
    26:31 declaration
    27:28 withLabel's field
    29:18 withLabel.label

Review: Where do the left-out fields come from now? Translcore rebuilds
the positional view from lbl_all: a field missing from `fields` is read
from the spread record when there is one, and is `undefined` otherwise.
Review: What else went? The record_label_definition type, the unused
type of Kept fields, and the tuple's optional flag, which was always
false.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Kbf3gKyiYMf54wPRxFdUWe
Signed-off-by: Christoph Knittel <ck@cca.io>

* Treat omitted mutable fields as allocated in the value restriction

Codex review: is_nonexpansive now saw only the written fields, so with
`type cell<'a> = {tag: int, mutable value?: 'a}` the binding
`let cell = {tag: 0}` generalized `'a`, and `cell.value` could be
written as option<int> and read as option<string>. Without a spread,
every label of the record is allocated, so all must be immutable; with
a spread, the check is unchanged. Master rejects the example as well;
the new super_errors fixture covers it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Kbf3gKyiYMf54wPRxFdUWe
Signed-off-by: Christoph Knittel <ck@cca.io>

---------

Signed-off-by: Christoph Knittel <ck@cca.io>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Pedro Castro <aspeddro@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Each JSX DOM element adds ~30 KB to the .cmt: omitted optional record fields become typed-tree expressions

2 participants