Avoid unnecessary work to improve performance #1830
Merged
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
That release is the first to implement parts of the region overlay handling in elisp. The change in the next commit depend on the ability to do just that; it allows decoupling hunk painting from section highlighting. And that in turn enables various significant performance improvements. So we have to do it - poor performance is most often voiced criticism. This had to happen eventually. There have been many small reasons why dropping support for older Emacsen would have benefited the maintainer and the majority of users. Now there is a big one. Note that one of the reasons I released 1.4.0 was to have a recent release which still works with older Emacsen. Remove all the backward compatibility definitions which now are no longer required.
When there is an active hunk-internal region, then no longer create the necessary overlays during section highlighting. Instead create these overlays by hooking into Emacs own region overlay mechanism. Also no longer make the regular region overlay invisible (doing that relied on an error which prevented the overlay from being visible). Instead just don't create that overlay if we create other overlays to represent the region. Beside the hunk-internal region overlays, this also includes the overlays that are created when highlighting sibling sections. However these sibling section overlays are still being created during highlighting.
In `magit-section-update-highlight' check whether the current section is the same as the one that was current before. If that is so do nothing. This makes moving inside diffs much cheaper. We keep track of the current section using the new variable `magit-section-highlighted-section'. Because sibling selection overlays are highlighted during highlighting and therefor not tracked as region overlays, their removal has to be forced by setting `magit-section-highlighted-section' to nil. Therefor we might still do some unnecessary re-highlighting after deactivating the region.
When a hidden hunk section is selected it is no longer highlighted, and when it is deselected it is no longer unhighlighted. In many cases this avoids unhighlighting and the rehighlighting again when moving back, both of which would be useless because the affected part of the hunk is not even visible. This also affects un-/refining. Instead hidden hunks that are not yet highlighted are highlighted when they are expanded. The heading of the hunk, which usually is visible, is un-/highlighted as before, when ever the section is de-/selected. When forgoing to unhighlight a hunk section add it to `magit-section-highlighted-sections' and remove it from `magit-section-unhighlight-sections'. Hunks are still the only sections which are ever added to these variables. Which means that all other sections are always un-/highlighted, even when that would not be necessary. The old reason for why we don't avoid un- and rehighlighting other sections is that doing so is believed to be cheap. Implementing the necessary housekeeping to avoid doing so might be more expensive even. There is a new reason now: if an ancestor section of a hidden hunk section whose body highlighting is not correct were marked were added to the list of fully highlighted sections, then the delayed highlighting of the hunk would fail. All members of `magit-section-highlighted-sections' are known to be fully and correctly highlighted. Other sections might or might not be fully and correctly highlighted, but we don't keep track of it.
The new function `magit-diff-un-/refine-hunk' takes care of everything now. Also remove the variable `magit-diff-refine-hunk-related', which was a temporary hack. Like in the last release and before, it once more is only possible to refine all hunks or the one and only current hunk. Remove the option to refine all selected hunks.
Previously these sections were first painted as unhighlighted and then painted again as highlighted. Never call `magit-diff-paint-hunk' directly in `magit-diff-wash-hunk'. Instead rely on `magit-section-show' triggering the (second) painting. That means that `magit-diff-paint-hunk' now has to be able to figure out whether it is expected to highlight or unhighlight. That could be somewhat expensive, so caller which know which is the case continue to pass on that information. But `magit-section-show' doesn't know. When `magit-diff-paint-hunk' is called without the HIGHLIGHT argument then it used the new `magit-section-selected-p' to figure out whether it should highlight or unhighlight. `magit-section-selected-p' needs to check whether the section it is called on is a member of the multi-section selection, which is expensive if we were to do it several times. So the functions that do unhighlighting now pass along that information, just like the functions that highlight already did.
When the region is active and point is at the end of a `list' section, then do not highlight its heading and those of its children. Without this the region overlay would be completely covered by heading overlays.
This better describes what kind of sibling sections these are: sibling sections which form a valid multi-section selection.
and add doc-string and move definition.
This was referenced Apr 29, 2015
|
If your distribution does not have Emacs 24.4, then please see https://github.com/magit/magit/wiki/FAQ#my-distribution-doesnt-have-emacs-244. Feel free to comment here. |
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.
Avoid re-highlighting when moving within a section, un-/highlighting of sections whose bodies are invisible, and many more such improvements.
Unfortunately it was a prerequisite for these changes to drop support for Emacsen before
24.4.