Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
Novanic opened this issue Feb 29, 2024 · 8 comments · May be fixed by #1807
Open

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

Novanic opened this issue Feb 29, 2024 · 8 comments · May be fixed by #1807

Comments

@Novanic
Copy link

Novanic commented Feb 29, 2024

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
Copy link
Author

Novanic commented Feb 29, 2024

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
Copy link

Malaydewangan09 commented Mar 9, 2024

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

@Novanic
Copy link
Author

Novanic commented Mar 13, 2024

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

@Malaydewangan09
Copy link

@Novanic cool, Thanks!

@Malaydewangan09
Copy link

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

@Novanic
Copy link
Author

Novanic commented Mar 18, 2024

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
Copy link
Author

Novanic commented Mar 18, 2024

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
Copy link

lobaorn commented Jun 1, 2024

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: micronaut-projects/micronaut-guice#9

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