Recently there are quite a few diffs hitting timeout when loaded in phabricator page. The reason is that those tests have huge amount of test results and it takes more than two minutes to finish rendering the user data part of the test results. And more than 90 percent of the time was spending on the shouldMergeBlocks calls. There is some unnecessary work in that function, especially first concatenate the lines and then split it using regex, which is a time killer.
Details
Details
- Reviewers
epriestley lifeihuang JoelB - Group Reviewers
Blessed Reviewers - Commits
- rPHU900e44191d52: Speed up text markup for test results
Load D1071019. It has more than 15k test results and always times out before. Now it loads in about 5 seconds. Click on 'show all test results' seems to bring up all the test results in right format.
Diff Detail
Diff Detail
- Lint
Lint Skipped - Unit
Tests Skipped
Event Timeline
Comment Actions
Broadly, I think the right fix for this problem is to stop rendering 15,000 unit test results, as they're presumably not useful to anyone. In the upstream, we're going to move all build/CI stuff into Harbormaster, but I'm not sure if Facebook is ever going to update Phabricator again so this might not be relevant for you guys.
This is a reasonable change in isolation, though. Some additional cleanup inline, feel free to send me a followup.
| src/markup/engine/PhutilRemarkupEngine.php | ||
|---|---|---|
| 191–195 | We could remove this, it doesn't make much sense to do this check here. I think this is vestigial. There's no reasonable way it will ever do anything useful, we've already called getMatchingLineCount() on the object so at a minimum it quacks like a BlockRule. | |
Comment Actions
Closed by commit rPHU900e44191d52 (authored by @sowedance, committed by @epriestley).