Page MenuHomePhabricator

Create just a single PhabricatorMarkupEngine when rendering tests
Needs RevisionPublic

Authored by andrewjcg on Oct 22 2013, 11:36 PM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Dec 26, 10:28 AM
Unknown Object (File)
Sat, Dec 21, 2:34 AM
Unknown Object (File)
Thu, Dec 19, 7:26 PM
Unknown Object (File)
Fri, Dec 6, 1:36 PM
Unknown Object (File)
Nov 27 2024, 7:32 AM
Unknown Object (File)
Nov 27 2024, 4:23 AM
Unknown Object (File)
Oct 19 2024, 11:14 PM
Unknown Object (File)
Oct 14 2024, 6:42 PM

Details

Reviewers
None
Group Reviewers
Blessed Reviewers
Summary

We've seen issues where diffs with comically large amounts test
failures can cause the diff to time out when loading. Part of the
issue appears to be that a new PhabricatorMarkupEngine is created
to render each test (which presumably prevents any cache hits?).

This diff just creates a single PhabricatorMarkupEngine and uses
that to marup all the result output from all tests.

Test Plan

Loaded a diff with hundreds of test failures.

Diff Detail

Branch
lots_o_tests
Lint
Lint Passed
Unit
Tests Passed

Event Timeline

Some suggestions inline, if they seem reasonable.

src/applications/differential/field/specification/DifferentialUnitFieldSpecification.php
74

If you use ::getEngine() instead of ::newDifferentialMarkupEngine(), you'll reuse the existing default engine, which is likely appropriate here. It won't include differential.custom-remarkup-rules or differential.custom-remarkup-block-rules, but those are slated for deprecation soon.

121

This is better than what we were doing before, but it's still basically an uncached, unbatched process.

You can cache and batch processing with PhabricatorMarkupOneOff. For each field, do:

$markup_object = id(new PhabricatorMarkupOneOff())
  ->setContent($userdata);
$engine->addObject($markup_object, 'default');

After adding all of the text, do;

$engine->process();

Then, retrieve the markup results with:

$output = $engine->getOutput($markup_object, 'default');

So, overall, it would look something like this:

$markup_objects = array();
foreach ($udata as $key => $test) {
  $obj = new oneoff ...
  $engine->add($obj, ...
  $markup_objects[$key] = $obj;
}
$engine->process();

foreach ($udata as $key => $test) {
  $fancy_userdata = $engine->getOutput($markup_objects[$key], ...
}

(Just pushing this back to your queue.)

Thanks for the feedback! I'll re-diff with those changes hopefully this weekend.

I think this was obsoleted by D7581.