Speed up getBean() for singletons with complex dependencies? #409

Closed
seancorfield opened this Issue Dec 16, 2015 · 3 comments

Comments

Projects
None yet
2 participants
@seancorfield
Member

seancorfield commented Dec 16, 2015

Can we safely add a top-level cache for singletons so that explicit calls to getBean() are very fast?

@seancorfield seancorfield self-assigned this Dec 16, 2015

@seancorfield seancorfield added this to the 4.0 milestone Dec 16, 2015

seancorfield added a commit that referenced this issue Dec 16, 2015

Minor tweak for #409
Ensure we don't overwrite any cached dependencies
@seancorfield

This comment has been minimized.

Show comment
Hide comment
@seancorfield

seancorfield Dec 16, 2015

Member

This provides a substantial speedup in the pathological case and doesn't appear to break anything so I'm inclined to close this out as "fixed" but I want to get feedback from @cameroncf first.

Member

seancorfield commented Dec 16, 2015

This provides a substantial speedup in the pathological case and doesn't appear to break anything so I'm inclined to close this out as "fixed" but I want to get feedback from @cameroncf first.

@cameroncf

This comment has been minimized.

Show comment
Hide comment
@cameroncf

cameroncf Dec 16, 2015

Contributor

I ran this on my stand alone test code and saw a pretty healthy jump in performance. This is definitely a big improvement - about 25-30x, which agrees with the numbers you saw.

I've also tried the updates DI/1 file out in a full blown "pathological" application that had problems around getBean() and there is a performance boost there too. Not quite as large since DI/1 is only a part of the larger app, but it seems to have sped things up overall by about 30% overall in that app.

I suspect others will experience some performance gain form these changes as well, though it will probably be far less dramatic than my somewhat extreme edge case.

I plan to do a lot of work in the app today with getBean() and removing the pathological nature of the code so I'll report back if I find any other negative side effects of this change. I looked at the code you changed though and it looks pretty safe.

Contributor

cameroncf commented Dec 16, 2015

I ran this on my stand alone test code and saw a pretty healthy jump in performance. This is definitely a big improvement - about 25-30x, which agrees with the numbers you saw.

I've also tried the updates DI/1 file out in a full blown "pathological" application that had problems around getBean() and there is a performance boost there too. Not quite as large since DI/1 is only a part of the larger app, but it seems to have sped things up overall by about 30% overall in that app.

I suspect others will experience some performance gain form these changes as well, though it will probably be far less dramatic than my somewhat extreme edge case.

I plan to do a lot of work in the app today with getBean() and removing the pathological nature of the code so I'll report back if I find any other negative side effects of this change. I looked at the code you changed though and it looks pretty safe.

@seancorfield

This comment has been minimized.

Show comment
Hide comment
@seancorfield

seancorfield Dec 16, 2015

Member

Thanks for your feedback @cameroncf -- I'll consider this "done" now! Feel free to post back with your findings tho' as it will be useful information for others (and for the 4.0 release notes!).

Member

seancorfield commented Dec 16, 2015

Thanks for your feedback @cameroncf -- I'll consider this "done" now! Feel free to post back with your findings tho' as it will be useful information for others (and for the 4.0 release notes!).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment