-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Activity startup becomes very slow after many tests using Compose #9043
Comments
Have you tried to disable animations?
robolectric/shadows/framework/src/main/java/org/robolectric/shadows/ShadowUiAutomation.java Line 75 in 3ab3afe
|
I just did. That makes no difference: test 100 will still process 121372 messages at startup. |
The first step to diagnose performance issues is to run the tests under a JVM profiler with CPU sampling enabled. This will show you where the time is spent. You could use a tool like VisualVM or YourKit. Although calling |
The issue is: it's not just the animations, they only make the problem more visible. Without doing anything other than starting the activity with any Compose function in it (even just an empty ComposeView), every activity start will add 4 messages to the queue that carry over to the next test. And I could run visualvm or similar, but we already know that the sheer amount of Looper messages is the problem. The issue is that I have no clue where they come from or why they survive activity destruction. |
@bvschaik if you want to see what tasks are run, you can set a breakpoint here in robolectric/shadows/framework/src/main/java/org/robolectric/shadows/ShadowPausedLooper.java Line 600 in 3ab3afe
If you look at the members of You could try to use that to get a sense of which tasks get posted. |
It's thousands of the same messages: I tried printing the stacktrace where they're coming from, by adding logging to: robolectric/shadows/framework/src/main/java/org/robolectric/shadows/ShadowPausedMessageQueue.java Line 194 in 81831fc
Those messages come from For reference, these are the messages processed in the first test:
These are the message processed in the subsequent tests:
I'm unsure how to proceed to debug this further. |
So it looks like the tasks are related to Conmpose's AndroidUiDispatcher. I am not an expert on Compose internals, but this appears to be a way to get coroutines to run in the Android UI thread. The next step is to figure out when AndroidUiDispatcher is used, and see if the behavior can be configured. |
Neither am I, and I suppose there isn't an expert on Compose internals that we can magically summon? 😉 What I've been able to gather:
I tried writing a shadow for AndroidUiDispatcher to ignore the call to I tried not advancing the compose clock using I tried calling AndroidUiDispatcher.Main.cancel() after the first test to hopefully reset the dispatcher - it had no effect. I tried dumping the memory after the first test and after the 100th test and comparing the objects in them, hoping to find some queue with thousands of messages in them, but that got me nowhere. I'm going to stop investigating - I spent a week on this already - and just apply the "forkEvery = 1" workaround for our tests. |
@bvschaik if you are interested in digging deeper, is there a repro of this bug that is possible to make available? Without being able to tinker with this, it may be difficult to progress on this issue. |
Nvm @bvschaik , I saw there is a repro of the issue already, I will take a look at that. |
I suspect this issue has the same root cause 755 AndroidUiDispatcher keeps a static reference to Choreographer.getInstance which is then reused among tests. This is bad because Robolectric resets the state of the Android environment between tests, including the current system time. See ShadowPausedChoreographer#reset Reusing the Choreographer between tests results in the Choreographer in use having an incorrect view of the current system time which leads to things generally going haywire. Running the provided sample shows a symptom of this as it starts spitting out many logs like
I can workaround this issue in the provided sample by resetting the AndroidUIDispatcher's Choreographer reference before each test:
I can't think of a great way to workaround this issue in Robolectric. In the past I've tried to change Robolectric's Choreographer resetter to just clear the local state as opposed to requiring a new instance, but its quite involved to do so in a safe way across all supported Android versions. I'll discuss with the compose team to see if we can figure out something. |
This is still ugly but I was able to simplify the workaround to the following:
|
Previously Robolectric just cleared the thread local Choreographer.getInstance between each test. This would cause issues with any code paths that kept static references to Choreographer.getInstance (like Compose's AndroidUiDispatcher), because those code paths would use stale Choreographer instances with inconsistent views of the current system time. This commit changes the Choreographer resetter to just reset the local state of each active Choreographer. Fixes #9043 PiperOrigin-RevId: 638065889
Previously Robolectric just cleared the thread local Choreographer.getInstance between each test. This would cause issues with any code paths that kept static references to Choreographer.getInstance (like Compose's AndroidUiDispatcher), because those code paths would use stale Choreographer instances with inconsistent views of the current system time. This commit changes the Choreographer resetter to just reset the local state of each active Choreographer. Fixes #9043 PiperOrigin-RevId: 638065889
Previously Robolectric just cleared the thread local Choreographer.getInstance between each test. This would cause issues with any code paths that kept static references to Choreographer.getInstance (like Compose's AndroidUiDispatcher), because those code paths would use stale Choreographer instances with inconsistent views of the current system time. This commit changes the Choreographer resetter to just reset the local state of each active Choreographer. Fixes #9043 PiperOrigin-RevId: 638065889
Previously Robolectric just cleared the thread local Choreographer.getInstance between each test. This would cause issues with any code paths that kept static references to Choreographer.getInstance (like Compose's AndroidUiDispatcher), because those code paths would use stale Choreographer instances with inconsistent views of the current system time. This commit changes the Choreographer resetter to just reset the local state of each active Choreographer. Fixes #9043 PiperOrigin-RevId: 638065889
Previously Robolectric just cleared the thread local Choreographer.getInstance between each test. This would cause issues with any code paths that kept static references to Choreographer.getInstance (like Compose's AndroidUiDispatcher), because those code paths would use stale Choreographer instances with inconsistent views of the current system time. This commit changes the Choreographer resetter to just reset the local state of each active Choreographer. Fixes #9043 PiperOrigin-RevId: 638065889
Previously Robolectric just cleared the thread local Choreographer.getInstance between each test. This would cause issues with any code paths that kept static references to Choreographer.getInstance (like Compose's AndroidUiDispatcher), because those code paths would use stale Choreographer instances with inconsistent views of the current system time. This commit changes the Choreographer resetter to just reset the local state of each active Choreographer. Fixes #9043 PiperOrigin-RevId: 638065889
Previously Robolectric just cleared the thread local Choreographer.getInstance between each test. This would cause issues with any code paths that kept static references to Choreographer.getInstance (like Compose's AndroidUiDispatcher), because those code paths would use stale Choreographer instances with inconsistent views of the current system time. This commit changes the Choreographer resetter to just reset the local state of each active Choreographer. Fixes #9043 PiperOrigin-RevId: 638065889
@bvschaik Robolectric 4.13 was released last night which I believe should fix this issue. Would you like to give it a try? |
@hoisie I tried it out but unfortunately the issue is still present, and possibly a bit worse than before. We have a project that contains around 450 Robolectric tests. Timings:
|
Previously Robolectric just cleared the thread local Choreographer.getInstance between each test. This would cause issues with any code paths that kept static references to Choreographer.getInstance (like Compose's AndroidUiDispatcher), because those code paths would use stale Choreographer instances with inconsistent views of the current system time. This commit changes the Choreographer resetter to just reset the local state of each active Choreographer. Fixes #9043 PiperOrigin-RevId: 638065889
Previously Robolectric just cleared the thread local Choreographer.getInstance between each test. This would cause issues with any code paths that kept static references to Choreographer.getInstance (like Compose's AndroidUiDispatcher), because those code paths would use stale Choreographer instances with inconsistent views of the current system time. This commit changes the Choreographer resetter to just reset the local state of each active Choreographer. Fixes #9043 PiperOrigin-RevId: 638065889
Previously Robolectric just cleared the thread local Choreographer.getInstance between each test. This would cause issues with any code paths that kept static references to Choreographer.getInstance (like Compose's AndroidUiDispatcher), because those code paths would use stale Choreographer instances with inconsistent views of the current system time. This commit changes the Choreographer resetter to just reset the local state of each active Choreographer. Fixes #9043 PiperOrigin-RevId: 638065889
Previously Robolectric just cleared the thread local Choreographer.getInstance between each test. This would cause issues with any code paths that kept static references to Choreographer.getInstance (like Compose's AndroidUiDispatcher), because those code paths would use stale Choreographer instances with inconsistent views of the current system time. This commit changes the Choreographer resetter to just reset the local state of each active Choreographer. Fixes #9043 PiperOrigin-RevId: 638065889
Previously Robolectric just cleared the thread local Choreographer.getInstance between each test. This would cause issues with any code paths that kept static references to Choreographer.getInstance (like Compose's AndroidUiDispatcher), because those code paths would use stale Choreographer instances with inconsistent views of the current system time. This commit changes the Choreographer resetter to just reset the local state of each active Choreographer. Fixes #9043 PiperOrigin-RevId: 638065889
Sorry for the confusion, the fix is not in 4.13. I just submitted it to the google branch now, hopefully it will be in upcoming releases. |
Description
Context: we're slowly migrating an app to Jetpack Compose. This app has an extensive test suite using Robolectric (~500 tests).
The problem: we noticed that the tests slow down to a crawl when executing all tests. Starting the activity (using ActivityScenario.launch) takes longer and longer: the first activity in the suite starts up instantly, activity in test 200 takes more than half a minute to start up.
Meanwhile, it is spamming the log with these messages, while all it is doing is starting an activity that displays a screen with a button..
I tracked it down to this line in ActivityController. Adding some debug logging revealed that the
shadowMainLooper.idleIfPaused();
call handles 1000+ more messages for activity startN+1
than for activity startN
. Meaning: the 100th test is processing 100,000+ messages before it starts.Something keeps hanging around after each test execution, but I'm not familiar enough with the internals of Android or Compose to figure out what exactly.
Note: this ONLY happens when using Compose. When using plain old layout XMLs, the number of messages is stable at 5 for each activity start.
Also interesting: having animations increase the number of messages by a lot. For example, every fragment transition adds around 400 messages to the next test.
Steps to Reproduce
Now, set MainActivity.useCompose to
false
, so the UI will use layouts instead of Compose.Robolectric & Android Version
Robolectric: 4.12.1
Android version: target 34
Compose version: 1.6.7
Link to a public git repo demonstrating the problem:
https://github.com/bvschaik/robolectric-compose-leak
Workaround
For now, we've set
forkEvery
to 1 to ensure each test starts with a clean environment, but that's of course not a solution.Possible related issues
I suspect the root cause of #7055 will be the same.
I tried the suggestions mentioned in that issue but none of them made any difference.
The text was updated successfully, but these errors were encountered: