Skip to content

Improve performance of unpack() with nameless repetitions#18803

Merged
ndossche merged 3 commits into
php:masterfrom
ndossche:unpack-repeats
Jun 10, 2025
Merged

Improve performance of unpack() with nameless repetitions#18803
ndossche merged 3 commits into
php:masterfrom
ndossche:unpack-repeats

Conversation

@ndossche
Copy link
Copy Markdown
Member

@ndossche ndossche 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
```
@ndossche ndossche requested a review from bukka as a code owner June 8, 2025 11:38
@ndossche ndossche mentioned this pull request Jun 8, 2025
@staabm
Copy link
Copy Markdown
Contributor

staabm commented Jun 8, 2025

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

@ndossche
Copy link
Copy Markdown
Member Author

ndossche commented Jun 8, 2025

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

@ndossche ndossche requested a review from Girgias June 9, 2025 16:27
Copy link
Copy Markdown
Member

@Girgias Girgias left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One minor suggestion, but this looks good to me.

Comment thread ext/standard/pack.c Outdated
Comment thread ext/standard/pack.c Outdated
@ndossche ndossche merged commit 559858c into php:master Jun 10, 2025
9 checks passed
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.

3 participants