PharBuilder icon indicating copy to clipboard operation
PharBuilder copied to clipboard

Performance improvements?

Open jiaojie1989 opened this issue 7 years ago • 6 comments

I have a cli application which has a 4000+ php files in its vendor directory, and I find it lasts more then 2 minutes to compile all files into one phar file (PHP version 5.6, AMD A8-9600/ 16G Mem/ Ubuntu 16.04).

Then I read the source. I find that adding files one by one makes poor performance, this can be solved by using Phar::buildFromDirectory or Phar::buildFromIterator and compress once with Phar::compressFiles.

I made a small change to the file app/PharBuilder.php. (https://github.com/jiaojie1989/PharBuilder/blob/master/app/PharBuilder.php) I just wonder if what I have done is ok? Please help me. Thanks.

jiaojie1989 avatar Jan 30 '18 14:01 jiaojie1989

Hi,

I have in the past an issue with the compression of a Phar when it's not done file by file (see my comment here: https://github.com/MacFJA/PharBuilder/blob/master/app/PharBuilder.php#L538)
(I don't know if the issue is still present in later PHP version. I can make some tests to check it.)

BTW, I like your solution ; but it's not very compatible with Windows, can you at least add a test on PHP_OS before running ulimit?

MacFJA avatar Jan 30 '18 20:01 MacFJA

Yeah, I will try to add a test on PHP_OS later. Thanks.

jiaojie1989 avatar Jan 31 '18 14:01 jiaojie1989

I have in the past an issue with the compression of a Phar when it's not done file by file

@MacFJA do you have an example where you have this? I saw a similar suggestion on Box2 and I tried to apply it but only saw major performance degradation (https://github.com/humbug/box/pull/29).

theofidry avatar Feb 02 '18 22:02 theofidry

@theofidry

Here some the test I run:

git clone https://github.com/jiaojie1989/PharBuilder.git --depth 2
git clone https://github.com/symfony/symfony-standard.git --branch v3.0.0 --depth 1

cd PharBuilder
git checkout 5fb2559c60a288784d8f377c6b7f0ac81a2573c4
composer install

cd ..

cd symfony-standard
composer install --no-scripts

php -d phar.readonly=0 ../PharBuilder/bin/phar-builder.php package -z --name app.phar -o ../ -s no -e web/app.php

The results:

OS: OS X (10.9.5) Kernel: Darwin 13.4.0 PHP: 7.1.11 Ulimit: 4864 Result: Fail -> Message from the cli: "unable to create temporary file"

OS: Ubuntu 16.04 Kernel: Linux 4.13.0 PHP: 7.0.22 Ulimit: 4864 (previous value was 1024, raised to match OS X and validate the code test) Result: Fail -> Message from the cli: "unable to create temporary file"

OS: OS X (10.9.5) Kernel: Darwin 13.4.0 PHP: 5.6.31 Ulimit: 4864 Result: Fail -> Message from the cli: "unable to create temporary file"


Related PHP bugs:

  • https://bugs.php.net/bug.php?id=53467
  • https://bugs.php.net/bug.php?id=58169

I'm interested in results in other cases.

MacFJA avatar Feb 03 '18 18:02 MacFJA

Hm so if I'm understanding this right you're not doing this for performance reasons but because the Phar::compress() hits a OS limitation about the number of files open? That's savage :/

Well I guess a workaround would be to have different compressing strategies depending of the number of files added... Still sounds like a pretty nasty bug that could be fixed at the PHAR extension level.

On the same note it's impressive to see the difference between buildFromIterator() and addFromString() (the former being 30x times faster in my case)

theofidry avatar Feb 05 '18 09:02 theofidry

Heads up: I actually manage to get it working on Box by bumping the OS limit of the max number of files opened (which is reverted afterwards obviously): https://github.com/humbug/box/blob/master/src/Console/Command/Compile.php#L624

theofidry avatar Sep 28 '18 09:09 theofidry