Page MenuHomePhabricator

Add a basic profiler to Herald transcripts
ClosedPublic

Authored by epriestley on May 31 2019, 2:33 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Mar 23, 7:23 AM
Unknown Object (File)
Sat, Mar 23, 7:22 AM
Unknown Object (File)
Sun, Mar 10, 7:00 PM
Unknown Object (File)
Feb 23 2024, 5:39 AM
Unknown Object (File)
Jan 27 2024, 6:42 AM
Unknown Object (File)
Jan 24 2024, 5:35 PM
Unknown Object (File)
Jan 8 2024, 10:03 PM
Unknown Object (File)
Dec 26 2023, 2:16 AM
Subscribers
None

Details

Summary

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

Repository
rP Phabricator
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

amckinley added inline comments.
src/applications/herald/engine/HeraldEngine.php
358

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
src/applications/herald/engine/HeraldEngine.php
358

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.