Skip to content
This repository has been archived by the owner on Jan 8, 2020. It is now read-only.

Zend\Loader\ClassMapAutoloader - Performance improvement #5716

Closed
fedys opened this issue Jan 13, 2014 · 22 comments
Closed

Zend\Loader\ClassMapAutoloader - Performance improvement #5716

fedys opened this issue Jan 13, 2014 · 22 comments

Comments

@fedys
Copy link

fedys commented Jan 13, 2014

array_merge() in Zend\Loader\ClassMapAutoloader::registerAutoloadMap() method causes significant performace lost. Replace $this->map = array_merge($this->map, $map); with foreach ($map as $key => $value) { $this->map[$key] = $value; }.

@Ocramius
Copy link
Member

@fedys is that true also for larger maps?

@fedys
Copy link
Author

fedys commented Jan 13, 2014

Absolutely, in my production environment the RESULTING map has over 2000 records with aprox. 40 custom modules(each provides its map). Performance gaing is 10-15ms after use of foreach(before: 190ms, after: 175ms).

@manuakasam
Copy link

http://3v4l.org/gAt9X 10 entries
http://3v4l.org/7Zo0k 100 entries
http://3v4l.org/NSKdT 1000 entries
http://3v4l.org/B7ajo 10000 entries

A little depending on the PHP Version, but most of the time foreach is faster.

@Ocramius
Copy link
Member

Just verified it as well here

ArrayTest\ArrayMergeEvent
classmaps-performance
Method Name Iterations Average Time Ops/s Relative
------------------------------ ---------- ------------ -------------- --------- ---------
arrayMergePerformance : [Baseline] [10,000 ] [0.0005519990921] [1,811.59718]
forEachPerformance : [10,000 ] [0.0003462410450] [2,888.16134] [62.72%]

That is... curious...

@Ocramius
Copy link
Member

According to @DaveRandom, the slowness is caused by copy-on-write going on at each call of array_merge. That's a lot of stuff being copied.

I've added a test that tries to do a single array_merge with multiple params, and the results are much much better:

ArrayTest\ArrayMergeEvent
  classmaps-performance
    Method Name                                 Iterations    Average Time      Ops/s    Relative
    ------------------------------  ----------  ------------ --------------   ---------  ---------
    singleArrayMerge              : [Baseline] [10,000    ] [0.0003203309298] [3,121.77160]
    singleArrayMergeCallUserFuncArray:            [10,000    ] [0.0003251281500] [3,075.71030] [101.50%]
    arrayMergePerformance         :            [10,000    ] [0.0005345391035] [1,870.77053] [166.87%]
    forEachPerformance            :            [10,000    ] [0.0003967431068] [2,520.52268] [123.85%]

@Ocramius
Copy link
Member

Composer doesn't seem to be affected since it has a single classmap when optimizing autoloading.

@ThaDafinser
Copy link
Contributor

👍 great work!

@franz-deleon
Copy link
Contributor

@Ocramius i am trying to figure out the code differences from your testds. mind sharing the actual code from the test above? thanks a bunch!!

@ThaDafinser
Copy link
Contributor

@franz-deleon something like this i think

//singleArrayMerge
array_merge($config1, $config2, $config3, ....$config22);

//singleArrayMergeCallUserFuncArray
call_user_func_array('array_merge', array($config1, $config2, ....$config22));

//arrayMergePerformance
$config = array();
$config = array_merge($config, $config1);
$config = array_merge($config, $config2);
//...
$config = array_merge($config, $config22);

@DASPRiD
Copy link
Member

DASPRiD commented Jan 17, 2014

What about the performance of array_replace(), or even $newArray + $oldArray?

@Thinkscape
Copy link
Member

What was puzzling to me, is that 5.5.8 is 36% faster with array_merge() than foreach.

Apparently 5.5.6 introduced: "Improved performance of array_merge() and func_get_args() by eliminating useless copying.", so the mystery is solved.

The performance gap might be huge enough to sanction a version_compare(). With each new PHP version it will be even more important.

@Thinkscape
Copy link
Member

@DASPRiD hahaha... it seems that array_replace() suffers from the same problem but I expected + operator to be way faster... guess what happened ? :-)

http://3v4l.org/BANT2#v5417

Anyways, + is now also an alternative, as classmaps are always assoc arrays.

@Ocramius
Copy link
Member

@Thinkscape the 5.5.8 improvement is magic by @dstogov

@Ocramius
Copy link
Member

@franz-deleon sorry, thought I had pasted a link. use https://gist.github.com/Ocramius/8399625 to write additional tests

@staabm
Copy link

staabm commented Jan 19, 2014

Also interessting that foreach performance dropped significantly since 5.5.6

@Ocramius
Copy link
Member

@fedys can you check my updated tests?

@fedys
Copy link
Author

fedys commented Jan 21, 2014

Your updated tests seem to cover all use cases. Please try replace key 'index' . $i with some random string with at least 50 chars to 100 chars. Do the same for the values.

$this->bigMap1['index123'] = 123 will become: $this->bigMap1['c4ca4238a0b923820dcc509a6f75849b6f4922f455'] = '621f1454cacf995530ea53652ddf8fb9908279ebbf1f9b250ba689db6a0222b'. This should be closer to real case.

@Ocramius
Copy link
Member

@fedys verified with larger maps, and it seems like the results stay consistent, with + being the fastest operation across platforms (mainly checking HHVM)

@fedys
Copy link
Author

fedys commented Jan 24, 2014

@Ocramius I agree with you. The + operator seems to be best choice. I have just noticed the arraySumPerformance() should be:

public function arraySumPerformance()
{
    $map = [];

    $map = $this->bigMap1 + $map;
    $map = $this->bigMap2 + $map;
    $map = $this->bigMap3 + $map;

    return $map;
}

which is semantically right and is a bit faster.

@Ocramius
Copy link
Member

Ocramius commented Apr 3, 2014

@fedys could you provide a PR for this enhancement?

@localheinz
Copy link
Member

Related to #6285.

@GeeH
Copy link

GeeH commented Jun 27, 2016

This issue has been closed as part of the bug migration program as outlined here - http://framework.zend.com/blog/2016-04-11-issue-closures.html

@GeeH GeeH closed this as completed Jun 27, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

10 participants