From 51b2a8b5eb053fd4d6f2b9d8b1554447d595cca0 Mon Sep 17 00:00:00 2001 From: Jiakai Zhang Date: Mon, 26 Jun 2023 16:47:38 +0100 Subject: [PATCH] Use per-app package list to avoid unnecessary dexpreopt. Starting from aosp/2594905, dexpreopt depends on `$PRODUCT_OUT/product_packages.txt`. When PRODUCT_PACKAGES changes, dexpreopt has to rerun for all apps. This is not ideal. After this change, dexpreopt uses a per-app product_packages.txt that is filtered by the app's dependencies, and it uses `rsync --checksum` to prevent the file's mtime from being changed if the contents don't change. This avoids unnecessary dexpreopt reruns. Bug: 288218403 Test: m Test: Change PRODUCT_PACKAGES and see no dexpreopt reruns. Change-Id: I5788a9ee987dfd0abfd7d91cbcef748452290004 --- dexpreopt/class_loader_context.go | 25 +++++++++++---------- dexpreopt/class_loader_context_test.go | 24 +++++++++++++------- dexpreopt/dexpreopt.go | 2 +- java/app_test.go | 2 +- java/dexpreopt.go | 31 ++++++++++++++++++++++++-- 5 files changed, 60 insertions(+), 24 deletions(-) diff --git a/dexpreopt/class_loader_context.go b/dexpreopt/class_loader_context.go index 6ead589dd..57c7ae851 100644 --- a/dexpreopt/class_loader_context.go +++ b/dexpreopt/class_loader_context.go @@ -310,7 +310,7 @@ func (clcMap ClassLoaderContextMap) addContext(ctx android.ModuleInstallPathCont // Nested class loader context shouldn't have conditional part (it is allowed only at the top level). for ver, _ := range nestedClcMap { if ver != AnySdkVersion { - clcPaths := ComputeClassLoaderContextDependencies(nestedClcMap) + _, clcPaths := ComputeClassLoaderContextDependencies(nestedClcMap) return fmt.Errorf("nested class loader context shouldn't have conditional part: %+v", clcPaths) } } @@ -553,27 +553,28 @@ func validateClassLoaderContextRec(sdkVer int, clcs []*ClassLoaderContext) (bool return true, nil } -// Returns a slice of build paths for all possible dependencies that the class loader context may -// refer to. +// Returns a slice of library names and a slice of build paths for all possible dependencies that +// the class loader context may refer to. // Perform a depth-first preorder traversal of the class loader context tree for each SDK version. -func ComputeClassLoaderContextDependencies(clcMap ClassLoaderContextMap) android.Paths { - var paths android.Paths +func ComputeClassLoaderContextDependencies(clcMap ClassLoaderContextMap) (names []string, paths android.Paths) { for _, clcs := range clcMap { - hostPaths := ComputeClassLoaderContextDependenciesRec(clcs) - paths = append(paths, hostPaths...) + currentNames, currentPaths := ComputeClassLoaderContextDependenciesRec(clcs) + names = append(names, currentNames...) + paths = append(paths, currentPaths...) } - return android.FirstUniquePaths(paths) + return android.FirstUniqueStrings(names), android.FirstUniquePaths(paths) } // Helper function for ComputeClassLoaderContextDependencies() that handles recursion. -func ComputeClassLoaderContextDependenciesRec(clcs []*ClassLoaderContext) android.Paths { - var paths android.Paths +func ComputeClassLoaderContextDependenciesRec(clcs []*ClassLoaderContext) (names []string, paths android.Paths) { for _, clc := range clcs { - subPaths := ComputeClassLoaderContextDependenciesRec(clc.Subcontexts) + subNames, subPaths := ComputeClassLoaderContextDependenciesRec(clc.Subcontexts) + names = append(names, clc.Name) paths = append(paths, clc.Host) + names = append(names, subNames...) paths = append(paths, subPaths...) } - return paths + return names, paths } // Class loader contexts that come from Make via JSON dexpreopt.config. JSON CLC representation is diff --git a/dexpreopt/class_loader_context_test.go b/dexpreopt/class_loader_context_test.go index 39b4652bd..7260abb64 100644 --- a/dexpreopt/class_loader_context_test.go +++ b/dexpreopt/class_loader_context_test.go @@ -97,10 +97,11 @@ func TestCLC(t *testing.T) { fixClassLoaderContext(m) - var havePaths android.Paths + var actualNames []string + var actualPaths android.Paths var haveUsesLibsReq, haveUsesLibsOpt []string if valid && validationError == nil { - havePaths = ComputeClassLoaderContextDependencies(m) + actualNames, actualPaths = ComputeClassLoaderContextDependencies(m) haveUsesLibsReq, haveUsesLibsOpt = m.UsesLibs() } @@ -112,19 +113,26 @@ func TestCLC(t *testing.T) { }) // Test that all expected build paths are gathered. - t.Run("paths", func(t *testing.T) { - wantPaths := []string{ + t.Run("names and paths", func(t *testing.T) { + expectedNames := []string{ + "a'", "a1", "a2", "a3", "android.hidl.base-V1.0-java", "android.hidl.manager-V1.0-java", "b", + "b1", "b2", "b3", "c", "c2", "d", "f", + } + expectedPaths := []string{ "out/soong/android.hidl.manager-V1.0-java.jar", "out/soong/android.hidl.base-V1.0-java.jar", "out/soong/a.jar", "out/soong/b.jar", "out/soong/c.jar", "out/soong/d.jar", "out/soong/a2.jar", "out/soong/b2.jar", "out/soong/c2.jar", "out/soong/a1.jar", "out/soong/b1.jar", "out/soong/f.jar", "out/soong/a3.jar", "out/soong/b3.jar", } - actual := havePaths.Strings() + actualPathsStrs := actualPaths.Strings() // The order does not matter. - sort.Strings(wantPaths) - sort.Strings(actual) - android.AssertArrayString(t, "", wantPaths, actual) + sort.Strings(expectedNames) + sort.Strings(actualNames) + android.AssertArrayString(t, "", expectedNames, actualNames) + sort.Strings(expectedPaths) + sort.Strings(actualPathsStrs) + android.AssertArrayString(t, "", expectedPaths, actualPathsStrs) }) // Test the JSON passed to construct_context.py. diff --git a/dexpreopt/dexpreopt.go b/dexpreopt/dexpreopt.go index 20737e307..f0e988e58 100644 --- a/dexpreopt/dexpreopt.go +++ b/dexpreopt/dexpreopt.go @@ -353,7 +353,7 @@ func dexpreoptCommand(ctx android.BuilderContext, globalSoong *GlobalSoongConfig } // Generate command that saves host and target class loader context in shell variables. - paths := ComputeClassLoaderContextDependencies(module.ClassLoaderContexts) + _, paths := ComputeClassLoaderContextDependencies(module.ClassLoaderContexts) rule.Command(). Text(`eval "$(`).Tool(globalSoong.ConstructContext). Text(` --target-sdk-version ${target_sdk_version}`). diff --git a/java/app_test.go b/java/app_test.go index cf7d17490..0f98416d2 100644 --- a/java/app_test.go +++ b/java/app_test.go @@ -2697,7 +2697,7 @@ func TestUsesLibraries(t *testing.T) { cmd := app.Rule("dexpreopt").RuleParams.Command android.AssertStringDoesContain(t, "dexpreopt app cmd context", cmd, "--context-json=") android.AssertStringDoesContain(t, "dexpreopt app cmd product_packages", cmd, - "--product-packages=out/soong/target/product/test_device/product_packages.txt") + "--product-packages=out/soong/.intermediates/app/android_common/dexpreopt/product_packages.txt") } func TestDexpreoptBcp(t *testing.T) { diff --git a/java/dexpreopt.go b/java/dexpreopt.go index e588c9ae3..998730e08 100644 --- a/java/dexpreopt.go +++ b/java/dexpreopt.go @@ -16,6 +16,7 @@ package java import ( "path/filepath" + "sort" "strings" "android/soong/android" @@ -390,11 +391,37 @@ func (d *dexpreopter) dexpreopt(ctx android.ModuleContext, dexJarFile android.Wr globalSoong := dexpreopt.GetGlobalSoongConfig(ctx) - // "product_packages.txt" is generated by `build/make/core/Makefile`. + // The root "product_packages.txt" is generated by `build/make/core/Makefile`. It contains a list + // of all packages that are installed on the device. We use `grep` to filter the list by the app's + // dependencies to create a per-app list, and use `rsync --checksum` to prevent the file's mtime + // from being changed if the contents don't change. This avoids unnecessary dexpreopt reruns. productPackages := android.PathForModuleInPartitionInstall(ctx, "", "product_packages.txt") + appProductPackages := android.PathForModuleOut(ctx, "dexpreopt", "product_packages.txt") + appProductPackagesStaging := appProductPackages.ReplaceExtension(ctx, "txt.tmp") + clcNames, _ := dexpreopt.ComputeClassLoaderContextDependencies(dexpreoptConfig.ClassLoaderContexts) + sort.Strings(clcNames) // The order needs to be deterministic. + productPackagesRule := android.NewRuleBuilder(pctx, ctx) + if len(clcNames) > 0 { + productPackagesRule.Command(). + Text("grep -F -x"). + FlagForEachArg("-e ", clcNames). + Input(productPackages). + FlagWithOutput("> ", appProductPackagesStaging). + Text("|| true") + } else { + productPackagesRule.Command(). + Text("rm -f").Output(appProductPackagesStaging). + Text("&&"). + Text("touch").Output(appProductPackagesStaging) + } + productPackagesRule.Command(). + Text("rsync --checksum"). + Input(appProductPackagesStaging). + Output(appProductPackages) + productPackagesRule.Restat().Build("product_packages", "dexpreopt product_packages") dexpreoptRule, err := dexpreopt.GenerateDexpreoptRule( - ctx, globalSoong, global, dexpreoptConfig, productPackages) + ctx, globalSoong, global, dexpreoptConfig, appProductPackages) if err != nil { ctx.ModuleErrorf("error generating dexpreopt rule: %s", err.Error()) return