-
-
Notifications
You must be signed in to change notification settings - Fork 539
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
Improve redraw performance #948
base: master
Are you sure you want to change the base?
Conversation
Does it make a noticeable difference? Did you do any measuring? |
I tried scrolling through revlog continuously, and batching up to 10 input events was enough to clear the input event congestion (for a debug build on my setup) |
ok I was asking if you can see a noticeable difference though? |
I checked out your branch. I cannot see any difference in a debug build compared to a debug build from master though. do you? |
I've recorded screencasts do demonstrate the case - in both recordings I was holding the down arrow key until the revlog passed ~1400 commit number. The master build keeps processing the congested events long after the 1400 commit mark and the version with event batching stops scrolling just in time, showing the absence of congested events. |
Honestly I think the only correct solution would be to move data fetching away from the UI thread, which would make this PR unnecessary. I'm new to rust, so I'm still trying to wrap my head around concurrency vs ownership. |
Not sure what you mean. What particular data fetching? Most expensive ones are run on a threadpool |
The revlog update in particular, it runs a |
well this is unlikely since this is only done once for every 1200 commits in a batch. did you measure this to be sure? if I only scroll inside the batch (so no |
Moving status updates off the UI thread seems to have worked out, redraw speed improved greatly and UI updates following repo status changes still work correctly, tested the case with "abort merge" command appearing after creating a merge commit. |
if we want to go forward with this we would need to bring it up to date with master |
This issue has been automatically marked as stale because it has not had any activity half a year. It will be closed in 14 days if no further activity occurs. Thank you for your contributions. |
This Pull Request fixes/closes #869.
It changes the following:
I followed the checklist:
make check
without errors