This repository was archived by the owner on Dec 15, 2022. It is now read-only.
Repository navigation
This repository was archived by the owner on Dec 15, 2022. It is now read-only.
Staging and unstaging files feels unresponsive #201
Description
Not sure this is due to shelling out to perform these actions, but I'm really noticing the lag when selecting a file to stage or unstage in the changed files list. I think we need to do something to make it feel more responsive... I favor making changes in the UI optimistically and rolling them back if there's an error. We could also show some kind of loading state but I don't think that's ideal.
Activity
BinaryMuse commentedon Mar 5, 2017
We've made a bunch of changes to the async queue and the model observer to improve performance. Other than #386, are you okay with closing this?
smashwilson commentedon Mar 5, 2017
Based on some Slack conversations with @nathansobo I'm guessing the answer is "no" 😉 He's suggested that we target <100ms as an acceptable time window to produce a visible response to user input, in accordance with Google's RAIL performance model. We're currently hovering around ~500ms, and that'll just get worse on large repositories or slower machines.
Offhand I can't think of anything else but #382 that would reliably bring us under that time window - but I also think #382 is reasonably attainable for public launch if we scope it to staging and unstaging operations.
smashwilson commentedon Mar 5, 2017
Just to brainstorm some other avenues to explore:
git statuscall in nodegit took much longer than shelling out..git/HEADhasn't been touched. The downsides here are (a) the most expensive call,git status, is also the one we'd need most often anyway, and (b) filesystem events are noisy and occasionally lossy, so we'd risk missing an event and displaying stale data.nathansobo commentedon Mar 5, 2017
I agree about focusing on staging and unstaging files and hunks and 100ms response being the target. Google has research backing up my intuition that it feels unsettling to interact in a fine-grained way with interfaces that don't update at least that fast. Even if we have to change the color of the hunk or something to confirm it being clicked, I really think we need some sort of response so staging doesn't feel like swimming in molasses.
smashwilson commentedon Apr 22, 2017
Automatic refresh (#560) is in. It helps somewhat, but I've noticed that on Windows this is really bad:
immediate-refresh.zip
The
git apply --cached -operation there alone takes 120ms. I'll profile on my Mac on Monday to see how we're doing on that front.#671 is up next in this effort.
BinaryMuse commentedon Apr 27, 2017
With #633, and #654, and #654, we're down to right around *(and often under) 100ms for a stage/unstage-to-rerender time.
#712 brings this down even more by allowing
statuscalls to be parallelized, saving not only the amount of time it takes to executestatus, but also the time lost by segregating the call tostatusaway from other read operations (so sometimes up to ~30-50ms total).This doesn't even include #700 yet, which will completely obviate the need to run certain commands at all in many cases.
BinaryMuse commentedon Apr 27, 2017
@smashwilson Curious if you have a similar waterfall on the Windows machine you used ot generate the graph above?
smashwilson commentedon Apr 28, 2017
@BinaryMuse Tomorrow's my Windows Day ™️, so I'll collect a fresh waterfall tomorrow morning with all of the improvements 👍
smashwilson commentedon Apr 28, 2017
Here's a typical Windows trace for a stage-line operation on the current master (5bdca1d):
master-stage-line.zip
Still not great but a good half-second faster 👍
Also, just for comparison, here's a waterfall of the same operation with master merged in to #700:
700-stage-line.zip
Almost another half-second off, even with the doubled operations 🎉
nathansobo commentedon May 1, 2017
Really appreciate the work y'all are doing to bring these times down. It will make a huge difference to the experience of staging in Atom.
smashwilson commentedon May 3, 2017
#700 is in! I've also added #727 to track a follow-on issue that's dampening its impact somewhat.
smashwilson commentedon May 11, 2017
I'm moving this to Complete on our ship project because we've shipped all the work we want to include in the public ship, but I'll leave the issue open for later prioritization of the caching improvements.