Fix issues with Android Studio Koala sync #562
Conversation
- And only use the wait for sync method on startup, as the manual sync triggers should be handled by the future provided by the API - And don't rely on the UI to tell the status of sync as there can be progress indicators that have nothing to do with sync. Signed-off-by: Ahmet Kerim Senol <[email protected]>
...as that doesn't seem to work anymore in headless mode. It's fine to rely on the progress indicator instead when exiting. Signed-off-by: Ahmet Kerim Senol <[email protected]>
|
@asodja please take a look. CC @joshfriend |
|
(this is in reference to a bug I filed with android studio: https://issuetracker.google.com/issues/346618772) |
| waitOnPreviousGradleSyncFinish(project); | ||
| waitOnBackgroundProcessesFinish(project); | ||
| if (isStartup) { | ||
| waitOnStartupSyncToFinish(project); |
There was a problem hiding this comment.
❓ How this behaves with all the indexing that happens after the sync now?
We deliberately used waitOnBackgroundProcessesFinish and waited on all processes to finish so also indexing was done before we triggered a next sync.
There was a problem hiding this comment.
Waiting for indexing shouldn't be necessary as:
- Triggering a new sync will pause any indexing. (Though newer versions of IDEA do allow indexing while syncing. Though I think that's another discussion due to that behavior potentially also affecting the IDE time measurements of the profiler.)
- Indexing is not considered part of the measured values.
This might not be the all around best approach, but I think the current approach is error-prone and warrants changing. We have progress indicators for things like Firebase Test Lab (or Run configurations) that has nothing to do with indexing and I've seen those "block" the profiler for no reason.
There was a problem hiding this comment.
Gradle "work time" is measured via tooling api , so we actually get 3 numbers from these measurements:
- Full sync time
- Gradle work time
- IDE work time: calculated as Full sync time - Gradle work time
So in that case indexing would count as IDE work time.
I think measuring full time (meaning additional idexing etc) that IDE needs to do project refresh is still useful, since that way we get better idea of real users experience.
I think it would be still nice we could wait on indexing to finish and not trigger sync before IDE has finish all it's work, since this always a big part of sync time.
There was a problem hiding this comment.
Is there any better way to wait for indexing and other work to finish right after the sync?
There was a problem hiding this comment.
I've asked this question on Jetbrains Slack, let's see if they have some guidance for us.
There was a problem hiding this comment.
I think measuring full time (meaning additional idexing etc) that IDE needs to do project refresh is still useful, since that way we get better idea of real users experience.
Yes that would be useful and I have been wondering how to measure this, however, gradle-profiler currently sometimes accidentally captures indexing which causes wild outliers in the data. Is this discussion about how to do that intentionally? If so, can that be a separate discussion so that this PR can be merged? I've tested this a few times with Koala and newer IDEs and it seems to work fine.
There was a problem hiding this comment.
@akerimsenol did you get any response from Jetbrains?
|
Just bumped into this :) |
|
I merged #579 that should make profiling work with Koala and Ladybug. It takes an idea from this PR to only use the wait for sync method on startup sync and not manual sync, other changes are a bit different. With that I will close this PR. |
No description provided.