I’m working on hunting down anything that allocates objects in the main rendering loop. Android’s garbage collector isn’t very fast (GC can pause animation for a second or more) or smart (it doesn’t treat short-lived objects as well as other VMs do), so it’s pretty important for making things smooth on the platform.
One thing I’ve found that actually creates a lot of objects is a few uses of SafeArrayList in AppStateManager. SafeArrayList itself isn’t bad, but calling addAll and removeAll allocate a lot of objects because those methods use Java iterators (have a look in DDMS and you’ll be surprised), so implementing them inline by hand avoids heap thrashing. These changes are small and help performance a lot. This isn’t off the latest SVN head, but this file may not have changed since my snapshot:
What do you think? I know it’s a drag not being able to use proper methods for addAll and removeAll, but sometimes you have to make sacrifices in the name of performance.
211,213c169,173
< List transfer = Arrays.asList(array);
< states.addAll(transfer);
< initializing.removeAll(transfer);
—
for (int i = 0; i < array.length; i++) {<br />
AppState state = array<i>;<br />
states.add(state);<br />
initializing.remove(state);<br />
}<br />
229c190,192
< terminating.removeAll(Arrays.asList(array));
—
for (int i = 0; i < array.length; i++) {<br />
terminating.remove(array<i>);<br />
}</i></i>
Thanks for this…but i must admit i’m a bit torn between choosing performance over code cleanness just because dalvik jvm sucks is not what one would expect…
@pspeed@Momoko_Fan and @Normen, what do we do? Once we get into this we gonna have a lot of this…inlining code just because it’s faster on android but does not make any difference on desktop.
I don’t think there’s going to be a lot of this for Android, actually. I’ve just started working on this in earnest but I think a handful of fixes will take care of most of it. I’m going to concentrate on the areas where it allocates objects every pass through the rendering loop even if idle (not otherwise changing anything in the scene). I’m not going to worry about cases where the app might unavoidably be allocating memory anyway (like adding/removing objects from the scene, changing materials, that sort of thing).
It might help performance on the desktop, too, though to a smaller extent. Allocating objects isn’t free on any Java platform.
I’ve just checked in a change that early-outs these methods if the array is empty. It’s a good change.
I count six allocations per update pass. On desktop Java you won’t even notice this. Allocations aren’t free but they are on the order of 4 or 5 method invocations… and these temporary objects are GC’ed effortlessly.
Your code is potentially much worse since adding a whole bunch of things one at a time doesn’t give the underlying collection a chance to allocate a full block for all of the items. And besides, it’s a wrongly directed micro-optimization when the code can be avoided completely.
Can we make statistics optional in the Android renderer? I see a lot of object creation whose stack trace looks similar to this (about 13 allocations per rendering loop pass). I don’t see anyway I can turn off stats or stub it out so the calls are NOPs.
I wonder if it still creates the objects if the stats aren’t cleared every frame. You could try removing the app state I added that clears the stats every frame and see.
I haven’t tried it, but looking at the code in OGLESShaderRenderer, it seems pretty hardwired to call Statistics for a lot of events, and a lot of those calls add an entry to a HashMap.
Oh, right. Yeah, that works, but it’s a little ugly what I have to do to override that app state – in my application constructor I have to re-create the state manager since I don’t have access to the instance of the state that I would need to pass in to AppStateManager#detach, and Application#initStateManager is private. But it’s obviously not a huge deal.
The only other allocations I can see in the idle rendering loop (for my use case at least) are in InputManager#invokeUpdateActions. The two enhanced Java for loops in there allocate an iterator each time through. If those could be wrapped in if statements to avoid the loop if the underlying collection has size == 0, then it cleans them right up.
@sbarta said:
The only other allocations I can see in the idle rendering loop (for my use case at least) are in InputManager#invokeUpdateActions. The two enhanced Java for loops in there allocate an iterator each time through. If those could be wrapped in if statements to avoid the loop if the underlying collection has size == 0, then it cleans them right up.
Those may be candidates for a different approach... otherwise you still take the hit every time there are things to update... which could be quite often. Like for the axisValues. I'd have to look deeper.
@pspeed said:
Those may be candidates for a different approach... otherwise you still take the hit every time there are things to update... which could be quite often. Like for the axisValues. I'd have to look deeper.
Obviously that depends on just how often this is, but even on Android a couple small memory allocations aren't the end of the world, so if a better fix isn't very easy, then the quick fix is still pretty good.
@sbarta said:
Obviously that depends on just how often this is, but even on Android a couple small memory allocations aren't the end of the world, so if a better fix isn't very easy, then the quick fix is still pretty good.
Yeah, but for example, in the case of axisValues, I think it's a non-change... extra work to do nothing because there will always be axisValues every time. Or it would be so rare that there aren't or something.
I'm not as comfortable mucking around in InputManager as I am with the app-state related stuff (since I reworked most of that recently anyway). But your suggested changes are not harmful... maybe someone will add them.
@Momoko_Fan said:
Does anything need to be done here?
Checking the collections for size before iterating in the two places in InputManager#invokeUpdateActions helps my use case. I made the changes in a patched version of the library I'm using and it works better.