Repository navigation
evaluator: Stop copying the whole heap on every formatter call - #261
Open
scottanderson wants to merge 1 commit into
Open
scottanderson wants to merge 1 commit into
scottanderson wants to merge 1 commit into
Conversation
Before calling a write formatter, a [[transform]] function or a [[format]] function, the pattern copied the whole heap and put the copy back afterwards. The cost of formatting was the number of patterns times the size of the heap, so a pattern with a large heap could take minutes to format. Formatters can still change globals and other existing heap cells, so their changes still have to be undone. Instead of copying everything, the evaluator now keeps a heap guard. The first time a cell that already existed is changed, the guard saves its contents. When the guard ends, it removes the cells created since it started and puts the saved cells back. The result is the same as before, but the cost only depends on what the formatter changes. Formatting 16384 patterns with a 16 MiB heap now takes about 50ms instead of about 20s.
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.
Problem
Before calling a write formatter, a
[[transform]]function or a[[format]]function, the pattern made a copy of the whole evaluator heap and put the copy back afterwards:So the time to format all patterns was the number of patterns times the size of the heap. In the ImHex-Patterns test runner,
mpq.hexpatonStar_103.exeruns in 25s, but then formatting its patterns takes 18.5 minutes.perfshowed 98.9% of that time inmemmove, called from the heap copy incallUserFormatFunc.What the copy protects
@each get a heap cell. The global scope is never popped, so these cells are still there when patterns are formatted after the run.So simply cutting the heap back to its old size is not enough. That would remove new cells but let changes to globals stay. The new
FormatHeapIsolationtest fails with that approach.Fix
The evaluator now has a heap guard (
pushHeapGuard()/popHeapGuard()):getHeapCellForWriting(). These writes happen in three places:setVariable,accessData(writes, and reads that grow a cell) andgetSection(HeapSectionId).The result is the same as before, but the cost only depends on what the formatter changes, not on the size of the heap. With nested guards, only the innermost guard needs to save a cell, since it restores the cell before outer guards can see it.
All three places in
pattern.hppnow use the guard: write formatters,[[transform]]and[[format]].Tests
FormatHeapGrowth: creates a 16 MiB global string and 16384 patterns with[[format]], then formats all of them. Fails if this takes longer than 5 seconds.FormatHeapIsolation: a formatter and a transform change a globalu32and a globalstr. A formatter that runs afterwards must still see the original values.FormatHeapIsolationAll 81 PatternLanguage tests pass.
Not changed
The old copy only covered the heap, and this PR keeps it that way. Changes a formatter makes to pattern-local storage, or to a pattern's size (for example, assigning a longer string to a global
str), were not undone before and still are not.