Page MenuHomePhabricator

Make Conduit API calls on `admin.phacility.com` reasonably easy to profile
Open, NormalPublic

Description

bin/host instances and other Phacility operations commands which operate on a list of instances make a Conduit instances.queryinstances call to admin.phacility.com with limit = 1000 after recent changes connected to T12218. This currently takes about a second to resolve.

It appears to be CPU time because the DarkConsole Services tab looks fine. However, it's currently hard to get a profile off the admin tier:

  • Although the XHProf extension is available, the XHProf UI is not installed on admin (and probably should not be).
  • You can not easily profile Conduit methods because it's a POST request.
  • XHProf does not build on PHP7 (and I'm now on PHP7 locally after T12101). This is a minor issue, but complicates things.
  • There isn't an easy way to download an XHProf profile (oh, there is, you just have to actually generate the profile first).
  • There isn't an easy way to upload an XHProf profile (T7954).

Event Timeline

Some possible approaches here:

  • Make phabricator_form() pass the value of __profile__ through, so if you profile a page all the forms on it profile. Then you can profile the console page, submit the form, and pick up the profile on the response page. We already do this for AJAX requests.
  • Provide an easy way to upload XHProf profiles into the XHProf application (so profiles can be reviewed on secure.phabricator.com), likely by allowing you to drag-and-drop an .xhprof profile into XHProf like we support .ics drops.

This is still a little bit of a pain but far more reasonable now, here's an actual profile:

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

Total cost is about 1.6s. Of this:

  • 227ms is spent formatting the result in pretty human-readable JSON. This code doesn't execute on a normal call (we just json_encode()).
  • Roughly 900ms is spent loading Property objects, policy checking and attaching properties. We could safely skip these policy checks, or generally just cache the dictionaries.
epriestley added a revision: Restricted Differential Revision.Nov 28 2017, 3:51 PM
epriestley added a commit: Restricted Diffusion Commit.Nov 28 2017, 7:14 PM

The "keep the profiler on across form submissions" code isn't working on secure.phabricator.com, even though it works locally and __profile__=page appears on the "Request" tab.

This might possibly be: enable_post_data_reading is off (?) and the plugins fire faster than we populate the POST data into $_REQUEST?

Yeah, I think the issue is:

  • We have enable_post_data_reading off to (mostly) support arbitrarily large HTTP VCS pushes (T4369).
  • That means $_POST does not populate automatically.
  • In AphrontApplicationConfiguration, we hook the profiler before we build $_POST.

I think we can reasonably build $_POST a little sooner.

Success! D20046 worked to fix the "profiler not sticking across form posts" issue on secure. 🐈