Page MenuHomePhabricator
Create Task
Maniphest T12319

Investigate an Owners-related performance issue on revision detail pages
Closed, ResolvedPublic
Actions

Description

An install is reporting that /Dxxx fatals after 30 seconds ("Maximum execution time of 30 seconds exceeded") in PhabricatorOwnersPackage.php:366.

Relevant details:

  • Change affects ~1500 paths.
  • About 25,000 paths are owned by a package.

This is a little muddy but the issue may be straightforward to identify and resolve.

Intermediate issues:

  • XHProf does not build on PHP7 (T9805) but I'm on PHP7 locally now (T12101).
  • Owners doesn't have lipsum support.

Revisions and Commits

rP Phabricator
D17427rPa9cd14674503 Filter archived packages out of the "controlling packages" query earlier
D17424rP3bea3fbb1289 When computing revision ownership, cache some intermediate results for…
D17423rPb9568646ac00 Add an owners package generator for Lipsum
D17422rP5cb4c76bef6b Add a lipsum generator for Badges
D17421rP3b8ccb0b78ee Add "--force" and "--quickly" flags to `bin/lipsum`
D17420rP99bcf5f112dc Make `bin/lipsum generate` hanldle generator keys and arguments more clearly
D17419rP1b2c047ce0df Correct spelling of "phabrictor" in Lipsum and elsewhere

Related Objects

Event Timeline

epriestley created this task.Feb 26 2017, 3:29 PM
Herald added a subscriber: eadler. · View Herald TranscriptFeb 26 2017, 3:29 PM
epriestley added a subscriber: jmeador.Feb 26 2017, 3:29 PM
epriestley added a revision: D17419: Correct spelling of "phabrictor" in Lipsum and elsewhere.Feb 27 2017, 12:06 PM
epriestley added a revision: D17420: Make `bin/lipsum generate` hanldle generator keys and arguments more clearly.Feb 27 2017, 12:20 PM
epriestley added a revision: D17421: Add "--force" and "--quickly" flags to `bin/lipsum`.Feb 27 2017, 1:14 PM
epriestley added a revision: D17422: Add a lipsum generator for Badges.Feb 27 2017, 1:18 PM
epriestley added a revision: D17423: Add an owners package generator for Lipsum.Feb 27 2017, 3:39 PM
epriestley added a comment.Feb 27 2017, 3:47 PM

After implementing PHP7 support for XHProf and updating Lipsum, I used bin/lipsum to generate 4,000 packages owning 150,000 paths. I think I have an approximately-reasonable reproduction case locally now, which generates this profile (6.5s for a change affecting about 100 files):

https://secure.phabricator.com/xhprof/profile/PHID-FILE-etty5kcw4os7w6pkjqsw/

I'll look at fixing that.

epriestley added a revision: D17424: When computing revision ownership, cache some intermediate results for performance.Feb 27 2017, 4:05 PM
epriestley added a commit: rP1b2c047ce0df: Correct spelling of "phabrictor" in Lipsum and elsewhere.Feb 27 2017, 5:09 PM
epriestley added a commit: rP99bcf5f112dc: Make `bin/lipsum generate` hanldle generator keys and arguments more clearly.
epriestley added a commit: rP3b8ccb0b78ee: Add "--force" and "--quickly" flags to `bin/lipsum`.
epriestley added a commit: rP5cb4c76bef6b: Add a lipsum generator for Badges.
epriestley added a commit: rPb9568646ac00: Add an owners package generator for Lipsum.Feb 27 2017, 5:11 PM
epriestley added a commit: rP3bea3fbb1289: When computing revision ownership, cache some intermediate results for….
epriestley added a comment.Feb 27 2017, 5:13 PM

@jmeador, I believe this is now resolved at HEAD. Let us know if you're still seeing issues after upgrading?

epriestley added a revision: D17427: Filter archived packages out of the "controlling packages" query earlier.Feb 27 2017, 8:11 PM
joshuaspence added a subscriber: joshuaspence.Feb 27 2017, 8:26 PM
epriestley added a commit: rPa9cd14674503: Filter archived packages out of the "controlling packages" query earlier.Feb 27 2017, 8:37 PM
epriestley closed this task as Resolved.Feb 27 2017, 9:46 PM
epriestley claimed this task.

Calling this resolved since the page is now tolerably bad (~5s) which more or less pulls this out of the realm of critical scaling issues.

Phabricator · Built by Phacility