setBaseApplicationInjector is being called twice at startup

[glenviewjeff]

When profiling my code between my first activity's onCreate and onResume methods, I see that setBaseApplicationInjector is being performed twice at startup. 

I assume this is an unnecessary bug and causing nearly double the startup time.  As you know, my startup time is 12 seconds on my EVO 4g.  I'm going to spend a little time trying to figure out how and why, but I assume Michale, you could readily confirm whether or not this is a bug.

Jeff

[glenviewjeff]

Here's some more detail.  I misspoke--it's not setBaseApplicationInjector that's being called twice, it's that setBaseApplicationInjector calls each module.config() method twice.

First from Elements.getElements(modules) in this line, where Roboguice is requesting static injection for each element.  Why is Roboguice doing this seemingly unnecessary block?
   for(Element element : Elements.getElements(modules))

then again on this line for the actual injection:
   final Injector rtrn = Guice.createInjector(stage, modules);

Jeff

[Michael Burton]

Calling module.config() twice is not in itself a problem.

http://code.google.com/p/roboguice/issues/detail?id=155

--
http://about.me/michaelburton

--
You received this message because you are subscribed to the Google Groups "roboguice" group.
To view this discussion on the web visit https://groups.google.com/d/msg/roboguice/-/8xXJAAbfbT8J.
To post to this group, send email to robo...@googlegroups.com.
To unsubscribe from this group, send email to roboguice+...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/roboguice?hl=en.

[glenviewjeff]

Michael, I recognize that calling configure twice does not "break" the bindings, but it does almost double the startup time. 

My app takes 12 seconds to startup right now, and 7.2 seconds of it is my module binding.  Unless there is a good reason to be annotating everything for static injection, then 3.6 seconds of this statup time is unnecessary.

--
http://about.me/michaelburton

To unsubscribe from this group, send email to roboguice+unsubscribe@googlegroups.com.

[Andrew Toulouse]

On the contrary, it's a problem, but expected behavior rather than a bug. Mobile devices need that performance. Remedying the double-binding performance issue would be productive.

--Andy

[glenviewjeff]

I filed an issue for this.  Can anyone explain why static injection is being requested for each element in the first place? 

http://code.google.com/p/roboguice/issues/detail?id=196

[Michael Burton]

I don't disagree, but I want to make sure I understand the problem correctly.  Do we have any direct evidence that shows that the second call to Module.configure() is taking ~3.6 seconds on jeff's device?

--
http://about.me/michaelburton

To view this discussion on the web visit https://groups.google.com/d/msg/roboguice/-/_RU2zmrDM54J.

To post to this group, send email to robo...@googlegroups.com.

To unsubscribe from this group, send email to roboguice+...@googlegroups.com.

[glenviewjeff]

So I modified Roboguice by removing the code that annotates everything for static injection, removed the test that tests that static injection works, and everything passed.  So it appears that this code is completely unnecessary.

If a user wants to use static injection in one of their classes, they should request it explicitly as shown in http://code.google.com/p/google-guice/wiki/Injections 

From what I can tell, there is no good reason for Roboguice to do this automatically, and certainly there is good reason not to do so.

[Manfred Moser]

So what are you seeing in terms of performance gains?

> --
> You received this message because you are subscribed to the Google Groups
> "roboguice" group.
> To view this discussion on the web visit

> https://groups.google.com/d/msg/roboguice/-/Xik06iBhPZ0J.

[glenviewjeff]

In my case, the startup time is reduced to about 70% of the time if the extra annotations are applied.  In other words, my original startup time was about 12 seconds, and my new time is about 8.5 seconds.  This reduction is as measured in CPU cycles with traceview.

I have to emphasize that nobody has given any reason why these annotations are applied in the first place, and I certainly can't think of any good reason to do so, even if it were computationally free of charge.  There's already a facility in Guice for people to annotate their classes to inject static members if they desire.  Besides, this behavior essentially breaks Guice's (albeit weak) contract that static injection won't occur unless you request that it do so explicitly.

[Manfred Moser]

I talked to Bob Lee at AnDevCon about them using Guice in the square
app and they evidently turn off some sort of stack trace generation or
so to improve startup. He promised to blog about it so we can do the
same..

Lets give him a week and if he has not blogged by then I will ping him.

manfred

> --
> You received this message because you are subscribed to the Google Groups
> "roboguice" group.
> To view this discussion on the web visit

> https://groups.google.com/d/msg/roboguice/-/Jjuz0lstS3MJ.

[Michael Burton]

Thanks for your research Jeff.

The code in question is there to support static injection of resources: http://code.google.com/p/roboguice/source/browse/roboguice/src/main/java/roboguice/RoboGuice.java#81

It uses an SPI visitor to visit only static injection requests, and if it's an InjectResource request then it does the appropriate resource injection. It does not add static injection to everything.

I was not aware of the performance overhead of that particular SPI call, and may need to do some additional investigation to see whether that feature is worth the tradeoff.

Mike

--
http://about.me/michaelburton

--
You received this message because you are subscribed to the Google Groups "roboguice" group.

To view this discussion on the web visit https://groups.google.com/d/msg/roboguice/-/Jjuz0lstS3MJ.

[Michael Burton]

awesome

--
http://about.me/michaelburton

[glenviewjeff]

Michael, to clarify in case it's not already clear, the overhead for this loop is definitely double, just not double the entire startup.  Depending upon how heavy ones binding computation is, the overhead could range between 0x and approaching 2x the entire startup time.

So do you mean that this loop is only to support static injection of an Android Resources class?  Couldn't a developer reasonably familiar with Guice just inject a reference to a @Singleton annotated class that owns a copy of Resources?  I can't think of any good reason to use static fields in Guice code in the first place instead of @Singleton, let alone with this costly startup step.

If it were up to me, I would eliminate the feature and if later requested, provide a separate library that has the feature enabled.  This would be similar to Guice with and without AOP.  Either way, I already modified my library to skip this step.

Regarding the that stack trace computation time that Manfred mentioned--maybe that's a runtime slowdown, but at startup, getting the stack trace via Throwable.nativeGetStackTrace only takes up 1.6% of the overall computation including children (inclusive time). 

In my startup code, 31% of the computation is spent in in StringBuilder.append/AbstractStringBUilder.append/String._getChars, 26% reflect/GenericSignatureParser.scanIdentifier.  All times inclusive.

[Michael Burton]

Manfred, can you follow up with Bob to see what this was about?  Given that square stopped using Guice, I'm doubting he'll ever get around to blogging it :)

Mike

[Jeff Axelrod]

I had assumed that what Bob was going to Blog about was Dagger given that it was released a few weeks after these emails were exchanged.  Dagger seems to be quite popular; there were a ton of attendees at the recent Google Hangout presentation.

 

--
You received this message because you are subscribed to a topic in the Google Groups "roboguice" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/roboguice/BPcFafTsW9c/unsubscribe.
To unsubscribe from this group and all its topics, send an email to roboguice+...@googlegroups.com.

To post to this group, send email to robo...@googlegroups.com.

Visit this group at http://groups.google.com/group/roboguice.
For more options, visit https://groups.google.com/groups/opt_out.