Browse Source

Use hashed subdir for soong_config modules

This is to differentiate soong intermediate directories for soong config
modules. This will help incremental build across different
devices.

Test result of building panther, building cheetah, and building panther
again:

Before this change
- build time: 02:57
- # of tasks: 31044

After this change
- build time: 01:48
- # of tasks: 1694

Build time includes build.ninja generating time (which is more than 1
minute), so the overriden artifacts become far fewer.

And "NINJA_ARGS='-n -d explain' m" only gave 4 "command line changed"
nodes.

Bug: 279362051
Test: see above
Change-Id: I4891cbe823ae21628465e5c6eb26a4837ccdd202
Inseob Kim 11 months ago
parent
commit
81b00a8db7
5 changed files with 355 additions and 5 deletions
  1. 6 0
      README.md
  2. 89 0
      android/module.go
  3. 5 1
      android/paths.go
  4. 55 2
      android/soong_config_modules.go
  5. 200 2
      android/soong_config_modules_test.go

+ 6 - 0
README.md

@@ -565,6 +565,12 @@ modules (`cc_defaults`, `java_defaults`, etc.), which can then be referenced
 by all of the vendor's other modules using the normal namespace and visibility
 rules.
 
+`soongConfigTraceMutator` enables modules affected by soong config variables to
+write outputs into a hashed directory path. It does this by recording accesses
+to soong config variables on each module, and then accumulating records of each
+module's all dependencies. `m soong_config_trace` builds information about
+hashes to `$OUT_DIR/soong/soong_config_trace.json`.
+
 ## Build logic
 
 The build logic is written in Go using the

+ 89 - 0
android/module.go

@@ -15,6 +15,9 @@
 package android
 
 import (
+	"crypto/md5"
+	"encoding/hex"
+	"encoding/json"
 	"fmt"
 	"net/url"
 	"os"
@@ -714,6 +717,31 @@ func SortedUniqueNamedPaths(l NamedPaths) NamedPaths {
 	return l[:k+1]
 }
 
+// soongConfigTrace holds all references to VendorVars. Uses []string for blueprint:"mutated"
+type soongConfigTrace struct {
+	Bools   []string `json:",omitempty"`
+	Strings []string `json:",omitempty"`
+	IsSets  []string `json:",omitempty"`
+}
+
+func (c *soongConfigTrace) isEmpty() bool {
+	return len(c.Bools) == 0 && len(c.Strings) == 0 && len(c.IsSets) == 0
+}
+
+// Returns hash of serialized trace records (empty string if there's no trace recorded)
+func (c *soongConfigTrace) hash() string {
+	// Use MD5 for speed. We don't care collision or preimage attack
+	if c.isEmpty() {
+		return ""
+	}
+	j, err := json.Marshal(c)
+	if err != nil {
+		panic(fmt.Errorf("json marshal of %#v failed: %#v", *c, err))
+	}
+	hash := md5.Sum(j)
+	return hex.EncodeToString(hash[:])
+}
+
 type nameProperties struct {
 	// The name of the module.  Must be unique across all modules.
 	Name *string
@@ -958,6 +986,10 @@ type commonProperties struct {
 
 	// Bazel conversion status
 	BazelConversionStatus BazelConversionStatus `blueprint:"mutated"`
+
+	// SoongConfigTrace records accesses to VendorVars (soong_config)
+	SoongConfigTrace     soongConfigTrace `blueprint:"mutated"`
+	SoongConfigTraceHash string           `blueprint:"mutated"`
 }
 
 // CommonAttributes represents the common Bazel attributes from which properties
@@ -3160,6 +3192,10 @@ func (m *moduleContext) ModuleSubDir() string {
 	return m.bp.ModuleSubDir()
 }
 
+func (m *moduleContext) ModuleSoongConfigHash() string {
+	return m.module.base().commonProperties.SoongConfigTraceHash
+}
+
 func (b *baseModuleContext) Target() Target {
 	return b.target
 }
@@ -3744,6 +3780,8 @@ func (m *moduleContext) TargetRequiredModuleNames() []string {
 
 func init() {
 	RegisterParallelSingletonType("buildtarget", BuildTargetSingleton)
+	RegisterParallelSingletonType("soongconfigtrace", soongConfigTraceSingletonFunc)
+	FinalDepsMutators(registerSoongConfigTraceMutator)
 }
 
 func BuildTargetSingleton() Singleton {
@@ -3925,3 +3963,54 @@ func (d *installPathsDepSet) ToList() InstallPaths {
 	}
 	return d.depSet.ToList().(InstallPaths)
 }
+
+func registerSoongConfigTraceMutator(ctx RegisterMutatorsContext) {
+	ctx.BottomUp("soongconfigtrace", soongConfigTraceMutator).Parallel()
+}
+
+// soongConfigTraceMutator accumulates recorded soong_config trace from children. Also it normalizes
+// SoongConfigTrace to make it consistent.
+func soongConfigTraceMutator(ctx BottomUpMutatorContext) {
+	trace := &ctx.Module().base().commonProperties.SoongConfigTrace
+	ctx.VisitDirectDeps(func(m Module) {
+		childTrace := &m.base().commonProperties.SoongConfigTrace
+		trace.Bools = append(trace.Bools, childTrace.Bools...)
+		trace.Strings = append(trace.Strings, childTrace.Strings...)
+		trace.IsSets = append(trace.IsSets, childTrace.IsSets...)
+	})
+	trace.Bools = SortedUniqueStrings(trace.Bools)
+	trace.Strings = SortedUniqueStrings(trace.Strings)
+	trace.IsSets = SortedUniqueStrings(trace.IsSets)
+
+	ctx.Module().base().commonProperties.SoongConfigTraceHash = trace.hash()
+}
+
+// soongConfigTraceSingleton writes a map from each module's config hash value to trace data.
+func soongConfigTraceSingletonFunc() Singleton {
+	return &soongConfigTraceSingleton{}
+}
+
+type soongConfigTraceSingleton struct {
+}
+
+func (s *soongConfigTraceSingleton) GenerateBuildActions(ctx SingletonContext) {
+	outFile := PathForOutput(ctx, "soong_config_trace.json")
+
+	traces := make(map[string]*soongConfigTrace)
+	ctx.VisitAllModules(func(module Module) {
+		trace := &module.base().commonProperties.SoongConfigTrace
+		if !trace.isEmpty() {
+			hash := module.base().commonProperties.SoongConfigTraceHash
+			traces[hash] = trace
+		}
+	})
+
+	j, err := json.Marshal(traces)
+	if err != nil {
+		ctx.Errorf("json marshal to %q failed: %#v", outFile, err)
+		return
+	}
+
+	WriteFileRule(ctx, outFile, string(j))
+	ctx.Phony("soong_config_trace", outFile)
+}

+ 5 - 1
android/paths.go

@@ -1475,7 +1475,11 @@ type ModuleOutPathContext interface {
 }
 
 func pathForModuleOut(ctx ModuleOutPathContext) OutputPath {
-	return PathForOutput(ctx, ".intermediates", ctx.ModuleDir(), ctx.ModuleName(), ctx.ModuleSubDir())
+	soongConfigHash := ""
+	if i, ok := ctx.(interface{ ModuleSoongConfigHash() string }); ok {
+		soongConfigHash = i.ModuleSoongConfigHash()
+	}
+	return PathForOutput(ctx, ".intermediates", ctx.ModuleDir(), ctx.ModuleName(), ctx.ModuleSubDir(), soongConfigHash)
 }
 
 // PathForModuleOut returns a Path representing the paths... under the module's

+ 55 - 2
android/soong_config_modules.go

@@ -421,6 +421,57 @@ func loadSoongConfigModuleTypeDefinition(ctx LoadHookContext, from string) map[s
 	}).(map[string]blueprint.ModuleFactory)
 }
 
+// tracingConfig is a wrapper to soongconfig.SoongConfig which records all accesses to SoongConfig.
+type tracingConfig struct {
+	config    soongconfig.SoongConfig
+	boolSet   map[string]bool
+	stringSet map[string]string
+	isSetSet  map[string]bool
+}
+
+func (c *tracingConfig) Bool(name string) bool {
+	c.boolSet[name] = c.config.Bool(name)
+	return c.boolSet[name]
+}
+
+func (c *tracingConfig) String(name string) string {
+	c.stringSet[name] = c.config.String(name)
+	return c.stringSet[name]
+}
+
+func (c *tracingConfig) IsSet(name string) bool {
+	c.isSetSet[name] = c.config.IsSet(name)
+	return c.isSetSet[name]
+}
+
+func (c *tracingConfig) getTrace() soongConfigTrace {
+	ret := soongConfigTrace{}
+
+	for k, v := range c.boolSet {
+		ret.Bools = append(ret.Bools, fmt.Sprintf("%q:%t", k, v))
+	}
+	for k, v := range c.stringSet {
+		ret.Strings = append(ret.Strings, fmt.Sprintf("%q:%q", k, v))
+	}
+	for k, v := range c.isSetSet {
+		ret.IsSets = append(ret.IsSets, fmt.Sprintf("%q:%t", k, v))
+	}
+
+	return ret
+}
+
+func newTracingConfig(config soongconfig.SoongConfig) *tracingConfig {
+	c := tracingConfig{
+		config:    config,
+		boolSet:   make(map[string]bool),
+		stringSet: make(map[string]string),
+		isSetSet:  make(map[string]bool),
+	}
+	return &c
+}
+
+var _ soongconfig.SoongConfig = (*tracingConfig)(nil)
+
 // configModuleFactory takes an existing soongConfigModuleFactory and a
 // ModuleType to create a new ModuleFactory that uses a custom loadhook.
 func configModuleFactory(factory blueprint.ModuleFactory, moduleType *soongconfig.ModuleType, bp2build bool) blueprint.ModuleFactory {
@@ -485,8 +536,8 @@ func configModuleFactory(factory blueprint.ModuleFactory, moduleType *soongconfi
 			// conditional on Soong config variables by reading the product
 			// config variables from Make.
 			AddLoadHook(module, func(ctx LoadHookContext) {
-				config := ctx.Config().VendorConfig(moduleType.ConfigNamespace)
-				newProps, err := soongconfig.PropertiesToApply(moduleType, conditionalProps, config)
+				tracingConfig := newTracingConfig(ctx.Config().VendorConfig(moduleType.ConfigNamespace))
+				newProps, err := soongconfig.PropertiesToApply(moduleType, conditionalProps, tracingConfig)
 				if err != nil {
 					ctx.ModuleErrorf("%s", err)
 					return
@@ -494,6 +545,8 @@ func configModuleFactory(factory blueprint.ModuleFactory, moduleType *soongconfi
 				for _, ps := range newProps {
 					ctx.AppendProperties(ps)
 				}
+
+				module.(Module).base().commonProperties.SoongConfigTrace = tracingConfig.getTrace()
 			})
 		}
 		return module, props

+ 200 - 2
android/soong_config_modules_test.go

@@ -16,6 +16,7 @@ package android
 
 import (
 	"fmt"
+	"path/filepath"
 	"testing"
 )
 
@@ -34,7 +35,8 @@ func soongConfigTestDefaultsModuleFactory() Module {
 type soongConfigTestModule struct {
 	ModuleBase
 	DefaultableModuleBase
-	props soongConfigTestModuleProperties
+	props      soongConfigTestModuleProperties
+	outputPath ModuleOutPath
 }
 
 type soongConfigTestModuleProperties struct {
@@ -49,7 +51,9 @@ func soongConfigTestModuleFactory() Module {
 	return m
 }
 
-func (t soongConfigTestModule) GenerateAndroidBuildActions(ModuleContext) {}
+func (t *soongConfigTestModule) GenerateAndroidBuildActions(ctx ModuleContext) {
+	t.outputPath = PathForModuleOut(ctx, "test")
+}
 
 var prepareForSoongConfigTestModule = FixtureRegisterWithContext(func(ctx RegistrationContext) {
 	ctx.RegisterModuleType("test_defaults", soongConfigTestDefaultsModuleFactory)
@@ -503,3 +507,197 @@ func TestSoongConfigModuleSingletonModule(t *testing.T) {
 		})
 	}
 }
+
+func TestSoongConfigModuleTrace(t *testing.T) {
+	bp := `
+		soong_config_module_type {
+			name: "acme_test",
+			module_type: "test",
+			config_namespace: "acme",
+			variables: ["board", "feature1", "FEATURE3", "unused_string_var"],
+			bool_variables: ["feature2", "unused_feature", "always_true"],
+			value_variables: ["size", "unused_size"],
+			properties: ["cflags", "srcs", "defaults"],
+		}
+
+		soong_config_module_type {
+			name: "acme_test_defaults",
+			module_type: "test_defaults",
+			config_namespace: "acme",
+			variables: ["board", "feature1", "FEATURE3", "unused_string_var"],
+			bool_variables: ["feature2", "unused_feature", "always_true"],
+			value_variables: ["size", "unused_size"],
+			properties: ["cflags", "srcs", "defaults"],
+		}
+
+		soong_config_string_variable {
+			name: "board",
+			values: ["soc_a", "soc_b", "soc_c"],
+		}
+
+		soong_config_string_variable {
+			name: "unused_string_var",
+			values: ["a", "b"],
+		}
+
+		soong_config_bool_variable {
+			name: "feature1",
+		}
+
+		soong_config_bool_variable {
+			name: "FEATURE3",
+		}
+
+		test_defaults {
+			name: "test_defaults",
+			cflags: ["DEFAULT"],
+		}
+
+		test {
+			name: "normal",
+			defaults: ["test_defaults"],
+		}
+
+		acme_test {
+			name: "board_1",
+			defaults: ["test_defaults"],
+			soong_config_variables: {
+				board: {
+					soc_a: {
+						cflags: ["-DSOC_A"],
+					},
+				},
+			},
+		}
+
+		acme_test {
+			name: "board_2",
+			defaults: ["test_defaults"],
+			soong_config_variables: {
+				board: {
+					soc_a: {
+						cflags: ["-DSOC_A"],
+					},
+				},
+			},
+		}
+
+		acme_test {
+			name: "size",
+			defaults: ["test_defaults"],
+			soong_config_variables: {
+				size: {
+					cflags: ["-DSIZE=%s"],
+				},
+			},
+		}
+
+		acme_test {
+			name: "board_and_size",
+			defaults: ["test_defaults"],
+			soong_config_variables: {
+				board: {
+					soc_a: {
+						cflags: ["-DSOC_A"],
+					},
+				},
+				size: {
+					cflags: ["-DSIZE=%s"],
+				},
+			},
+		}
+
+		acme_test_defaults {
+			name: "board_defaults",
+			soong_config_variables: {
+				board: {
+					soc_a: {
+						cflags: ["-DSOC_A"],
+					},
+				},
+			},
+		}
+
+		acme_test_defaults {
+			name: "size_defaults",
+			soong_config_variables: {
+				size: {
+					cflags: ["-DSIZE=%s"],
+				},
+			},
+		}
+
+		test {
+			name: "board_and_size_with_defaults",
+			defaults: ["board_defaults", "size_defaults"],
+		}
+    `
+
+	fixtureForVendorVars := func(vars map[string]map[string]string) FixturePreparer {
+		return FixtureModifyProductVariables(func(variables FixtureProductVariables) {
+			variables.VendorVars = vars
+		})
+	}
+
+	preparer := fixtureForVendorVars(map[string]map[string]string{
+		"acme": {
+			"board":    "soc_a",
+			"size":     "42",
+			"feature1": "true",
+			"feature2": "false",
+			// FEATURE3 unset
+			"unused_feature":    "true", // unused
+			"unused_size":       "1",    // unused
+			"unused_string_var": "a",    // unused
+			"always_true":       "true",
+		},
+	})
+
+	t.Run("soong config trace hash", func(t *testing.T) {
+		result := GroupFixturePreparers(
+			preparer,
+			PrepareForTestWithDefaults,
+			PrepareForTestWithSoongConfigModuleBuildComponents,
+			prepareForSoongConfigTestModule,
+			FixtureRegisterWithContext(func(ctx RegistrationContext) {
+				ctx.FinalDepsMutators(registerSoongConfigTraceMutator)
+			}),
+			FixtureWithRootAndroidBp(bp),
+		).RunTest(t)
+
+		// Hashes of modules not using soong config should be empty
+		normal := result.ModuleForTests("normal", "").Module().(*soongConfigTestModule)
+		AssertDeepEquals(t, "normal hash", normal.base().commonProperties.SoongConfigTraceHash, "")
+		AssertDeepEquals(t, "normal hash out", normal.outputPath.RelativeToTop().String(), "out/soong/.intermediates/normal/test")
+
+		board1 := result.ModuleForTests("board_1", "").Module().(*soongConfigTestModule)
+		board2 := result.ModuleForTests("board_2", "").Module().(*soongConfigTestModule)
+		size := result.ModuleForTests("size", "").Module().(*soongConfigTestModule)
+
+		// Trace mutator sets soong config trace hash correctly
+		board1Hash := board1.base().commonProperties.SoongConfigTrace.hash()
+		board1Output := board1.outputPath.RelativeToTop().String()
+		AssertDeepEquals(t, "board hash calc", board1Hash, board1.base().commonProperties.SoongConfigTraceHash)
+		AssertDeepEquals(t, "board hash path", board1Output, filepath.Join("out/soong/.intermediates/board_1", board1Hash, "test"))
+
+		sizeHash := size.base().commonProperties.SoongConfigTrace.hash()
+		sizeOutput := size.outputPath.RelativeToTop().String()
+		AssertDeepEquals(t, "size hash calc", sizeHash, size.base().commonProperties.SoongConfigTraceHash)
+		AssertDeepEquals(t, "size hash path", sizeOutput, filepath.Join("out/soong/.intermediates/size", sizeHash, "test"))
+
+		// Trace should be identical for modules using the same set of variables
+		AssertDeepEquals(t, "board trace", board1.base().commonProperties.SoongConfigTrace, board2.base().commonProperties.SoongConfigTrace)
+		AssertDeepEquals(t, "board hash", board1.base().commonProperties.SoongConfigTraceHash, board2.base().commonProperties.SoongConfigTraceHash)
+
+		// Trace hash should be different for different sets of soong variables
+		AssertBoolEquals(t, "board hash not equal to size hash", board1.base().commonProperties.SoongConfigTraceHash == size.commonProperties.SoongConfigTraceHash, false)
+
+		boardSize := result.ModuleForTests("board_and_size", "").Module().(*soongConfigTestModule)
+		boardSizeDefaults := result.ModuleForTests("board_and_size_with_defaults", "").Module()
+
+		// Trace should propagate
+		AssertDeepEquals(t, "board_size hash calc", boardSize.base().commonProperties.SoongConfigTrace.hash(), boardSize.base().commonProperties.SoongConfigTraceHash)
+		AssertDeepEquals(t, "board_size trace", boardSize.base().commonProperties.SoongConfigTrace, boardSizeDefaults.base().commonProperties.SoongConfigTrace)
+		AssertDeepEquals(t, "board_size hash", boardSize.base().commonProperties.SoongConfigTraceHash, boardSizeDefaults.base().commonProperties.SoongConfigTraceHash)
+	})
+}