From c0e42d5dfc265345037b980b981824bc83b9e8b0 Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Thu, 1 Feb 2024 16:42:36 -0800 Subject: [PATCH 1/2] Optimize InstallPath.String() Every InstallPath will have String() called on it eventually, often more than once if it is in a slice that is sorted. Precompute the full path so it can be returned from InstallPath.String() without recomputing every time. Test: paths_test.go Change-Id: I1ed0a3801806854356865c0a5fc35d5cf6d349fe --- android/paths.go | 55 +++++++++++++++++++++++++++--------------------- 1 file changed, 31 insertions(+), 24 deletions(-) diff --git a/android/paths.go b/android/paths.go index 95f53ea4c..61c125811 100644 --- a/android/paths.go +++ b/android/paths.go @@ -1661,6 +1661,8 @@ type InstallPath struct { // makePath indicates whether this path is for Soong (false) or Make (true). makePath bool + + fullPath string } // Will panic if called from outside a test environment. @@ -1673,7 +1675,12 @@ func ensureTestOnly() { func (p InstallPath) RelativeToTop() Path { ensureTestOnly() - p.soongOutDir = OutSoongDir + if p.makePath { + p.soongOutDir = OutDir + } else { + p.soongOutDir = OutSoongDir + } + p.fullPath = filepath.Join(p.soongOutDir, p.path) return p } @@ -1691,12 +1698,7 @@ var _ WritablePath = InstallPath{} func (p InstallPath) writablePath() {} func (p InstallPath) String() string { - if p.makePath { - // Make path starts with out/ instead of out/soong. - return filepath.Join(p.soongOutDir, "../", p.path) - } else { - return filepath.Join(p.soongOutDir, p.path) - } + return p.fullPath } // PartitionDir returns the path to the partition where the install path is rooted at. It is @@ -1726,6 +1728,7 @@ func (p InstallPath) Join(ctx PathContext, paths ...string) InstallPath { func (p InstallPath) withRel(rel string) InstallPath { p.basePath = p.basePath.withRel(rel) + p.fullPath = filepath.Join(p.fullPath, rel) return p } @@ -1769,6 +1772,25 @@ func osAndArch(ctx ModuleInstallPathContext) (OsType, ArchType) { return os, arch } +func pathForPartitionInstallDir(ctx PathContext, partition, partitionPath string, makePath bool) InstallPath { + fullPath := ctx.Config().SoongOutDir() + if makePath { + // Make path starts with out/ instead of out/soong. + fullPath = filepath.Join(fullPath, "../", partitionPath) + } else { + fullPath = filepath.Join(fullPath, partitionPath) + } + + return InstallPath{ + basePath: basePath{partitionPath, ""}, + soongOutDir: ctx.Config().soongOutDir, + partitionDir: partitionPath, + partition: partition, + makePath: makePath, + fullPath: fullPath, + } +} + func pathForInstall(ctx PathContext, os OsType, arch ArchType, partition string, pathComponents ...string) InstallPath { @@ -1805,27 +1827,12 @@ func pathForInstall(ctx PathContext, os OsType, arch ArchType, partition string, reportPathError(ctx, err) } - base := InstallPath{ - basePath: basePath{partitionPath, ""}, - soongOutDir: ctx.Config().soongOutDir, - partitionDir: partitionPath, - partition: partition, - } - - if ctx.Config().KatiEnabled() { - base.makePath = true - } - + base := pathForPartitionInstallDir(ctx, partition, partitionPath, ctx.Config().KatiEnabled()) return base.Join(ctx, pathComponents...) } func pathForNdkOrSdkInstall(ctx PathContext, prefix string, paths []string) InstallPath { - base := InstallPath{ - basePath: basePath{prefix, ""}, - soongOutDir: ctx.Config().soongOutDir, - partitionDir: prefix, - makePath: false, - } + base := pathForPartitionInstallDir(ctx, "", prefix, false) return base.Join(ctx, paths...) } From 984223fd04e0ae95488e207a518bd6e6de8618b6 Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Thu, 1 Feb 2024 17:10:23 -0800 Subject: [PATCH 2/2] Optimize mutator contexts Mutator contexts are created for every module, and in the case of transition mutator contexts for every dependency of every module. Unlike the Blueprint mutator contexts that they wrap, the Soong mutator contexts can be relatively large. Add global pools for all of them that can avoid the repeated allocations. Test: SOONG_PROFILE_MEM=/tmp/mem.pprof m nothing Change-Id: I64a5f3c91292cff6352300f99c11ac50c713f96d --- android/arch.go | 2 ++ android/mutator.go | 47 +++++++++++++++++++++++++++++++++++++++------- 2 files changed, 42 insertions(+), 7 deletions(-) diff --git a/android/arch.go b/android/arch.go index c39db0200..b09a3022f 100644 --- a/android/arch.go +++ b/android/arch.go @@ -426,6 +426,7 @@ func osMutator(bpctx blueprint.BottomUpMutatorContext) { // filters out non-Soong modules. Now that we've handled them, create a // normal android.BottomUpMutatorContext. mctx := bottomUpMutatorContextFactory(bpctx, module, false) + defer bottomUpMutatorContextPool.Put(mctx) base := module.base() @@ -572,6 +573,7 @@ func archMutator(bpctx blueprint.BottomUpMutatorContext) { // filters out non-Soong modules. Now that we've handled them, create a // normal android.BottomUpMutatorContext. mctx := bottomUpMutatorContextFactory(bpctx, module, false) + defer bottomUpMutatorContextPool.Put(mctx) base := module.base() diff --git a/android/mutator.go b/android/mutator.go index 22e91603c..0ff4f48ea 100644 --- a/android/mutator.go +++ b/android/mutator.go @@ -15,6 +15,8 @@ package android import ( + "sync" + "github.com/google/blueprint" ) @@ -328,29 +330,52 @@ type BottomUpMutatorContext interface { SetVariationProvider(module blueprint.Module, provider blueprint.AnyProviderKey, value interface{}) } +// An outgoingTransitionContextImpl and incomingTransitionContextImpl is created for every dependency of every module +// for each transition mutator. bottomUpMutatorContext and topDownMutatorContext are created once for every module +// for every BottomUp or TopDown mutator. Use a global pool for each to avoid reallocating every time. +var ( + outgoingTransitionContextPool = sync.Pool{ + New: func() any { return &outgoingTransitionContextImpl{} }, + } + incomingTransitionContextPool = sync.Pool{ + New: func() any { return &incomingTransitionContextImpl{} }, + } + bottomUpMutatorContextPool = sync.Pool{ + New: func() any { return &bottomUpMutatorContext{} }, + } + + topDownMutatorContextPool = sync.Pool{ + New: func() any { return &topDownMutatorContext{} }, + } +) + type bottomUpMutatorContext struct { bp blueprint.BottomUpMutatorContext baseModuleContext finalPhase bool } +// callers must immediately follow the call to this function with defer bottomUpMutatorContextPool.Put(mctx). func bottomUpMutatorContextFactory(ctx blueprint.BottomUpMutatorContext, a Module, finalPhase bool) BottomUpMutatorContext { moduleContext := a.base().baseModuleContextFactory(ctx) - - return &bottomUpMutatorContext{ + mctx := bottomUpMutatorContextPool.Get().(*bottomUpMutatorContext) + *mctx = bottomUpMutatorContext{ bp: ctx, baseModuleContext: moduleContext, finalPhase: finalPhase, } + return mctx } func (x *registerMutatorsContext) BottomUp(name string, m BottomUpMutator) MutatorHandle { finalPhase := x.finalPhase f := func(ctx blueprint.BottomUpMutatorContext) { if a, ok := ctx.Module().(Module); ok { - m(bottomUpMutatorContextFactory(ctx, a, finalPhase)) + mctx := bottomUpMutatorContextFactory(ctx, a, finalPhase) + defer bottomUpMutatorContextPool.Put(mctx) + m(mctx) } } mutator := &mutator{name: x.mutatorName(name), bottomUpMutator: f} @@ -514,7 +539,9 @@ func (c *outgoingTransitionContextImpl) DeviceConfig() DeviceConfig { func (a *androidTransitionMutator) OutgoingTransition(bpctx blueprint.OutgoingTransitionContext, sourceVariation string) string { if m, ok := bpctx.Module().(Module); ok { - ctx := &outgoingTransitionContextImpl{ + ctx := outgoingTransitionContextPool.Get().(*outgoingTransitionContextImpl) + defer outgoingTransitionContextPool.Put(ctx) + *ctx = outgoingTransitionContextImpl{ archModuleContext: m.base().archModuleContextFactory(bpctx), bp: bpctx, } @@ -543,7 +570,9 @@ func (c *incomingTransitionContextImpl) DeviceConfig() DeviceConfig { func (a *androidTransitionMutator) IncomingTransition(bpctx blueprint.IncomingTransitionContext, incomingVariation string) string { if m, ok := bpctx.Module().(Module); ok { - ctx := &incomingTransitionContextImpl{ + ctx := incomingTransitionContextPool.Get().(*incomingTransitionContextImpl) + defer incomingTransitionContextPool.Put(ctx) + *ctx = incomingTransitionContextImpl{ archModuleContext: m.base().archModuleContextFactory(bpctx), bp: bpctx, } @@ -555,7 +584,9 @@ func (a *androidTransitionMutator) IncomingTransition(bpctx blueprint.IncomingTr func (a *androidTransitionMutator) Mutate(ctx blueprint.BottomUpMutatorContext, variation string) { if am, ok := ctx.Module().(Module); ok { - a.mutator.Mutate(bottomUpMutatorContextFactory(ctx, am, a.finalPhase), variation) + mctx := bottomUpMutatorContextFactory(ctx, am, a.finalPhase) + defer bottomUpMutatorContextPool.Put(mctx) + a.mutator.Mutate(mctx, variation) } } @@ -578,7 +609,9 @@ func (x *registerMutatorsContext) TopDown(name string, m TopDownMutator) Mutator f := func(ctx blueprint.TopDownMutatorContext) { if a, ok := ctx.Module().(Module); ok { moduleContext := a.base().baseModuleContextFactory(ctx) - actx := &topDownMutatorContext{ + actx := topDownMutatorContextPool.Get().(*topDownMutatorContext) + defer topDownMutatorContextPool.Put(actx) + *actx = topDownMutatorContext{ bp: ctx, baseModuleContext: moduleContext, }