DVCS Graph tweaks #4986
DVCS Graph tweaks #4986
Conversation
Previously, the graph was pressed hard up against the left edge. This commit adds some space to either side of the graph so it's more pleasing on the eye.
Removes duplicate dimension values from RevisionGrid, keeping those in DvcsGraph.
Hoists constant array to field. Reuse adjacent colours list via field. Assert always on UI thread so that this is safe.
Previously the list of junction colours would be created within nested for loops, resulting in potentially very many redundant collections. This makes the collection a field and reuses it. We assert we are on the UI thread so that this is always safe.
Codecov Report
@@ Coverage Diff @@
## master #4986 +/- ##
==========================================
+ Coverage 34.23% 34.27% +0.04%
==========================================
Files 546 546
Lines 43708 43647 -61
Branches 6023 6014 -9
==========================================
Hits 14962 14962
+ Misses 28007 27947 -60
+ Partials 739 738 -1 |
| public readonly List<Junction> Ancestors = new List<Junction>(); | ||
| public readonly List<Junction> Descendants = new List<Junction>(); | ||
| public readonly List<Junction> Ancestors = new List<Junction>(capacity: 2); | ||
| public readonly List<Junction> Descendants = new List<Junction>(capacity: 2); |
There was a problem hiding this comment.
I can understand a node having only two ancestors, but can it not have more than 2 descendants?
There was a problem hiding this comment.
When loading the GE repo in GE, this change avoids ~2MB of redundant heap allocation.
These values don't represent hard limits as the lists can resize if necessary.
The assumption here is that the vast majority of nodes wouldn't have more than two ancestors or descendants.
List<T> has a default capacity of 16 (possibly implementation specific), we save 14 items in each of 2 collections on every node. On GE we have just over 10,000 commits.
So for GE on a 64 bit process allocations are reduced by 14*2*10,000*8 = 1,920,000 bytes.
Maybe we should go further. If we use default capacities of 1 then we would save 2*10,000*8 = 160,000 additional bytes (8% more), but have to resize the array for merge commits etc.
There was a problem hiding this comment.
Thank you for the explanation 👍
This PR contains small commits I've cherry-picked out of another PR related to revision loading performance. Probably easiest to review commit-by-commit as the overall diff is a tad confusing in places.
Changes proposed in this pull request:
IReadOnlyList<T>Screenshots
The only visual change is the padding around the graph. Notice how the 'margin' on the left/right matches that at the top afterwards.
Before
After
What did I do to test the code and ensure quality:
I'd prefer this not to be squashed, as any problems will be easier to identify via bisect with small commits.
Has been tested on: