Android: Cache android studio project entries

Android: Cache android studio project entries

- Add entry.AllEntries() in preparation for iterating through deps.
- Caching _ProjectEntry objects to avoid duplicate computation.
- Dedup template variables between androidTest and main.
- Minor clean-up.

BUG=620034
Review-Url: https://codereview.chromium.org/2697313004
Cr-Commit-Position: refs/heads/master@{#451352}
Committed: https://chromium.googlesource.com/chromium/src/+/8281f251376f2a21ad28737f5c7416f10c276d6f

[Peter Wen, 2017-02-16 18:08:48]

wnwen@chromium.org changed reviewers:
+ agrieve@chromium.org

[Peter Wen, 2017-02-16 18:08:48]

Hi Andrew, PTAL. This gets us ready to add a flag to enable single module apks.

Single module apks not only greatly reduce sync time (20s -> 1s), but also
greatly reduce android studio build time (XXs -> 1-5s). As we move towards using
AS resources and hopefully even AS builds, this is more important than ever.

cc'ing sakal-chromium@ and dgn@

[agrieve, 2017-02-17 03:07:01]

just nits. lgtm

https://codereview.chromium.org/2697313004/diff/20001/build/android/gradle/ge...
File build/android/gradle/generate_gradle.py (right):

https://codereview.chromium.org/2697313004/diff/20001/build/android/gradle/ge...
build/android/gradle/generate_gradle.py:120: def _Dedup(entries):
nit: this doesn't save any lines over just calling _DedupAgainst() directly. I'd
remove it.

https://codereview.chromium.org/2697313004/diff/20001/build/android/gradle/ge...
build/android/gradle/generate_gradle.py:126: """Return unique entries in
iterable given existing entries."""
You return a list, so I'd say that rather than iterable.

I think the point of this is that it maintains the order of entries (or else you
could just do set(A) - set(B). So that's worth saying as well.

https://codereview.chromium.org/2697313004/diff/20001/build/android/gradle/ge...
build/android/gradle/generate_gradle.py:168: return hash(self.GnTarget())
nit: I think it's generally better to leave these how they were. I don't see
what it buys us to go through a getter

https://codereview.chromium.org/2697313004/diff/20001/build/android/gradle/ge...
build/android/gradle/generate_gradle.py:226: def SubdirJavaFiles(self):
Is this the same as saying generated java files? If so, I think that would be
clearer.

https://codereview.chromium.org/2697313004/diff/20001/build/android/gradle/ge...
build/android/gradle/generate_gradle.py:232: def AllEntries(self):
This is worth adding pydoc for.

https://codereview.chromium.org/2697313004/diff/20001/build/android/gradle/ge...
build/android/gradle/generate_gradle.py:237:
deps.extend([_ProjectEntry.FromBuildConfigPath(p)
nit: shouldn't need the [].

https://codereview.chromium.org/2697313004/diff/20001/build/android/gradle/ge...
build/android/gradle/generate_gradle.py:241: all_entries += dep.AllEntries()
I think it might be easier to follow if you de-dupe as you go rather than adding
all and then removing again. E.g.:

for dep in deps:
  for e in dep.AllEntries():
    if e not in all_entries:
      all_entries.append(e)

Note: Doing an O(n) lookup in the list is fine, as n will be small.

[Peter Wen, 2017-02-17 16:37:26]

Thanks for the review!

https://codereview.chromium.org/2697313004/diff/20001/build/android/gradle/ge...
File build/android/gradle/generate_gradle.py (right):

https://codereview.chromium.org/2697313004/diff/20001/build/android/gradle/ge...
build/android/gradle/generate_gradle.py:120: def _Dedup(entries):
On 2017/02/17 03:07:01, agrieve (OOO until Feb 21) wrote:
> nit: this doesn't save any lines over just calling _DedupAgainst() directly.
I'd
> remove it.

Done.

https://codereview.chromium.org/2697313004/diff/20001/build/android/gradle/ge...
build/android/gradle/generate_gradle.py:126: """Return unique entries in
iterable given existing entries."""
On 2017/02/17 03:07:01, agrieve (OOO until Feb 21) wrote:
> You return a list, so I'd say that rather than iterable.
> 
> I think the point of this is that it maintains the order of entries (or else
you
> could just do set(A) - set(B). So that's worth saying as well.

Done.

https://codereview.chromium.org/2697313004/diff/20001/build/android/gradle/ge...
build/android/gradle/generate_gradle.py:168: return hash(self.GnTarget())
On 2017/02/17 03:07:01, agrieve (OOO until Feb 21) wrote:
> nit: I think it's generally better to leave these how they were. I don't see
> what it buys us to go through a getter

Done.

We don't do any checks/calculations in the getter, reverted.

https://codereview.chromium.org/2697313004/diff/20001/build/android/gradle/ge...
build/android/gradle/generate_gradle.py:226: def SubdirJavaFiles(self):
On 2017/02/17 03:07:01, agrieve (OOO until Feb 21) wrote:
> Is this the same as saying generated java files? If so, I think that would be
> clearer.

Done.

https://codereview.chromium.org/2697313004/diff/20001/build/android/gradle/ge...
build/android/gradle/generate_gradle.py:232: def AllEntries(self):
On 2017/02/17 03:07:01, agrieve (OOO until Feb 21) wrote:
> This is worth adding pydoc for.

Done.

https://codereview.chromium.org/2697313004/diff/20001/build/android/gradle/ge...
build/android/gradle/generate_gradle.py:237:
deps.extend([_ProjectEntry.FromBuildConfigPath(p)
On 2017/02/17 03:07:01, agrieve (OOO until Feb 21) wrote:
> nit: shouldn't need the [].

Done.

https://codereview.chromium.org/2697313004/diff/20001/build/android/gradle/ge...
build/android/gradle/generate_gradle.py:241: all_entries += dep.AllEntries()
On 2017/02/17 03:07:01, agrieve (OOO until Feb 21) wrote:
> I think it might be easier to follow if you de-dupe as you go rather than
adding
> all and then removing again. E.g.:
> 
> for dep in deps:
>   for e in dep.AllEntries():
>     if e not in all_entries:
>       all_entries.append(e)
> 
> Note: Doing an O(n) lookup in the list is fine, as n will be small.

Switched to using sets instead, it's better to sort everything after
calculations for the generated build.gradle file to be nice (in-order dedup
doesn't work quite as nicely as I originally hoped).

[Peter Wen, 2017-02-17 16:37:31]

The CQ bit was checked by wnwen@chromium.org

[Peter Wen, 2017-02-17 16:37:31]

The patchset sent to the CQ was uploaded after l-g-t-m from agrieve@chromium.org
Link to the patchset: https://codereview.chromium.org/2697313004/#ps40001
(title: "Remove dup add docs")

[commit-bot: I haz the power, 2017-02-17 16:38:06]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-17 19:20:53]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1487349451751430,
"parent_rev": "54d2cfed0cfc008975147b3481a19cc892edf56b", "commit_rev":
"8281f251376f2a21ad28737f5c7416f10c276d6f"}

[commit-bot: I haz the power, 2017-02-17 19:22:17]

Description was changed from

==========
Android: Cache android studio project entries

- Add entry.AllEntries() in preparation for iterating through deps.
- Caching _ProjectEntry objects to avoid duplicate computation.
- Dedup template variables between androidTest and main.
- Minor clean-up.

BUG=620034
==========

to

==========
Android: Cache android studio project entries

- Add entry.AllEntries() in preparation for iterating through deps.
- Caching _ProjectEntry objects to avoid duplicate computation.
- Dedup template variables between androidTest and main.
- Minor clean-up.

BUG=620034

Review-Url: https://codereview.chromium.org/2697313004
Cr-Commit-Position: refs/heads/master@{#451352}
Committed:
https://chromium.googlesource.com/chromium/src/+/8281f251376f2a21ad28737f5c74...
==========

[commit-bot: I haz the power, 2017-02-17 19:22:19]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/8281f251376f2a21ad28737f5c74...