Merge "cosmetic: warmup runs made clearer" am: 3a7126408b

Original change: https://android-review.googlesource.com/c/platform/build/bazel/+/2537137

Change-Id: I7434d26ffafb266cc73fcd7d65eee65187180933
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>

2 files changed, 100 insertions, 101 deletions

diff --git a/scripts/incremental_build/cuj_catalog.py b/scripts/incremental_build/cuj_catalog.py
index e2d4868e..6db3c8d8 100644
--- a/scripts/incremental_build/cuj_catalog.py
+++ b/scripts/incremental_build/cuj_catalog.py
@@ -25,6 +25,7 @@ import uuid
 from enum import Enum
 from pathlib import Path
 from typing import Callable, Optional
+from typing import Final
 from typing import TypeAlias
 
 import util
@@ -106,8 +107,12 @@ class CujGroup:
     if len(self.steps) < 2:
       return f'{self.steps[0].verb} {self.description}'.strip()
     return ' '.join(
-      [f'({chr(ord("a") + i)}) {step.verb} {self.description}'.strip() for
-       i, step in enumerate(self.steps)])
+        [f'({chr(ord("a") + i)}) {step.verb} {self.description}'.strip() for
+         i, step in enumerate(self.steps)])
+
+
+Warmup: Final[CujGroup] = CujGroup('WARMUP',
+                                   [CujStep('no change', lambda: None)])
 
 
 class InWorkspace(Enum):
@@ -122,7 +127,7 @@ class InWorkspace(Enum):
   @staticmethod
   def ws_counterpart(src_path: Path) -> Path:
     return util.get_out_dir().joinpath('soong/workspace').joinpath(
-      de_src(src_path))
+        de_src(src_path))
 
   def verifier(self, src_path: Path) -> Verifier:
     @skip_when_soong_only
@@ -146,7 +151,7 @@ class InWorkspace(Enum):
 
       if self != actual:
         raise AssertionError(
-          f'{ws_path} expected {self.name} but got {actual.name}')
+            f'{ws_path} expected {self.name} but got {actual.name}')
       logging.info(f'VERIFIED {de_src(ws_path)} {self.name}')
 
     return f
@@ -181,8 +186,8 @@ def modify_revert(file: Path, text: str = '//BOGUS line\n') -> CujGroup:
       f.truncate()
 
   return CujGroup(de_src(file), [
-    CujStep('modify', add_line),
-    CujStep('revert', revert)
+      CujStep('modify', add_line),
+      CujStep('revert', revert)
   ])
 
 
@@ -202,8 +207,8 @@ def create_delete(file: Path, ws: InWorkspace,
   def create():
     if file.exists():
       raise RuntimeError(
-        f'File {file} already exists. Interrupted an earlier run?\n'
-        'TIP: `repo status` and revert changes!!!')
+          f'File {file} already exists. Interrupted an earlier run?\n'
+          'TIP: `repo status` and revert changes!!!')
     file.parent.mkdir(parents=True, exist_ok=True)
     file.touch(exist_ok=False)
     with open(file, mode="w") as f:
@@ -216,8 +221,8 @@ def create_delete(file: Path, ws: InWorkspace,
       file.unlink(missing_ok=False)
 
   return CujGroup(de_src(file), [
-    CujStep('create', create, ws.verifier(file)),
-    CujStep('delete', delete, InWorkspace.OMISSION.verifier(file)),
+      CujStep('create', create, ws.verifier(file)),
+      CujStep('delete', delete, InWorkspace.OMISSION.verifier(file)),
   ])
 
 
@@ -227,8 +232,8 @@ def create_delete_bp(bp_file: Path) -> CujGroup:
   an Android.bp file.
   """
   return create_delete(
-    bp_file, InWorkspace.SYMLINK,
-    'filegroup { name: "test-bogus-filegroup", srcs: ["**/*.md"] }')
+      bp_file, InWorkspace.SYMLINK,
+      'filegroup { name: "test-bogus-filegroup", srcs: ["**/*.md"] }')
 
 
 def delete_restore(original: Path, ws: InWorkspace) -> CujGroup:
@@ -252,12 +257,12 @@ def delete_restore(original: Path, ws: InWorkspace) -> CujGroup:
     original.rename(copied)
 
   return CujGroup(de_src(original), [
-    CujStep('delete',
-            move_to_tempdir_to_mimic_deletion,
-            InWorkspace.OMISSION.verifier(original)),
-    CujStep('restore',
-            lambda: copied.rename(original),
-            ws.verifier(original))
+      CujStep('delete',
+              move_to_tempdir_to_mimic_deletion,
+              InWorkspace.OMISSION.verifier(original)),
+      CujStep('restore',
+              lambda: copied.rename(original),
+              ws.verifier(original))
   ])
 
 
@@ -274,7 +279,7 @@ def replace_link_with_dir(p: Path):
   create_dir: CujStep
   delete_dir: CujStep
   create_dir, delete_dir, *tail = create_delete_bp(
-    p.joinpath('Android.bp')).steps
+      p.joinpath('Android.bp')).steps
   assert len(tail) == 0
 
   def replace_it():
@@ -282,11 +287,11 @@ def replace_link_with_dir(p: Path):
     create_dir.apply_change()
 
   return CujGroup(cd.description, [
-    create_file,
-    CujStep(f'{de_src(p)}/Android.bp instead of',
-            replace_it,
-            create_dir.verify),
-    delete_dir
+      create_file,
+      CujStep(f'{de_src(p)}/Android.bp instead of',
+              replace_it,
+              create_dir.verify),
+      delete_dir
   ])
 
 
@@ -311,14 +316,14 @@ def content_verfiers(
   def contains():
     if not search():
       raise AssertionError(
-        f'{de_src(ws_build_file)} expected to contain {content}')
+          f'{de_src(ws_build_file)} expected to contain {content}')
     logging.info(f'VERIFIED {de_src(ws_build_file)} contains {content}')
 
   @skip_when_soong_only
   def does_not_contain():
     if search():
       raise AssertionError(
-        f'{de_src(ws_build_file)} not expected to contain {content}')
+          f'{de_src(ws_build_file)} not expected to contain {content}')
     logging.info(f'VERIFIED {de_src(ws_build_file)} does not contain {content}')
 
   return contains, does_not_contain
@@ -329,22 +334,22 @@ def modify_revert_kept_build_file(build_file: Path) -> CujGroup:
   step1, step2, *tail = modify_revert(build_file, content).steps
   assert len(tail) == 0
   ws_build_file = InWorkspace.ws_counterpart(build_file).with_name(
-    'BUILD.bazel')
+      'BUILD.bazel')
   merge_prover, merge_disprover = content_verfiers(ws_build_file, content)
   return CujGroup(de_src(build_file), [
-    CujStep(step1.verb,
-            step1.apply_change,
-            _sequence(step1.verify, merge_prover)),
-    CujStep(step2.verb,
-            step2.apply_change,
-            _sequence(step2.verify, merge_disprover))
+      CujStep(step1.verb,
+              step1.apply_change,
+              _sequence(step1.verify, merge_prover)),
+      CujStep(step2.verb,
+              step2.apply_change,
+              _sequence(step2.verify, merge_disprover))
   ])
 
 
 def create_delete_kept_build_file(build_file: Path) -> CujGroup:
   content = f'//BOGUS {uuid.uuid4()}\n'
   ws_build_file = InWorkspace.ws_counterpart(build_file).with_name(
-    'BUILD.bazel')
+      'BUILD.bazel')
   if build_file.name == 'BUILD.bazel':
     ws = InWorkspace.NOT_UNDER_SYMLINK
   elif build_file.name == 'BUILD':
@@ -359,32 +364,32 @@ def create_delete_kept_build_file(build_file: Path) -> CujGroup:
   step1, step2, *tail = create_delete(build_file, ws, content).steps
   assert len(tail) == 0
   return CujGroup(de_src(build_file), [
-    CujStep(step1.verb,
-            step1.apply_change,
-            _sequence(step1.verify, merge_prover)),
-    CujStep(step2.verb,
-            step2.apply_change,
-            _sequence(step2.verify, merge_disprover))
+      CujStep(step1.verb,
+              step1.apply_change,
+              _sequence(step1.verify, merge_prover)),
+      CujStep(step2.verb,
+              step2.apply_change,
+              _sequence(step2.verify, merge_disprover))
   ])
 
 
 def create_delete_unkept_build_file(build_file) -> CujGroup:
   content = f'//BOGUS {uuid.uuid4()}\n'
   ws_build_file = InWorkspace.ws_counterpart(build_file).with_name(
-    'BUILD.bazel')
+      'BUILD.bazel')
   step1: CujStep
   step2: CujStep
   step1, step2, *tail = create_delete(
-    build_file, InWorkspace.SYMLINK, content).steps
+      build_file, InWorkspace.SYMLINK, content).steps
   assert len(tail) == 0
   _, merge_disprover = content_verfiers(ws_build_file, content)
   return CujGroup(de_src(build_file), [
-    CujStep(step1.verb,
-            step1.apply_change,
-            _sequence(step1.verify, merge_disprover)),
-    CujStep(step2.verb,
-            step2.apply_change,
-            _sequence(step2.verify, merge_disprover))
+      CujStep(step1.verb,
+              step1.apply_change,
+              _sequence(step1.verify, merge_disprover)),
+      CujStep(step2.verb,
+              step2.apply_change,
+              _sequence(step2.verify, merge_disprover))
   ])
 
 
@@ -407,9 +412,9 @@ def _kept_build_cujs() -> list[CujGroup]:
               pkg.joinpath('BUILD.bazel')]
 
   return [
-    *[create_delete_kept_build_file(build_file) for build_file in examples],
-    create_delete(pkg.joinpath('BUILD/kept-dir'), InWorkspace.SYMLINK),
-    modify_revert_kept_build_file(util.any_file_under(kept, 'BUILD'))]
+      *[create_delete_kept_build_file(build_file) for build_file in examples],
+      create_delete(pkg.joinpath('BUILD/kept-dir'), InWorkspace.SYMLINK),
+      modify_revert_kept_build_file(util.any_file_under(kept, 'BUILD'))]
 
 
 def _unkept_build_cujs() -> list[CujGroup]:
@@ -417,15 +422,15 @@ def _unkept_build_cujs() -> list[CujGroup]:
   unkept = src('bionic')
   pkg = util.any_dir_under(unkept, *PKG)
   return [
-    *[create_delete_unkept_build_file(build_file) for build_file in [
-      pkg.joinpath('BUILD'),
-      pkg.joinpath('BUILD.bazel'),
-    ]],
-    *[create_delete(build_file, InWorkspace.OMISSION) for build_file in [
-      unkept.joinpath('bogus-unkept/BUILD'),
-      unkept.joinpath('bogus-unkept/BUILD.bazel'),
-    ]],
-    create_delete(pkg.joinpath('BUILD/unkept-dir'), InWorkspace.SYMLINK)
+      *[create_delete_unkept_build_file(build_file) for build_file in [
+          pkg.joinpath('BUILD'),
+          pkg.joinpath('BUILD.bazel'),
+      ]],
+      *[create_delete(build_file, InWorkspace.OMISSION) for build_file in [
+          unkept.joinpath('bogus-unkept/BUILD'),
+          unkept.joinpath('bogus-unkept/BUILD.bazel'),
+      ]],
+      create_delete(pkg.joinpath('BUILD/unkept-dir'), InWorkspace.SYMLINK)
   ]
 
 
@@ -447,52 +452,48 @@ def get_cujgroups() -> list[CujGroup]:
   '''))
 
   android_bp_cujs = [
-    modify_revert(src('Android.bp')),
-    *[create_delete_bp(d.joinpath('Android.bp')) for d in
-      [ancestor, pkg_free, leaf_pkg_free]]
+      modify_revert(src('Android.bp')),
+      *[create_delete_bp(d.joinpath('Android.bp')) for d in
+        [ancestor, pkg_free, leaf_pkg_free]]
   ]
   mixed_build_launch_cujs = [
-    modify_revert(src('bionic/libc/tzcode/asctime.c')),
-    modify_revert(src('bionic/libc/stdio/stdio.cpp')),
-    modify_revert(src('packages/modules/adb/daemon/main.cpp')),
-    modify_revert(src('frameworks/base/core/java/android/view/View.java')),
+      modify_revert(src('bionic/libc/tzcode/asctime.c')),
+      modify_revert(src('bionic/libc/stdio/stdio.cpp')),
+      modify_revert(src('packages/modules/adb/daemon/main.cpp')),
+      modify_revert(src('frameworks/base/core/java/android/view/View.java')),
   ]
   unreferenced_file_cujs = [
-    *[create_delete(d.joinpath('unreferenced.txt'), InWorkspace.SYMLINK) for
-      d in [ancestor, pkg]],
-    *[create_delete(d.joinpath('unreferenced.txt'), InWorkspace.UNDER_SYMLINK)
-      for d
-      in [pkg_free, leaf_pkg_free]]
+      *[create_delete(d.joinpath('unreferenced.txt'), InWorkspace.SYMLINK) for
+        d in [ancestor, pkg]],
+      *[create_delete(d.joinpath('unreferenced.txt'), InWorkspace.UNDER_SYMLINK)
+        for d
+        in [pkg_free, leaf_pkg_free]]
   ]
 
   def clean():
     if ui.get_user_input().log_dir.is_relative_to(util.get_top_dir()):
       raise AssertionError(
-        f'specify a different LOG_DIR: {ui.get_user_input().log_dir}')
+          f'specify a different LOG_DIR: {ui.get_user_input().log_dir}')
     if util.get_out_dir().exists():
       shutil.rmtree(util.get_out_dir())
 
   return [
-    CujGroup('', [CujStep('clean', clean)]),
-    CujGroup('', [CujStep('no change', lambda: None)]),
-
-    create_delete(src('bionic/libc/tzcode/globbed.c'),
-                  InWorkspace.UNDER_SYMLINK),
-
-    # TODO (usta): find targets that should be affected
-    *[delete_restore(f, InWorkspace.SYMLINK) for f in [
-      util.any_file('version_script.txt'),
-      util.any_file('AndroidManifest.xml')]],
-
-    *unreferenced_file_cujs,
-    *mixed_build_launch_cujs,
-    *android_bp_cujs,
-    *_unkept_build_cujs(),
-    *_kept_build_cujs(),
-    replace_link_with_dir(pkg.joinpath('bogus.txt')),
-    # TODO(usta): add a dangling symlink
+      CujGroup('', [CujStep('clean', clean)]),
+      Warmup,
+
+      create_delete(src('bionic/libc/tzcode/globbed.c'),
+                    InWorkspace.UNDER_SYMLINK),
+
+      # TODO (usta): find targets that should be affected
+      *[delete_restore(f, InWorkspace.SYMLINK) for f in [
+          util.any_file('version_script.txt'),
+          util.any_file('AndroidManifest.xml')]],
+
+      *unreferenced_file_cujs,
+      *mixed_build_launch_cujs,
+      *android_bp_cujs,
+      *_unkept_build_cujs(),
+      *_kept_build_cujs(),
+      replace_link_with_dir(pkg.joinpath('bogus.txt')),
+      # TODO(usta): add a dangling symlink
   ]
-
-
-def warmup_index() -> int:
-  return 1
diff --git a/scripts/incremental_build/incremental_build.py b/scripts/incremental_build/incremental_build.py
index b95d3a84..b43e0fb1 100644
--- a/scripts/incremental_build/incremental_build.py
+++ b/scripts/incremental_build/incremental_build.py
@@ -32,9 +32,9 @@ from typing import Mapping
 
 import cuj_catalog
 import perf_metrics
+import pretty
 import ui
 import util
-import pretty
 
 MAX_RUN_COUNT: int = 5
 
@@ -213,13 +213,11 @@ def main():
 
   run_dir_gen = util.next_path(user_input.log_dir.joinpath(util.RUN_DIR_PREFIX))
 
-  def run_cuj_group(cuj_group: cuj_catalog.CujGroup, is_warmup: bool):
+  def run_cuj_group(cuj_group: cuj_catalog.CujGroup):
     for cujstep in cuj_group.steps:
       desc = cujstep.verb
       desc = f'{desc} {cuj_group.description}'.strip()
       desc = f'{desc} {user_input.description}'.strip()
-      if is_warmup:
-        desc = f'WARMUP {desc}'
       logging.info('START %s %s [%s]', build_type.name,
                    ' '.join(user_input.targets), desc)
       cujstep.apply_change()
@@ -234,9 +232,9 @@ def main():
 
   for build_type in user_input.build_types:
     # warm-up run reduces variations attributable to OS caches
-    run_cuj_group(cuj_catalog.get_cujgroups()[cuj_catalog.warmup_index()], True)
+    run_cuj_group(cuj_catalog.Warmup)
     for i in user_input.chosen_cujgroups:
-      run_cuj_group(cuj_catalog.get_cujgroups()[i], False)
+      run_cuj_group(cuj_catalog.get_cujgroups()[i])
 
   perf_metrics.tabulate_metrics_csv(user_input.log_dir)
   perf_metrics.display_tabulated_metrics(user_input.log_dir)