Details
-
Type:
Improvement
-
Status:
Resolved
-
Priority:
Major
-
Resolution: Fixed
-
Affects Version/s: 2.14.1
-
Fix Version/s: 2.15
-
Component/s: PicoContainer (Java)
-
Labels:None
-
Number of attachments :
Description
This method
DefaultPicoContainer.getInstance()
currently does this
getModifiableComponentAdapterList().contains(componentAdapter);
which involves scraping over an array. This is rather slow for a big container and is proving to be a bottleneck in our application.
If you have n containers being instantiated then you'll do contains n times on a list o
which will be o(n^2) calls to equals.
Two simple fixes spring to mind:
- Keep the component adapters in a set instead - if you use a linked hash set then you'd get exactly the same iteration order but contains() will be fast.
- Use the keyToAdapterCache map instead like so:
getComponentKeyToAdapterCache().get(componentAdapter.getComponentKey()) == componentAdapter
A more involved alternative might be to admit that a given ComponentAdapter needs to know its parent PicoContainer and hence keep a back-reference so you could do componentAdapater.getPicoContainer() == this.
Activity
I've had a think and attempted to write a performance test that shows the performance I'm seeing.
Its a bit slow but it proves the point and hopefully wouldn't give too many spurious errors.
Chris Simmons
added a comment - I've had a think and attempted to write a performance test that shows the performance I'm seeing.
Its a bit slow but it proves the point and hopefully wouldn't give too many spurious errors. I've noticed exactly the same thing as Chris - there's a serious drop of performance, when Pico is frequently being asked for any component in a bigger container. CPU snapshots from profiler confirm that the problem lays in List.contains(componentAdapter) calls.
Are there plans to fix this issue soon?
Maybe I could contribute the fix (which I don't have yet, of course 
) ?
Michal Skowronek
added a comment - I've noticed exactly the same thing as Chris - there's a serious drop of performance, when Pico is frequently being asked for any component in a bigger container. CPU snapshots from profiler confirm that the problem lays in List.contains(componentAdapter) calls.
Are there plans to fix this issue soon?
Maybe I could contribute the fix (which I don't have yet, of course ) ? fixed as suggested
Paul Hammant
added a comment - fixed as suggested 
Chris, Do you have some load tests that really show the problem? Why am I asking? You might find it easy to try out alternates, to say which one provably makes the issue go away.
Issues for us are that Unit tests are classically good at guarding functionality, and not so good at proving non-functional things. We can do some stuff to make sure thread-realted stuff us good, but performance is really hard with JUnit (etC).
Thoughts?