Page MenuHomePhabricator
Create Task
Maniphest T7776

Loading delta between two diffs in Differential can result in diffs for large files failing to load
Open, Needs TriagePublic
Actions

Description

I can consistently repro this problem with Differential, and have (so far) found two different cases that repro this.

If I load the page for the revision (/DXXXX), all files load within a couple of seconds. However, if I use the "Show Diff" button in the "Revision Update History" section of the page to look at the delta between two diffs (ending up on a URL like /DXXXX?vs=YYYY&id=ZZZZ&whitespace=ignore-all#toc), then the following happens:

Most files load just fine, but several files stay stuck on "Loading..." indefinitely:

Screenshot_2015-04-07_14.59.07.png (177×1 px, 19 KB)

By looking at the network tab of devtools, I can see that a request to /differential/changeset/ 500s with the response:

>>> UNRECOVERABLE FATAL ERROR <<<

Maximum execution time of 30 seconds exceeded

/phab_path/libphutil/src/utils/utils.php:711


┻━┻ ︵ ¯\_(ツ)_/¯ ︵ ┻━┻

This also causes many JS errors, presumably because of the file that hasn't loaded:

Uncaught Error: JX.$('C2844557NL0') call matched no nodes.

Potentially helpful data: We're using git. In both the repro cases I found, the files that fail to load are >10k lines (whereas files that are <10k lines load fine). But again, these >10k line files load fine in the normal revision view -- it's only in the delta-between-two-diffs view that problems occur.

If you need any more info to debug or repro, let me know.

Revisions and Commits

rP Phabricator
D12730rPabe28e628a7c Improve performance of generating synthetic changesets

Related Objects

Event Timeline

jhurwitz created this task.Apr 7 2015, 10:10 PM
jhurwitz raised the priority of this task from to Needs Triage.
jhurwitz updated the task description. (Show Details)
jhurwitz added a project: Differential.
jhurwitz added a subscriber: jhurwitz.
dgoldstein added a subscriber: dgoldstein.Apr 8 2015, 1:19 AM
epriestley added a subscriber: epriestley.Apr 11 2015, 9:18 AM

This is probably 2-3 hours of work at the outside, but I'd have to spend 30 minutes poking around with a profiler to be sure. There's a small chance it's more involved, but I believe we're just doing something needlessly inefficient when computing diffs-of-diffs and that there's nothing fundamentally hard or involved here.

• sshannin added a subscriber: • sshannin.Apr 19 2015, 4:00 PM
jhurwitz mentioned this in T7895: Make HIGHLIGHT_BYTE_LIMIT configurable.Apr 23 2015, 9:20 PM
jhurwitz added a comment.Apr 23 2015, 9:22 PM

In case it helps: I set debug.time-limit to 25s and got this stacktrace.

AphrontApplicationConfiguration.php:196 PhabricatorStartup->onDebugTick()
AphrontApplicationConfiguration.php:121 AphrontApplicationConfiguration->processRequest()
index.php:19 AphrontApplicationConfiguration->runHTTPRequest()
epriestley added a revision: D12730: Improve performance of generating synthetic changesets.May 6 2015, 1:02 AM
epriestley added a commit: rPabe28e628a7c: Improve performance of generating synthetic changesets.May 6 2015, 11:43 PM
angie added a project: Restricted Project.May 20 2015, 5:15 PM
angie added subscribers: aroravishal, • rohansharmaa.May 20 2015, 5:20 PM
epriestley added a comment.Sep 10 2015, 12:41 AM

Did D12730 resolve this?

dgoldstein added a comment.Sep 10 2015, 12:43 AM

@jhurwitz do you remember a diff that we saw this on?

eadler added a subscriber: eadler.Apr 14 2016, 5:39 AM
eadler added a project: Restricted Project.Aug 5 2016, 5:23 PM
Phabricator · Built by Phacility