search

google / guice

Guice (pronounced 'juice') is a lightweight dependency injection framework for Java 11 and above, brought to you by Google.
https://github.com/google/guice
Apache License 2.0
12.48k stars 1.67k forks source link

High memory consumption to obtain a Singleton instance via Injector or Provider #1802

Open Novanic opened 7 months ago

Novanic commented 7 months ago

I found out that Guice causes a high amount of memory allocations when I try to get a Singleton instance via Injector#getInstance(...). It causes a memory consumption of about 4,5 MB when the Singleton is accessed 10.000 times via Injector#getInstance(...). That can happen for example within static methods or factories in legacy applications or applications which are currently migrating to Guice. With using javax.inject.Provider it looks a bit better with about 3 MB, but that is also too high to just get a Singleton instance. The main problem seems to be the creation of Object arrays. Can that get optimized?

I have attached screenshots of the profiler results.

testInjector testProvider

Novanic commented 7 months ago

I have already found the cause of the problem. The problem is, that every access via Injector#getInstance(...) or Provider#get() is creating an com.google.inject.internal.InternalContext object -> which is creating a java.util.IdentityHashMap object -> which is creating an Object array with space for 64 items. This object array requires about 270 bytes multiplied with 10.000 calls = 2,7 MB. The clou: The IdentityHashMap isn't used when Injector#getInstance(...) or Provider#get() is called by the application. ;-) Therefore the 2,7 MB are wasted without a need. The solution could be to lazy initialize the member variable InternalContext#constructionContexts within the InternalContext#getConstructionContext(...) method. The member variable is only accessed by this method and the method isn't called when using Injector#getInstance(...) or Provider#get().

Can that get fixed for Guice 7.x.x and 6.x.x? Many people and I require it for Guice 6.x.x because Injector#getInstance(...) is potentially used within older applications which aren't yet moved to jakarta.

Malaydewangan09 commented 7 months ago

@Novanic Is this still open Can I take up this ?

Novanic commented 6 months ago

@Malaydewangan09 Yes, it is still open and I would appreciate it if it would get fixed. :-)

Malaydewangan09 commented 6 months ago

@Novanic cool, Thanks!

Malaydewangan09 commented 6 months ago

Hey , @Novanic any updates on #1807 Is it good to go, or there are some changes to be made ?

Novanic commented 6 months ago

Hello @Malaydewangan09 , I have now re-tested the test case with your fix #1807 and it looks very good, it solves the problem. I think we need to find someone how can approve and merge the fix (for Guice 7.x.x and 6.x.x).

Usage via Injector: testInjector-fixed

Usage via Provider: testProvider-fixed

Novanic commented 6 months ago

Hello @sameb I saw that you are often committing to the Guice repository. :-) Can you take a look to check if the linked pull-request #1807 can get approved and merged (for Guice 7.x.x and 6.x.x)? It's a small change which improves the memory consumption a lot. Thank you in advance for your help and best regards. :-)

lobaorn commented 4 months ago

Hey @Novanic @Malaydewangan09 , letting you know since you may be targeting Guice 7.0.0, about the new Micronaut Guice project, that depending on your usecase could be a useful replacement: https://github.com/micronaut-projects/micronaut-guice/issues/9