Screen optimizations #160
Open
Screen optimizations #160
Conversation
This file contains 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
Since 0.8.1 pyte does not support Python 2.x anymore so it makes sense to upgrade one of its dev dependencies, pyperf.
Receive via environ the geometry of the screen to test with a default of 24 lines by 80 columns. Add this and the input file into Runner's metadata so it is preserved in the log file (if any)
Implement three more benchmark scenarios for testing screen.display, screen.reset and screen.resize. For the standard 24x80 geometry, these methods have a negligible cost however of larger geometries, they can be up to 100 times slower than stream.feed so benchmarking them is important. Changed how the metadata is stored so on each bench_func call we encode which scenario are we testing, with which screen class and geometry.
A shell script to test all the captured input files and run them under different terminal geometries (24x80, 240x800, 2400x8000, 24x8000 and 2400x80). These settings aim to stress pyte with larger and larger screens (by a 10 factor on both dimensions and on each dimension separately).
The input files in the tests/captured must be loaded with ByteStream and not Stream, otherwise the \r are lost and the benchmark results may not reflect real scenarios.
The former `for x in range(...)` implementation iterated over the all the possibly indexes (for columns and lines) wasting cyclies because some of those indexes (and in some cases most) pointed to non-existing entries. These non-existing entries were faked and a default character was returned in place. This commit instead makes display to iterate over the existing entries. When gaps between to entries are detected, the gap is filled with the same default character without having to pay for indexing non-entries. Note: I found that in the current implementation of screen, screen.buffer may have entries (chars in a line) outside of the width of the screen. At the display method those are filtered out however I'm not sure if this is not a real bug that was uncovered because never we iterated over the data entries. If this is true, we may be wasting space as we keep in memory chars that are outside of the screen.
Python generators (yield) and function calls are slower then normal for-loops. Improve screen.display by x1 to x1.8 times faster by inlining the code.
The assert that checks the width of each char is removed from screen.display and put it into the tests. This ensures that our test suite maintains the same quality and at the same time we make screen.display ~x1.7 faster.
Instead of computing it on each screen.display, compute the width of the char once on screen.draw and store it in the Char tuple. This makes screen.display ~x1.10 to ~x1.20 faster and it makes stream.feed only ~x1.01 slower in the worst case. This negative impact is due the change on screen.draw but measurements on my lab show inconsistent results (stream.feed didn't show a consistent performance regression and ~x1.01 slower was the worst value that I've got).
Fetch some attributes that were frequently accessed in the for-loop of screen.draw avoiding accessing them on each iteration. Most of them remain constant within the draw() method anyways. Others, like cursor.x, cursor.y and line are updated infrequently inside the for-loop so it still faster pre-fetch them outside and update them if needed than accessing them on each iteration. Benchmark results show stream.feed is x1.20 to x2.0 faster with these optimizations. Benchmark files that have more control sequences (like htop, mc and vim) have a lower improvement as the parsing of these sequences dominates the runtime of stream.feed.
Instead of checking if cursor_x > columns at the end of iteration and set cursor_x to the minimum of (cursor_x and columns), delay that decision to the begin of the next iteration or at the end of the for-loop. This removes one "if" statement at the end of the for-loop and allows us to use the local variable cursor_x all the time without having to update cursor.x. Only this happens before insert_characters() and at the end of the draw() method when the cursor.x is been visible by code outside draw() and therefore must be updated with the latest value of cursor_x. This optimization makes stream.feed between x1.05 and x1.14 faster. As in any optimization on draw(), the use cases that gets more improvements are the ones that have very few control sequences in their input (so stream.feed is dominated by screen.draw and not be stream._parse_fsm)
Make Char mutable (and ordinary object) so we can modify each char in place avoiding calling _replace. This commit only changed the Char class and implements some methods to emulate the namedtuple API. Theoretically it could be possible to emulate the whole namedtuple API but it is unclear if it worth. In this scenario, user code may break. Using a plain object instead of a namedtuple added a regression on memory usage of x1.20 for htop and mc benchmark files when HistoryScreen was used. The rest of the benchmarks didn't change significantly (but it is expected to be slightly more inefficient).
Reduce the memory footprint reusing/sharing the same CharStyle object among different Char instances. A specialized _replace_data changes the data and width of the char but not its style. This reduces the footprint between x1.05 and x1.30 with respect the 0.8.1-memory.json baseline results.
…ter; regress on mem) Instead of calling _replace() to create a new Char object, modify the existing one. For that, the Line (ex StaticDefaultDict) is in charge to fetch the char and do the modifications. If no Char is found, only then a Char is created and inserted in the Line (dict). See write_data(). In some cases we need to get a Char, read it and then update it so a copy of the Line's default char is returned and added to the line. A copy is required because now the Char are mutable. See char_at(). Changed the API of Char: _asdict renamed as as_dict and _replace as copy_and_change; removed _replace_data and added a copy method. The constructor also changed: it is required data, width and style. The former way to construct a Char can be done with from_attributes class method. This commits improved the runtime of stream.feed by x1.20 to x1.90 (faster) however a regression on the memory footprint was found (between x1.10 and x1.50). I don't have an explanation for this last point.
The test was using a legacy API of screen.buffer when the buffer was a dense matrix. Now it is sparse we cannot use len(screen.buffer) anymore or buffer[-1] either.
This improvement impacts slighly negatively over small geometries (x1.01 to x1.05 slower) but improves on larger geometries and for almost all the cases of HistoryScreen (x1.10 to x1.20)
It is handy to get some stats about the layout and chars locations in the lines/buffer and see how sparse they are. The statistics are not part of the stable API so they may change between versions.
This layer of abstraction will allow use to changes on the real buffer without breaking the public API.
Because screen._buffer is a defaultdict, an access to a non-existent element has the side effect
screen.default_char is always the space character so instead of using screen.default_char for padding we use the space character directly.
Because the default char of a line is always the space character, we don't need to overwrite it with screen.default_char, just we need to change its style.
If the next line of the margin's top was not empty, the for-loop used that entry to override the top line working as expected. But when the next line of the top was empty, the top line was untouched so an explicit pop is required. A similar issue happen on reverse_index. Both bugs were covered due a side effect to iterating over screen.buffer: on each line lookup, if no such exist, a new line is added to the buffer. This added new line then was used to override the top on an index() call.
If the line's default char and the cursor's attributes are the same, instead of writing spaces into the line, delete the chars. This should be equivalent from user's perspective. This applies to erase_characters, erase_in_line and erase_in_display. This optimization reduced the number of false lines (lines without any char) and the number of blank lines (lines with only spaces). With less entries in the buffer, the rest of the iterations can take advantage of the sparsity of the buffer and process much less.
We avoid a full scan and instead we do a sparse iteration. This also avoids adding empty lines into the buffer as real entries when a non-existing entry has the same effect and it consumes less memory.
By default Screen tracks which lines are dirty and should be of interest for the user. This functionality may not be of interest for all the use cases and this tracking is expensibe, specially for large geometries. If track_dirty_lines is set to False, the screen.dirty attribute becomes a dummy or null set that it is always empty saving memory and time.
Instead of using None as a special case, set the margins to (0, lines-1) by default. This may break user code if the user is expecting None as a valid value or if it is setting it. If required we could make screen.margins a property and hide the internal implementation.
Using a defaultdict can easily introduce false entries in the buffer making it less sparse. The new Buffer class supports all the dict's operations but it does not add an entry if the key is missing. To add a new entry (a new line), do a buffer.line_at(y). This is equivalent to dict.setdefault(y, new_line()) but avoids the call to new_line() if an entry y exists.
When the graphic attributes are disabled, select_graphic_rendition always set the default style. With this, the lines' default char and the cursor's char will always match and the screen will optimize the erase* methods.
…ttom empty lines This is an optimization over screen.display where it is possible to strip left/right spaces of the lines and/or filter top/bottom whole empty lines. It is implemented in an opportunistic fashion so it may not fully strip/filter all that it is expected. In particular, lines with left or right spaces with non-default attributes are not stripped; lines that contains only spaces at the top or bottom are not filtered (even if lstrip/rstrip is set). This implementation is meant to be used when the screen has very large geometries and screen.display wastes too much time and memory on padding large lines and/or filling with a lot of empty lines.
|
@superbobry , the PR is quite long but I have no problem to review it with you, piece by piece. Despite its length I think that the changes are an solid improvement (ok, I'm a little biased here). A possible first round for the review process could be:
This walkthrough covers ~30% of the PR. From there we can think in the next rounds for review. Thank for your time! |
superbobry
force-pushed
the
master
branch
2 times, most recently
from
6cea8ec to
259ee02
Compare
|
Bump - any chance of reviewing this? I've also been experiencing performance issues (I'm using feed). It's 26 commits behind but it looks pretty easy to sync up. |
Hi @Moult , I'm ok to assist in the review of the PR but I'm not sure if there is intention to merge it by the owners of the project. Until that, doing a rebase + fixing the conflict may yield no result. If you want, you could try to use the PR directly to see if there is an improvement in the performance or not. Depending of your use case, you may also try a fork that I made where I removed features in order to be a little faster. PR branch (compatible with the functionality of |
|
Thanks! I've actually tested it and I can confirm there are really, really significant speedups to feed(), and as far as I can tell (at least in the realm of NetHack ttyrecs) there are no regressions (I did spot incorrect example code for looping through BufferView though in the docstring). I don't use display so I can't really commment there. One minor (major?) issue for me is that the Char namedtuple has changed into a class. I write a ttyrec player, and so to implement a "rewind" feature, I copy the contents of buffer (or now, since there is a BufferView, I copy _buffer). The structure of buffer used to be basically dict[dict[namedtuple]] whereas now it's dict[dict[class[namedtuple, namedtuple]]] and that absolutely kills performance if one needed to do a copy of buffer 500,000 times like myself. I naively reintroduced the Char namedtuple and tweaked write_data to use it and it seems to work fine again, and now super fast buffer copying via this code works. I'd love to know if this has side effects or why a Char class was chosen: # Example copy which is only fast if Char is a namedtuple, and not possible if Char is a class
buffer_copy = {y: dict(row) for y, row in self.screen._buffer.items()} |
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.
What is this PR about?
Optimization. My goal was to make
pytefaster and lighter speciallyfor large geometries (think in a screen of 240x800 or 2400x8000).
Results (overview of the results)
For large geometries (240x800, 2400x8000),
Screen.displayruns several ordersof magnitude faster and consumes between 1.10 and 50.0 times less memory.
For smaller geometries the minimum improvement was of 2 times faster.
Stream.feedis now between 1.10 and 7.30 times faster and ifScreenis tuned, the speedup is between 1.14 and 12.0.However there is a regression for the
mc.inputtest of up to 4 timesslower.
For memory usage,
Stream.feedis between 1.10 and 17.0 times lighterand up to 44.0 times lighter if
Screenis tuned.Screen.reset is between 1.10 and 1.50 slower but several cases improve
if the
Screenis tuned (but not all).Context (background)
byexample executes snippets
of code using real interpreters (Python, Ruby, Java) and capturing the
output so then
byexamplecan check if it is the expected or not.While most of the interpreters are "terminal-naive", some are
"terminal-aware" and they will output escape and control sequences
which obviously are not of interest.
byexampleusespytefor handling those cases (thanks for such greatlib!!).
Unfortunately using a screen introduces artifacts due the hard
boundaries of the screen (24x80). Think in very long lines that are
"unexpectedly" cut into two lines (cut that happen because the screen
has a finite width).
A simple and elegant solution could create screen for larger geometries
where these artifacts are much more rare.
Sadly, while
pyteimplements a sparse buffer, most of its algorithmsare not aware and they don't take advantage of that making the terminal
emulation really slow and consuming a lot of memory.
This is the motivation for this PR: make it perform better!!
Note for the reviewers
This PR is not trivial. The commits are simple of understand (as much as
I could) but still it is a quite large PR.
So I will be available for discussion and explanation of each commit so
I can guide the review process.
Contributions
Screen.display,Screen.resizeandScreen.resetunder different geometries (24x80,240x800, 2400x8000, 24x8000, 2400x80). With these the benchmark takes
much more time (sorry!) but it gives a deeper view of how
pyteworks.Streaminstead ofByteStream. The use of the former led to an incorrect interpretationof the new lines; the use of
ByteStreamfixed that and it is alignedwith the
test_input_output.pytests.Screen.displayto work (approx) linearly with the inputand not with the size of the screen (quadratic). Improved by a lot
both for runtime and memory (specially for large geometries).
Screen.compressed_displaythat works similar toScreen.displaybut it allows to "strip" empty space from the left orright and "filter" empty lines on top and bottom of the screen
reducing time and memory.
Screen.drawwith caching of attributes and methods (thesame optimizations already present in
Stream._parser_fsm).Char's foreground, bold, blink (...) into a separatednamedtupleCharStyle. When possible, reuse the same style formultiple characters reducing the memory usage at the expense of an
additional lookup (instead of
char.fgyou havechar.style.fg).Chara mutable object allowing changes in thedataandwidthfields to be in-place instead of creating a newCharobject.Screen.indexandScreen.reverse_indexwhich improved indirectly
Screen.drawandStream.feed.Screen.resizeScreen.tabstopScreenHistory.prev_pageandScreenHistory.next_page.Screen.insert_characters,Screen.delete_characters,Screen.insert_linesandScreen.delete_characterswhich improved the performance of "terminalaware" programs.
Screen's buffer and lines to have insight aboutthe sparsity and usage of these elements. (The API is not not standard
like
DebugScreen).Screen.bufferreturn aBufferView.Retrieve of lines from it yield
LineViewinstead ofLineobjects.This adds an overhead on user code but allows a separation between the
public part and the internals. Iterate over
LineViewstill yieldsCharobjects as usual (to much high penalty otherwise).Screen.history'stopandbottomqueues returnLineViewand notLineobjectsScreen._bufferadictand not adefaultdict. This prevent adding entries unintentionally which wouldmake the buffer less sparse and therefore slow.
attributes of the screen, do not write explicit spaces on erase methods
(
Screen.erase_characters,Screen.erase_in_lineandScreen.erase_in_display)disable_display_graphicisTruepreventScreen.select_graphic_renditionto change the cursor attributes(style). If the cursor attrs don't change, we can optimize the erase
methods. The flag is
Falseby default.but just remove the chars from the buffer. This makes speedup other
algorithms and maintain high the sparsity (and consume less memory).
track_dirty_linesisFalseuse aNullSetforScreen.dirtyattribute to not consume any
memory and discard any element, disabling effectively the dirty
functionality. This saves time and memory for large geometries.
The flag is
Trueby default.Screen.marginalways aMarginobject so we can avoidchecking if it is
Noneor not.Compatibility changes
The following are changes in the API that may break user code. A special
care was taken to avoid this situation.
Charis not longer anamedtupleso things like_replacearegone. If necessary we could reimplement the API of
namedtuplebut Idon't think users will use.
Charis mutable but the user must not relay on this: changes tocharacter will have undefined behaviour. The user must use always the
API provided by
Screen.Charnot longer has attributes forfg,bg,bold. Instead, it has asingle read-only
CharStyle. TheCharclass implementsfg,bg,boldas properties to do the lookup to the style behind the scene. User
code should not break then.
Screen.buffernow is a property that returns aBufferViewwith asimilar API to a dictionary. It yields
LineViewobjects instead ofLineobjects. These in turn yieldCharobjects (not views). User canstill iterate over the lines and chars as if the buffer were a dense
array and not a sparse array as it is really.
Like any view, these are valid until the next modification of the
screen. This change may break user code if it uses
bufferin anotherway.
topandbottomofScreenHistory.historycontainLineViewand notLineobjects. This may break user code.Screen.marginis always aMarginobject: theNonevalue is notlonger supported`.
TL;DR - Numbers overview
The following is a overview of the numbers got. To make this post
as short as possible, the some results were omitted
(rows omitted are marked with
:::).Full benchmark results are left attached in this commit. People are encouraged to do
their own benchmarks for cross validation.
Screen.displayScreen.displaywas optimized to generate large chunks of spacesvery quickly.
For large geometries, this has an huge impact on the performance:
Screen.displaytakes advantage of the sparsity of the screen and thereforeit was indirectly beneficed by the optimizations done across
Screento avoid filling it with false entries.
Screen.displayit was also optimized on memory (tracemalloc) avoidingthen append of each space character separately when they could be
appended in a single chunk.
The only two regressions are:
Not sure why this happen.
Stream.feedstream.feedwas not modified but its runtime depends onScreen'sperformance.
For terminal programs that just write into then terminal, like
cat-gpl3andfind-etc,stream.feedmerely sends then inputto
Screen.drawfor rendering.The method
Screen.drawwas optimized to avoid the modificationof the cursor internally and update it only at the exit. This saved a
few lookups.
While not been frequently called,
Screen.indexwas the next bottleneckfor
Screen.draw: it moves all the lines of the screen which it meansthat all the entries of the buffer are rewritten.
Screen.indexandScreen.reverse_indexwere optimized to take advantageof the sparsity and to avoid adding false entries.
This resulted on a speedup across the tests:
The
mc.inputhowever took much more time.When
track_dirty_linesisFalseanddisable_display_graphicisTrue,the overall performance increases even further.
On memory there is an improvement too:
The following are the tests that show regression on memory usage.
When
track_dirty_linesisFalseanddisable_display_graphicisTrue, this is even better:However, we still have some regressions:
Screen.resetFor
Screen.resetwe have a regressions, some minor, some not-so-muchminor:
However when
track_dirty_linesisFalseanddisable_display_graphicisTrue,the things improves (but we still have regressions):