Skip to content

Commit

Permalink
fix: Migrate from amphp/parallel-functions to amphp/parallel (#1216)
Browse files Browse the repository at this point in the history
In #723 I disabled the parallelization by default as building the Box PHAR with parallelization was x1000 slower than without. 

Since then, a number of things changed in PHP-Scoper and in preparation to adapt the way the parallelization is done, a few things changed in Box too. I am not sure what element made a difference but according to PHPBench, enabling parallelization was reducing the build time from `13.932s` to `9.243s` so a ~33% speed improvement.

I however gave it more though I as thinking serializing closures is a bad idea of a source of too many problems. Currently using parallelization in Box results in errors due to data being `readonly`. I do not remember exactly in what part `laravel/serializable-closure`  messes up but I also remembered it was a limitation of the library (and in fairness, the library is really not at fault, it tries really hard to hack its way through to patch a very annoying PHP limitation, so it does what it can).

But I kept finding those issues too annoying and I think the solution is also quite simple: to use AMPHP at a level lower which would avoid to have to serialize closures altogether.

Closes #552.
Closes #602 (the memory issue seems gone according to the PHPBench results).
Closes #1160 (superseded).

With those changes, we get the following results:

| subject                                      | memory   | mode   | rstdev | stdev    |
| - | - | - | - | - |
| with PHP-Scoper; no parallel processing | 940.391mb | 13.932s | ±0.88%  | 122.973ms |
| with PHP-Scoper; parallel processing | 47.957mb | 6.668s | ±0.64% | 42.899ms |

**In other words the parallelization goes from `9.243s` to `6.668s`, so a further ~28% improvement making it ~52% faster than without parallelization.**
  • Loading branch information
theofidry authored Nov 27, 2023
1 parent b55a381 commit 0384de9
Show file tree
Hide file tree
Showing 13 changed files with 1,156 additions and 275 deletions.
7 changes: 4 additions & 3 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -487,18 +487,19 @@ $(REQUIREMENT_CHECKER_EXTRACT):

$(SCOPED_BOX_BIN): $(SCOPED_BOX_DEPS)
@echo "$(YELLOW_COLOR)Compile Box.$(NO_COLOR)"
$(BOX) compile --ansi --no-parallel
@# Use parallelization
$(BOX) compile --ansi

rm $(TMP_SCOPED_BOX_BIN) || true
mv -v bin/box.phar $(TMP_SCOPED_BOX_BIN)

@echo "$(YELLOW_COLOR)Compile Box with the isolated Box PHAR.$(NO_COLOR)"
php $(TMP_SCOPED_BOX_BIN) compile --ansi --no-parallel
php $(TMP_SCOPED_BOX_BIN) compile --ansi

mv -fv bin/box.phar box

@echo "$(YELLOW_COLOR)Test the PHAR which has been created by the isolated PHAR.$(NO_COLOR)"
./box compile --ansi --no-parallel
./box compile --ansi

mv -fv box bin/box.phar
rm $(TMP_SCOPED_BOX_BIN)
Expand Down
5 changes: 3 additions & 2 deletions bin/bench-test.php
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@

if ($parallelTime <= $maxParallelTimeTarget) {
echo 'OK.'.PHP_EOL;

exit(0);
}

Expand All @@ -59,5 +60,5 @@

echo 'Failed!'.PHP_EOL;
echo 'Missed the target by '.$relativeDifference.'%'.PHP_EOL;
// TODO: https://github.com/box-project/box/issues/552
exit(0);

exit(1);
2 changes: 1 addition & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
"ext-mbstring": "*",
"ext-phar": "*",
"composer-plugin-api": "^2.2",
"amphp/parallel-functions": "^1.1",
"amphp/parallel": "^2.0",
"composer/semver": "^3.3.2",
"composer/xdebug-handler": "^3.0.3",
"fidry/console": "^0.6.0",
Expand Down
Loading

0 comments on commit 0384de9

Please sign in to comment.