memoization of step heaps, effectively preallocating weak arrays#16
Open
nilsbecker wants to merge 4 commits intodbuenzli:masterfrom
Open
memoization of step heaps, effectively preallocating weak arrays#16nilsbecker wants to merge 4 commits intodbuenzli:masterfrom
nilsbecker wants to merge 4 commits intodbuenzli:masterfrom
Conversation
Owner
It's not evil but it invalidates the thread safety properties mentioned in the documentation... Interesting though. |
Author
|
i see. i guess to make something like this thread safe it would be better to work with immutable values? not sure how this would be done, or whether it can be done efficiently. about speed: i noticed that the speedup factor is larger when the call graph is small. it seems that a constant, relatively small overhead per step creation is removed, and this is most noticeable for small call graphs. |
the idea was to save on gc load by letting weak arrays expire by themselves test.ml passes. the non-emptyness of the arrays above Wa.len does not seem to be a problem
this saves a lot of allocation/gc of weak arrays and gives speedups up to 40% in overhead dominated simulation tests
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.
this is another performance improvement suggestion. it's entirely possible that this breaks the JS backend. i have not tried.
weak array creations are reduced further by keeping old weak array heaps from previous steps around in a primitive stack. in my benchmarks where steps are fired in a tight loop, the commit cee407f decreased the runtime of a simulation between 20 and 40%. similar improvements are also seen in the minibenchmark https://gist.github.com/nilsbecker/2ec8d0a136e2fcc20184#file-react_minibenchmark-ml . compared to react 1.2.0, in my specific use cases, this in some cases gives a speedup of around 2 times.