JS runtime: Use classes for Ref and Arena instead of closures#1332
Merged
JS runtime: Use classes for Ref and Arena instead of closures#1332
Conversation
jiribenes
commented
Mar 21, 2026
|
|
||
| const TOPLEVEL_K = (x, ks) => { throw { computationIsDone: true, result: x } } | ||
| const TOPLEVEL_KS = { prompt: 0, arena: Arena(), rest: null } | ||
| const TOPLEVEL_KS = { stack: null, prompt: 0, arena: new Arena(), rest: null } |
Contributor
Author
There was a problem hiding this comment.
Explicitly setting stack: null seems to help V8 so that it knows it's always the same "type".
Same for the one created in RESET.
Contributor
Author
|
I think this is largely uncontroversial, so I'll merge this -- we can always revert later before the release if the Effekt Plots decide to go into an undesirable direction. |
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.
Previously, we:
var, we allocated a new closure for its ownsetmethodRESET, we allocated two closures for theirfreshandarenamethods... what if we, uh, didn't do that?
This could have been resolved both with prototypes and classes, but I chose to use classes since intent is important and classes are better recognisable by tooling. (and by JSDoc which is relevant for #1218)
The benchmark results are pretty strong: geomean is like a 4-5% speedup on my machine, and all benchmarks got faster, yet!
These are the biggest changes (ran with hyperfine, warmup = 5, N ≥ 10, inputs from the
config_js.txtfile), everything else is negligible as per the Welch t-test (using mean, stddev & number of runs; the tests where hyperfine complained about outliers are deemed as not significant):