Open dead-claudia opened 5 years ago
I will note that git
itself reads the diff correctly and doesn't break.
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.
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.
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.
I agree, I think deferred sizing is the only way around this, the computation of everything just takes too long.
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
.
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.
Perhaps related #508
@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.
Platform: macOS High Sierra GitUp version: 1.0.11
Steps to reproduce:
buggy-repo
directory inside of it.master
branch is active and selected.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.