@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.

Fix explosive runtime of detectCopiedCode()

Summary:
Fixes T5041. Pretty sure this is the issue: if a diff contains a large number of identical lines longer than 30 characters, we end up paying O(N^2) for each set.

Instead, when N > 16, opt to pay 0.

Test Plan: Added a test which dropped from ~100s to ~0 after changes (this diff includes a reduced-strenght version of the test, since parsing a 4,000 line diff is a little bit pricey).

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T5041

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

+47
+10
src/applications/differential/parser/DifferentialChangesetParser.php
··· 1176 1176 $added = array_map('trim', $hunk->getAddedLines()); 1177 1177 for (reset($added); list($line, $code) = each($added); ) { 1178 1178 if (isset($map[$code])) { // We found a long matching line. 1179 + 1180 + if (count($map[$code]) > 16) { 1181 + // If there are a large number of identical lines in this diff, 1182 + // don't try to figure out where this block came from: the 1183 + // analysis is O(N^2), since we need to compare every line 1184 + // against every other line. Even if we arrive at a result, it 1185 + // is unlikely to be meaningful. See T5041. 1186 + continue 2; 1187 + } 1188 + 1179 1189 $best_length = 0; 1180 1190 foreach ($map[$code] as $val) { // Explore all candidates. 1181 1191 list($file, $orig_line) = $val;
+37
src/applications/differential/storage/__tests__/DifferentialDiffTestCase.php
··· 15 15 ipull($copies, 1)); 16 16 } 17 17 18 + public function testDetectSlowCopiedCode() { 19 + // This tests that the detector has a reasonable runtime when a diff 20 + // contains a very large number of identical lines. See T5041. 21 + 22 + $parser = new ArcanistDiffParser(); 23 + 24 + $line = str_repeat('x', 60); 25 + $oline = '-'.$line."\n"; 26 + $nline = '+'.$line."\n"; 27 + 28 + $n = 1000; 29 + $oblock = str_repeat($oline, $n); 30 + $nblock = str_repeat($nline, $n); 31 + 32 + $raw_diff = <<<EODIFF 33 + diff --git a/dst b/dst 34 + new file mode 100644 35 + index 0000000..1234567 36 + --- /dev/null 37 + +++ b/dst 38 + @@ -0,0 +1,{$n} @@ 39 + {$nblock} 40 + diff --git a/src b/src 41 + deleted file mode 100644 42 + index 123457..0000000 43 + --- a/src 44 + +++ /dev/null 45 + @@ -1,{$n} +0,0 @@ 46 + {$oblock} 47 + EODIFF; 48 + 49 + $diff = DifferentialDiff::newFromRawChanges($parser->parseDiff($raw_diff)); 50 + 51 + $this->assertTrue(true); 52 + } 53 + 54 + 18 55 }