Skip to content
This repository has been archived by the owner on Apr 20, 2021. It is now read-only.

Feature/performance #23

Merged
merged 6 commits into from
Jul 27, 2016
Merged

Feature/performance #23

merged 6 commits into from
Jul 27, 2016

Conversation

passsy
Copy link
Owner

@passsy passsy commented Jul 22, 2016

  • Elimination of unnecessary Object[] allocation due to varargs.
  • Reduce superCallback instances to one per method call instead of one per layer

I tested caching the superCall object per method as well as caching the iterator. But the results haven't been noticeable on Dalvik and Art. The allocation of the Object[] was the main reason for bad performance on Dalvik.

Results on Genymotion Android 5.0:

Normal 5 Layer inheritance 50.000 iterations
V/PerformanceTestActivity: 5 stages: getCacheDir() in 115.0ms
V/PerformanceTestActivity: 5 stages: getResources() in 3.0ms
V/PerformanceTestActivity: 5 stages: getDelegate() in 1.0ms
V/PerformanceTestActivity: 5 stages: getFile() in 97.0ms
V/PerformanceTestActivity: 5 stages: onSaveInstanceState() in 287.0ms
CompositeAndroid 0.3.0 5 plugins 50.000 iterations with optimizations
V/CompositePerformanceTestActivity: 5 plugins: getCacheDir() in 256.0ms
V/CompositePerformanceTestActivity: 5 plugins: getCacheDir() in 90.0ms  (direct)
V/CompositePerformanceTestActivity: 5 plugins: getResources() in 143.0ms
V/CompositePerformanceTestActivity: 5 plugins: getResources() in 2.0ms (direct) *
V/CompositePerformanceTestActivity: 5 plugins: onSaveInstanceState() in 389.0ms
V/CompositePerformanceTestActivity: 5 plugins: onSaveInstanceState() in 247.0ms (direct)
CompositeAndroid 0.2.1 5 plugins 50.000 iterations with optimizations
V/CompositePerformanceTestActivity: 5 plugins: getCacheDir() in 364.0ms
V/CompositePerformanceTestActivity: 5 plugins: getCacheDir() in 96.0ms  (direct)
V/CompositePerformanceTestActivity: 5 plugins: getResources() in 238.0ms
V/CompositePerformanceTestActivity: 5 plugins: getResources() in 2.0ms (direct) *
V/CompositePerformanceTestActivity: 5 plugins: onSaveInstanceState() in 596.0ms
V/CompositePerformanceTestActivity: 5 plugins: onSaveInstanceState() in 350.0ms (direct)

* Compiler optimizations for getters as we see it for getResources() are not applied when doing composition. This makes the overhead much larger but mostly acceptable.

(direct) calls call the super method directly without any delegate logic and without calling any plugin. The diff between direct and non direct calls is the overhead from CompositeAndroid which is between 50% and 400%. Nothing to worry.

I also tried optimizations via reflection, not calling a plugin when the plugin doesn't override the calling method. This results in a big overhead for the first call but all future calls get very close the results of the normal 4 layer inheritance. My tests showed it's only worth the initial overhead after over 1.000.000 calls.

I also discovered that some methods are far more called than others. Especially getApplicationInfo() with often 100.000 times per Activity and getResources() a few thousand times. getApplicationInfo() is used inside the View constructor. When inflating a layout this gets called thousands of times. Therefore I'm looking forward to the reflection solution for those performance critical methods in a separate PR.

}
});
};
return superCall.call();

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I understand why you are allocating a function just to call it immediately. Is there something I'm missing?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you missed the recursive call where the listener is passed into the ActivityPlugin#getContext(CallFun0<Context>) in line 128

@ScottPierce
Copy link

I appreciate the work on this, but I still feel like this is very heavy. It seems like this framework, at any stage of the lifecycle of any component shouldn't be allocating any objects.

In my mind, the only object allocations that should be done are the list of components, and the components themselves. Beyond that, I'm not sure I understand why this is doing anything more than iterating through the list of components and properly handling their responses.

@passsy
Copy link
Owner Author

passsy commented Jul 23, 2016

Let me explain why the allocation is required:

The easiest solution, iterating over the plugins works for the easiest cases when the plugins just want to know when a method gets called. That's exactly what Navi does.
CompositeAndroid goes one step further allowing plugins to skip the super call. Skipping a super call results in not calling earlier added plugins and also not calling super of AppCompatActivity. It basically a rebuild of inheritance. And I haven't found a way to build this without this additional allocation 😉

Check the early return sample in the wiki for a more detailed explanation.

@ScottPierce
Copy link

I see. Now it's making more sense, some of the decisions that were made.

So when you mention it that way, it seems like you view it as an important feature? I wouldn't intuitively expect that it'd behave like that. Why is it desirable that it behaves that way?

@passsy
Copy link
Owner Author

passsy commented Jul 25, 2016

Well, you have to know the history. I wanted composition for Activities. I was very confident the Navi project is the right tool to accomplish my goals. But it turned out that Navi isn't able to work with return types by design. For Navi it's not clear what would happen when two "plugins" would return a value. For example when calling #onCreateOptionsMenu.

This problem doesn't exists when using inheritance. I didn't want to reinvent the wheel so I've rebuild inheritance for plugins. And now it's easy to convert a level of inheritance into a plugin and use it only when required with composition.

Another important factor is the ability to execute code before calling super. Navi set defaults like the onResume event always gets fired after super whereas the onPause event get fired before super. I had a case where I had to execute code before super.onResume() and had no chance doing this.

The solution to this problem was: rebuild inheritance and let super of a plugins actually call super of the Activity. When not calling super in plugins, super of the Activity doesn't get called.

I'd call this desirable because you have full control from a plugin. You could write a plugin which reimplements setContentView inflating the layouts not into the root container but inside a layout of your choice (and do not call super). I also think mocking could be done easily with this library i.e. by overriding getSystemService() to fake the absence/presence of a service.

What have you expected? Any recommendation how I should clarify this behavior?

@passsy passsy merged commit a2fea1a into master Jul 27, 2016
@passsy passsy deleted the feature/performance branch July 27, 2016 09:36
@passsy passsy mentioned this pull request Jul 27, 2016
@passsy passsy restored the feature/performance branch August 26, 2016 13:50
@passsy passsy deleted the feature/performance branch September 14, 2016 11:57
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants