[go: nahoru, domu]

Android: Turn off caching for jar_utils

Since ninja already checks timestamps and only re-compiles what is
needed in a new build, this caching is of limited benefit. Caching
also fails for some bots. Turning it off in order to avoid bot
failures.

Each dependency analysis run will be about a minute slower without
caching, but since it is usually run on a bot continuously, the impact
for devs is very limited.

Bug: 1099522,1470044,b/294782170
Change-Id: Ibcc115549a856f78a8b2c2aae61940f9dea6f2bc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4790864
Auto-Submit: Peter Wen <wnwen@chromium.org>
Reviewed-by: Samuel Huang <huangs@chromium.org>
Commit-Queue: Peter Wen <wnwen@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1184774}
diff --git a/build/android/gyp/bytecode_processor.py b/build/android/gyp/bytecode_processor.py
index 11bcda8..64a3253e5 100755
--- a/build/android/gyp/bytecode_processor.py
+++ b/build/android/gyp/bytecode_processor.py
@@ -7,6 +7,7 @@
 
 import argparse
 import collections
+import json
 import logging
 import os
 import pathlib
@@ -28,6 +29,10 @@
 # TODO(crbug.com/1099522): Remove this and make jdeps on by default.
 _USE_JDEPS = False
 
+# This is temporary in case another revert & debug session is necessary.
+# TODO(crbug.com/1099522): Remove this when jdeps is on by default.
+_DEBUG = False
+
 
 def _ShouldIgnoreDep(dep_name: str):
   if 'gen.base_module.R' in dep_name:
@@ -35,9 +40,8 @@
   return False
 
 
-def _ParseDepGraph(jar_path: str, output_dir: str):
-  output = jar_utils.run_jdeps(build_output_dir=pathlib.Path(output_dir),
-                               filepath=pathlib.Path(jar_path))
+def _ParseDepGraph(jar_path: str):
+  output = jar_utils.run_jdeps(pathlib.Path(jar_path))
   assert output is not None, f'Unable to parse jdep for {jar_path}'
   dep_graph = collections.defaultdict(set)
   # pylint: disable=line-too-long
@@ -62,7 +66,7 @@
     dep_from = parsed[0]
     dep_to = parsed[2]
     dep_graph[dep_from].add(dep_to)
-  return dep_graph
+  return dep_graph, output
 
 
 def _GnTargetToBuildFilePath(gn_target: str):
@@ -93,23 +97,20 @@
   logging.info('Parsing %d direct classpath jars', len(sdk_classpath_jars))
   sdk_classpath_deps = set()
   for jar in sdk_classpath_jars:
-    deps = jar_utils.extract_full_class_names_from_jar(
-        build_output_dir=pathlib.Path(output_dir), jar_path=pathlib.Path(jar))
+    deps = jar_utils.extract_full_class_names_from_jar(jar)
     sdk_classpath_deps.update(deps)
 
   logging.info('Parsing %d direct classpath jars', len(direct_classpath_jars))
   direct_classpath_deps = set()
   for jar in direct_classpath_jars:
-    deps = jar_utils.extract_full_class_names_from_jar(
-        build_output_dir=pathlib.Path(output_dir), jar_path=pathlib.Path(jar))
+    deps = jar_utils.extract_full_class_names_from_jar(jar)
     direct_classpath_deps.update(deps)
 
   logging.info('Parsing %d full classpath jars', len(full_classpath_jars))
   full_classpath_deps = set()
   dep_to_target = collections.defaultdict(set)
   for jar, target in zip(full_classpath_jars, full_classpath_gn_targets):
-    deps = jar_utils.extract_full_class_names_from_jar(
-        build_output_dir=pathlib.Path(output_dir), jar_path=pathlib.Path(jar))
+    deps = jar_utils.extract_full_class_names_from_jar(jar)
     full_classpath_deps.update(deps)
     for dep in deps:
       dep_to_target[dep].add(target)
@@ -117,7 +118,7 @@
   transitive_deps = full_classpath_deps - direct_classpath_deps
 
   missing_classes: Dict[str, str] = {}
-  dep_graph = _ParseDepGraph(input_jar, output_dir)
+  dep_graph, output = _ParseDepGraph(input_jar)
   logging.info('Finding missing deps from %d classes', len(dep_graph))
   # dep_graph.keys() is a list of all the classes in the current input_jar. Skip
   # all of these to avoid checking dependencies in the same target (e.g. A
@@ -155,6 +156,19 @@
         for dep_to in deps_to:
           dep_from = missing_classes[dep_to]
           print(f'     ** {dep_to} (needed by {dep_from})')
+      if _DEBUG:
+        gn_target_name = gn_target.split(':', 1)[-1]
+        debug_fname = f'/tmp/bytecode_processor_debug_{gn_target_name}.json'
+        with open(debug_fname, 'w') as f:
+          print(gn_target, file=f)
+          print(input_jar, file=f)
+          print(json.dumps(sorted(sdk_classpath_deps), indent=4), file=f)
+          print(json.dumps(sorted(direct_classpath_deps), indent=4), file=f)
+          print(json.dumps(sorted(full_classpath_deps), indent=4), file=f)
+          print(json.dumps(sorted(transitive_deps), indent=4), file=f)
+          print(json.dumps(missing_classes, sort_keys=True, indent=4), file=f)
+          print(output, file=f)
+        print(f'DEBUG FILE: {debug_fname}')
       if warnings_as_errors:
         sys.exit(1)
 
diff --git a/build/android/gyp/util/dep_utils.py b/build/android/gyp/util/dep_utils.py
index 10ef243..370e358 100644
--- a/build/android/gyp/util/dep_utils.py
+++ b/build/android/gyp/util/dep_utils.py
@@ -223,7 +223,7 @@
 
         full_class_names.update(
             jar_utils.extract_full_class_names_from_jar(
-                self._abs_build_output_dir, abs_unprocessed_jar_path))
+                abs_unprocessed_jar_path))
 
     return full_class_names
 
diff --git a/build/android/gyp/util/jar_utils.py b/build/android/gyp/util/jar_utils.py
index eabae59..05ecdbf 100644
--- a/build/android/gyp/util/jar_utils.py
+++ b/build/android/gyp/util/jar_utils.py
@@ -3,12 +3,10 @@
 # found in the LICENSE file.
 """Methods to run tools over jars and cache their output."""
 
-import dataclasses
-import functools
 import logging
 import pathlib
 import zipfile
-from typing import List, Optional
+from typing import List, Optional, Union
 
 from util import build_utils
 
@@ -36,59 +34,6 @@
     return False
 
 
-@dataclasses.dataclass
-class CacheFile:
-  jar_path: pathlib.Path
-  cache_suffix: str
-  build_output_dir: pathlib.Path
-  src_dir: pathlib.Path = _SRC_PATH
-
-  def __post_init__(self):
-    # Ensure that all paths are absolute so that relative_to works correctly.
-    self.jar_path = self.jar_path.resolve()
-    self.build_output_dir = self.build_output_dir.resolve()
-    self.src_dir = self.src_dir.resolve()
-
-  @functools.cached_property
-  def cache_path(self):
-    """Return a cache path for the jar that is always in the output dir.
-
-      Example:
-      - Given:
-        src_path = /cr/src
-        build_output_dir = /cr/src/out/Debug
-        cache_suffix = .jdeps
-      - filepath = /cr/src/out/Debug/a/d/file.jar
-        Returns: /cr/src/out/Debug/a/d/file.jar.jdeps
-      - filepath = /cr/src/out/b/c/file.jar
-        Returns: /cr/src/out/Debug/gen/b/c/file.jar.jdeps
-      - filepath = /random/path/file.jar
-        Returns: /cr/src/out/Debug/gen/abs/random/path/file.jar.jdeps
-      """
-    path = self.jar_path.with_suffix(self.jar_path.suffix + self.cache_suffix)
-    if _is_relative_to(path, self.build_output_dir):
-      # already in the outdir, no need to adjust cache path
-      return path
-    if _is_relative_to(self.jar_path, _SRC_PATH):
-      return self.build_output_dir / 'gen' / path.relative_to(_SRC_PATH)
-    return self.build_output_dir / 'gen/abs' / path.relative_to(path.anchor)
-
-  def is_valid(self):
-    return (self.cache_path.exists() and self.jar_path.exists()
-            and self.cache_path.stat().st_mtime > self.jar_path.stat().st_mtime)
-
-  def read(self):
-    with open(self.cache_path) as f:
-      return f.read()
-
-  def write(self, content: str):
-    # If the jar file is in //src but not in the output dir or outside //src
-    # then the reparented dirs within the output dir need to be created first.
-    self.cache_path.parent.mkdir(parents=True, exist_ok=True)
-    with open(self.cache_path, 'w') as f:
-      f.write(content)
-
-
 def _should_ignore(jar_path: pathlib.Path) -> bool:
   for ignored_jar_path in _IGNORED_JAR_PATHS:
     if ignored_jar_path in str(jar_path):
@@ -98,33 +43,14 @@
 
 def run_jdeps(filepath: pathlib.Path,
               *,
-              build_output_dir: pathlib.Path,
-              jdeps_path: pathlib.Path = _JDEPS_PATH,
-              src_path: pathlib.Path = _SRC_PATH) -> Optional[str]:
-  """Runs jdeps on the given filepath and returns the output.
-
-    Uses a simple file cache for the output of jdeps. If the jar file's mtime is
-    older than the jdeps cache then just use the cached content instead.
-    Otherwise jdeps is run again and the output used to update the file cache.
-
-    Tested Nov 2nd, 2022:
-    - With all cache hits, script takes 13 seconds.
-    - Without the cache, script takes 1 minute 14 seconds.
-    """
-  # Some __compile_java targets do not generate a .jar file, skipping these
-  # does not affect correctness.
+              jdeps_path: pathlib.Path = _JDEPS_PATH) -> Optional[str]:
+  """Runs jdeps on the given filepath and returns the output."""
   if not filepath.exists() or _should_ignore(filepath):
+    # Some __compile_java targets do not generate a .jar file, skipping these
+    # does not affect correctness.
     return None
 
-  cache_file = CacheFile(jar_path=filepath,
-                         cache_suffix='.jdeps_cache',
-                         build_output_dir=build_output_dir,
-                         src_dir=src_path)
-  if cache_file.is_valid():
-    return cache_file.read()
-
-  # Cache either doesn't exist or is older than the jar file.
-  output = build_utils.CheckOutput([
+  return build_utils.CheckOutput([
       str(jdeps_path),
       '-verbose:class',
       '--multi-release',  # Some jars support multiple JDK releases.
@@ -132,20 +58,10 @@
       str(filepath),
   ])
 
-  cache_file.write(output)
-  return output
 
-
-def extract_full_class_names_from_jar(build_output_dir: pathlib.Path,
-                                      jar_path: pathlib.Path) -> List[str]:
+def extract_full_class_names_from_jar(
+    jar_path: Union[str, pathlib.Path]) -> List[str]:
   """Returns set of fully qualified class names in passed-in jar."""
-
-  cache_file = CacheFile(jar_path=jar_path,
-                         cache_suffix='.class_name_cache',
-                         build_output_dir=build_output_dir)
-  if cache_file.is_valid():
-    return cache_file.read().splitlines()
-
   out = set()
   with zipfile.ZipFile(jar_path) as z:
     for zip_entry_name in z.namelist():
@@ -161,10 +77,7 @@
         full_java_class = full_java_class[0:dollar_index]
 
       out.add(full_java_class)
-  out = sorted(out)
-
-  cache_file.write('\n'.join(out))
-  return out
+  return sorted(out)
 
 
 def parse_full_java_class(source_path: pathlib.Path) -> str:
diff --git a/tools/android/dependency_analysis/generate_json_dependency_graph.py b/tools/android/dependency_analysis/generate_json_dependency_graph.py
index e5ed265..58251d2 100755
--- a/tools/android/dependency_analysis/generate_json_dependency_graph.py
+++ b/tools/android/dependency_analysis/generate_json_dependency_graph.py
@@ -337,10 +337,7 @@
     jdeps_process_number = math.ceil(multiprocessing.cpu_count() / 2)
     with multiprocessing.Pool(jdeps_process_number) as pool:
         jdeps_outputs = pool.map(
-            functools.partial(jar_utils.run_jdeps,
-                              jdeps_path=jdeps_path,
-                              src_path=src_path,
-                              build_output_dir=args.build_output_dir),
+            functools.partial(jar_utils.run_jdeps, jdeps_path=jdeps_path),
             target_jars.values())
 
     logging.info('Parsing jdeps output...')