Repository navigation
Reuse ordinary Lua function artifacts after fresh compile-time execution - #1585
Merged
Merged
Conversation
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. |
Member
Author
|
@codex review |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. 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". |
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.
Ordinary map function edits previously rebuilt the entire Lua backend even when retained frontend IM was available. This adds a post-compile-time artifact path for same-shape primitive function edits with ordinary calls, globals/arrays, real/boolean control flow and preserved names. Fresh compile-time execution and object injection still run; changed owners alone are materialised, lowered and emitted, and unchanged callers keep their bindings.
Generated NodeGraphs/NodeCodec contracts own body snapshots and symbolic operands. Cold admission requires equality with both batch-lowered IM and the batch-emitted definition. Request-local semantic receipts guard compiler facts, non-admitted bodies and admitted shapes; frontend rebuilds invalidate the snapshot. Entry/init/replay products retain batch manifest ownership. Unsupported structural, generic/class, vararg, helper-dependent arithmetic, stack-trace and optimisation cases remain explicitly observable batch paths.
Also repairs Flatten's initializer metadata to follow the replacement assignment, marks IM source fields in the schema, tracks owned function flags, and distinguishes compile-time/runtime lowering in work receipts. No emitted runtime representation or language semantics change.
Validation:
This remains an incremental rollout: fresh compile-time work still copies the whole IM graph, semantic validation scans global state, and script assembly/ABI hashing/payload comparison process full output. Cold snapshot admission adds startup work. The docs and backlog record these remaining costs and producer migrations; this PR does not claim a fully incremental compiler or subsecond reloads.
Controlled warm-edit timing: Windows x64, Java 27, 6 GiB heap, ParallelGC. Three separate JVM trials per map/variant in baseline/symbol/symbol/baseline/baseline/symbol order, exact previous master
1c23fb384c0f6c96da55218a996703378014ee51versus reviewed headef9e5cb3d87d37920f22aa37290c3c6139ccad19; identical current harness, dependencies, exported fixture and flags.Total ranges: Castle Fight old 9.29–9.77 s / new 6.35–6.44 s; Zombie Defense old 5.39–6.06 s / new 4.66–6.32 s. Zombie Defense ranges overlap and one new trial was slower than the old median. Three trials show a local median gain, not a p95 guarantee. Editor queueing and game pickup/application are excluded. All twelve trials matched independent fresh Lua and full manifests and produced one-function payloads. New backend runtime lowering/emission: 1/1 functions; baseline Castle Fight: 36,757/35,723; baseline Zombie Defense: 30,624/29,761. Each new request also lowered one compile-time owner freshly. Reproduce with the checked-in SymbolHotReloadProfile driver and edit specs; full local receipts:
de.peeeq.wurstscript/test-output/symbol-final-benchmarks.json.Cold admission adds work: independent fresh-model compilation oracles took median 8.11 → 14.08 s on Castle Fight and 4.94 → 9.29 s on Zombie Defense. These are compilation-stage observations after model loading, not end-to-end startup measurements. Warm results above use completed admission snapshots. Full private IM copying remains 60,018 functions / 2,339,305 nodes on Castle Fight and 54,551 / 1,519,344 on Zombie Defense. Removing that compile-time bridge and global receipt scans remains necessary for the subsecond target.
Runtime support assessment (separate from compiler speed): all four CF rejections involve closure lineage/identity ambiguity or reuse of closure storage that existing objects may still read. Partial updates commonly retain removed callbacks, skip changed main/package initialization, preserve old initial values, or cannot reconstruct newly added captures on already-created closures. Warning-free receipt is not an in-game whole-commit proof, and the sample includes test-only changes. This is not yet broad everyday-commit coverage. Next coverage work should minimize actual callback/initialization commits, distinguish compiler identity churn from genuine migration requirements, and prove changed behavior in stock Lua and the game rather than relaxing rejection guards. The history campaign used the full backend for all 24 pairs; the speedup claim comes from the separately identified existing-function body probes.