Freezing while trying to browse a repo's history

[dead-claudia]

Platform: macOS High Sierra
GitUp version: 1.0.11

Steps to reproduce:

1. Download this tarball
2. Extract it and navigate to the buggy-repo directory inside of it.
3. Open GitUp from within that directory.
4. Ensure the master branch is active and selected.
5. Enter "Quick View" on the head of the master branch.

Expected: a sensible view to open.

Actual: it freezes with 100% CPU usage and a severe memory leak - hits 1GB for me in about 15 seconds based on just watching on Activity Monitor.

[dead-claudia]

I will note that git itself reads the diff correctly and doesn't break.

[lucasderraugh]

Interesting, it's hitting an infinite loop on the following:

#0	0x0000000101373080 in -[GIDiffContentsViewController tableView:heightOfRow:]
#1	0x00007fff435662c8 in -[NSTableView _sendDelegateHeightOfRow:] ()
#2	0x00007fff4354df0b in -[NSTableView _safeSendDelegateHeightOfRow:] ()
#3	0x00007fff4354de2e in -[NSTableView _uncachedRectHeightOfRow:] ()
#4	0x00007fff4354dbbf in -[_NSTableRowHeightStorage _cacheRowHeights] ()
#5	0x00007fff433c3009 in -[_NSTableRowHeightStorage _ensureRowHeights] ()
#6	0x00007fff433c2f2b in -[_NSTableRowHeightStorage computeTableHeightForNumberOfRows:] ()
#7	0x00007fff433c23ab in -[NSTableView _minimumFrameSize] ()
#8	0x00007fff433c1973 in -[NSTableView tile] ()
#9	0x00007fff433edb5d in -[NSTableView _rectOfRowAssumingRowExists:] ()
#10	0x00007fff433ed95c in -[NSTableView rectOfRow:] ()
#11	0x0000000101368d6f in -[GIDiffContentsViewController setTopVisibleDelta:offset:]

Seems like some height calculation isn't happy. I'll keep investigating what could cause this.

[lucasderraugh]

Ah I just didn't wait long enough. So it's indeed just processing the diff of all the changes. On my machine it does actually get to showing the diff at some point. One thing to note is that unified takes much longer than the split diff. This will take some profiling to figure out if there is something about the text computation that can take less time. Without that, we would need a way to dynamically calculate partial heights and then fault in full heights when they come on screen. Perhaps NSTableView estimated row heights would be useful here.

As it stands, I don't believe there aren't any true memory leaks per se (though I agree it's a lot of memory). There are 3000+ items in that diff and with the current architecture, the entire diff has to be calculated per row in the diff view. These are cached and you'll notice that if you leave and come back, the time to display is almost instant because of the caching. I'm not sure I have the time to think of a different architecture for NSTableView lazy height computations and to test it out, but this is essentially what would be needed if anyone else is willing to tackle the problem.

[zwaldowski]

I’ve been investigating leaning a little more on the native text controls for doing this. Those would get us non-contiguous and asynchronous layout in far less code. Grafting that on to amortize the CTFramesetter would be tricky, but the stuff I’ve done to convince the table view into deferred sizing could translate over and make things a lot better either way.

[lucasderraugh]

I agree, I think deferred sizing is the only way around this, the computation of everything just takes too long.

[dead-claudia]

BTW, I've had it a couple times crash from (I'm guessing) an OOM on my machine. It doesn't complete, but I only have 8GB of RAM total, with usually 4-5GB when idle.

As I noted, Git doesn't hang. Just verified it's basically instant on git diff.

[lucasderraugh]

Ya, most of the time is spent formatting the diff, not reading the text itself (why git diff is fast). We ultimately need a lazy way of doing this that doesn't attempt to format the text for an exact size.

[lucasderraugh]

Perhaps related #508

[lolgear]

@lucasderraugh
Something like:

- (NSInteger)getCurrentPage {
  return 0;
}

- (NSRange)getPageRange {
  return NSMakeRange(MIN(self.getCurrentPage * 10, _deltas.count), MIN(10, _deltas.count));
}

- (NSArray *)getPagedDeltas {
  return [_deltas subarrayWithRange:self.getPageRange];
}

Fixed the problem.