Sync process optimizations #5219
Conversation
github-actions
bot
added
product: Android Studio
idanakav
force-pushed
the
sync-optimize
branch
from
08243ad
to
a878147
Compare
There was a problem hiding this comment.
Thanks for the contribution!
The main ideas look good, but I don't think we can cache the bazel info between runs. Command-line flags affect the outcome (e.g. --server_javabase), which can be in any of multiple locations and bazelrc files. Caching for a single run seems okay, since a lot of the flags that change the info output are startup flags and would invalidate most of the sync anyway.
So, if you could change the logic in BlazeInfoProvider to invalidate every run, I think it's a great PR!
| BlazeContext context, | ||
| List<String> syncFlags, | ||
| SyncMode syncMode) { | ||
| boolean useBazelInfoRunner = !BlazeInfoProvider.isEnabled() || syncMode == SyncMode.FULL; |
There was a problem hiding this comment.
Maybe we could just pass the sync mode to the BlazeInfoProvider and have it invalidate itself (or pretend that it has invalidated) on a full sync? I quite like the idea of "everything goes through the provider".
There was a problem hiding this comment.
The idea was to keep BlazeInfoProvider useful to other use cases apart from the sync, but if we are not planning to extend it, we can make it more coupled to the sync and pass the sync mode as well.
base/src/com/google/idea/blaze/base/command/info/BlazeInfoProvider.java
Outdated
Show resolved
Hide resolved
Computing the working set involves running few git commands, including git diff, it can be skipped if the "Expand Sync to Working Set" option is disabled. The working set will be computed on each `collectProjectState` call, given that this method is triggered three times per sync operation, disabling unnecessary working set computations can reduce the sync times roughly the amount of time taken by 3 git diff runs.
idanakav
force-pushed
the
sync-optimize
branch
from
a878147
to
aaad8dd
Compare
| if(!isEnabled()) { | ||
| return; | ||
| } | ||
| BlazeInfoProvider.getInstance(project).invalidate(); |
There was a problem hiding this comment.
Should we check for null here? In case we de-register the provider accidentally and forget to de-register the invalidator.
idanakav
force-pushed
the
sync-optimize
branch
from
aaad8dd
to
c662eb6
Compare
Currently, BlazeInfo will be computed 5 times during incremental sync, this commit introduces logic to cache it and re-use the computed value during incremental syncs. `BlazeInfoProvider` was introduced in order to avoid changing `BlazeInfoRunner` and keep its responsibility to solely execute the info command. Logic is guarded by `blaze.info.provider.enabled` which is disabled by default.
idanakav
force-pushed
the
sync-optimize
branch
from
c662eb6
to
c12b09d
Compare
github-actions
bot
removed
the
awaiting-review
|
Tested it locally in IJ Ultimate and saw no issues. |
Checklist
Issue number: #5217
More details can be found in the issue and on each commit.