taplytics / Taplytics-Android-SDK

Taplytics Android SDK
http://taplytics.com
19 stars 9 forks source link

Traceview finds that albatross.onActivityCreated() takes lots of CPU time #24

Closed raymondctc closed 8 years ago

raymondctc commented 8 years ago

Hi developers,

We are using Taplytics 1.9.8 at the moment and we found that some Taplytics function takes too much time to run on UI thread and thus making the UI unresponsive, especially on slow devices.

Here is the screen capture of the traceview

It takes around 420ms for albatross.onActivityCreated() to be finished.

VicV commented 8 years ago

@raymondctc where are you calling StartTaplytics?

albatross is an extension ActivityLifecycleCallbacks. In the onActivityCreated function, we execute a runnable on our own executor, which kicks off a network call to get the Taplytics configuration. This should not be happening on the UI thread at all. I'll take a look into it from my side, but I am curious if you're doing something different with regards to how you're setting up Taplytics.

Note: Today is a holiday here in Toronto for me, so I will have to get back to you tomorrow for this if I don't get to it later today.

Thanks for bringing this up!

raymondctc commented 8 years ago

Hi @VicV ,

I see. We do what the document suggested, to put startTaplytics inside Application class onCreate(). But we initialize it on a separate thread pool.

Executors.newFixedThreadPool(getNumberOfCores()).submit(r -> {
        Taplytics.startTaplytics(...);
});

Not sure if this is related

VicV commented 8 years ago

It may be. Any chance you can do it on the application thread and give me the results? Will be looking at this tomorrow.

On Aug 1, 2016 10:11 PM, "Raymond Chan" notifications@github.com wrote:

Hi @VicV https://github.com/VicV ,

I see. We do what the document suggested, to put startTaplytics inside Application class onCreate(). But we initialize it on a separate thread pool.

Executors.newFixedThreadPool(getNumberOfCores()).submit(r -> { Taplytics.startTaplytics(...); });

Not sure if this is related

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/taplytics/Taplytics-Android-SDK/issues/24#issuecomment-236773146, or mute the thread https://github.com/notifications/unsubscribe-auth/ABPbJSmGLn0gGHkKomVtiqs2OPfFp2NYks5qbqc-gaJpZM4JZVW1 .

raymondctc commented 8 years ago

Hi @VicV ,

I have tried to move back Taplytics.startTaplytics() to main thread, the result is similar. https://s31.postimg.org/4z6mqvnu3/Screen_Shot_2016_08_02_at_8_10_47_PM.png

FYI, my device is RedMi 1, running on Android API 17 (JellyBean 4.2.2)

VicV commented 8 years ago

Thanks @raymondctc,

Currently working on a new release and will get to this ASAP.

VicV commented 8 years ago

Note: Closing this as the "async" starting option alleviated a lot of issues.

Regarding this without the async option: Right now the taplytics executor runs on the current available thread, so this function does operate on the main(UI) thread.

I've opened a ticket internally to switch where this executor operates, however I am first going to write many tests revolving around that before I change anything given that it is such a large change.

So, its on the roadmap. Thanks for brining this up!