diff options
author | Colin Cross <ccross@android.com> | 2019-09-23 15:55:30 -0700 |
---|---|---|
committer | Colin Cross <ccross@android.com> | 2019-11-21 14:48:07 -0800 |
commit | d9c1c8fbcb242d2aacebbd9dc28702b402fe44f4 (patch) | |
tree | a2eccd26193e970726af5fd4e6799b79a571855a | |
parent | a95304ec10b3b518fd47cd7677c4d46b124fdde9 (diff) | |
download | soong-d9c1c8fbcb242d2aacebbd9dc28702b402fe44f4.tar.gz |
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
Fixes: 144948629
Change-Id: I8409e43011a4754e095ad1b368570a2ba8d75e50
Merged-In: I8409e43011a4754e095ad1b368570a2ba8d75e50
(cherry picked from commit 1a527688d67b9349f4ea5735916c35bdb7f1847f)
-rw-r--r-- | genrule/genrule.go | 378 | ||||
-rw-r--r-- | genrule/genrule_test.go | 101 |
2 files changed, 331 insertions, 148 deletions
diff --git a/genrule/genrule.go b/genrule/genrule.go index 87e6747e9..55c7e628e 100644 --- a/genrule/genrule.go +++ b/genrule/genrule.go @@ -17,6 +17,7 @@ package genrule import ( "fmt" "io" + "strconv" "strings" "github.com/google/blueprint" @@ -37,10 +38,20 @@ 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.HostBinToolVariable("sboxCmd", "sbox") + + pctx.HostBinToolVariable("soongZip", "soong_zip") + pctx.HostBinToolVariable("zipSync", "zipsync") } type SourceFileGenerator interface { @@ -110,9 +121,9 @@ type Module struct { taskGenerator taskFunc - deps android.Paths - rule blueprint.Rule - rawCommand string + deps android.Paths + rule blueprint.Rule + rawCommands []string exportedIncludeDirs android.Paths @@ -120,15 +131,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 { @@ -167,10 +183,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{} @@ -275,120 +291,154 @@ func (g *Module) GenerateAndroidBuildActions(ctx android.ModuleContext) { } } - task := g.taskGenerator(ctx, String(g.properties.Cmd), srcFiles) - - for _, out := range task.out { - addLocationLabel(out.Rel(), []string{filepath.Join("__SBOX_OUT_DIR__", out.Rel())}) - } - - referencedDepfile := false + var copyFrom android.Paths + var outputFiles android.WritablePaths + var zipArgs strings.Builder - rawCommand, err := android.Expand(task.cmd, func(name string) (string, error) { - // report the error directly without returning an error to android.Expand to catch multiple errors in a - // single run - reportError := func(fmt string, args ...interface{}) (string, error) { - ctx.PropertyErrorf("cmd", fmt, args...) - return "SOONG_ERROR", nil + 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())}) } - switch name { - case "location": - if len(g.properties.Tools) == 0 && len(g.properties.Tool_files) == 0 { - return reportError("at least one `tools` or `tool_files` is required if $(location) is used") - } - paths := locationLabels[firstLabel] - if len(paths) == 0 { - return reportError("default label %q has no files", firstLabel) - } else if len(paths) > 1 { - return reportError("default label %q has multiple files, use $(locations %s) to reference it", - firstLabel, firstLabel) - } - return locationLabels[firstLabel][0], nil - case "in": - return "${in}", nil - case "out": - return "__SBOX_OUT_FILES__", nil - case "depfile": - referencedDepfile = true - if !Bool(g.properties.Depfile) { - return reportError("$(depfile) used without depfile property") + referencedDepfile := false + + rawCommand, err := android.Expand(task.cmd, func(name string) (string, error) { + // report the error directly without returning an error to android.Expand to catch multiple errors in a + // single run + reportError := func(fmt string, args ...interface{}) (string, error) { + ctx.PropertyErrorf("cmd", fmt, args...) + return "SOONG_ERROR", nil } - return "__SBOX_DEPFILE__", nil - case "genDir": - return "__SBOX_OUT_DIR__", nil - default: - if strings.HasPrefix(name, "location ") { - label := strings.TrimSpace(strings.TrimPrefix(name, "location ")) - if paths, ok := locationLabels[label]; ok { - if len(paths) == 0 { - return reportError("label %q has no files", label) - } else if len(paths) > 1 { - return reportError("label %q has multiple files, use $(locations %s) to reference it", - label, label) - } - return paths[0], nil - } else { - return reportError("unknown location label %q", label) + + switch name { + case "location": + if len(g.properties.Tools) == 0 && len(g.properties.Tool_files) == 0 { + return reportError("at least one `tools` or `tool_files` is required if $(location) is used") + } + paths := locationLabels[firstLabel] + if len(paths) == 0 { + return reportError("default label %q has no files", firstLabel) + } else if len(paths) > 1 { + return reportError("default label %q has multiple files, use $(locations %s) to reference it", + firstLabel, firstLabel) + } + return locationLabels[firstLabel][0], nil + case "in": + return "${in}", nil + case "out": + return "__SBOX_OUT_FILES__", nil + case "depfile": + referencedDepfile = true + if !Bool(g.properties.Depfile) { + return reportError("$(depfile) used without depfile property") } - } else if strings.HasPrefix(name, "locations ") { - label := strings.TrimSpace(strings.TrimPrefix(name, "locations ")) - if paths, ok := locationLabels[label]; ok { - if len(paths) == 0 { - return reportError("label %q has no files", label) + return "__SBOX_DEPFILE__", nil + case "genDir": + return "__SBOX_OUT_DIR__", nil + default: + if strings.HasPrefix(name, "location ") { + label := strings.TrimSpace(strings.TrimPrefix(name, "location ")) + if paths, ok := locationLabels[label]; ok { + if len(paths) == 0 { + return reportError("label %q has no files", label) + } else if len(paths) > 1 { + return reportError("label %q has multiple files, use $(locations %s) to reference it", + label, label) + } + return paths[0], nil + } else { + return reportError("unknown location label %q", label) + } + } else if strings.HasPrefix(name, "locations ") { + label := strings.TrimSpace(strings.TrimPrefix(name, "locations ")) + if paths, ok := locationLabels[label]; ok { + if len(paths) == 0 { + return reportError("label %q has no files", label) + } + return strings.Join(paths, " "), nil + } else { + return reportError("unknown locations label %q", label) } - return strings.Join(paths, " "), nil } else { - return reportError("unknown locations label %q", label) + return reportError("unknown variable '$(%s)'", name) } - } else { - return reportError("unknown variable '$(%s)'", name) } + }) + + if err != nil { + ctx.PropertyErrorf("cmd", "%s", err.Error()) + return } - }) - if err != nil { - ctx.PropertyErrorf("cmd", "%s", err.Error()) - return - } + if Bool(g.properties.Depfile) && !referencedDepfile { + ctx.PropertyErrorf("cmd", "specified depfile=true but did not include a reference to '${depfile}' in cmd") + return + } - if Bool(g.properties.Depfile) && !referencedDepfile { - ctx.PropertyErrorf("cmd", "specified depfile=true but did not include a reference to '${depfile}' in cmd") - } + // tell the sbox command which directory to use as its sandbox root + buildDir := android.PathForOutput(ctx).String() + sandboxPath := shared.TempDirForOutDir(buildDir) - // tell the sbox command which directory to use as its sandbox root - buildDir := android.PathForOutput(ctx).String() - sandboxPath := shared.TempDirForOutDir(buildDir) + // recall that Sprintf replaces percent sign expressions, whereas dollar signs expressions remain as written, + // to be replaced later by ninja_strings.go + depfilePlaceholder := "" + if Bool(g.properties.Depfile) { + depfilePlaceholder = "$depfileArgs" + } - // recall that Sprintf replaces percent sign expressions, whereas dollar signs expressions remain as written, - // to be replaced later by ninja_strings.go - depfilePlaceholder := "" - if Bool(g.properties.Depfile) { - depfilePlaceholder = "$depfileArgs" - } + // Escape the command for the shell + rawCommand = "'" + strings.Replace(rawCommand, "'", `'\''`, -1) + "'" + 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) - 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) + ruleParams := blueprint.RuleParams{ + Command: sandboxCommand, + CommandDeps: []string{"$sboxCmd"}, + } + args := []string{"allouts"} + if Bool(g.properties.Depfile) { + ruleParams.Deps = blueprint.DepsGCC + args = append(args, "depfileArgs") + } + name := "generator" + if task.shards > 1 { + name += strconv.Itoa(task.shard) + } + rule := ctx.Rule(pctx, name, ruleParams, args...) - ruleParams := blueprint.RuleParams{ - Command: sandboxCommand, - CommandDeps: []string{"$sboxCmd"}, - } - args := []string{"allouts"} - if Bool(g.properties.Depfile) { - ruleParams.Deps = blueprint.DepsGCC - args = append(args, "depfileArgs") + 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...) + } } - g.rule = ctx.Rule(pctx, "generator", ruleParams, args...) - g.generateSourceFile(ctx, task) + 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() + if len(g.outputFiles) > 0 { + g.outputDeps = append(g.outputDeps, g.outputFiles[0]) + } } -func (g *Module) generateSourceFile(ctx android.ModuleContext, task generateTask) { +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") @@ -403,9 +453,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, @@ -420,11 +474,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) - } - g.outputDeps = append(g.outputDeps, task.out[0]) } // Collect information for opening IDE project files in java/jdeps.go. @@ -446,7 +495,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) { @@ -483,47 +532,80 @@ 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) - - sandboxOutfile := pathToSandboxOut(outFile, genDir) - sandboxOuts = append(sandboxOuts, sandboxOutfile) - - command, err := android.Expand(rawCommand, func(name string) (string, error) { - switch name { - case "in": - return in.String(), nil - case "out": - return sandboxOutfile, nil - default: - return "$(" + name + ")", nil - } - }) - if err != nil { - ctx.PropertyErrorf("cmd", err.Error()) + 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 } - // escape the command in case for example it contains '#', an odd number of '"', etc - command = fmt.Sprintf("bash -c %v", proptools.ShellEscape(command)) - commands = append(commands, command) - } - fullCommand := strings.Join(commands, " && ") + for _, in := range shard { + outFile := android.GenPathWithExt(ctx, "gensrcs", in, String(properties.Output_extension)) + sandboxOutfile := pathToSandboxOut(outFile, genDir) - return generateTask{ - in: srcFiles, - out: outFiles, - sandboxOuts: sandboxOuts, - cmd: fullCommand, + 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) { + switch name { + case "in": + return in.String(), nil + case "out": + return sandboxOutfile, nil + default: + return "$(" + name + ")", nil + } + }) + if err != nil { + ctx.PropertyErrorf("cmd", err.Error()) + } + + // escape the command in case for example it contains '#', an odd number of '"', etc + command = fmt.Sprintf("bash -c %v", proptools.ShellEscape(command)) + commands = append(commands, command) + } + fullCommand := strings.Join(commands, " && ") + + 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 { @@ -535,12 +617,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) @@ -548,12 +635,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) diff --git a/genrule/genrule_test.go b/genrule/genrule_test.go index 0b6952f30..e8dc3b55b 100644 --- a/genrule/genrule_test.go +++ b/genrule/genrule_test.go @@ -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"}, + 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"}, + 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"} |