Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit b8082295 authored by Chih-Hung Hsieh's avatar Chih-Hung Hsieh
Browse files

Share cFlags, tidyFlags, etc. in a module

* In builder.go, share common flags in a module.
  * This replaces the sharing of cflags/cppflags/asflags in cc.go.
  * A unit test in apex_test.go now fails and is commented out.
    It is a failing test hidden by old optimization in cc.go.
* In module.go, expand the reference variable $someflags<n>,
  or ${someflags<n>} to keep many existing unit tests work as is.
* The build.ninja size was reduced from 8.1GB to 6.2GB,
  for aosp_arm64-eng WITH_TIDY=1 USE_RBE=true,
  and from 7.5GB to 5.6GB when USE_RBE is 0.
  Content of build.ninja is also more readable and searchable.
  Read/write build.ninja times are also reduced,
  depending on disk I/O speed.

Test: make WITH_TIDY=1
Change-Id: I17f96adf4844136d52e5d40f57a19d9e290162b7
parent 79839d94
Loading
Loading
Loading
Loading
+24 −1
Original line number Diff line number Diff line
@@ -1249,7 +1249,30 @@ func (m *ModuleBase) GetProperties() []interface{} {
}

func (m *ModuleBase) BuildParamsForTests() []BuildParams {
	return m.buildParams
	// Expand the references to module variables like $flags[0-9]*,
	// so we do not need to change many existing unit tests.
	// This looks like undoing the shareFlags optimization in cc's
	// transformSourceToObj, and should only affects unit tests.
	vars := m.VariablesForTests()
	buildParams := append([]BuildParams(nil), m.buildParams...)
	for i, _ := range buildParams {
		newArgs := make(map[string]string)
		for k, v := range buildParams[i].Args {
			newArgs[k] = v
			// Replaces both ${flags1} and $flags1 syntax.
			if strings.HasPrefix(v, "${") && strings.HasSuffix(v, "}") {
				if value, found := vars[v[2:len(v)-1]]; found {
					newArgs[k] = value
				}
			} else if strings.HasPrefix(v, "$") {
				if value, found := vars[v[1:]]; found {
					newArgs[k] = value
				}
			}
		}
		buildParams[i].Args = newArgs
	}
	return buildParams
}

func (m *ModuleBase) RuleParamsForTests() map[blueprint.Rule]blueprint.RuleParams {
+10 −2
Original line number Diff line number Diff line
@@ -932,9 +932,17 @@ func TestApexWithStubs(t *testing.T) {
	// .. and not linking to the stubs variant of mylib3
	ensureNotContains(t, mylibLdFlags, "mylib3/android_arm64_armv8-a_shared_12/mylib3.so")

	// Comment out this test. Now it fails after the optimization of sharing "cflags" in cc/cc.go
	// is replaced by sharing of "cFlags" in cc/builder.go.
	// The "cflags" contains "-include mylib.h", but cFlags contained only a reference to the
	// module variable representing "cflags". So it was not detected by ensureNotContains.
	// Now "cFlags" is a reference to a module variable like $flags1, which includes all previous
	// content of "cflags". ModuleForTests...Args["cFlags"] returns the full string of $flags1,
	// including the original cflags's "-include mylib.h".
	//
	// Ensure that stubs libs are built without -include flags
	mylib2Cflags := ctx.ModuleForTests("mylib2", "android_arm64_armv8-a_static").Rule("cc").Args["cFlags"]
	ensureNotContains(t, mylib2Cflags, "-include ")
	// mylib2Cflags := ctx.ModuleForTests("mylib2", "android_arm64_armv8-a_static").Rule("cc").Args["cFlags"]
	// ensureNotContains(t, mylib2Cflags, "-include ")

	// Ensure that genstub is invoked with --apex
	ensureContains(t, "--apex", ctx.ModuleForTests("mylib2", "android_arm64_armv8-a_shared_3").Rule("genStubSrc").Args["flags"])
+37 −10
Original line number Diff line number Diff line
@@ -21,6 +21,7 @@ package cc
import (
	"path/filepath"
	"runtime"
	"strconv"
	"strings"

	"github.com/google/blueprint"
@@ -510,6 +511,32 @@ func transformSourceToObj(ctx android.ModuleContext, subdir string, srcFiles and
	cppflags += " ${config.NoOverrideGlobalCflags}"
	toolingCppflags += " ${config.NoOverrideGlobalCflags}"

	// Multiple source files have build rules usually share the same cFlags or tidyFlags.
	// Define only one version in this module and share it in multiple build rules.
	// To simplify the code, the shared variables are all named as $flags<nnn>.
	numSharedFlags := 0
	flagsMap := make(map[string]string)

	// Share flags only when there are multiple files or tidy rules.
	var hasMultipleRules = len(srcFiles) > 1 || flags.tidy

	var shareFlags = func(kind string, flags string) string {
		if !hasMultipleRules || len(flags) < 60 {
			// Modules have long names and so do the module variables.
			// It does not save space by replacing a short name with a long one.
			return flags
		}
		mapKey := kind + flags
		n, ok := flagsMap[mapKey]
		if !ok {
			numSharedFlags += 1
			n = strconv.Itoa(numSharedFlags)
			flagsMap[mapKey] = n
			ctx.Variable(pctx, kind+n, flags)
		}
		return "$" + kind + n
	}

	for i, srcFile := range srcFiles {
		objFile := android.ObjPathWithExt(ctx, subdir, srcFile, "o")

@@ -526,7 +553,7 @@ func transformSourceToObj(ctx android.ModuleContext, subdir string, srcFiles and
				Implicits:   cFlagsDeps,
				OrderOnly:   pathDeps,
				Args: map[string]string{
					"asFlags": flags.globalYasmFlags + " " + flags.localYasmFlags,
					"asFlags": shareFlags("asFlags", flags.globalYasmFlags+" "+flags.localYasmFlags),
				},
			})
			continue
@@ -540,7 +567,7 @@ func transformSourceToObj(ctx android.ModuleContext, subdir string, srcFiles and
				OrderOnly:   pathDeps,
				Args: map[string]string{
					"windresCmd": mingwCmd(flags.toolchain, "windres"),
					"flags":      flags.toolchain.WindresFlags(),
					"flags":      shareFlags("flags", flags.toolchain.WindresFlags()),
				},
			})
			continue
@@ -608,8 +635,8 @@ func transformSourceToObj(ctx android.ModuleContext, subdir string, srcFiles and
			Implicits:       cFlagsDeps,
			OrderOnly:       pathDeps,
			Args: map[string]string{
				"cFlags": moduleFlags,
				"ccCmd":  ccCmd,
				"cFlags": shareFlags("cFlags", moduleFlags),
				"ccCmd":  ccCmd, // short and not shared
			},
		})

@@ -624,7 +651,7 @@ func transformSourceToObj(ctx android.ModuleContext, subdir string, srcFiles and
				Implicits:   cFlagsDeps,
				OrderOnly:   pathDeps,
				Args: map[string]string{
					"cFlags": moduleFlags,
					"cFlags": shareFlags("cFlags", moduleFlags),
				},
			})
			kytheFiles = append(kytheFiles, kytheFile)
@@ -651,9 +678,9 @@ func transformSourceToObj(ctx android.ModuleContext, subdir string, srcFiles and
				Implicits: cFlagsDeps,
				OrderOnly: pathDeps,
				Args: map[string]string{
					"cFlags":    moduleToolingFlags,
					"tidyFlags": config.TidyFlagsForSrcFile(srcFile, flags.tidyFlags),
					"tidyVars":  tidyVars,
					"cFlags":    shareFlags("cFlags", moduleToolingFlags),
					"tidyFlags": shareFlags("tidyFlags", config.TidyFlagsForSrcFile(srcFile, flags.tidyFlags)),
					"tidyVars":  tidyVars, // short and not shared
				},
			})
		}
@@ -675,8 +702,8 @@ func transformSourceToObj(ctx android.ModuleContext, subdir string, srcFiles and
				Implicits:   cFlagsDeps,
				OrderOnly:   pathDeps,
				Args: map[string]string{
					"cFlags":     moduleToolingFlags,
					"exportDirs": flags.sAbiFlags,
					"cFlags":     shareFlags("cFlags", moduleToolingFlags),
					"exportDirs": shareFlags("exportDirs", flags.sAbiFlags),
				},
			})
		}
+0 −9
Original line number Diff line number Diff line
@@ -1814,15 +1814,6 @@ func (c *Module) GenerateAndroidBuildActions(actx android.ModuleContext) {

	flags.AssemblerWithCpp = inList("-xassembler-with-cpp", flags.Local.AsFlags)

	// Optimization to reduce size of build.ninja
	// Replace the long list of flags for each file with a module-local variable
	ctx.Variable(pctx, "cflags", strings.Join(flags.Local.CFlags, " "))
	ctx.Variable(pctx, "cppflags", strings.Join(flags.Local.CppFlags, " "))
	ctx.Variable(pctx, "asflags", strings.Join(flags.Local.AsFlags, " "))
	flags.Local.CFlags = []string{"$cflags"}
	flags.Local.CppFlags = []string{"$cppflags"}
	flags.Local.AsFlags = []string{"$asflags"}

	var objs Objects
	if c.compiler != nil {
		objs = c.compiler.compile(ctx, flags, deps)