Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add fast path for TerminalRow.setChar #628

Merged
merged 1 commit into from
Jun 21, 2018

Conversation

michalbednarski
Copy link
Contributor

Added fast path for TerminalRow.setChar for case where theres 1-to-1 mapping between characters and columns (that is no UTF-16 surrogate pairs and characters with width != 1)

Trying to fix #603, please test @evg-zhabotinsky

In my tests this highly improved performance of cat 500KBfile.txt test.

Still slower than Android Terminal Emulator, but difference is now much smaller and seems to be matter of redraw throttling, without redrawing Termux is now faster (For test commented out line in Termux and Android Terminal Emulator, press volume key in such version to redraw screen)

@evg-zhabotinsky
Copy link
Contributor

evg-zhabotinsky commented Mar 20, 2018

Huh. So it was mostly paranoid unicode handling and not syncing?
Works fine for me, ~5MB hexdump file cating time is down to ~1.5 minutes from ~1 hour before the fix. vim also starts up instantly now. Could be faster, but at that point human is the bottleneck most of the time.

Still, it's a stopgap, and the other code path still needs to be optimized. Suggest keeping the issue open until then.

Copy link

@its-pointless its-pointless left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works well here unless im geting placebo effect... Nice.

@fornwall
Copy link
Member

@michalbednarski Sorry for the delay here! Looks interesting, nice gains without messing up the code.

I think mHasNonOneWidthOrSurrogateChars should be reset to false the clear() method, since otherwise over time all rows will end up having this set to true (as lines are reused after a while in the circular buffer).

@michalbednarski
Copy link
Contributor Author

Rebased and added to flag reset to clear.

@fornwall fornwall merged commit 90b6f93 into termux:master Jun 21, 2018
@fornwall
Copy link
Member

Thanks! This will be in v0.62 of the app, to be released in a few days.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Terminal output speed issues
4 participants