From 895969de075d66b8c11cce7ba663ec91e392d3a3 Mon Sep 17 00:00:00 2001 From: Googler Date: Wed, 2 Aug 2023 13:19:48 -0700 Subject: [PATCH] BEGIN_PUBLIC Add support for android_binary.startup_profiles attribute. END_PUBLIC Add support for android_binary.startup_profiles attribute. This implements the blaze portion of Phase 2 of what is described here: https://docs.google.com/document/d/14ZXSjMK8V1slSCxigGp7B9tCRO_YeXXdNSNSeDzXSxU/edit?usp=sharing In particular, this adds the startup_profiles attribute which enables optimizers to use both the baseline_profiles and startup_profiles to perform optimizations. The wiring is vaguely what's described here: [] RELNOTES: Supports for android_binary.startup_profiles attribute. PiperOrigin-RevId: 553237487 Change-Id: Ie16e4d3aedb49f9e217d0f8475389720a6ee7066 --- .../rules/android/BazelAndroidSemantics.java | 9 ++- .../lib/rules/android/AndroidBinary.java | 48 ++++++++--- .../lib/rules/android/AndroidSemantics.java | 6 +- .../lib/rules/android/ProguardHelper.java | 81 +++++++++++++------ 4 files changed, 108 insertions(+), 36 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/android/BazelAndroidSemantics.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/android/BazelAndroidSemantics.java index e92beee32c1e7c..836d8b36d4d38a 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/android/BazelAndroidSemantics.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/android/BazelAndroidSemantics.java @@ -142,7 +142,14 @@ public Artifact compileBaselineProfile( /* Bazel does not currently support baseline profiles in the final apk. */ @Override - public Artifact mergeBaselineProfiles(RuleContext ruleContext, String baselineProfileDir) { + public Artifact mergeBaselineProfiles( + RuleContext ruleContext, String baselineProfileDir, boolean includeStartupProfiles) { + return null; + } + + /* Bazel does not currently support baseline profiles in the final apk. */ + @Override + public Artifact mergeStartupProfiles(RuleContext ruleContext, String baselineProfileDir) { return null; } } diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinary.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinary.java index 382868680633cd..1aa9b51a1fc9d5 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinary.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinary.java @@ -185,7 +185,12 @@ private static void validateRuleContext(RuleContext ruleContext, AndroidDataCont ruleContext.attributeError( "min_sdk_version", "Target is not permitted to set min_sdk_version"); } - + if (ruleContext.attributes().isAttributeValueExplicitlySpecified("startup_profiles") + && Allowlist.hasAllowlist(ruleContext, "allow_baseline_profiles_optimizer_integration") + && !Allowlist.isAvailable(ruleContext, "allow_baseline_profiles_optimizer_integration")) { + ruleContext.attributeError( + "startup_profiles", "Target is not permitted to use startup_profiles."); + } if (ruleContext.getFragment(JavaConfiguration.class).enforceProguardFileExtension() && ruleContext.attributes().has(ProguardHelper.PROGUARD_SPECS)) { List pathsWithUnexpectedExtension = @@ -534,14 +539,24 @@ public static RuleConfiguredTargetBuilder createAndroidBinary( BaselineProfileProvider baselineprofileProvider = ruleContext.getPrerequisite("application_resources", BaselineProfileProvider.PROVIDER); + Artifact startupProfile = null; Artifact baselineProfile = null; String baselineProfileDir = ruleContext.getLabel().getName() + "-baseline-profile/"; if (baselineprofileProvider == null && Allowlist.hasAllowlist(ruleContext, "allow_baseline_profiles_optimizer_integration") && Allowlist.isAvailable(ruleContext, "allow_baseline_profiles_optimizer_integration")) { - baselineProfile = androidSemantics.mergeBaselineProfiles(ruleContext, baselineProfileDir); + if (!proguardSpecs.isEmpty()) { + // This is only needed for optimized builds since otherwise the dexer doesn't process this. + startupProfile = androidSemantics.mergeStartupProfiles(ruleContext, baselineProfileDir); + } + baselineProfile = + androidSemantics.mergeBaselineProfiles( + ruleContext, + baselineProfileDir, + // Include startup profiles if the optimizer is disabled since profiles won't be + // merged in the optimizer. + proguardSpecs.isEmpty()); } - ProguardOutput proguardOutput = applyProguard( ruleContext, @@ -551,6 +566,7 @@ public static RuleConfiguredTargetBuilder createAndroidBinary( proguardSpecs, proguardMapping, proguardOutputMap, + startupProfile, baselineProfile, baselineProfileDir); @@ -611,6 +627,7 @@ public static RuleConfiguredTargetBuilder createAndroidBinary( proguardOutputMap, postProcessingOutputMap, proguardOutput.getLibraryJar(), + proguardOutput.getStartupProfileRewritten(), !proguardSpecs.isEmpty()); DexPostprocessingOutput dexPostprocessingOutput = @@ -627,7 +644,7 @@ public static RuleConfiguredTargetBuilder createAndroidBinary( // Compute the final DEX files by appending Java 8 legacy .dex if used. final Artifact finalClassesDex; - Java8LegacyDexOutput java8LegacyDexOutput; + Java8LegacyDexOutput java8LegacyDexOutput = null; ImmutableList finalShardDexZips = dexingOutput.shardDexZips; if (androidDexInfo != null) { finalClassesDex = androidDexInfo.getFinalClassesDexZip(); @@ -699,7 +716,7 @@ public static RuleConfiguredTargetBuilder createAndroidBinary( if (baselineprofileProvider != null) { // This happens when baseline profiles are provided via starlark. artProfileZip = baselineprofileProvider.getArtProfileZip(); - } else if (baselineProfile == null) { + } else if (baselineProfile == null && startupProfile == null) { // This happens when optimizer profile rewriting isn't enabled. artProfileZip = androidSemantics.getArtProfileForApk( @@ -710,11 +727,13 @@ public static RuleConfiguredTargetBuilder createAndroidBinary( androidSemantics.compileBaselineProfile( ruleContext, finalClassesDex, - // Minified symbols are emitted when rewriting, so map isn't needed. - /* proguardOutputMap= */ null, - proguardOutput.getBaselineProfileOut() == null + // Minified symbols are emitted when rewriting, so only use map for symbols which + // weren't passed to bytecode optimizer (if it exists). + java8LegacyDexOutput == null ? null : java8LegacyDexOutput.getMap(), + // At this point, either baseline profile here also contains startup-profiles, if any. + proguardOutput.getMergedBaselineProfileRewritten() == null ? baselineProfile - : proguardOutput.getBaselineProfileOut(), + : proguardOutput.getMergedBaselineProfileRewritten(), baselineProfileDir); } @@ -1083,6 +1102,7 @@ private static ProguardOutput applyProguard( ImmutableList proguardSpecs, Artifact proguardMapping, @Nullable Artifact proguardOutputMap, + @Nullable Artifact startupProfile, @Nullable Artifact baselineProfile, String baselineProfileDir) throws InterruptedException { @@ -1132,6 +1152,7 @@ private static ProguardOutput applyProguard( javaSemantics, getProguardOptimizationPasses(ruleContext), proguardOutputMap, + startupProfile, baselineProfile, baselineProfileDir); } @@ -1164,7 +1185,8 @@ private static ProguardOutput createEmptyProguardAction( semantics, proguardOutputMap, /* libraryJar= */ null, - /* baselineProfileOutput= */ null); + /* startupProfileRewritten= */ null, + /* mergedBaselineProfileRewritten= */ null); outputs.addAllToSet(failures); ruleContext.registerAction( new FailAction( @@ -1361,6 +1383,7 @@ private static DexingOutput dex( @Nullable Artifact proguardOutputMap, @Nullable Artifact postProcessingOutputMap, @Nullable Artifact libraryJar, + @Nullable Artifact startupProfile, boolean isOptimizedBuild) throws InterruptedException, RuleErrorException { FilesToRunProvider optimizingDexer = ruleContext.getExecutablePrerequisite(":optimizing_dexer"); @@ -1439,7 +1462,10 @@ private static DexingOutput dex( .addExecPath("--pg-map", proguardOutputMap) .addExecPath("--pg-map-output", postProcessingOutputMap); } - + if (startupProfile != null && nativeMultidex) { + dexAction.addInput(startupProfile); + dexCommand.addExecPath("--startup-profile", startupProfile); + } // TODO(b/261110876): Pass min SDK through here based on the value in the merged manifest. The // current value is statically defined for the entire depot. // We currently set the minimum SDK version to 21 if you are doing native multidex as that is diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidSemantics.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidSemantics.java index fe0bdf8c1928d8..dee3a84ff65c49 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidSemantics.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidSemantics.java @@ -119,7 +119,11 @@ Artifact getArtProfileForApk( String baselineProfileDir); /** The merged baseline profiles from the {@code baseline_profiles} attribute. */ - Artifact mergeBaselineProfiles(RuleContext ruleContext, String baselineProfileDir); + Artifact mergeBaselineProfiles( + RuleContext ruleContext, String baselineProfileDir, boolean includeStartupProfiles); + + /** The merged startup profiles from the {@code startup_profiles} attribute. */ + Artifact mergeStartupProfiles(RuleContext ruleContext, String baselineProfileDir); /** The artifact for ART profile information, given a particular merged profile. */ Artifact compileBaselineProfile( diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/ProguardHelper.java b/src/main/java/com/google/devtools/build/lib/rules/android/ProguardHelper.java index 08388e2888280f..1fbcd9b92604df 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/ProguardHelper.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/ProguardHelper.java @@ -63,7 +63,8 @@ public static final class ProguardOutput { @Nullable private final Artifact constantStringObfuscatedMapping; @Nullable private final Artifact libraryJar; private final Artifact config; - @Nullable private final Artifact baselineProfileOut; + @Nullable private final Artifact startupProfileRewritten; + @Nullable private final Artifact mergedBaselineProfileRewritten; public ProguardOutput( Artifact outputJar, @@ -74,7 +75,8 @@ public ProguardOutput( @Nullable Artifact constantStringObfuscatedMapping, @Nullable Artifact libraryJar, Artifact config, - @Nullable Artifact baselineProfileOut) { + @Nullable Artifact startupProfileRewritten, + @Nullable Artifact mergedBaselineProfileRewritten) { this.outputJar = checkNotNull(outputJar); this.mapping = mapping; this.protoMapping = protoMapping; @@ -83,11 +85,12 @@ public ProguardOutput( this.constantStringObfuscatedMapping = constantStringObfuscatedMapping; this.libraryJar = libraryJar; this.config = config; - this.baselineProfileOut = baselineProfileOut; + this.startupProfileRewritten = startupProfileRewritten; + this.mergedBaselineProfileRewritten = mergedBaselineProfileRewritten; } public static ProguardOutput createEmpty(Artifact outputJar) { - return new ProguardOutput(outputJar, null, null, null, null, null, null, null, null); + return new ProguardOutput(outputJar, null, null, null, null, null, null, null, null, null); } public Artifact getOutputJar() { @@ -132,8 +135,12 @@ public Artifact getConfig() { return config; } - public Artifact getBaselineProfileOut() { - return baselineProfileOut; + public Artifact getMergedBaselineProfileRewritten() { + return mergedBaselineProfileRewritten; + } + + public Artifact getStartupProfileRewritten() { + return startupProfileRewritten; } /** Adds the output artifacts to the given set builder. */ @@ -264,7 +271,8 @@ public static ProguardOutput getProguardOutputs( JavaSemantics semantics, @Nullable Artifact proguardOutputMap, @Nullable Artifact libraryJar, - @Nullable Artifact baselineProfileOutput) + @Nullable Artifact startupProfileRewritten, + @Nullable Artifact mergedBaselineProfileRewritten) throws InterruptedException { boolean mappingRequested = genProguardMapping(ruleContext.attributes()); @@ -293,7 +301,8 @@ public static ProguardOutput getProguardOutputs( proguardConstantStringMap, libraryJar, proguardConfigOutput, - baselineProfileOutput); + startupProfileRewritten, + mergedBaselineProfileRewritten); } /** @@ -313,8 +322,10 @@ public static ProguardOutput getProguardOutputs( * @param optimizationPasses if not null specifies to break proguard up into multiple passes with * the given number of optimization passes. * @param proguardOutputMap mapping generated by Proguard if requested. could be null. - * @param baselineProfileIn Profile to pass the optimizer to perform feedback-directed - * optimization. + * @param startupProfileIn Profiles provided by the startup_profiles attribute. Their existence + * enables feedback directed optimization in optimization tools. These are rewritten by the + * optimizer. + * @param baselineProfileIn Profile to pass the optimizer to rewrite through optimization. * @param baselineProfileDir Directory to write baseline profile artifacts if baseline profile is * provided. */ @@ -332,6 +343,7 @@ public static ProguardOutput createOptimizationActions( JavaSemantics semantics, @Nullable Integer optimizationPasses, @Nullable Artifact proguardOutputMap, + @Nullable Artifact startupProfileIn, @Nullable Artifact baselineProfileIn, String baselineProfileDir) throws InterruptedException { @@ -373,10 +385,14 @@ public static ProguardOutput createOptimizationActions( libraryJars = NestedSetBuilder.create(Order.STABLE_ORDER, filteredLibraryJar); libraryJar = filteredLibraryJar; } - Artifact baselineProfileOut = null; - if (baselineProfileIn != null) { - baselineProfileOut = ruleContext.getBinArtifact(baselineProfileDir + "rewritten-prof.txt"); - } + Artifact startupProfileRewritten = + startupProfileIn == null + ? null + : ruleContext.getBinArtifact(baselineProfileDir + "rewritten-startup-prof.txt"); + Artifact baselineProfileRewritten = + baselineProfileIn == null && startupProfileIn == null + ? null + : ruleContext.getBinArtifact(baselineProfileDir + "rewritten-merged-prof.txt"); ProguardOutput output = getProguardOutputs( proguardOutputJar, @@ -386,7 +402,8 @@ public static ProguardOutput createOptimizationActions( semantics, proguardOutputMap, libraryJar, - baselineProfileOut); + startupProfileRewritten, + baselineProfileRewritten); JavaConfiguration javaConfiguration = ruleContext.getConfiguration().getFragment(JavaConfiguration.class); @@ -412,8 +429,10 @@ public static ProguardOutput createOptimizationActions( output.getUsage(), output.getConstantStringObfuscatedMapping(), output.getConfig(), + startupProfileIn, + startupProfileRewritten, baselineProfileIn, - baselineProfileOut, + baselineProfileRewritten, mnemonic); proguardAction .setProgressMessage("Trimming binary with %s: %s", mnemonic, ruleContext.getLabel()) @@ -455,8 +474,10 @@ public static ProguardOutput createOptimizationActions( /* proguardUsage */ null, /* constantStringObfuscatedMapping */ null, /* proguardConfigOutput */ null, + startupProfileIn, + /* startupProfileRewritten */ null, baselineProfileIn, - /* baselineProfileOutput */ null, + /* baselineProfileRewritten */ null, mnemonic); initialAction .setProgressMessage("Trimming binary with %s: Verification/Shrinking Pass", mnemonic) @@ -537,8 +558,10 @@ public static ProguardOutput createOptimizationActions( output.getUsage(), output.getConstantStringObfuscatedMapping(), output.getConfig(), + /* startupProfileInput */ null, + startupProfileRewritten, /* baselineProfileInput */ null, - baselineProfileOut, + baselineProfileRewritten, mnemonic); finalAction .setProgressMessage( @@ -600,8 +623,10 @@ private static Artifact createSingleOptimizationAction( /* proguardUsage */ null, /* constantStringObfuscatedMapping */ null, /* proguardConfigOutput */ null, + /* startupProfileInput */ null, + /* startupProfileRewritten */ null, /* baselineProfileInput */ null, - /* baselineProfileOutput */ null, + /* baselineProfileRewritten */ null, mnemonic); optimizationAction .setProgressMessage( @@ -635,8 +660,10 @@ private static void defaultAction( @Nullable Artifact proguardUsage, @Nullable Artifact constantStringObfuscatedMapping, @Nullable Artifact proguardConfigOutput, + @Nullable Artifact startupProfileInput, + @Nullable Artifact startupProfileRewritten, @Nullable Artifact baselineProfileInput, - @Nullable Artifact baselineProfileOutput, + @Nullable Artifact baselineProfileRewritten, String mnemonic) { builder @@ -708,13 +735,21 @@ private static void defaultAction( builder.addOutput(proguardConfigOutput); commandLine.addExecPath("-printconfiguration", proguardConfigOutput); } + if (startupProfileInput != null) { + builder.addInput(startupProfileInput); + commandLine.addExecPath("-startupprofile", startupProfileInput); + } + if (startupProfileRewritten != null) { + builder.addOutput(startupProfileRewritten); + commandLine.addExecPath("-printstartupprofile", startupProfileRewritten); + } if (baselineProfileInput != null) { builder.addInput(baselineProfileInput); commandLine.addExecPath("-baselineprofile", baselineProfileInput); } - if (baselineProfileOutput != null) { - builder.addOutput(baselineProfileOutput); - commandLine.addExecPath("-printbaselineprofile", baselineProfileOutput); + if (baselineProfileRewritten != null) { + builder.addOutput(baselineProfileRewritten); + commandLine.addExecPath("-printbaselineprofile", baselineProfileRewritten); } }