Add cacheFile option to compileProtoDefSync - #175
Open
u9g wants to merge 1 commit into
Open
Conversation
compileProtoDefSync evals the generated code, so every consumer process
regenerates and reparses the full protocol on startup, and eval is
invisible to V8's on-disk compile cache.
With { cacheFile } the compiler saves the generated module to that path
on first compile and loads it back with require on later runs. The
require path calls module.enableCompileCache() (Node 22.8+, harmless
no-op earlier), so V8 caches the parsed protocol across processes —
that, not skipping generation, is where most of the time goes: a full
Minecraft Java 26.1 client (four states, both directions via
node-minecraft-protocol) drops from ~340ms to ~170ms of protocol setup
per process on an idle M-series Mac.
The cached module exports (native, PartialReadError) => factories,
mirroring the locals the eval closes over, so no globals are involved
(bedrock-protocol's pregenerated data files need global.PartialReadError
today and could migrate to this). Invalidation stays with the caller,
who knows what determines the generated code; any load or write failure
falls back to the normal in-process compile.
Contributor
Author
|
Should we add a default file path? i didn’t in this pr but if we did then everyone who uses protodef gets this caching for free |
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.
Problem
compileProtoDefSync()evals the generated read/write/sizeOf code, so every consumer process pays full code generation and a full V8 parse of the generated source on startup —evalcan never hit V8's on-disk compile cache.Change
compileProtoDefSync({ cacheFile }):cacheFile(temp file + rename, so a concurrent process never requires a half-written file), then load itrequirethe file and skip generationmodule.enableCompileCache()(Node 22.8+; wrapped in try/catch so older Node degrades to a plain require, andNODE_DISABLE_COMPILE_CACHE=1opts out) — this is where most of the win is: V8 caches the parsed protocol code across processes, which theevalpath structurally cannotMeasured with node-minecraft-protocol on an idle M-series Mac, full Minecraft Java 26.1 client protocol setup (4 states × both directions): ~340ms → ~170ms per process once caches are warm.
The cached module exports
(native, PartialReadError) => ctxfactories, mirroring the locals the existingevalcloses over — no globals. Same idea as bedrock-protocol's pregenerateddata/<version>/{read,write,size}.jsfiles (which requireglobal.PartialReadErrortoday), offered as a first-class API it could migrate to.Invalidation is deliberately the caller's job: the caller knows what determines the generated code (protocol JSON, custom types, package versions) and encodes it in the file path. node-minecraft-protocol is the intended first consumer — PR to follow once this lands.
Verification
npm test: 500 passing (3 new: cache write, cache load with byte-identical output, unwritable-path fallback), standard clean.