Use phony ninja outputs to reduce file-size am: 2bae13b095

Original change: https://android-review.googlesource.com/c/platform/build/blueprint/+/2449271

Change-Id: Id6d6187eef662ab628c268caa9b26391df075da5
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>

context.go

@@ -3999,59 +3999,46 @@ func (c *Context) WriteBuildFile(w io.StringWriter) error {
 
 		nw := newNinjaWriter(w)
 
-		err = c.writeBuildFileHeader(nw)
-		if err != nil {
+		if err = c.writeBuildFileHeader(nw); err != nil {
 			return
 		}
 
-		err = c.writeNinjaRequiredVersion(nw)
-		if err != nil {
+		if err = c.writeNinjaRequiredVersion(nw); err != nil {
 			return
 		}
 
-		err = c.writeSubninjas(nw)
-		if err != nil {
+		if err = c.writeSubninjas(nw); err != nil {
 			return
 		}
 
 		// TODO: Group the globals by package.
 
-		err = c.writeGlobalVariables(nw)
-		if err != nil {
+		if err = c.writeGlobalVariables(nw); err != nil {
 			return
 		}
 
-		err = c.writeGlobalPools(nw)
-		if err != nil {
+		if err = c.writeGlobalPools(nw); err != nil {
 			return
 		}
 
-		err = c.writeBuildDir(nw)
-		if err != nil {
+		if err = c.writeBuildDir(nw); err != nil {
 			return
 		}
 
-		err = c.writeGlobalRules(nw)
-		if err != nil {
+		if err = c.writeGlobalRules(nw); err != nil {
 			return
 		}
 
-		err = c.writeAllModuleActions(nw)
-		if err != nil {
+		if err = c.writeAllModuleActions(nw); err != nil {
 			return
 		}
 
-		err = c.writeAllSingletonActions(nw)
-		if err != nil {
+		if err = c.writeAllSingletonActions(nw); err != nil {
 			return
 		}
 	})
 
-	if err != nil {
 	return err
-	}
-
-	return nil
 }
 
 type pkgAssociation struct {
@@ -4332,9 +4319,10 @@ func (s moduleSorter) Swap(i, j int) {
 }
 
 func (c *Context) writeAllModuleActions(nw *ninjaWriter) error {
+	c.BeginEvent("modules")
+	defer c.EndEvent("modules")
 	headerTemplate := template.New("moduleHeader")
-	_, err := headerTemplate.Parse(moduleHeaderTemplate)
-	if err != nil {
+	if _, err := headerTemplate.Parse(moduleHeaderTemplate); err != nil {
 		// This is a programming error.
 		panic(err)
 	}
@@ -4345,6 +4333,11 @@ func (c *Context) writeAllModuleActions(nw *ninjaWriter) error {
 	}
 	sort.Sort(moduleSorter{modules, c.nameInterface})
 
+	phonys := c.extractPhonys(modules)
+	if err := c.writeLocalBuildActions(nw, phonys); err != nil {
+		return err
+	}
+
 	buf := bytes.NewBuffer(nil)
 
 	for _, module := range modules {
@@ -4371,28 +4364,23 @@ func (c *Context) writeAllModuleActions(nw *ninjaWriter) error {
 			"pos":       relPos,
 			"variant":   module.variant.name,
 		}
-		err = headerTemplate.Execute(buf, infoMap)
-		if err != nil {
+		if err := headerTemplate.Execute(buf, infoMap); err != nil {
 			return err
 		}
 
-		err = nw.Comment(buf.String())
-		if err != nil {
+		if err := nw.Comment(buf.String()); err != nil {
 			return err
 		}
 
-		err = nw.BlankLine()
-		if err != nil {
+		if err := nw.BlankLine(); err != nil {
 			return err
 		}
 
-		err = c.writeLocalBuildActions(nw, &module.actionDefs)
-		if err != nil {
+		if err := c.writeLocalBuildActions(nw, &module.actionDefs); err != nil {
 			return err
 		}
 
-		err = nw.BlankLine()
-		if err != nil {
+		if err := nw.BlankLine(); err != nil {
 			return err
 		}
 	}
@@ -4401,6 +4389,8 @@ func (c *Context) writeAllModuleActions(nw *ninjaWriter) error {
 }
 
 func (c *Context) writeAllSingletonActions(nw *ninjaWriter) error {
+	c.BeginEvent("singletons")
+	defer c.EndEvent("singletons")
 	headerTemplate := template.New("singletonHeader")
 	_, err := headerTemplate.Parse(singletonHeaderTemplate)
 	if err != nil {
@@ -4470,6 +4460,131 @@ func (c *Context) SetBeforePrepareBuildActionsHook(hookFn func() error) {
 	c.BeforePrepareBuildActionsHook = hookFn
 }
 
+// phonyCandidate represents the state of a set of deps that decides its eligibility
+// to be extracted as a phony output
+type phonyCandidate struct {
+	sync.Mutex
+	frequency int       // the number of buildDef instances that use this set
+	phony     *buildDef // the phony buildDef that wraps the set
+	first     *buildDef // the first buildDef that uses this set
+	key       string    // a unique identifier for the set
+}
+
+func (c *phonyCandidate) less(other *phonyCandidate) bool {
+	if c.frequency == other.frequency {
+		if len(c.phony.OrderOnly) == len(other.phony.OrderOnly) {
+			return c.key < other.key
+		}
+		return len(c.phony.OrderOnly) < len(other.phony.OrderOnly)
+	}
+	return c.frequency < other.frequency
+}
+
+// keyForPhonyCandidate gives a unique identifier for a set of deps.
+// We are not using hash because string concatenation proved cheaper.
+// If any of the deps use a variable, we return an empty string to signal
+// that this set of deps is ineligible for extraction.
+func keyForPhonyCandidate(deps []ninjaString) string {
+	s := make([]string, len(deps))
+	for i, d := range deps {
+		if len(d.Variables()) != 0 {
+			return ""
+		}
+		s[i] = d.Value(nil)
+	}
+	return strings.Join(s, "\n")
+}
+
+// scanBuildDef is called for every known buildDef `b` that has a non-empty `b.OrderOnly`.
+// If `b.OrderOnly` is not present in `candidates`, it gets stored.
+// But if `b.OrderOnly` already exists in `candidates`, then `b.OrderOnly`
+// (and phonyCandidate#first.OrderOnly) will be replaced with phonyCandidate#phony.Outputs
+func scanBuildDef(wg *sync.WaitGroup, candidates *sync.Map, phonyCount *atomic.Uint32, b *buildDef) {
+	defer wg.Done()
+	key := keyForPhonyCandidate(b.OrderOnly)
+	if key == "" {
+		return
+	}
+	if v, loaded := candidates.LoadOrStore(key, &phonyCandidate{
+		frequency: 1,
+		first:     b,
+		key:       key,
+	}); loaded {
+		m := v.(*phonyCandidate)
+		func() {
+			m.Lock()
+			defer m.Unlock()
+			if m.frequency == 1 {
+				// this is the second occurrence and hence it makes sense to
+				// extract it as a phony output
+				phonyCount.Add(1)
+				m.phony = &buildDef{
+					Rule: Phony,
+					// We are using placeholder because we don't have a deterministic
+					// name for the phony output; m.key is unique and could be used but
+					// it's rather long (and has characters we would need to escape)
+					Outputs:  make([]ninjaString, 1),
+					Inputs:   m.first.OrderOnly, //we could also use b.OrderOnly
+					Optional: true,
+				}
+				// the previously recorded build-def, which first had these deps as its
+				// order-only deps, should now use this phony output instead
+				m.first.OrderOnly = m.phony.Outputs
+				m.first = nil
+			}
+			m.frequency += 1
+			b.OrderOnly = m.phony.Outputs
+		}()
+	}
+}
+
+// extractPhonys searches for common sets of order-only dependencies across all
+// buildDef instances in the provided moduleInfo instances. Each such
+// common set forms a new buildDef representing a phony output that then becomes
+// the sole order-only dependency of those buildDef instances
+func (c *Context) extractPhonys(infos []*moduleInfo) *localBuildActions {
+	c.BeginEvent("extract_phonys")
+	defer c.EndEvent("extract_phonys")
+
+	candidates := sync.Map{} //used as map[key]*candidate
+	phonyCount := atomic.Uint32{}
+	wg := sync.WaitGroup{}
+	for _, info := range infos {
+		for _, b := range info.actionDefs.buildDefs {
+			if len(b.OrderOnly) > 0 {
+				wg.Add(1)
+				go scanBuildDef(&wg, &candidates, &phonyCount, b)
+			}
+		}
+	}
+	wg.Wait()
+
+	//now filter candidates with freq > 1
+	phonys := make([]*phonyCandidate, 0, phonyCount.Load())
+	candidates.Range(func(_ any, v any) bool {
+		candidate := v.(*phonyCandidate)
+		if candidate.frequency > 1 {
+			phonys = append(phonys, candidate)
+		}
+		return true
+	})
+
+	phonyBuildDefs := make([]*buildDef, len(phonys))
+	c.EventHandler.Do("name", func() {
+		// sorting for determinism
+		sort.Slice(phonys, func(i int, j int) bool {
+			return phonys[i].less(phonys[j])
+		})
+		for index, p := range phonys {
+			// use the index to set the name for the phony output
+			p.phony.Outputs[0] = literalNinjaString(fmt.Sprintf("phony-%d", index))
+			phonyBuildDefs[index] = p.phony
+		}
+	})
+
+	return &localBuildActions{buildDefs: phonyBuildDefs}
+}
+
 func (c *Context) writeLocalBuildActions(nw *ninjaWriter,
 	defs *localBuildActions) error {
 

context_test.go

@@ -1131,6 +1131,7 @@ func TestPackageIncludes(t *testing.T) {
 		},
 	}
 	for _, tc := range testCases {
+		t.Run(tc.desc, func(t *testing.T) {
 			ctx := NewContext()
 			// Register mock FS
 			ctx.MockFileSystem(mockFs)
@@ -1146,12 +1147,128 @@ func TestPackageIncludes(t *testing.T) {
 				t.Errorf("Expected errors: %s, got errors: %s\n", tc.expectedErr, actualErrs)
 			}
 			if tc.expectedErr != "" {
-			continue // expectedDir check not necessary
+				return // expectedDir check not necessary
 			}
 			actualBpFile := ctx.moduleGroupFromName("foo", nil).modules.firstModule().relBlueprintsFile
 			if tc.expectedDir != filepath.Dir(actualBpFile) {
 				t.Errorf("Expected foo from %s, got %s\n", tc.expectedDir, filepath.Dir(actualBpFile))
 			}
+		})
 	}
 
 }
+
+func TestExtractPhonys(t *testing.T) {
+	outputs := func(names ...string) []ninjaString {
+		r := make([]ninjaString, len(names))
+		for i, name := range names {
+			r[i] = literalNinjaString(name)
+		}
+		return r
+	}
+	b := func(output string, inputs []string, orderOnlyDeps []string) *buildDef {
+		return &buildDef{
+			Outputs:   outputs(output),
+			Inputs:    outputs(inputs...),
+			OrderOnly: outputs(orderOnlyDeps...),
+		}
+	}
+	m := func(bs ...*buildDef) *moduleInfo {
+		return &moduleInfo{actionDefs: localBuildActions{buildDefs: bs}}
+	}
+	type testcase struct {
+		modules        []*moduleInfo
+		expectedPhonys []*buildDef
+		conversions    map[string][]ninjaString
+	}
+	testCases := []testcase{{
+		modules: []*moduleInfo{
+			m(b("A", nil, []string{"d"})),
+			m(b("B", nil, []string{"d"})),
+		},
+		expectedPhonys: []*buildDef{
+			b("phony-0", []string{"d"}, nil),
+		},
+		conversions: map[string][]ninjaString{
+			"A": outputs("phony-0"),
+			"B": outputs("phony-0"),
+		},
+	}, {
+		modules: []*moduleInfo{
+			m(b("A", nil, []string{"a"})),
+			m(b("B", nil, []string{"b"})),
+		},
+	}, {
+		modules: []*moduleInfo{
+			m(b("A", nil, []string{"a"})),
+			m(b("B", nil, []string{"b"})),
+			m(b("C", nil, []string{"a"})),
+		},
+		expectedPhonys: []*buildDef{b("phony-0", []string{"a"}, nil)},
+		conversions: map[string][]ninjaString{
+			"A": outputs("phony-0"),
+			"B": outputs("b"),
+			"C": outputs("phony-0"),
+		},
+	}, {
+		modules: []*moduleInfo{
+			m(b("A", nil, []string{"a", "b"}),
+				b("B", nil, []string{"a", "b"})),
+			m(b("C", nil, []string{"a", "c"}),
+				b("D", nil, []string{"a", "c"})),
+		},
+		expectedPhonys: []*buildDef{
+			b("phony-0", []string{"a", "b"}, nil),
+			b("phony-1", []string{"a", "c"}, nil)},
+		conversions: map[string][]ninjaString{
+			"A": outputs("phony-0"),
+			"B": outputs("phony-0"),
+			"C": outputs("phony-1"),
+			"D": outputs("phony-1"),
+		},
+	}}
+	for index, tc := range testCases {
+		t.Run(fmt.Sprintf("TestCase-%d", index), func(t *testing.T) {
+			ctx := NewContext()
+			actualPhonys := ctx.extractPhonys(tc.modules)
+			if len(actualPhonys.variables) != 0 {
+				t.Errorf("No variables expected but found %v", actualPhonys.variables)
+			}
+			if len(actualPhonys.rules) != 0 {
+				t.Errorf("No rules expected but found %v", actualPhonys.rules)
+			}
+			if e, a := len(tc.expectedPhonys), len(actualPhonys.buildDefs); e != a {
+				t.Errorf("Expected %d build statements but got %d", e, a)
+			}
+			for i := 0; i < len(tc.expectedPhonys); i++ {
+				a := actualPhonys.buildDefs[i]
+				e := tc.expectedPhonys[i]
+				if !reflect.DeepEqual(e.Outputs, a.Outputs) {
+					t.Errorf("phonys expected %v but actualPhonys %v", e.Outputs, a.Outputs)
+				}
+				if !reflect.DeepEqual(e.Inputs, a.Inputs) {
+					t.Errorf("phonys expected %v but actualPhonys %v", e.Inputs, a.Inputs)
+				}
+			}
+			find := func(k string) *buildDef {
+				for _, m := range tc.modules {
+					for _, b := range m.actionDefs.buildDefs {
+						if reflect.DeepEqual(b.Outputs, outputs(k)) {
+							return b
+						}
+					}
+				}
+				return nil
+			}
+			for k, conversion := range tc.conversions {
+				actual := find(k)
+				if actual == nil {
+					t.Errorf("Couldn't find %s", k)
+				}
+				if !reflect.DeepEqual(actual.OrderOnly, conversion) {
+					t.Errorf("expected %s.OrderOnly = %v but got %v", k, conversion, actual.OrderOnly)
+				}
+			}
+		})
+	}
+}

go.mod

@@ -1,3 +1,3 @@
 module github.com/google/blueprint
 
-go 1.13
+go 1.18