Page MenuHomePhabricator

Add a basic profiler to Herald transcripts

Authored by epriestley on May 31 2019, 2:33 PM.



Ref T13298. Add a simple profiler as a starting point to catch any egregiously expensive rules or conditions.

This doesn't profile rule actions, so if "Add subscriber" (or whatever) is outrageously expensive it won't show up on the profile. Right now, actions get evaluated inside the Adapter so they're hard to profile. A future change could likely dig them out without too much trouble. I generally expect actions to be less expensive than conditions.

This also can't pin down a specific condition being expensive, but if you see that H123 takes 20s to evaluate you can probably guess that the giant complicated regex is the expensive part.

Test Plan

Screen Shot 2019-05-31 at 7.24.39 AM.png (416×1 px, 47 KB)

Diff Detail

rP Phabricator
Lint Not Applicable
Tests Not Applicable

Event Timeline

amckinley added inline comments.

We don't expect to ever hit this; this is just to prevent the profiler from losing its mind if someone has a broken custom Herald rule or something, right?

This revision is now accepted and ready to land.Jun 4 2019, 9:26 PM

Right. There might be some crazy pathway where we can hit this upstream -- maybe "Another Herald Rule" causing initial evaluation of a rule with a "Content added" rule on a commit which hits a Conduit exception because of a bad repository cluster configuration -- but this isn't routine and the exception handling is so the profiler doesn't explode when it exits with a nonempty stack or report something wildly misleading that throws us off the scent of the actual issue.