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

Commit 1a527688 authored by Colin Cross's avatar Colin Cross
Browse files

Shard gensrcs modules into multiple commands

gensrcs modules use a single command to convert all of the sources,
which can lead to command line length limits, long critical path
times, and excessive rebuilds.  Shard them into multiple rules,
defaulting to 100 commands in each.

Test: TestGenSrcs
Test: m
Fixes: 70221552
Fixes: 138438756
Fixes: 138829091
Change-Id: I8409e43011a4754e095ad1b368570a2ba8d75e50
parent 0a2f719b
Loading
Loading
Loading
Loading
+248 −161
Original line number Diff line number Diff line
@@ -17,6 +17,7 @@ package genrule
import (
	"fmt"
	"io"
	"strconv"
	"strings"

	"github.com/google/blueprint"
@@ -37,11 +38,21 @@ func init() {

var (
	pctx = android.NewPackageContext("android/soong/genrule")

	gensrcsMerge = pctx.AndroidStaticRule("gensrcsMerge", blueprint.RuleParams{
		Command:        "${soongZip} -o ${tmpZip} @${tmpZip}.rsp && ${zipSync} -d ${genDir} ${tmpZip}",
		CommandDeps:    []string{"${soongZip}", "${zipSync}"},
		Rspfile:        "${tmpZip}.rsp",
		RspfileContent: "${zipArgs}",
	}, "tmpZip", "genDir", "zipArgs")
)

func init() {
	pctx.Import("android/soong/android")
	pctx.HostBinToolVariable("sboxCmd", "sbox")

	pctx.HostBinToolVariable("soongZip", "soong_zip")
	pctx.HostBinToolVariable("zipSync", "zipsync")
}

type SourceFileGenerator interface {
@@ -114,7 +125,7 @@ type Module struct {

	deps        android.Paths
	rule        blueprint.Rule
	rawCommand string
	rawCommands []string

	exportedIncludeDirs android.Paths

@@ -122,15 +133,20 @@ type Module struct {
	outputDeps  android.Paths

	subName string
	subDir  string
}

type taskFunc func(ctx android.ModuleContext, rawCommand string, srcFiles android.Paths) generateTask
type taskFunc func(ctx android.ModuleContext, rawCommand string, srcFiles android.Paths) []generateTask

type generateTask struct {
	in          android.Paths
	out         android.WritablePaths
	copyTo      android.WritablePaths
	genDir      android.WritablePath
	sandboxOuts []string
	cmd         string
	shard       int
	shards      int
}

func (g *Module) GeneratedSourceFiles() android.Paths {
@@ -169,10 +185,10 @@ func (g *Module) GenerateAndroidBuildActions(ctx android.ModuleContext) {
	if len(g.properties.Export_include_dirs) > 0 {
		for _, dir := range g.properties.Export_include_dirs {
			g.exportedIncludeDirs = append(g.exportedIncludeDirs,
				android.PathForModuleGen(ctx, ctx.ModuleDir(), dir))
				android.PathForModuleGen(ctx, g.subDir, ctx.ModuleDir(), dir))
		}
	} else {
		g.exportedIncludeDirs = append(g.exportedIncludeDirs, android.PathForModuleGen(ctx, ""))
		g.exportedIncludeDirs = append(g.exportedIncludeDirs, android.PathForModuleGen(ctx, g.subDir))
	}

	locationLabels := map[string][]string{}
@@ -277,8 +293,11 @@ func (g *Module) GenerateAndroidBuildActions(ctx android.ModuleContext) {
		}
	}

	task := g.taskGenerator(ctx, String(g.properties.Cmd), srcFiles)
	var copyFrom android.Paths
	var outputFiles android.WritablePaths
	var zipArgs strings.Builder

	for _, task := range g.taskGenerator(ctx, String(g.properties.Cmd), srcFiles) {
		for _, out := range task.out {
			addLocationLabel(out.Rel(), []string{filepath.Join("__SBOX_OUT_DIR__", out.Rel())})
		}
@@ -355,6 +374,7 @@ func (g *Module) GenerateAndroidBuildActions(ctx android.ModuleContext) {

		if Bool(g.properties.Depfile) && !referencedDepfile {
			ctx.PropertyErrorf("cmd", "specified depfile=true but did not include a reference to '${depfile}' in cmd")
			return
		}

		// tell the sbox command which directory to use as its sandbox root
@@ -368,12 +388,11 @@ func (g *Module) GenerateAndroidBuildActions(ctx android.ModuleContext) {
			depfilePlaceholder = "$depfileArgs"
		}

	genDir := android.PathForModuleGen(ctx)
		// Escape the command for the shell
		rawCommand = "'" + strings.Replace(rawCommand, "'", `'\''`, -1) + "'"
	g.rawCommand = rawCommand
	sandboxCommand := fmt.Sprintf("$sboxCmd --sandbox-path %s --output-root %s -c %s %s $allouts",
		sandboxPath, genDir, rawCommand, depfilePlaceholder)
		g.rawCommands = append(g.rawCommands, rawCommand)
		sandboxCommand := fmt.Sprintf("rm -rf %s && $sboxCmd --sandbox-path %s --output-root %s -c %s %s $allouts",
			task.genDir, sandboxPath, task.genDir, rawCommand, depfilePlaceholder)

		ruleParams := blueprint.RuleParams{
			Command:     sandboxCommand,
@@ -384,13 +403,60 @@ func (g *Module) GenerateAndroidBuildActions(ctx android.ModuleContext) {
			ruleParams.Deps = blueprint.DepsGCC
			args = append(args, "depfileArgs")
		}
	g.rule = ctx.Rule(pctx, "generator", ruleParams, args...)
		name := "generator"
		if task.shards > 1 {
			name += strconv.Itoa(task.shard)
		}
		rule := ctx.Rule(pctx, name, ruleParams, args...)

	g.generateSourceFile(ctx, task)
		g.generateSourceFile(ctx, task, rule)

		if len(task.copyTo) > 0 {
			outputFiles = append(outputFiles, task.copyTo...)
			copyFrom = append(copyFrom, task.out.Paths()...)
			zipArgs.WriteString(" -C " + task.genDir.String())
			zipArgs.WriteString(android.JoinWithPrefix(task.out.Strings(), " -f "))
		} else {
			outputFiles = append(outputFiles, task.out...)
		}
	}

func (g *Module) generateSourceFile(ctx android.ModuleContext, task generateTask) {
	if len(copyFrom) > 0 {
		ctx.Build(pctx, android.BuildParams{
			Rule:      gensrcsMerge,
			Implicits: copyFrom,
			Outputs:   outputFiles,
			Args: map[string]string{
				"zipArgs": zipArgs.String(),
				"tmpZip":  android.PathForModuleGen(ctx, g.subDir+".zip").String(),
				"genDir":  android.PathForModuleGen(ctx, g.subDir).String(),
			},
		})
	}

	g.outputFiles = outputFiles.Paths()

	// For <= 6 outputs, just embed those directly in the users. Right now, that covers >90% of
	// the genrules on AOSP. That will make things simpler to look at the graph in the common
	// case. For larger sets of outputs, inject a phony target in between to limit ninja file
	// growth.
	if len(g.outputFiles) <= 6 {
		g.outputDeps = g.outputFiles
	} else {
		phonyFile := android.PathForModuleGen(ctx, "genrule-phony")

		ctx.Build(pctx, android.BuildParams{
			Rule:   blueprint.Phony,
			Output: phonyFile,
			Inputs: g.outputFiles,
		})

		g.outputDeps = android.Paths{phonyFile}
	}

}

func (g *Module) generateSourceFile(ctx android.ModuleContext, task generateTask, rule blueprint.Rule) {
	desc := "generate"
	if len(task.out) == 0 {
		ctx.ModuleErrorf("must have at least one output file")
@@ -405,9 +471,13 @@ func (g *Module) generateSourceFile(ctx android.ModuleContext, task generateTask
		depFile = android.PathForModuleGen(ctx, task.out[0].Rel()+".d")
	}

	if task.shards > 1 {
		desc += " " + strconv.Itoa(task.shard)
	}

	params := android.BuildParams{
		Rule:            g.rule,
		Description:     "generate",
		Rule:            rule,
		Description:     desc,
		Output:          task.out[0],
		ImplicitOutputs: task.out[1:],
		Inputs:          task.in,
@@ -422,28 +492,6 @@ func (g *Module) generateSourceFile(ctx android.ModuleContext, task generateTask
	}

	ctx.Build(pctx, params)

	for _, outputFile := range task.out {
		g.outputFiles = append(g.outputFiles, outputFile)
	}

	// For <= 6 outputs, just embed those directly in the users. Right now, that covers >90% of
	// the genrules on AOSP. That will make things simpler to look at the graph in the common
	// case. For larger sets of outputs, inject a phony target in between to limit ninja file
	// growth.
	if len(task.out) <= 6 {
		g.outputDeps = g.outputFiles
	} else {
		phonyFile := android.PathForModuleGen(ctx, "genrule-phony")

		ctx.Build(pctx, android.BuildParams{
			Rule:   blueprint.Phony,
			Output: phonyFile,
			Inputs: g.outputFiles,
		})

		g.outputDeps = android.Paths{phonyFile}
	}
}

// Collect information for opening IDE project files in java/jdeps.go.
@@ -465,7 +513,7 @@ func (g *Module) AndroidMk() android.AndroidMkData {
		SubName:    g.subName,
		Extra: []android.AndroidMkExtraFunc{
			func(w io.Writer, outputFile android.Path) {
				fmt.Fprintln(w, "LOCAL_ADDITIONAL_DEPENDENCIES :=", strings.Join(g.outputFiles.Strings(), " "))
				fmt.Fprintln(w, "LOCAL_ADDITIONAL_DEPENDENCIES :=", strings.Join(g.outputDeps.Strings(), " "))
			},
		},
		Custom: func(w io.Writer, name, prefix, moduleDir string, data android.AndroidMkData) {
@@ -502,16 +550,40 @@ func pathToSandboxOut(path android.Path, genDir android.Path) string {
func NewGenSrcs() *Module {
	properties := &genSrcsProperties{}

	taskGenerator := func(ctx android.ModuleContext, rawCommand string, srcFiles android.Paths) generateTask {
		commands := []string{}
		outFiles := android.WritablePaths{}
		genDir := android.PathForModuleGen(ctx)
		sandboxOuts := []string{}
		for _, in := range srcFiles {
			outFile := android.GenPathWithExt(ctx, "", in, String(properties.Output_extension))
			outFiles = append(outFiles, outFile)
	taskGenerator := func(ctx android.ModuleContext, rawCommand string, srcFiles android.Paths) []generateTask {
		genDir := android.PathForModuleGen(ctx, "gensrcs")
		shardSize := defaultShardSize
		if s := properties.Shard_size; s != nil {
			shardSize = int(*s)
		}

		shards := android.ShardPaths(srcFiles, shardSize)
		var generateTasks []generateTask

		for i, shard := range shards {
			var commands []string
			var outFiles android.WritablePaths
			var copyTo android.WritablePaths
			var shardDir android.WritablePath
			var sandboxOuts []string

			if len(shards) > 1 {
				shardDir = android.PathForModuleGen(ctx, strconv.Itoa(i))
			} else {
				shardDir = genDir
			}

			for _, in := range shard {
				outFile := android.GenPathWithExt(ctx, "gensrcs", in, String(properties.Output_extension))
				sandboxOutfile := pathToSandboxOut(outFile, genDir)

				if len(shards) > 1 {
					shardFile := android.GenPathWithExt(ctx, strconv.Itoa(i), in, String(properties.Output_extension))
					copyTo = append(copyTo, outFile)
					outFile = shardFile
				}

				outFiles = append(outFiles, outFile)
				sandboxOuts = append(sandboxOuts, sandboxOutfile)

				command, err := android.Expand(rawCommand, func(name string) (string, error) {
@@ -534,15 +606,24 @@ func NewGenSrcs() *Module {
			}
			fullCommand := strings.Join(commands, " && ")

		return generateTask{
			in:          srcFiles,
			generateTasks = append(generateTasks, generateTask{
				in:          shard,
				out:         outFiles,
				copyTo:      copyTo,
				genDir:      shardDir,
				sandboxOuts: sandboxOuts,
				cmd:         fullCommand,
				shard:       i,
				shards:      len(shards),
			})
		}

		return generateTasks
	}

	return generatorFactory(taskGenerator, properties)
	g := generatorFactory(taskGenerator, properties)
	g.subDir = "gensrcs"
	return g
}

func GenSrcsFactory() android.Module {
@@ -554,12 +635,17 @@ func GenSrcsFactory() android.Module {
type genSrcsProperties struct {
	// extension that will be substituted for each output file
	Output_extension *string

	// maximum number of files that will be passed on a single command line.
	Shard_size *int64
}

const defaultShardSize = 100

func NewGenRule() *Module {
	properties := &genRuleProperties{}

	taskGenerator := func(ctx android.ModuleContext, rawCommand string, srcFiles android.Paths) generateTask {
	taskGenerator := func(ctx android.ModuleContext, rawCommand string, srcFiles android.Paths) []generateTask {
		outs := make(android.WritablePaths, len(properties.Out))
		sandboxOuts := make([]string, len(properties.Out))
		genDir := android.PathForModuleGen(ctx)
@@ -567,12 +653,13 @@ func NewGenRule() *Module {
			outs[i] = android.PathForModuleGen(ctx, out)
			sandboxOuts[i] = pathToSandboxOut(outs[i], genDir)
		}
		return generateTask{
		return []generateTask{{
			in:          srcFiles,
			out:         outs,
			genDir:      android.PathForModuleGen(ctx),
			sandboxOuts: sandboxOuts,
			cmd:         rawCommand,
		}
		}}
	}

	return generatorFactory(taskGenerator, properties)
+98 −3
Original line number Diff line number Diff line
@@ -57,6 +57,7 @@ func testContext(config android.Config, bp string,
	ctx := android.NewTestArchContext()
	ctx.RegisterModuleType("filegroup", android.ModuleFactoryAdaptor(android.FileGroupFactory))
	ctx.RegisterModuleType("genrule", android.ModuleFactoryAdaptor(GenRuleFactory))
	ctx.RegisterModuleType("gensrcs", android.ModuleFactoryAdaptor(GenSrcsFactory))
	ctx.RegisterModuleType("genrule_defaults", android.ModuleFactoryAdaptor(defaultsFactory))
	ctx.RegisterModuleType("tool", android.ModuleFactoryAdaptor(toolFactory))
	ctx.PreArchMutators(android.RegisterDefaultsPreArchMutators)
@@ -109,6 +110,9 @@ func testContext(config android.Config, bp string,
		"tool_file2": nil,
		"in1":        nil,
		"in2":        nil,
		"in1.txt":    nil,
		"in2.txt":    nil,
		"in3.txt":    nil,
	}

	for k, v := range fs {
@@ -491,11 +495,102 @@ func TestGenruleCmd(t *testing.T) {
			}

			gen := ctx.ModuleForTests("gen", "").Module().(*Module)
			if g, w := gen.rawCommand, "'"+test.expect+"'"; w != g {
			if g, w := gen.rawCommands[0], "'"+test.expect+"'"; w != g {
				t.Errorf("want %q, got %q", w, g)
			}
		})
	}
}

func TestGenSrcs(t *testing.T) {
	testcases := []struct {
		name string
		prop string

		allowMissingDependencies bool

		err   string
		cmds  []string
		deps  []string
		files []string
	}{
		{
			name: "gensrcs",
			prop: `
				tools: ["tool"],
				srcs: ["in1.txt", "in2.txt"],
				cmd: "$(location) $(in) > $(out)",
			`,
			cmds: []string{
				"'bash -c '\\''out/tool in1.txt > __SBOX_OUT_DIR__/in1.h'\\'' && bash -c '\\''out/tool in2.txt > __SBOX_OUT_DIR__/in2.h'\\'''",
			},
			deps:  []string{buildDir + "/.intermediates/gen/gen/gensrcs/in1.h", buildDir + "/.intermediates/gen/gen/gensrcs/in2.h"},
			files: []string{buildDir + "/.intermediates/gen/gen/gensrcs/in1.h", buildDir + "/.intermediates/gen/gen/gensrcs/in2.h"},
		},
		{
			name: "shards",
			prop: `
				tools: ["tool"],
				srcs: ["in1.txt", "in2.txt", "in3.txt"],
				cmd: "$(location) $(in) > $(out)",
				shard_size: 2,
			`,
			cmds: []string{
				"'bash -c '\\''out/tool in1.txt > __SBOX_OUT_DIR__/in1.h'\\'' && bash -c '\\''out/tool in2.txt > __SBOX_OUT_DIR__/in2.h'\\'''",
				"'bash -c '\\''out/tool in3.txt > __SBOX_OUT_DIR__/in3.h'\\'''",
			},
			deps:  []string{buildDir + "/.intermediates/gen/gen/gensrcs/in1.h", buildDir + "/.intermediates/gen/gen/gensrcs/in2.h", buildDir + "/.intermediates/gen/gen/gensrcs/in3.h"},
			files: []string{buildDir + "/.intermediates/gen/gen/gensrcs/in1.h", buildDir + "/.intermediates/gen/gen/gensrcs/in2.h", buildDir + "/.intermediates/gen/gen/gensrcs/in3.h"},
		},
	}

	for _, test := range testcases {
		t.Run(test.name, func(t *testing.T) {
			config := android.TestArchConfig(buildDir, nil)
			bp := "gensrcs {\n"
			bp += `name: "gen",` + "\n"
			bp += `output_extension: "h",` + "\n"
			bp += test.prop
			bp += "}\n"

			ctx := testContext(config, bp, nil)

			_, errs := ctx.ParseFileList(".", []string{"Android.bp"})
			if errs == nil {
				_, errs = ctx.PrepareBuildActions(config)
			}
			if errs == nil && test.err != "" {
				t.Fatalf("want error %q, got no error", test.err)
			} else if errs != nil && test.err == "" {
				android.FailIfErrored(t, errs)
			} else if test.err != "" {
				if len(errs) != 1 {
					t.Errorf("want 1 error, got %d errors:", len(errs))
					for _, err := range errs {
						t.Errorf("   %s", err.Error())
					}
					t.FailNow()
				}
				if !strings.Contains(errs[0].Error(), test.err) {
					t.Fatalf("want %q, got %q", test.err, errs[0].Error())
				}
				return
			}

			gen := ctx.ModuleForTests("gen", "").Module().(*Module)
			if g, w := gen.rawCommands, test.cmds; !reflect.DeepEqual(w, g) {
				t.Errorf("want %q, got %q", w, g)
			}

			if g, w := gen.outputDeps.Strings(), test.deps; !reflect.DeepEqual(w, g) {
				t.Errorf("want deps %q, got %q", w, g)
			}

			if g, w := gen.outputFiles.Strings(), test.files; !reflect.DeepEqual(w, g) {
				t.Errorf("want files %q, got %q", w, g)
			}
		})
	}

}

@@ -529,8 +624,8 @@ func TestGenruleDefaults(t *testing.T) {
	gen := ctx.ModuleForTests("gen", "").Module().(*Module)

	expectedCmd := "'cp ${in} __SBOX_OUT_FILES__'"
	if gen.rawCommand != expectedCmd {
		t.Errorf("Expected cmd: %q, actual: %q", expectedCmd, gen.rawCommand)
	if gen.rawCommands[0] != expectedCmd {
		t.Errorf("Expected cmd: %q, actual: %q", expectedCmd, gen.rawCommands[0])
	}

	expectedSrcs := []string{"in1"}