Skip to content

Improve performance of unpack() with nameless repetitions #18803

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

nielsdos
Copy link
Member

@nielsdos nielsdos commented Jun 8, 2025

We can avoid creating temporary strings, and then reparsing them into numbers with zend_symtable_update() by using zend_hash_index_update() directly.

For the following benchmark on an i7-4790:

$file = str_repeat('A', 100000);
for ($i=0;$i<100;$i++) unpack('C*',$file);

I get:

Benchmark 1: ./sapi/cli/php test.php
  Time (mean ± σ):      85.8 ms ±   1.8 ms    [User: 74.5 ms, System: 10.4 ms]
  Range (min … max):    83.8 ms …  92.4 ms    33 runs

Benchmark 2: ./sapi/cli/php_old test.php
  Time (mean ± σ):     318.3 ms ±   2.7 ms    [User: 306.7 ms, System: 9.9 ms]
  Range (min … max):   314.9 ms … 321.6 ms    10 runs

Summary
  ./sapi/cli/php test.php ran
    3.71 ± 0.08 times faster than ./sapi/cli/php_old test.php

On an i7-1185G7 I get:

Benchmark 1: ./sapi/cli/php test.php
  Time (mean ± σ):      60.1 ms ±   0.7 ms    [User: 47.8 ms, System: 12.0 ms]
  Range (min … max):    59.2 ms …  63.8 ms    48 runs

  Warning: Statistical outliers were detected. Consider re-running this benchmark on a quiet system without any interferences from other programs. It might help to use the '--warmup' or '--prepare' options.

Benchmark 2: ./sapi/cli/php_old test.php
  Time (mean ± σ):     220.8 ms ±   2.2 ms    [User: 209.6 ms, System: 10.7 ms]
  Range (min … max):   218.5 ms … 224.5 ms    13 runs

Summary
  ./sapi/cli/php test.php  ran
    3.67 ± 0.06 times faster than ./sapi/cli/php_old test.php

We can avoid creating temporary strings, and then reparsing them into
numbers with zend_symtable_update() by using zend_hash_index_update()
directly.

For the following benchmark on an i7-4790:
```php
$file = str_repeat('A', 100000);
for ($i=0;$i<100;$i++) unpack('C*',$file);
```

I get:
```
Benchmark 1: ./sapi/cli/php y.php
  Time (mean ± σ):      85.8 ms ±   1.8 ms    [User: 74.5 ms, System: 10.4 ms]
  Range (min … max):    83.8 ms …  92.4 ms    33 runs

Benchmark 2: ./sapi/cli/php_old y.php
  Time (mean ± σ):     318.3 ms ±   2.7 ms    [User: 306.7 ms, System: 9.9 ms]
  Range (min … max):   314.9 ms … 321.6 ms    10 runs

Summary
  ./sapi/cli/php y.php ran
    3.71 ± 0.08 times faster than ./sapi/cli/php_old y.php
```

On an i7-1185G7 I get:
```
Benchmark 1: ./sapi/cli/php test.php
  Time (mean ± σ):      60.1 ms ±   0.7 ms    [User: 47.8 ms, System: 12.0 ms]
  Range (min … max):    59.2 ms …  63.8 ms    48 runs

  Warning: Statistical outliers were detected. Consider re-running this benchmark on a quiet system without any interferences from other programs. It might help to use the '--warmup' or '--prepare' options.

Benchmark 2: ./sapi/cli/php_old test.php
  Time (mean ± σ):     220.8 ms ±   2.2 ms    [User: 209.6 ms, System: 10.7 ms]
  Range (min … max):   218.5 ms … 224.5 ms    13 runs

Summary
  ./sapi/cli/php test.php  ran
    3.67 ± 0.06 times faster than ./sapi/cli/php_old test.php
```
@staabm
Copy link
Contributor

staabm commented Jun 8, 2025

nice to see the benchmark was useful. thanks for looking into it. 🙏

@nielsdos
Copy link
Member Author

nielsdos commented Jun 8, 2025

Looks like there's a missing null check somewhere, I'll look tomorrow or this evening.

@nielsdos nielsdos requested a review from Girgias June 9, 2025 16:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants