Android: Cache android studio project entries

build/android/gradle/generate_gradle.py

Index: build/android/gradle/generate_gradle.py
diff --git a/build/android/gradle/generate_gradle.py b/build/android/gradle/generate_gradle.py
index 2b529f0b6e30206e53bff013ffc1ac2b97945b84..8b6577839a005a3370b6d93cbdee63568aa1c730 100755
--- a/build/android/gradle/generate_gradle.py
+++ b/build/android/gradle/generate_gradle.py
@@ -117,39 +117,67 @@ def _QueryForAllGnTargets(output_dir):
   return ret
 
 
+def _Dedup(entries):

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

nit: this doesn't save any lines over just calling _DedupAgainst() directly. I'd
remove it.

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

Done.

+  """Return unique entries in iterable in the same order."""
+  return _DedupAgainst(entries, [])
+
+
+def _DedupAgainst(entries, existing_entries):
+  """Return unique entries in iterable given existing entries."""

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

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.

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

Done.

+  seen = set(existing_entries)
+  unique_entries = []
+  for entry in entries:
+    if entry not in seen:
+      unique_entries.append(entry)
+      seen.add(entry)
+  return unique_entries
+
+
 class _ProjectEntry(object):
   """Helper class for project entries."""
+
+  _cached_entries = {}
+
   def __init__(self, gn_target):
-    assert gn_target.startswith('//'), gn_target
-    if ':' not in gn_target:
-      gn_target = '%s:%s' % (gn_target, os.path.basename(gn_target))
+    # Use _ProjectEntry.FromGnTarget instead for caching.
     self._gn_target = gn_target
     self._build_config = None
     self._java_files = None
+    self._all_entries = None
     self.android_test_entry = None
 
   @classmethod
+  def FromGnTarget(cls, gn_target):
+    assert gn_target.startswith('//'), gn_target
+    if ':' not in gn_target:
+      gn_target = '%s:%s' % (gn_target, os.path.basename(gn_target))
+    if gn_target not in cls._cached_entries:
+      cls._cached_entries[gn_target] = cls(gn_target)
+    return cls._cached_entries[gn_target]
+
+  @classmethod
   def FromBuildConfigPath(cls, path):
     prefix = 'gen/'
     suffix = '.build_config'
     assert path.startswith(prefix) and path.endswith(suffix), path
     subdir = path[len(prefix):-len(suffix)]
-    return cls('//%s:%s' % (os.path.split(subdir)))
+    gn_target = '//%s:%s' % (os.path.split(subdir))
+    return cls.FromGnTarget(gn_target)
 
   def __hash__(self):
-    return hash(self._gn_target)
+    return hash(self.GnTarget())

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

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

[Peter Wen, 2017/02/17 16:37:25]

Done.

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

 
   def __eq__(self, other):
-    return self._gn_target == other.GnTarget()
+    return self.GnTarget() == other.GnTarget()
 
   def GnTarget(self):
     return self._gn_target
 
   def NinjaTarget(self):
-    return self._gn_target[2:]
+    return self.GnTarget()[2:]
 
   def GnBuildConfigTarget(self):
-    return '%s__build_config' % self._gn_target
+    return '%s__build_config' % self.GnTarget()
 
   def NinjaBuildConfigTarget(self):
     return '%s__build_config' % self.NinjaTarget()
@@ -195,6 +223,26 @@ class _ProjectEntry(object):
       self._java_files = java_files
     return self._java_files
 
+  def SubdirJavaFiles(self):

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

Is this the same as saying generated java files? If so, I think that would be
clearer.

[Peter Wen, 2017/02/17 16:37:25]

Done.

+    return [p for p in self.JavaFiles() if not p.startswith('..')]
+
+  def PrebuiltJars(self):
+    return self.Gradle()['dependent_prebuilt_jars']
+
+  def AllEntries(self):

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

This is worth adding pydoc for.

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

Done.

+    if self._all_entries is None:
+      logging.debug('Generating entries for %s', self.GnTarget())
+      deps = [_ProjectEntry.FromBuildConfigPath(p)
+          for p in self.Gradle()['dependent_android_projects']]
+      deps.extend([_ProjectEntry.FromBuildConfigPath(p)

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

nit: shouldn't need the [].

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

Done.

+          for p in self.Gradle()['dependent_java_projects']])
+      all_entries = []
+      for dep in deps:
+        all_entries += dep.AllEntries()

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

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:25]

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).

+      all_entries.append(self)
+      self._all_entries = _Dedup(all_entries)
+    return self._all_entries
+
 
 class _ProjectContextGenerator(object):
   """Helper class to generate gradle build files"""
@@ -216,7 +264,7 @@ class _ProjectContextGenerator(object):
     return jni_libs
 
   def _GenJavaDirs(self, entry):
-    java_dirs, excludes = _CreateJavaSourceDir(
+    java_dirs, excludes = _ComputeJavaSourceDirsAndExcludes(
         constants.GetOutDirectory(), entry.JavaFiles())
     if self.Srcjars(entry):
       java_dirs.append(
@@ -230,18 +278,19 @@ class _ProjectContextGenerator(object):
     return res_dirs
 
   def _GenCustomManifest(self, entry):
-    """Returns the path to the generated AndroidManifest.xml."""
-    javac = entry.Javac()
-    resource_packages = javac['resource_packages']
-    output_file = os.path.join(
-        self.EntryOutputDir(entry), 'AndroidManifest.xml')
+    """Returns the path to the generated AndroidManifest.xml.
 
+    Gradle uses package id from manifest when generating R.class. So, we need
+    to generate a custom manifest if we let gradle process resources. We cannot
+    simply set android.defaultConfig.applicationId because it is not supported
+    for library targets."""
+    resource_packages = entry.Javac().get('resource_packages')
     if not resource_packages:
-      logging.error('Target ' + entry.GnTarget() + ' includes resources from '
+      logging.debug('Target ' + entry.GnTarget() + ' includes resources from '
           'unknown package. Unable to process with gradle.')
       return _DEFAULT_ANDROID_MANIFEST_PATH
     elif len(resource_packages) > 1:
-      logging.error('Target ' + entry.GnTarget() + ' includes resources from '
+      logging.debug('Target ' + entry.GnTarget() + ' includes resources from '
           'multiple packages. Unable to process with gradle.')
       return _DEFAULT_ANDROID_MANIFEST_PATH
 
@@ -249,6 +298,8 @@ class _ProjectContextGenerator(object):
     variables['compile_sdk_version'] = self.build_vars['android_sdk_version']
     variables['package'] = resource_packages[0]
 
+    output_file = os.path.join(
+        self.EntryOutputDir(entry), 'AndroidManifest.xml')
     data = self.jinja_processor.Render(_TemplatePath('manifest'), variables)
     _WriteFile(output_file, data)
 
@@ -270,9 +321,8 @@ class _ProjectContextGenerator(object):
     generated_inputs = []
     generated_inputs.extend(self.Srcjars(entry))
     generated_inputs.extend(_RebasePath(entry.ResZips()))
-    generated_inputs.extend(
-        p for p in entry.JavaFiles() if not p.startswith('..'))
-    generated_inputs.extend(entry.Gradle()['dependent_prebuilt_jars'])
+    generated_inputs.extend(entry.SubdirJavaFiles())
+    generated_inputs.extend(entry.PrebuiltJars())
     return generated_inputs
 
   def Generate(self, entry):
@@ -284,22 +334,14 @@ class _ProjectContextGenerator(object):
     variables['res_dirs'] = self._Relativize(entry, self._GenResDirs(entry))
     android_manifest = entry.Gradle().get('android_manifest')
     if not android_manifest:
-      # Gradle uses package id from manifest when generating R.class. So, we
-      # need to generate a custom manifest if we let gradle process resources.
-      # We cannot simply set android.defaultConfig.applicationId because it is
-      # not supported for library targets.
-      if variables['res_dirs']:
-        android_manifest = self._GenCustomManifest(entry)
-      else:
-        android_manifest = _DEFAULT_ANDROID_MANIFEST_PATH
+      android_manifest = self._GenCustomManifest(entry)
     variables['android_manifest'] = self._Relativize(entry, android_manifest)
+    # TODO(agrieve): Add an option to use interface jars and see if that speeds
+    # things up at all.
+    variables['prebuilts'] = self._Relativize(entry, entry.PrebuiltJars())
     deps = [_ProjectEntry.FromBuildConfigPath(p)
             for p in entry.Gradle()['dependent_android_projects']]
     variables['android_project_deps'] = [d.ProjectName() for d in deps]
-    # TODO(agrieve): Add an option to use interface jars and see if that speeds
-    # things up at all.
-    variables['prebuilts'] = self._Relativize(
-        entry, entry.Gradle()['dependent_prebuilt_jars'])
     deps = [_ProjectEntry.FromBuildConfigPath(p)
             for p in entry.Gradle()['dependent_java_projects']]
     variables['java_project_deps'] = [d.ProjectName() for d in deps]
@@ -350,7 +392,7 @@ def _ComputeExcludeFilters(wanted_files, unwanted_files, parent_dir):
   return excludes
 
 
-def _CreateJavaSourceDir(output_dir, java_files):
+def _ComputeJavaSourceDirsAndExcludes(output_dir, java_files):
   """Computes the list of java source directories and exclude patterns.
 
   1. Computes the root java source directories from the list of files.
@@ -461,6 +503,10 @@ def _GenerateGradleFile(entry, generator, build_vars, jinja_processor):
   if entry.android_test_entry:
     variables['android_test'] = generator.Generate(
         entry.android_test_entry)
+    for key, value in variables['android_test'].iteritems():
+      if isinstance(value, list):
+        variables['android_test'][key] = _DedupAgainst(
+            value, variables['main'][key])
 
   return jinja_processor.Render(
       _TemplatePath(target_type.split('_')[0]), variables)
@@ -600,7 +646,7 @@ def main():
     targets = [re.sub(r'_junit_tests$', '_junit_tests__java_binary', t)
                for t in targets]
 
-  main_entries = [_ProjectEntry(t) for t in targets]
+  main_entries = [_ProjectEntry.FromGnTarget(t) for t in targets]
 
   logging.warning('Building .build_config files...')
   _RunNinja(output_dir, [e.NinjaBuildConfigTarget() for e in main_entries])