Repository navigation
Optimistic rendering #382
Description
To improve the perceived responsiveness of git operations, we can use an optimistic rendering strategy throughout the package. When a git operation is requested, the UI should reflect the expected outcome immediately rather than waiting for the operation to actually complete.
For example:
- A user double-clicks on a file to stage it.
- The staging action is enqueued in the AsyncQueue of git operations.
- The file moves from the unstaged list to the staged list on the very next render.
- At some later event loop tick, the git operation completes, and the view accurately reflects the current states of the git index and working directory tree again.
Considerations
We'll need a consistent strategy for handling errors and failed operations. It would be jarring to stage a file and have it keep jumping back up to the unstaged list.
On a related note, we should decide how far we want to go to handle dependent operations and cancellability. The easiest route would be to ensure that optimistically rendered elements are read-only (you can't unstage an optimistically staged file until the index catches up). It would be possible, if complicated, to handle these though: if an operation hasn't begun yet it could be removed from the queue, for example, or operations could be explicitly linked to dependent state so that failed operations would propagate correctly... at some point we'll need to put a finite depth on that rabbit-hole.
It'd be nice to give users some subtle feedback that pending operations are still in progress. One idea would be to have a throbber in the status bar that animates while the view is known to be out of sync. We could also add animation or other indications directly to optimistically-rendered elements; for example, a filename rendered optimistically in the staged list could have its icon greyed out or at a lower alpha value and fade it in once the real index catches up.
Operations that touch the network (fetches, pushes and pulls) should likely be excluded from optimistic rendering and instead follow the current approach of disabling UI elements and blocking. Because these are "expected" to take more time to complete, optimistically rendering their results would cause doubt that they actually did anything. There's some grey area here about other operations like commit that I'd like to hear others' thoughts on.
Implementation
- Introduce an
Operationhierarchy into our models to represent a pending action. EachOperationimplements two methods: one that returns a Promise performing the git command, and one that mutates the state of model objects (through aRepositoryargument, maybe) to reflect the change it will perform. Mutated model objects should likely be marked in some way (isOptimistic() === true?) to enable UI feedback. - Rather than storing Promises directly,
AsyncQueuestores and executesOperations. - When the
Repositoryis queried for its state, eachOperationcurrently within the queue is given the opportunity to mutate its return value. The mutated state is then reported to the components for rendering.
Based on discussions with @kuychaco, @BinaryMuse and @nathansobo.
Activity
nathansobo commentedon Dec 20, 2016
This sounds cool, but before plunging down this route I wonder if we should consider how hard it would be to just make the real interactions with Git faster. Based on some discussion with @as-cii, it seems like we might be submitting more Git operations to the queue than strictly necessary right now in response to actions. I haven't investigated personally so he may be able to comment further. Also, would it make sense to try
node-gitagain for a select set of operations where low latency is critical, such as staging files and hunks? Perhaps they've resolved their stability issues or perhaps less intensive usage by our application will mean that we don't encounter them. Finally, I wonder how much of the unresponsive feeling might be attributable to blocking the UI on large DOM updates. Basically, I think like we should be sure that simpler optimizations are off the table before adding what seems like a decent chunk of complexity.If we do determine that improving performance either isn't enough or would be more expensive than introducing this optimistic model, then this sounds pretty good. I agree that rolling back when errors occur is going to be the trickiest, though it seems like the most critical place to apply this approach is in staging files and hunks, where errors should be rare. It seems like if there were an error during a staging operation, we'd want to communicate it to the user, in which case rolling back the optimistic update might not feel that jarring. If you agree with me there, maybe we could hold off on exposing the pending UI state. I guess the diabolical case is the user is on a network drive or some other stupidly slow file system and thinks they've staged something when they haven't, but I'm not convinced this possibility warrants the complexity of exposing the pending state in the UI. Most of the time the updates should be flushed well before the user could tab out of Atom to check the state independently.
smashwilson commentedon Dec 20, 2016
I'm definitely 👍 on going the nodegit/libgit2 route again.
What I'd really like to do is revisit the inclusion of an async git API in Atom proper. There's a lot of up-front learning on my part before I could start tackling that, though: on Chromium's threading model, git's threading model, and catching up on all of the obstacles that were encountered during the first attempt. I'd also be nervous going native because the failure cases (C++ crashes) are harder to diagnose.
With that said, shelling out always feels like a smell to me, so if we could get to a reliable, native git interface, that would be ✨ 💯
I'd mentally pegged that as a pretty big undertaking, and this as a medium one that would still deliver value if/when we had more mature git interop. It is quite possible I have those backwards 😁
nathansobo commentedon Dec 21, 2016
I'm reluctant to commit to a Git API in Atom until our usage of it in the GitHub package stabilizes. In the meantime I would rather expose a service so we have finer-grained versioning control. As soon as we've been happily using enough of the API that we're confident we don't want changes then it would make sense to introduce into core. In the meantime I do think we should deprecate the existing API and push people toward a service exposed by this package as soon as it ships
smashwilson commentedon Dec 22, 2016
Excellent point about being able to version the interface. Now that I think about it, exposing git and GitHub operations through the service API of a package is probably the way to go anyway. As long as we expose documentation about it somewhere that package authors can discover, that gives us more flexibility for little cost.
Definitely. Mostly what I'd like to avoid is having two sources of truth for git operations - bundling libgit2 twice, possibly with different versions...