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

Slow aliases and factory set methods #112

Closed
malinink opened this issue Apr 1, 2016 · 9 comments
Closed

Slow aliases and factory set methods #112

malinink opened this issue Apr 1, 2016 · 9 comments

Comments

@malinink
Copy link
Contributor

malinink commented Apr 1, 2016

In new SM3 there is couple of methods that are more slowly than in SM2

Difference in time is great (about 8x slower on this method)
All other comparisons I provide exactly on that function.

The reason is that is it a lot of factories and aliases that pass to SM with functions setAlias and setFactory. Behind the scene one key value pair meld with config in configure method.

I see 2 ways to solve problem:

  • add functions such as setAliases and etc...
public function setAliases($aliasTargetPairs)
{
    $this->configure(['aliases' => $aliasTargetPairs]);
}
  • Modify function setAlias to something like that: it was a bad idea, skip that
public function setAlias($alias, $target)
{
    $this->validateOverrides(['aliases' => [$alias => $target]]);
    $this->aliases[$alias] = $target;
}

First one is more elegant and have maximum speed(10x faster then in SM2, 75x faster current SM3). But we need to rewrite other components.
Second one is twice slower than SM2, but there is no need to modify other components.

Of course we could apply both of them.
I think that first one is best idea, I would like to provide PR for this issue, if it is resonable.

Also I could provide PR to to 2.7 branch of SM witch allows modify other components to use that methods.

@malinink
Copy link
Contributor Author

malinink commented Apr 6, 2016

After some checks I found that:

  • main problem is in calling resolveAliases in configure method at bad condition
  • resolveAliases could be optimized for case when new aliases do not intersect with aliases that already present in SM.

I'll provide PR that consists both of changes.
Also I add one more test for recursive aliases and benchmark for set methods.

All changes that provide my PR improve set methods speeding and does not affect the speed of the other SM methods.

setFactory is now 9x faster
setAlias (for not intersecting case) is 3x faster

@malinink
Copy link
Contributor Author

malinink commented Apr 6, 2016

Also it is still a good question is it a good idea to add

public function setAliases($aliasTargetPairs)
{
    $this->configure(['aliases' => $aliasTargetPairs]);
}

and setFactories methods.

Now only two classes will have improvements of that:

As a result we will have much faster load of application.

@snapshotpl
Copy link
Contributor

Great improvement!

@malinink
Copy link
Contributor Author

malinink commented Apr 6, 2016

I provide PR with setAliases and setFactories methods for both master and release-2.7 branches

@weierophinney
Copy link
Member

There's no reason for the new methods on v3, as you can use configure () already to accomplish the same thing. As for v2, we do not want to add new features at this time, unless they aid with migration to v3.

@malinink
Copy link
Contributor Author

malinink commented Apr 6, 2016

@weierophinney
For example here
We should use something like that?

$serviceManager->configure([
    'factories' => $this->factories,
]);

I thought it was a bad idea.

Nevertheless, I move setAliases/setFactories methods to another PR. (should I close them?)
P.S. with new SM3 default mvc application loads slower then with SM2, that was the main reason of that functions

The main PR do not add new features, just improvements.

@pine3ree
Copy link
Contributor

@malinink

FYI, since it's related to an issue of mine (sorry I have read your issue report just now)

I had the same performance problems after migrating to zend-servicemanager3. After a little investigation I came to the same conclusion about setAlias and setFactory in vie helper config classes.

I added 2 PRs to solve the performance overhead for my use case.

These do not solve the slowness of those methods, just avoid using them when using service-manager3 and use its newly added batch configure() method instead.

related discussions:

kind regards

@malinink
Copy link
Contributor Author

@pine3ree
Thanks for looking from another side on that problem. I will trace your PR, and will remove mine on merge.

@pine3ree
Copy link
Contributor

@malinink
My PRs just circumvent the performance issues where they become very evident but they do not fix the slowness of v3 set(Alias|Factory) methods. So i believe you should keep your issue report / fixes open.

kind regards

Ocramius added a commit that referenced this issue Jun 1, 2016
Ocramius added a commit that referenced this issue Jun 1, 2016
Before:

+-----------------------------+----------------------------------+-------+--------+------+-----+-------------+-------------+-------------+-------------+-------------+----------+--------+-------------+
| benchmark                   | subject                          | group | params | revs | its | mem         | best        | mean        | mode        | worst       | stdev    | rstdev | diff        |
+-----------------------------+----------------------------------+-------+--------+------+-----+-------------+-------------+-------------+-------------+-------------+----------+--------+-------------+
| FetchCachedServicesBench    | benchFetchFactory1               |       | []     | 1000 | 20  | 964,792b    | 2.764μs     | 2.875μs     | 2.847μs     | 2.977μs     | 0.071μs  | 2.45%  | +2.64%      |
| FetchCachedServicesBench    | benchFetchInvokable1             |       | []     | 1000 | 20  | 964,792b    | 2.702μs     | 2.801μs     | 2.781μs     | 2.939μs     | 0.060μs  | 2.13%  | 0.00%       |
| FetchCachedServicesBench    | benchFetchService1               |       | []     | 1000 | 20  | 964,792b    | 2.831μs     | 2.948μs     | 2.970μs     | 3.047μs     | 0.068μs  | 2.31%  | +5.25%      |
| FetchCachedServicesBench    | benchFetchAlias1                 |       | []     | 1000 | 20  | 964,792b    | 2.686μs     | 2.812μs     | 2.838μs     | 2.908μs     | 0.065μs  | 2.32%  | +0.40%      |
| FetchCachedServicesBench    | benchFetchRecursiveAlias1        |       | []     | 1000 | 20  | 964,800b    | 2.812μs     | 2.911μs     | 2.930μs     | 3.021μs     | 0.067μs  | 2.32%  | +3.95%      |
| FetchCachedServicesBench    | benchFetchRecursiveAlias2        |       | []     | 1000 | 20  | 964,800b    | 2.747μs     | 2.862μs     | 2.935μs     | 2.961μs     | 0.074μs  | 2.59%  | +2.18%      |
| FetchCachedServicesBench    | benchFetchAbstractFactoryService |       | []     | 1000 | 20  | 964,808b    | 13.010μs    | 13.501μs    | 13.659μs    | 14.150μs    | 0.299μs  | 2.21%  | +382.09%    |
| FetchNewServiceManagerBench | benchFetchServiceManagerCreation |       | []     | 100  | 20  | 24,030,120b | 1,925.660μs | 1,962.191μs | 1,936.676μs | 2,009.680μs | 28.444μs | 1.45%  | +69,965.74% |
| FetchNewServicesBench       | benchFetchFactory1               |       | []     | 1000 | 10  | 967,480b    | 13.060μs    | 13.409μs    | 13.214μs    | 14.046μs    | 0.303μs  | 2.26%  | +378.82%    |
| FetchNewServicesBench       | benchBuildFactory1               |       | []     | 1000 | 10  | 967,480b    | 11.992μs    | 12.426μs    | 12.285μs    | 12.991μs    | 0.310μs  | 2.49%  | +343.69%    |
| FetchNewServicesBench       | benchFetchInvokable1             |       | []     | 1000 | 10  | 967,480b    | 13.611μs    | 14.086μs    | 14.041μs    | 14.667μs    | 0.291μs  | 2.07%  | +402.98%    |
| FetchNewServicesBench       | benchBuildInvokable1             |       | []     | 1000 | 10  | 967,480b    | 12.400μs    | 12.837μs    | 13.069μs    | 13.199μs    | 0.296μs  | 2.31%  | +358.40%    |
| FetchNewServicesBench       | benchFetchService1               |       | []     | 1000 | 10  | 967,480b    | 2.723μs     | 2.814μs     | 2.762μs     | 2.939μs     | 0.068μs  | 2.42%  | +0.46%      |
| FetchNewServicesBench       | benchFetchFactoryAlias1          |       | []     | 1000 | 10  | 967,480b    | 12.683μs    | 13.027μs    | 12.956μs    | 13.508μs    | 0.251μs  | 1.93%  | +365.15%    |
| FetchNewServicesBench       | benchBuildFactoryAlias1          |       | []     | 1000 | 10  | 967,480b    | 12.385μs    | 12.893μs    | 12.766μs    | 13.410μs    | 0.322μs  | 2.50%  | +360.39%    |
| FetchNewServicesBench       | benchFetchRecursiveFactoryAlias1 |       | []     | 1000 | 10  | 967,496b    | 12.296μs    | 12.566μs    | 12.421μs    | 13.015μs    | 0.239μs  | 1.90%  | +348.70%    |
| FetchNewServicesBench       | benchBuildRecursiveFactoryAlias1 |       | []     | 1000 | 10  | 967,496b    | 12.272μs    | 12.825μs    | 13.105μs    | 13.331μs    | 0.356μs  | 2.77%  | +357.96%    |
| FetchNewServicesBench       | benchFetchRecursiveFactoryAlias2 |       | []     | 1000 | 10  | 967,496b    | 12.286μs    | 12.649μs    | 12.569μs    | 13.152μs    | 0.271μs  | 2.15%  | +351.68%    |
| FetchNewServicesBench       | benchBuildRecursiveFactoryAlias2 |       | []     | 1000 | 10  | 967,496b    | 12.125μs    | 12.582μs    | 12.716μs    | 13.166μs    | 0.340μs  | 2.71%  | +349.27%    |
| FetchNewServicesBench       | benchFetchAbstractFactoryFoo     |       | []     | 1000 | 10  | 967,488b    | 12.851μs    | 13.358μs    | 13.600μs    | 13.813μs    | 0.351μs  | 2.63%  | +376.99%    |
| FetchNewServicesBench       | benchBuildAbstractFactoryFoo     |       | []     | 1000 | 10  | 967,488b    | 12.266μs    | 12.705μs    | 12.633μs    | 13.135μs    | 0.295μs  | 2.33%  | +353.65%    |
| SetNewServicesBench         | benchSetFactory                  |       | []     | 1000 | 10  | 992,128b    | 12.394μs    | 12.755μs    | 12.657μs    | 13.189μs    | 0.237μs  | 1.86%  | +355.45%    |
| SetNewServicesBench         | benchSetAlias                    |       | []     | 1000 | 10  | 992,128b    | 28.579μs    | 29.372μs    | 29.209μs    | 30.831μs    | 0.590μs  | 2.01%  | +948.80%    |
| SetNewServicesBench         | benchSetAliasOverrided           |       | []     | 1000 | 10  | 992,136b    | 76.288μs    | 77.842μs    | 77.077μs    | 81.144μs    | 1.435μs  | 1.84%  | +2,679.58%  |
+-----------------------------+----------------------------------+-------+--------+------+-----+-------------+-------------+-------------+-------------+-------------+----------+--------+-------------+

After:

+-----------------------------+----------------------------------+-------+--------+------+-----+-------------+-------------+-------------+-------------+-------------+----------+--------+-------------+
| benchmark                   | subject                          | group | params | revs | its | mem         | best        | mean        | mode        | worst       | stdev    | rstdev | diff        |
+-----------------------------+----------------------------------+-------+--------+------+-----+-------------+-------------+-------------+-------------+-------------+----------+--------+-------------+
| FetchCachedServicesBench    | benchFetchFactory1               |       | []     | 1000 | 20  | 963,704b    | 2.727μs     | 2.858μs     | 2.847μs     | 2.983μs     | 0.064μs  | 2.25%  | +7.11%      |
| FetchCachedServicesBench    | benchFetchInvokable1             |       | []     | 1000 | 20  | 963,704b    | 2.577μs     | 2.705μs     | 2.689μs     | 2.829μs     | 0.081μs  | 3.00%  | +1.38%      |
| FetchCachedServicesBench    | benchFetchService1               |       | []     | 1000 | 20  | 963,704b    | 2.597μs     | 2.668μs     | 2.636μs     | 2.766μs     | 0.046μs  | 1.74%  | 0.00%       |
| FetchCachedServicesBench    | benchFetchAlias1                 |       | []     | 1000 | 20  | 963,704b    | 2.597μs     | 2.721μs     | 2.702μs     | 2.852μs     | 0.084μs  | 3.07%  | +1.99%      |
| FetchCachedServicesBench    | benchFetchRecursiveAlias1        |       | []     | 1000 | 20  | 963,712b    | 2.622μs     | 2.725μs     | 2.795μs     | 2.829μs     | 0.079μs  | 2.89%  | +2.13%      |
| FetchCachedServicesBench    | benchFetchRecursiveAlias2        |       | []     | 1000 | 20  | 963,712b    | 2.647μs     | 2.760μs     | 2.725μs     | 2.893μs     | 0.064μs  | 2.31%  | +3.44%      |
| FetchCachedServicesBench    | benchFetchAbstractFactoryService |       | []     | 1000 | 20  | 963,720b    | 12.608μs    | 13.053μs    | 13.206μs    | 13.434μs    | 0.254μs  | 1.94%  | +389.20%    |
| FetchNewServiceManagerBench | benchFetchServiceManagerCreation |       | []     | 100  | 20  | 24,029,952b | 1,699.350μs | 1,733.026μs | 1,720.312μs | 1,790.900μs | 24.906μs | 1.44%  | +64,849.89% |
| FetchNewServicesBench       | benchFetchFactory1               |       | []     | 1000 | 10  | 966,392b    | 12.609μs    | 13.165μs    | 13.185μs    | 13.559μs    | 0.266μs  | 2.02%  | +393.38%    |
| FetchNewServicesBench       | benchBuildFactory1               |       | []     | 1000 | 10  | 966,392b    | 11.787μs    | 12.125μs    | 11.949μs    | 12.514μs    | 0.262μs  | 2.16%  | +354.41%    |
| FetchNewServicesBench       | benchFetchInvokable1             |       | []     | 1000 | 10  | 966,392b    | 13.441μs    | 13.798μs    | 13.877μs    | 14.157μs    | 0.209μs  | 1.51%  | +417.12%    |
| FetchNewServicesBench       | benchBuildInvokable1             |       | []     | 1000 | 10  | 966,392b    | 12.281μs    | 12.605μs    | 12.432μs    | 13.029μs    | 0.256μs  | 2.03%  | +372.42%    |
| FetchNewServicesBench       | benchFetchService1               |       | []     | 1000 | 10  | 966,392b    | 2.810μs     | 2.864μs     | 2.834μs     | 2.958μs     | 0.049μs  | 1.72%  | +7.33%      |
| FetchNewServicesBench       | benchFetchFactoryAlias1          |       | []     | 1000 | 10  | 966,392b    | 11.910μs    | 12.376μs    | 12.187μs    | 12.974μs    | 0.351μs  | 2.84%  | +363.82%    |
| FetchNewServicesBench       | benchBuildFactoryAlias1          |       | []     | 1000 | 10  | 966,392b    | 12.000μs    | 12.354μs    | 12.329μs    | 12.672μs    | 0.176μs  | 1.42%  | +363.01%    |
| FetchNewServicesBench       | benchFetchRecursiveFactoryAlias1 |       | []     | 1000 | 10  | 966,408b    | 11.691μs    | 12.134μs    | 12.277μs    | 12.501μs    | 0.262μs  | 2.16%  | +354.76%    |
| FetchNewServicesBench       | benchBuildRecursiveFactoryAlias1 |       | []     | 1000 | 10  | 966,408b    | 12.137μs    | 12.516μs    | 12.344μs    | 12.971μs    | 0.277μs  | 2.21%  | +369.09%    |
| FetchNewServicesBench       | benchFetchRecursiveFactoryAlias2 |       | []     | 1000 | 10  | 966,408b    | 11.863μs    | 12.326μs    | 12.412μs    | 12.721μs    | 0.295μs  | 2.40%  | +361.95%    |
| FetchNewServicesBench       | benchBuildRecursiveFactoryAlias2 |       | []     | 1000 | 10  | 966,408b    | 11.780μs    | 12.177μs    | 12.138μs    | 12.648μs    | 0.275μs  | 2.26%  | +356.38%    |
| FetchNewServicesBench       | benchFetchAbstractFactoryFoo     |       | []     | 1000 | 10  | 966,400b    | 12.754μs    | 13.143μs    | 12.982μs    | 13.743μs    | 0.311μs  | 2.37%  | +392.58%    |
| FetchNewServicesBench       | benchBuildAbstractFactoryFoo     |       | []     | 1000 | 10  | 966,400b    | 11.730μs    | 12.002μs    | 11.872μs    | 12.480μs    | 0.237μs  | 1.98%  | +349.82%    |
| SetNewServicesBench         | benchSetFactory                  |       | []     | 1000 | 10  | 991,040b    | 11.968μs    | 12.282μs    | 12.419μs    | 12.579μs    | 0.204μs  | 1.66%  | +360.32%    |
| SetNewServicesBench         | benchSetAlias                    |       | []     | 1000 | 10  | 991,040b    | 28.803μs    | 29.834μs    | 29.417μs    | 30.981μs    | 0.703μs  | 2.36%  | +1,018.11%  |
| SetNewServicesBench         | benchSetAliasOverrided           |       | []     | 1000 | 10  | 991,048b    | 74.980μs    | 76.211μs    | 75.839μs    | 78.355μs    | 0.944μs  | 1.24%  | +2,756.22%  |
+-----------------------------+----------------------------------+-------+--------+------+-----+-------------+-------------+-------------+-------------+-------------+----------+--------+-------------+
Ocramius added a commit that referenced this issue Jun 1, 2016
Ocramius added a commit that referenced this issue Jun 1, 2016
Ocramius added a commit that referenced this issue Jun 1, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants