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

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

Closed
raymondctc opened this issue Aug 1, 2016 · 6 comments

Comments

@raymondctc
Copy link

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

VicV commented Aug 1, 2016

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

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

VicV commented Aug 2, 2016

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
#24 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABPbJSmGLn0gGHkKomVtiqs2OPfFp2NYks5qbqc-gaJpZM4JZVW1
.

@raymondctc
Copy link
Author

raymondctc commented Aug 3, 2016

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

VicV commented Aug 3, 2016

Thanks @raymondctc,

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

@VicV
Copy link
Contributor

VicV commented Aug 11, 2016

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!

@VicV VicV closed this as completed Aug 11, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants