Skip to content

Commit 948d258

Browse files
authored
Fix the configuration calculation (#132)
- We previously we doing some calculation based on `$numberOfProcesses` and `$numberOfProcessesDefined`. In reality those calculation were not related directly to those values, but about the fact that child processes we spawned. Indeed when they are not, i.e. we do the processing in the main process, then the segment size and the number of segment becomes irrelevant. I think the logging should be updated to reflect this and better document when child processes are spawned, but this is out of scope of this PR. To address the direct issue though, we now pass directly `bool $shouldSpawnChildProcesses` - The calculation of the segment size and number of segments was incorrect, most likely due to some confusion as to when it is run in the main process or not. - The total number of batches was incorrect: - when no child processes are spawned, it is not the segment size that matters but the total number of items - when child processes are spawned, it may be that the last process does not need to run its maximum number of batches if there is not enough items
1 parent 7851501 commit 948d258

File tree

4 files changed

+201
-172
lines changed

4 files changed

+201
-172
lines changed

src/Configuration.php

+51-8
Original file line numberDiff line numberDiff line change
@@ -35,14 +35,12 @@ final class Configuration
3535
private int $totalNumberOfBatches;
3636

3737
/**
38-
* @param positive-int $numberOfProcesses
3938
* @param 0|positive-int $numberOfItems
4039
* @param positive-int $segmentSize
4140
* @param positive-int $batchSize
4241
*/
4342
public function __construct(
44-
bool $numberOfProcessesDefined,
45-
int $numberOfProcesses,
43+
bool $shouldSpawnChildProcesses,
4644
int $numberOfItems,
4745
int $segmentSize,
4846
int $batchSize
@@ -59,11 +57,24 @@ public function __construct(
5957
),
6058
);
6159

62-
$this->segmentSize = 1 === $numberOfProcesses && !$numberOfProcessesDefined
63-
? $numberOfItems
64-
: $segmentSize;
65-
$this->numberOfSegments = (int) (1 === $numberOfProcesses ? 1 : ceil($numberOfItems / $segmentSize));
66-
$this->totalNumberOfBatches = (int) (ceil($segmentSize / $batchSize) * $this->numberOfSegments);
60+
if ($shouldSpawnChildProcesses) {
61+
$this->segmentSize = $segmentSize;
62+
$this->numberOfSegments = (int) ceil($numberOfItems / $segmentSize);
63+
$this->totalNumberOfBatches = self::calculateTotalNumberOfBatches(
64+
$numberOfItems,
65+
$segmentSize,
66+
$batchSize,
67+
$this->numberOfSegments,
68+
);
69+
} else {
70+
// The segments are what define the sizes of the sub-processes. When
71+
// executing only the main process, then there is no use for
72+
// segments.
73+
// See https://github.com/webmozarts/console-parallelization#segments
74+
$this->segmentSize = 1;
75+
$this->numberOfSegments = 1;
76+
$this->totalNumberOfBatches = (int) ceil($numberOfItems / $batchSize);
77+
}
6778
}
6879

6980
/**
@@ -89,4 +100,36 @@ public function getTotalNumberOfBatches(): int
89100
{
90101
return $this->totalNumberOfBatches;
91102
}
103+
104+
/**
105+
* @param 0|positive-int $numberOfItems
106+
* @param positive-int $segmentSize
107+
* @param positive-int $batchSize
108+
* @param positive-int $numberOfSegments
109+
*
110+
* @return positive-int
111+
*/
112+
private static function calculateTotalNumberOfBatches(
113+
int $numberOfItems,
114+
int $segmentSize,
115+
int $batchSize,
116+
int $numberOfSegments
117+
): int {
118+
if ($numberOfSegments >= 2) {
119+
// It "should" be `$numberOfSegments - 1`. However, it actually does
120+
// not matter as the expression L128 is just going to give a
121+
// negative number adjusting the final result correctly.
122+
// So we keep this simpler expression, although a bit less intuitive,
123+
// to avoid to have to configure Infection to not mutate this piece.
124+
$numberOfCompleteSegments = $numberOfSegments;
125+
$totalNumberOfBatches = ((int) ceil($segmentSize / $batchSize)) * $numberOfSegments;
126+
} else {
127+
$numberOfCompleteSegments = 0;
128+
$totalNumberOfBatches = 0;
129+
}
130+
131+
$totalNumberOfBatches += (int) ceil(($numberOfItems - $numberOfCompleteSegments * $segmentSize) / $batchSize);
132+
133+
return $totalNumberOfBatches;
134+
}
92135
}

src/ParallelExecutor.php

+9-8
Original file line numberDiff line numberDiff line change
@@ -219,9 +219,15 @@ private function executeMainProcess(
219219

220220
$numberOfItems = $itemIterator->getNumberOfItems();
221221

222-
$config = new Configuration(
223-
$isNumberOfProcessesDefined,
222+
$shouldSpawnChildProcesses = self::shouldSpawnChildProcesses(
223+
$numberOfItems,
224+
$segmentSize,
224225
$numberOfProcesses,
226+
$parallelizationInput->isNumberOfProcessesDefined(),
227+
);
228+
229+
$config = new Configuration(
230+
$shouldSpawnChildProcesses,
225231
$numberOfItems,
226232
$segmentSize,
227233
$batchSize,
@@ -243,12 +249,7 @@ private function executeMainProcess(
243249

244250
$logger->startProgress($numberOfItems);
245251

246-
if (self::shouldSpawnChildProcesses(
247-
$numberOfItems,
248-
$segmentSize,
249-
$numberOfProcesses,
250-
$parallelizationInput->isNumberOfProcessesDefined(),
251-
)) {
252+
if ($shouldSpawnChildProcesses) {
252253
$this
253254
->createProcessLauncher(
254255
$segmentSize,

0 commit comments

Comments
 (0)