Skip to content

Conversation

@h4kuna
Copy link
Contributor

@h4kuna h4kuna commented Sep 25, 2018

First PR is here, where was discussion.

Q: The issue is that this mutates the internal pointer.
A: created benchmark and implementation does not change internal pointer.

Q: about performance for small vs big arrays
A: look like implementation is every time faster

This implementation is approx twice faster than cycle foreach.

Benchmark is on my gist.

Results on my machine.

array_key_last_symfony
0.1387140750885

array_key_last
0.055586814880371

@BackEndTea
Copy link
Contributor

BackEndTea commented Sep 25, 2018

end changes the internal pointer of the array. And the 'official' 7.3 array_key_last (and first), don't change the internal pointer.

The foreach approach was taken to make sure that the polyfill behaves the same as the official version.

I should learn how to read better 😅

@nicolas-grekas
Copy link
Member

Actually it doesn't change the internal pointer, because the "end" operates on a copy of the array (argument is passed by value and not by reference to the function)

@stof
Copy link
Member

stof commented Sep 25, 2018

but on PHP 5.x, the reference mismatch forces to create a copy of the array, and that's also not the behavior of the core method.

@stof
Copy link
Member

stof commented Sep 25, 2018

your benchmark is checking time, but does not check memory

@h4kuna
Copy link
Contributor Author

h4kuna commented Sep 26, 2018

@stof you have right. Could you check if the benchmarks are relevant?

Array size is 2000, on site is execute limit 3s for php.

Memory usage

Here is benchmark.

php 5.6.29

Array size: 2000
array_key_last_symfony: time: 1.5190160274506 s, memory: 80 B
array_key_last: time: 0.70938301086426 s, memory: 112 B

php 7+ return this result

Array size: 2000
array_key_last_symfony: time: 0.29880118370056 s, memory: 0 B
array_key_last: time: 0.13791918754578 s, memory: 0 B

Peak of memory

array_key_last

  • php 7.0.1: 140.03 kB
  • php 5.6.29: 468.34 kB

array_key_last_symfony

  • php 7.0.1: 140.02 kB
  • php 5.6.29: 468.34 kB

@nicolas-grekas
Copy link
Member

Thank you @h4kuna.

@nicolas-grekas nicolas-grekas merged commit 77aa2ff into symfony:master Sep 30, 2018
nicolas-grekas added a commit that referenced this pull request Sep 30, 2018
This PR was merged into the 1.9-dev branch.

Discussion
----------

array_key_last: improving performance

First PR is [here](symfony/polyfill-php73#1), where was discussion.

Q: The issue is that this mutates the internal pointer.
A: created [benchmark](symfony/polyfill-php73#1 (comment)) and implementation does not change internal pointer.

Q: about performance for small vs big arrays
A: look like implementation is [every time faster](symfony/polyfill-php73#1 (comment))

This implementation is approx twice faster than cycle foreach.

Benchmark is [on my gist](https://gist.github.com/h4kuna/6353ce99bd45b9c5fcdf49ef6d0e3da8).

Results on my machine.

```
array_key_last_symfony
0.1387140750885

array_key_last
0.055586814880371
```

Commits
-------

77aa2ff array_key_last: improving performance
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants