Page MenuHomePhabricator

Phabricator web UI is unusable when patch gets too big
Closed, WontfixPublic

Description

Here is an example of such a monster patch: https://phabricator.haskell.org/D246

On my machine, just loading the page pegs my CPU for a while. After it quiesces, I can scroll a bit, but now most UI interactions (e.g. clicking a line number to add an inline comment) have a three to four second lag. This is on Chrome; Firefox is almost nearly completely unusable.

I'm not really sure how you'd go about fixing this; maybe lightening up on the JavaScript/HTML styling on really big pages to something minimal but which works. But maybe at least, the tools should warn if your diff is too big for the tools to handle reasonably.

Related Objects

Event Timeline

ezyang raised the priority of this task from to Needs Triage.
ezyang updated the task description. (Show Details)
ezyang added a project: Phabricator.
ezyang added a subscriber: ezyang.

We don't expect patches of this size to be reviewed in Phabricator. Arcanist should warn before submission.

https://secure.phabricator.com/book/phabflavor/article/writing_reviewable_code/

epriestley claimed this task.

We collapse text in large diffs by default, but show files with inline comments (including lint messages). Because this diff has 250 lint messages, most of the files expand, defeating the "this is a large diff, so collapse to speed it up" feature. D9991 is an example of a diff on this install which is larger (affects more files) but loads very quickly because files collapse by default (as it does not have lint messages). A potential workaround for this is to use --nolint when running arc diff to suppress linting.

In theory we could try to relax the "inlines" rule and try to detect when there are too many inlines, or provide more UI options, but I think this case is very unusual (we haven't seen other similar reports) and the additional complexity cost isn't worth improving this edge case.

See also Differential: Large Changes for some discussion about reviewing (and not reviewing) large changes. I'm not sure if the linked revision is really applicable there, but it seems like a lot of it might have been machine-transformed and be unlikely to receive detailed human review.

Sorry for all the noise, but is this WONTFIX for having Arcanist warn when the diff is really big?

Arcanist does warn when it's a big diff.