Skip to content

Commit a75e2c2

Browse files
sirbrilligsirbrillig
committed
Batch phpcs calls across files into a single invocation
Replace the 2N per-file phpcs launches in runGitWorkflow and runSvnWorkflow with a single batch invocation. All modified and unmodified file contents are written to a temp directory and phpcs is run once on all of them, eliminating the startup overhead cost that scaled linearly with the number of changed files. - Add getPhpcsOutputForGitBatch / getPhpcsOutputForSvnBatch to ShellOperator - Implement batch methods in UnixShell using a temp dir layout (new/ and old/) - Override batch methods in TestShell to delegate to existing per-file mocks - Rewrite runGitWorkflow and runSvnWorkflow with pre-batch/batch/filter phases - Add and update tests for the new batch behavior
1 parent 5c9f6b2 commit a75e2c2

6 files changed

Lines changed: 579 additions & 20 deletions

File tree

‎PhpcsChanged/Cli.php‎

Lines changed: 261 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -229,12 +229,142 @@ function runSvnWorkflow(array $svnFiles, CliOptions $options, ShellOperator $she
229229

230230
loadCache($cache, $shell, $options->toArray());
231231

232-
$phpcsMessages = array_map(function(string $svnFile) use ($options, $shell, $cache, $debug): PhpcsMessages {
233-
return runSvnWorkflowForFile($svnFile, $options, $shell, $cache, $debug);
234-
}, $svnFiles);
232+
$phpcsStandard = $options->phpcsStandard;
233+
$warningSeverity = $options->warningSeverity;
234+
$errorSeverity = $options->errorSeverity;
235235

236-
saveCache($cache, $shell, $options->toArray());
236+
// Pre-batch phase: determine which files need phpcs scans
237+
$needsModifiedPhpcs = [];
238+
$needsUnmodifiedPhpcs = [];
239+
$modifiedOutputs = [];
240+
$unmodifiedOutputs = [];
241+
$isNewFileMap = [];
242+
$modifiedHashMap = [];
243+
$revisionIdMap = [];
244+
245+
foreach ($svnFiles as $svnFile) {
246+
try {
247+
if (! $shell->isReadable($svnFile)) {
248+
throw new ShellException("Cannot read file '{$svnFile}'");
249+
}
250+
251+
$modifiedFileHash = '';
252+
$modifiedCached = null;
253+
if (isCachingEnabled($options->toArray())) {
254+
$modifiedFileHash = $shell->getFileHash($svnFile);
255+
$modifiedCached = $cache->getCacheForFile($svnFile, 'new', $modifiedFileHash, $phpcsStandard ?? '', $warningSeverity ?? '', $errorSeverity ?? '');
256+
$debug(($modifiedCached !== null ? 'Using' : 'Not using') . " cache for modified file '{$svnFile}' at hash '{$modifiedFileHash}', and standard '{$phpcsStandard}'");
257+
}
258+
$modifiedHashMap[$svnFile] = $modifiedFileHash;
259+
260+
if ($modifiedCached !== null) {
261+
$modifiedOutputs[$svnFile] = $modifiedCached;
262+
} else {
263+
$needsModifiedPhpcs[] = $svnFile;
264+
}
265+
266+
$revisionId = $shell->getSvnRevisionId($svnFile);
267+
$isNewFile = $shell->doesUnmodifiedFileExistInSvn($svnFile);
268+
$isNewFileMap[$svnFile] = $isNewFile;
269+
$revisionIdMap[$svnFile] = $revisionId;
270+
if ($isNewFile) {
271+
$debug("File '{$svnFile}' is new; unmodified version will not be scanned.");
272+
}
273+
274+
if (! $isNewFile) {
275+
$unmodifiedCached = null;
276+
if (isCachingEnabled($options->toArray())) {
277+
$unmodifiedCached = $cache->getCacheForFile($svnFile, 'old', $revisionId, $phpcsStandard ?? '', $warningSeverity ?? '', $errorSeverity ?? '');
278+
$debug(($unmodifiedCached !== null ? 'Using' : 'Not using') . " cache for unmodified file '{$svnFile}' at revision '{$revisionId}', and standard '{$phpcsStandard}'");
279+
}
280+
281+
if ($unmodifiedCached !== null) {
282+
$unmodifiedOutputs[$svnFile] = $unmodifiedCached;
283+
} else {
284+
$needsUnmodifiedPhpcs[] = $svnFile;
285+
}
286+
}
287+
} catch( ShellException $err ) {
288+
$shell->printError($err->getMessage());
289+
$shell->exitWithCode(1);
290+
throw $err; // Just in case we do not actually exit, like in tests
291+
}
292+
}
293+
294+
// Batch phase: single phpcs invocation for all uncached files
295+
$batchTime = 0.0;
296+
$batchSize = count($needsModifiedPhpcs) + count($needsUnmodifiedPhpcs);
297+
if ($batchSize > 0) {
298+
try {
299+
$batchStartTime = microtime(true);
300+
$batchResults = $shell->getPhpcsOutputForSvnBatch($needsModifiedPhpcs, $needsUnmodifiedPhpcs);
301+
$batchTime = microtime(true) - $batchStartTime;
302+
} catch( \Exception $err ) {
303+
$shell->printError($err->getMessage());
304+
$shell->exitWithCode(1);
305+
throw $err; // Just in case we do not actually exit, like in tests
306+
}
307+
308+
foreach ($needsModifiedPhpcs as $svnFile) {
309+
$modifiedOutputs[$svnFile] = $batchResults['new'][$svnFile] ?? '';
310+
if (isCachingEnabled($options->toArray())) {
311+
$cache->setCacheForFile($svnFile, 'new', $modifiedHashMap[$svnFile], $phpcsStandard ?? '', $warningSeverity ?? '', $errorSeverity ?? '', $modifiedOutputs[$svnFile]);
312+
}
313+
}
314+
315+
foreach ($needsUnmodifiedPhpcs as $svnFile) {
316+
$unmodifiedOutputs[$svnFile] = $batchResults['old'][$svnFile] ?? '';
317+
if (isCachingEnabled($options->toArray())) {
318+
$cache->setCacheForFile($svnFile, 'old', $revisionIdMap[$svnFile], $phpcsStandard ?? '', $warningSeverity ?? '', $errorSeverity ?? '', $unmodifiedOutputs[$svnFile]);
319+
}
320+
}
321+
}
237322

323+
$timePerFile = $batchSize > 0 ? $batchTime / $batchSize : 0.0;
324+
325+
// Filter phase: compute new messages per file
326+
$phpcsMessages = [];
327+
foreach ($svnFiles as $svnFile) {
328+
$fileName = $shell->getFileNameFromPath($svnFile);
329+
try {
330+
$modifiedOutput = $modifiedOutputs[$svnFile] ?? '';
331+
$modifiedFilePhpcsMessages = PhpcsMessages::fromPhpcsJson($modifiedOutput, $fileName);
332+
$modifiedFilePhpcsMessages->setTiming($fileName, $timePerFile);
333+
$hasNewPhpcsMessages = count($modifiedFilePhpcsMessages->getMessages()) > 0;
334+
335+
if (! $hasNewPhpcsMessages) {
336+
throw new NoChangesException("Modified file '{$svnFile}' has no PHPCS messages; skipping");
337+
}
338+
339+
$unifiedDiff = $shell->getSvnUnifiedDiff($svnFile);
340+
$isNewFile = $isNewFileMap[$svnFile] ?? false;
341+
342+
if ($isNewFile) {
343+
$debug('Skipping the linting of the unmodified file as it is a new file.');
344+
$phpcsMessages[] = getNewPhpcsMessages($unifiedDiff, PhpcsMessages::fromPhpcsJson('', $fileName), $modifiedFilePhpcsMessages);
345+
continue;
346+
}
347+
348+
$unmodifiedOutput = $unmodifiedOutputs[$svnFile] ?? '';
349+
$phpcsMessages[] = getNewPhpcsMessages($unifiedDiff, PhpcsMessages::fromPhpcsJson($unmodifiedOutput, $fileName), $modifiedFilePhpcsMessages);
350+
} catch( NoChangesException $err ) {
351+
$debug($err->getMessage());
352+
$unifiedDiff = '';
353+
$unmodifiedFilePhpcsOutput = '';
354+
$modifiedFilePhpcsMessages = PhpcsMessages::fromPhpcsJson('');
355+
$phpcsMessages[] = getNewPhpcsMessages(
356+
$unifiedDiff,
357+
PhpcsMessages::fromPhpcsJson($unmodifiedFilePhpcsOutput, $fileName),
358+
$modifiedFilePhpcsMessages
359+
);
360+
} catch( \Exception $err ) {
361+
$shell->printError($err->getMessage());
362+
$shell->exitWithCode(1);
363+
throw $err; // Just in case we do not actually exit, like in tests
364+
}
365+
}
366+
367+
saveCache($cache, $shell, $options->toArray());
238368
$shell->clearCaches();
239369
return PhpcsMessages::merge($phpcsMessages);
240370
}
@@ -336,12 +466,135 @@ function runGitWorkflow(CliOptions $options, ShellOperator $shell, CacheManager
336466

337467
loadCache($cache, $shell, $options->toArray());
338468

339-
$phpcsMessages = array_map(function(string $gitFile) use ($options, $shell, $cache, $debug): PhpcsMessages {
340-
return runGitWorkflowForFile($gitFile, $options, $shell, $cache, $debug);
341-
}, $options->files);
469+
$phpcsStandard = $options->phpcsStandard;
470+
$warningSeverity = $options->warningSeverity;
471+
$errorSeverity = $options->errorSeverity;
342472

343-
saveCache($cache, $shell, $options->toArray());
473+
// Pre-batch phase: determine which files need phpcs scans
474+
$needsModifiedPhpcs = [];
475+
$needsUnmodifiedPhpcs = [];
476+
$modifiedOutputs = [];
477+
$unmodifiedOutputs = [];
478+
$isNewFileMap = [];
479+
$modifiedHashMap = [];
480+
$unmodifiedHashMap = [];
481+
482+
foreach ($options->files as $gitFile) {
483+
try {
484+
if (! $shell->isReadable($gitFile)) {
485+
throw new ShellException("Cannot read file '{$gitFile}'");
486+
}
487+
488+
$modifiedHash = '';
489+
$modifiedCached = null;
490+
if (isCachingEnabled($options->toArray())) {
491+
$modifiedHash = $shell->getGitHashOfModifiedFile($gitFile);
492+
$modifiedCached = $cache->getCacheForFile($gitFile, 'new', $modifiedHash, $phpcsStandard ?? '', $warningSeverity ?? '', $errorSeverity ?? '');
493+
$debug(($modifiedCached !== null ? 'Using' : 'Not using') . " cache for modified file '{$gitFile}' at hash '{$modifiedHash}', and standard '{$phpcsStandard}'");
494+
}
495+
$modifiedHashMap[$gitFile] = $modifiedHash;
496+
497+
if ($modifiedCached !== null) {
498+
$modifiedOutputs[$gitFile] = $modifiedCached;
499+
} else {
500+
$needsModifiedPhpcs[] = $gitFile;
501+
}
502+
503+
$isNewFile = $shell->doesUnmodifiedFileExistInGit($gitFile);
504+
$isNewFileMap[$gitFile] = $isNewFile;
505+
if ($isNewFile) {
506+
$debug("File '{$gitFile}' is new; unmodified version will not be scanned.");
507+
}
508+
509+
if (! $isNewFile) {
510+
$unmodifiedHash = '';
511+
$unmodifiedCached = null;
512+
if (isCachingEnabled($options->toArray())) {
513+
$unmodifiedHash = $shell->getGitHashOfUnmodifiedFile($gitFile);
514+
$unmodifiedCached = $cache->getCacheForFile($gitFile, 'old', $unmodifiedHash, $phpcsStandard ?? '', $warningSeverity ?? '', $errorSeverity ?? '');
515+
$debug(($unmodifiedCached !== null ? 'Using' : 'Not using') . " cache for unmodified file '{$gitFile}' at hash '{$unmodifiedHash}', and standard '{$phpcsStandard}'");
516+
}
517+
$unmodifiedHashMap[$gitFile] = $unmodifiedHash;
518+
519+
if ($unmodifiedCached !== null) {
520+
$unmodifiedOutputs[$gitFile] = $unmodifiedCached;
521+
} else {
522+
$needsUnmodifiedPhpcs[] = $gitFile;
523+
}
524+
}
525+
} catch(ShellException $err) {
526+
$shell->printError($err->getMessage());
527+
$shell->exitWithCode(1);
528+
throw $err; // Just in case we do not actually exit
529+
}
530+
}
344531

532+
// Batch phase: single phpcs invocation for all uncached files
533+
$batchTime = 0.0;
534+
$batchSize = count($needsModifiedPhpcs) + count($needsUnmodifiedPhpcs);
535+
if ($batchSize > 0) {
536+
try {
537+
$batchStartTime = microtime(true);
538+
$batchResults = $shell->getPhpcsOutputForGitBatch($needsModifiedPhpcs, $needsUnmodifiedPhpcs);
539+
$batchTime = microtime(true) - $batchStartTime;
540+
} catch(\Exception $err) {
541+
$shell->printError($err->getMessage());
542+
$shell->exitWithCode(1);
543+
throw $err; // Just in case we do not actually exit
544+
}
545+
546+
foreach ($needsModifiedPhpcs as $gitFile) {
547+
$modifiedOutputs[$gitFile] = $batchResults['new'][$gitFile] ?? '';
548+
if (isCachingEnabled($options->toArray())) {
549+
$cache->setCacheForFile($gitFile, 'new', $modifiedHashMap[$gitFile], $phpcsStandard ?? '', $warningSeverity ?? '', $errorSeverity ?? '', $modifiedOutputs[$gitFile]);
550+
}
551+
}
552+
553+
foreach ($needsUnmodifiedPhpcs as $gitFile) {
554+
$unmodifiedOutputs[$gitFile] = $batchResults['old'][$gitFile] ?? '';
555+
if (isCachingEnabled($options->toArray())) {
556+
$cache->setCacheForFile($gitFile, 'old', $unmodifiedHashMap[$gitFile], $phpcsStandard ?? '', $warningSeverity ?? '', $errorSeverity ?? '', $unmodifiedOutputs[$gitFile]);
557+
}
558+
}
559+
}
560+
561+
$timePerFile = $batchSize > 0 ? $batchTime / $batchSize : 0.0;
562+
563+
// Filter phase: compute new messages per file
564+
$phpcsMessages = [];
565+
foreach ($options->files as $gitFile) {
566+
try {
567+
$modifiedOutput = $modifiedOutputs[$gitFile] ?? '';
568+
$modifiedFilePhpcsMessages = PhpcsMessages::fromPhpcsJson($modifiedOutput, $gitFile);
569+
$modifiedFilePhpcsMessages->setTiming($gitFile, $timePerFile);
570+
571+
$unifiedDiff = '';
572+
$unmodifiedFilePhpcsOutput = '';
573+
if (count($modifiedFilePhpcsMessages->getMessages()) === 0) {
574+
throw new NoChangesException("Modified file '{$gitFile}' has no PHPCS messages; skipping");
575+
}
576+
577+
$isNewFile = $isNewFileMap[$gitFile] ?? false;
578+
if (! $isNewFile) {
579+
$debug('Checking the unmodified file with PHPCS since the file is not new and contains some messages.');
580+
$unifiedDiff = $shell->getGitUnifiedDiff($gitFile);
581+
$unmodifiedFilePhpcsOutput = $unmodifiedOutputs[$gitFile] ?? '';
582+
} else {
583+
$debug('Skipping the linting of the unmodified file as it is a new file.');
584+
}
585+
586+
$phpcsMessages[] = getNewPhpcsMessages($unifiedDiff, PhpcsMessages::fromPhpcsJson($unmodifiedFilePhpcsOutput, $gitFile), $modifiedFilePhpcsMessages);
587+
} catch( NoChangesException $err ) {
588+
$debug($err->getMessage());
589+
$phpcsMessages[] = PhpcsMessages::fromPhpcsJson('');
590+
} catch(\Exception $err) {
591+
$shell->printError($err->getMessage());
592+
$shell->exitWithCode(1);
593+
throw $err; // Just in case we do not actually exit
594+
}
595+
}
596+
597+
saveCache($cache, $shell, $options->toArray());
345598
$shell->clearCaches();
346599
return PhpcsMessages::merge($phpcsMessages);
347600
}

‎PhpcsChanged/ShellOperator.php‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,4 +50,26 @@ public function getGitMergeBase(): string;
5050
public function getSvnRevisionId(string $fileName): string;
5151

5252
public function getSvnUnifiedDiff(string $fileName): string;
53+
54+
/**
55+
* Run phpcs once on all modified and unmodified versions of the given git files.
56+
* New files should not appear in $unmodifiedFileNames.
57+
*
58+
* @param string[] $modifiedFileNames files needing modified (new) phpcs output
59+
* @param string[] $unmodifiedFileNames files needing unmodified (old) phpcs output
60+
* @return array{new: array<string,string>, old: array<string,string>}
61+
* Each sub-array maps originalPath => single-file phpcs JSON string
62+
*/
63+
public function getPhpcsOutputForGitBatch(array $modifiedFileNames, array $unmodifiedFileNames): array;
64+
65+
/**
66+
* Run phpcs once on all modified and unmodified versions of the given svn files.
67+
* New files should not appear in $unmodifiedFileNames.
68+
*
69+
* @param string[] $modifiedFileNames files needing modified (new) phpcs output
70+
* @param string[] $unmodifiedFileNames files needing unmodified (old) phpcs output
71+
* @return array{new: array<string,string>, old: array<string,string>}
72+
* Each sub-array maps originalPath => single-file phpcs JSON string
73+
*/
74+
public function getPhpcsOutputForSvnBatch(array $modifiedFileNames, array $unmodifiedFileNames): array;
5375
}

0 commit comments

Comments
 (0)