Repository navigation
Conversation
Contributor
Author
|
@codex review |
JonoPrest
added a commit
that referenced
this pull request
Nov 3, 2026
The entry names #9190, so the changelog points at this PR.
Changelog💅 Polish
Semantic Diff1 commit · Full page Remember type path lookups within a compilation3096465 · JonoPrest · Nov 03, 2026, 07:59 UTC
A direct-mapped table now remembers successful lookups and unchanged normalizations. An entry is only reused while the environment tables the lookup read are physically the same (env.types for a Pident, env.components for a Pdot, env.modules and env.components for a normalization), and only within the request that stored it. Changed normalizations are computed again, so paths in types are allocated as before. Paths with a functor application aren't cached, the memo is skipped while loading .cmi files is disabled, and it is emptied when a request ends and when a loaded module is replaced. The two tables start empty and grow from 64 to 4096 slots as entries are stored, and a replaced entry is updated in place, so a small compilation allocates little for them: fixed 4096 slot tables made each runtime file allocate about 3% more words than master. Now the file that grows most, Dom.res, allocates 0.24% more, and the runtime and Belt in total 0.06% less. On the corpus 94% of lookups and 96% of normalizations hit, and the output is byte identical to master. Review
|
Codex Review SummaryThis 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. Delightful! 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". |
JonoPrest
marked this pull request as ready for review
November 3, 2026 07:45
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #9190 +/- ##
==========================================
+ Coverage 81.92% 81.97% +0.04%
==========================================
Files 456 457 +1
Lines 65464 65646 +182
==========================================
+ Hits 53631 53812 +181
- Misses 11833 11834 +1
🚀 New features to boost your workflow:
|
rescript
@rescript/belt
@rescript/darwin-arm64
@rescript/linux-arm64
@rescript/linux-x64
@rescript/runtime
@rescript/win32-x64
commit: |
JonoPrest
added a commit
that referenced
this pull request
Nov 3, 2026
The entry names #9190, so the changelog points at this PR. Signed-off-by: Jono Prest <[email protected]>
JonoPrest
force-pushed
the
jono/type-path-memo
branch
from
November 3, 2026 07:59
06d84a7 to
d5690b3
Compare
|
Developer playground preview: https://rescript-lang.github.io/rescript/dev-playground/?version=pr-9190 |
JonoPrest
force-pushed
the
jono/type-path-memo
branch
4 times, most recently
from
November 9, 2026 07:18
27cb0bf to
4bb2b5d
Compare
🎉 Compiler benchmarksAllocated wordsIn total: 683,552,688 → 683,045,157 (-0.07%).
160 more changes
InstructionsIn total: 9,664,672,297 → 9,630,314,898 (-0.36%). 🎉 Improvement: 4 benchmarks execute more than 1% fewer instructions.
11 more changes
Output sizeIn total: 25,569,598 → 25,571,454 (+0.01%). None of the 13 benchmarks changed by 0.1% or more. Pull request 3096465 compared with base b82d944 in this run. |
JonoPrest
force-pushed
the
jono/type-path-memo
branch
4 times, most recently
from
November 9, 2026 13:05
e23d28f to
dfe4362
Compare
Why:
Typechecking a 552 module React app calls Env.find_type_full 1.76M
times, and normalize_path 666k times from expand_abbrev. Most calls
look up the same few paths against the same environment: `option`
alone is 528k of them, followed by `string`, `array`, `Jsx.element`.
Each lookup walks the open layers of env.types, and expanding a type
that isn't an abbreviation also normalizes its path, which raises and
catches Not_found to return the path unchanged.
A direct-mapped table now remembers successful lookups and unchanged
normalizations. An entry is only reused while the environment tables the
lookup read are physically the same (env.types for a Pident, env.components
for a Pdot, env.modules and env.components for a normalization), and only
within the request that stored it. Changed normalizations are computed
again, so paths in types are allocated as before. Paths with a functor
application aren't cached, the memo is skipped while loading .cmi files is
disabled, and it is emptied when a request ends and when a loaded module is
replaced.
The two tables start empty and grow from 64 to 4096 slots as entries are
stored, and a replaced entry is updated in place, so a small compilation
allocates little for them: fixed 4096 slot tables made each runtime file
allocate about 3% more words than master. Now the file that grows most,
Dom.res, allocates 0.24% more, and the runtime and Belt in total 0.06% less.
On the corpus 94% of lookups and 96% of normalizations hit, and the output
is byte identical to master.
Review: Can an entry be stale? The tables an entry depends on are
immutable, so the same physical table gives the same answer. New unit
tests bind the same path differently in two environments within one
request (recursive type placeholders, recursive module approximations,
an alias) and fail if any of these checks is removed.
Review: Can growing or reusing an entry return a wrong result? Growth moves
each entry to the slot of its path in the larger table, and a lookup still
checks the path, the epoch and the tables of each entry it finds. An entry
is only referenced from its slot, so updating it in place changes no result
a caller holds. A unit test looks up 3000 types across the growth and fails
if a replaced entry keeps its old result.
Signed-off-by: Jono Prest <[email protected]>
JonoPrest
force-pushed
the
jono/type-path-memo
branch
from
November 9, 2026 16:43
dfe4362 to
3096465
Compare
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.
Typechecking a 552 module React app calls
Env.find_type_full1.76M times. Most of those resolve the same few paths against the same environment:optionalone is 528k calls, thenstring,array,Jsx.element. Each one walks the open layers ofenv.types. On top of that, expanding a type that isn't an abbreviation (a variant, record or abstract type) normalizes its path, which raises and catchesNot_foundto return the path unchanged, 666k times.This adds a small direct-mapped memo for both, per request:
env.typesfor aPident,env.componentsfor aPdot, andenv.modulesplusenv.componentsfor a normalization. These tables are immutable, so the same table gives the same answer..cmifiles is disabled, and it is emptied when a request ends or a loaded module is replaced.On the corpus below, 94% of lookups and 96% of normalizations hit.
Clean builds, 8 vCPU Linux VM, median of interleaved runs:
The gain is on code that leans on many small type lookups. Backend code without JSX doesn't change.
Testing:
ounit_type_path_memo_tests.mlbinds the same path differently in two environments within one request (recursive type placeholders, a recursive module approximation, an alias). Each test fails if its check is removed from the memo.type_path_lookup_test.rescovers shadowing, local opens, aliases, functors, first class modules, recursive types and recursive modules. Its output matches master..cmi,.cmj,.astand.cmtare byte identical to master across three production codebases and their dependencies.make testandmake test-rewatchpass.