@recaptime-dev's working patches + fork for Phorge, a community fork of Phabricator. (Upstream dev and stable branches are at upstream/main and upstream/stable respectively.) hq.recaptime.dev/wiki/Phorge
phorge phabricator
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

Add a basic profiler to Herald transcripts

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: {F6473407}

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13298

Differential Revision: https://secure.phabricator.com/D20566

+261 -3
+8
src/applications/herald/adapter/HeraldAdapter.php
··· 130 130 131 131 abstract public function getHeraldName(); 132 132 133 + final public function willGetHeraldField($field_key) { 134 + // This method is called during rule evaluation, before we engage the 135 + // Herald profiler. We make sure we have a concrete implementation so time 136 + // spent loading fields out of the classmap is not mistakenly attributed to 137 + // whichever field happens to evaluate first. 138 + $this->requireFieldImplementation($field_key); 139 + } 140 + 133 141 public function getHeraldField($field_key) { 134 142 return $this->requireFieldImplementation($field_key) 135 143 ->getHeraldFieldValue($this->getObject());
+95
src/applications/herald/controller/HeraldTranscriptController.php
··· 77 77 $this->buildTransactionsTranscriptPanel( 78 78 $object, 79 79 $xscript), 80 + $this->buildProfilerTranscriptPanel($xscript), 80 81 ); 81 82 } 82 83 ··· 596 597 return $box_view; 597 598 } 598 599 600 + 601 + private function buildProfilerTranscriptPanel(HeraldTranscript $xscript) { 602 + $viewer = $this->getViewer(); 603 + 604 + $object_xscript = $xscript->getObjectTranscript(); 605 + 606 + $profile = $object_xscript->getProfile(); 607 + 608 + // If this is an older transcript without profiler information, don't 609 + // show anything. 610 + if ($profile === null) { 611 + return null; 612 + } 613 + 614 + $profile = isort($profile, 'elapsed'); 615 + $profile = array_reverse($profile); 616 + 617 + $phids = array(); 618 + foreach ($profile as $frame) { 619 + if ($frame['type'] === 'rule') { 620 + $phids[] = $frame['key']; 621 + } 622 + } 623 + $handles = $viewer->loadHandles($phids); 624 + 625 + $field_map = HeraldField::getAllFields(); 626 + 627 + $rows = array(); 628 + foreach ($profile as $frame) { 629 + $cost = $frame['elapsed']; 630 + $cost = 1000000 * $cost; 631 + $cost = pht('%sus', new PhutilNumber($cost)); 632 + 633 + $type = $frame['type']; 634 + switch ($type) { 635 + case 'rule': 636 + $type_display = pht('Rule'); 637 + break; 638 + case 'field': 639 + $type_display = pht('Field'); 640 + break; 641 + default: 642 + $type_display = $type; 643 + break; 644 + } 645 + 646 + $key = $frame['key']; 647 + switch ($type) { 648 + case 'field': 649 + $field_object = idx($field_map, $key); 650 + if ($field_object) { 651 + $key_display = $field_object->getHeraldFieldName(); 652 + } else { 653 + $key_display = $key; 654 + } 655 + break; 656 + case 'rule': 657 + $key_display = $handles[$key]->renderLink(); 658 + break; 659 + default: 660 + $key_display = $key; 661 + break; 662 + } 663 + 664 + $rows[] = array( 665 + $type_display, 666 + $key_display, 667 + $cost, 668 + pht('%s', new PhutilNumber($frame['count'])), 669 + ); 670 + } 671 + 672 + $table_view = id(new AphrontTableView($rows)) 673 + ->setHeaders( 674 + array( 675 + pht('Type'), 676 + pht('What'), 677 + pht('Cost'), 678 + pht('Count'), 679 + )) 680 + ->setColumnClasses( 681 + array( 682 + null, 683 + 'wide', 684 + 'right', 685 + 'right', 686 + )); 687 + 688 + $box_view = id(new PHUIObjectBoxView()) 689 + ->setHeaderText(pht('Profile')) 690 + ->setTable($table_view); 691 + 692 + return $box_view; 693 + } 599 694 600 695 }
+148 -3
src/applications/herald/engine/HeraldEngine.php
··· 16 16 private $forbiddenActions = array(); 17 17 private $skipEffects = array(); 18 18 19 + private $profilerStack = array(); 20 + private $profilerFrames = array(); 21 + 19 22 public function setDryRun($dry_run) { 20 23 $this->dryRun = $dry_run; 21 24 return $this; ··· 137 140 ->setName($object->getHeraldName()) 138 141 ->setType($object->getAdapterContentType()) 139 142 ->setFields($this->fieldCache) 140 - ->setAppliedTransactionPHIDs($xaction_phids); 143 + ->setAppliedTransactionPHIDs($xaction_phids) 144 + ->setProfile($this->getProfile()); 141 145 142 146 $this->transcript->setObjectTranscript($object_transcript); 143 147 ··· 329 333 break; 330 334 } 331 335 332 - $match = $this->doesConditionMatch($rule, $condition, $object); 336 + // Here, we're profiling the cost to match the condition value against 337 + // whatever test is configured. Normally, this cost should be very 338 + // small (<<1ms) since it amounts to a single comparison: 339 + // 340 + // [ Task author ][ is any of ][ alice ] 341 + // 342 + // However, it may be expensive in some cases, particularly if you 343 + // write a rule with a very creative regular expression that backtracks 344 + // explosively. 345 + // 346 + // At time of writing, the "Another Herald Rule" field is also 347 + // evaluated inside the matching function. This may be arbitrarily 348 + // expensive (it can prompt us to execute any finite number of other 349 + // Herald rules), although we'll push the profiler stack appropriately 350 + // so we don't count the evaluation time against this rule in the final 351 + // profile. 352 + 353 + $caught = null; 354 + 355 + $this->pushProfilerRule($rule); 356 + try { 357 + $match = $this->doesConditionMatch($rule, $condition, $object); 358 + } catch (Exception $ex) { 359 + $caught = $ex; 360 + } 361 + $this->popProfilerRule($rule); 362 + 363 + if ($caught) { 364 + throw $ex; 365 + } 333 366 334 367 if (!$all && $match) { 335 368 $reason = pht('Any condition matched.'); ··· 421 454 422 455 public function getObjectFieldValue($field) { 423 456 if (!array_key_exists($field, $this->fieldCache)) { 424 - $this->fieldCache[$field] = $this->object->getHeraldField($field); 457 + $adapter = $this->object; 458 + 459 + $adapter->willGetHeraldField($field); 460 + 461 + $caught = null; 462 + 463 + $this->pushProfilerField($field); 464 + try { 465 + $value = $adapter->getHeraldField($field); 466 + } catch (Exception $ex) { 467 + $caught = $ex; 468 + } 469 + $this->popProfilerField($field); 470 + 471 + if ($caught) { 472 + throw $caught; 473 + } 474 + 475 + $this->fieldCache[$field] = $value; 425 476 } 426 477 427 478 return $this->fieldCache[$field]; ··· 636 687 637 688 return $is_forbidden; 638 689 } 690 + 691 + /* -( Profiler )----------------------------------------------------------- */ 692 + 693 + private function pushProfilerField($field_key) { 694 + return $this->pushProfilerStack('field', $field_key); 695 + } 696 + 697 + private function popProfilerField($field_key) { 698 + return $this->popProfilerStack('field', $field_key); 699 + } 700 + 701 + private function pushProfilerRule(HeraldRule $rule) { 702 + return $this->pushProfilerStack('rule', $rule->getPHID()); 703 + } 704 + 705 + private function popProfilerRule(HeraldRule $rule) { 706 + return $this->popProfilerStack('rule', $rule->getPHID()); 707 + } 708 + 709 + private function pushProfilerStack($type, $key) { 710 + $this->profilerStack[] = array( 711 + 'type' => $type, 712 + 'key' => $key, 713 + 'start' => microtime(true), 714 + ); 715 + 716 + return $this; 717 + } 718 + 719 + private function popProfilerStack($type, $key) { 720 + if (!$this->profilerStack) { 721 + throw new Exception( 722 + pht( 723 + 'Unable to pop profiler stack: profiler stack is empty.')); 724 + } 725 + 726 + $frame = last($this->profilerStack); 727 + if (($frame['type'] !== $type) || ($frame['key'] !== $key)) { 728 + throw new Exception( 729 + pht( 730 + 'Unable to pop profiler stack: expected frame of type "%s" with '. 731 + 'key "%s", but found frame of type "%s" with key "%s".', 732 + $type, 733 + $key, 734 + $frame['type'], 735 + $frame['key'])); 736 + } 737 + 738 + // Accumulate the new timing information into the existing profile. If this 739 + // is the first time we've seen this particular rule or field, we'll 740 + // create a new empty frame first. 741 + 742 + $elapsed = microtime(true) - $frame['start']; 743 + $frame_key = sprintf('%s/%s', $type, $key); 744 + 745 + if (!isset($this->profilerFrames[$frame_key])) { 746 + $current = array( 747 + 'type' => $type, 748 + 'key' => $key, 749 + 'elapsed' => 0, 750 + 'count' => 0, 751 + ); 752 + } else { 753 + $current = $this->profilerFrames[$frame_key]; 754 + } 755 + 756 + $current['elapsed'] += $elapsed; 757 + $current['count']++; 758 + 759 + $this->profilerFrames[$frame_key] = $current; 760 + 761 + array_pop($this->profilerStack); 762 + } 763 + 764 + private function getProfile() { 765 + if ($this->profilerStack) { 766 + $frame = last($this->profilerStack); 767 + $frame_type = $frame['type']; 768 + $frame_key = $frame['key']; 769 + $frame_count = count($this->profilerStack); 770 + 771 + throw new Exception( 772 + pht( 773 + 'Unable to retrieve profile: profiler stack is not empty. The '. 774 + 'stack has %s frame(s); the final frame has type "%s" and key '. 775 + '"%s".', 776 + new PhutilNumber($frame_count), 777 + $frame_type, 778 + $frame_key)); 779 + } 780 + 781 + return array_values($this->profilerFrames); 782 + } 783 + 639 784 640 785 }
+10
src/applications/herald/storage/transcript/HeraldObjectTranscript.php
··· 7 7 protected $name; 8 8 protected $fields; 9 9 protected $appliedTransactionPHIDs; 10 + protected $profile; 10 11 11 12 public function setPHID($phid) { 12 13 $this->phid = $phid; ··· 46 47 47 48 public function getFields() { 48 49 return $this->fields; 50 + } 51 + 52 + public function setProfile(array $profile) { 53 + $this->profile = $profile; 54 + return $this; 55 + } 56 + 57 + public function getProfile() { 58 + return $this->profile; 49 59 } 50 60 51 61 public function setAppliedTransactionPHIDs($phids) {