Use phony ninja outputs to reduce file-size

1. scan if any set of order-only deps are repeated 2. if so extract them as a phony output to be shared Test: m libc Bug: NA Change-Id: I0689111b97bbbd1f3b26650e8ae2e0a4ffb5085e
2023-02-21 23:08:34 -05:00 · 2023-02-21 23:08:34 -05:00 · 2bae13b095
commit 2bae13b095
parent ad7bcc7b50
3 changed files with 289 additions and 57 deletions
--- a/context.go
+++ b/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
+	return err
 }

 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 {

--- a/context_test.go
+++ b/context_test.go
@ -1131,27 +1131,144 @@ func TestPackageIncludes(t *testing.T) {
 		},
 	}
 	for _, tc := range testCases {
-		ctx := NewContext()
-		// Register mock FS
-		ctx.MockFileSystem(mockFs)
-		// Register module types
-		ctx.RegisterModuleType("foo_module", newFooModule)
-		RegisterPackageIncludesModuleType(ctx)
-		// Add include tags for test case
-		ctx.AddIncludeTags(tc.includeTags...)
-		// Run test
-		_, actualErrs := ctx.ParseFileList(".", []string{"dir1/Android.bp", "dir2/Android.bp"}, nil)
-		// Evaluate
-		if !strings.Contains(fmt.Sprintf("%s", actualErrs), fmt.Sprintf("%s", tc.expectedErr)) {
-			t.Errorf("Expected errors: %s, got errors: %s\n", tc.expectedErr, actualErrs)
-		}
-		if tc.expectedErr != "" {
-			continue // 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))
-		}
+		t.Run(tc.desc, func(t *testing.T) {
+			ctx := NewContext()
+			// Register mock FS
+			ctx.MockFileSystem(mockFs)
+			// Register module types
+			ctx.RegisterModuleType("foo_module", newFooModule)
+			RegisterPackageIncludesModuleType(ctx)
+			// Add include tags for test case
+			ctx.AddIncludeTags(tc.includeTags...)
+			// Run test
+			_, actualErrs := ctx.ParseFileList(".", []string{"dir1/Android.bp", "dir2/Android.bp"}, nil)
+			// Evaluate
+			if !strings.Contains(fmt.Sprintf("%s", actualErrs), fmt.Sprintf("%s", tc.expectedErr)) {
+				t.Errorf("Expected errors: %s, got errors: %s\n", tc.expectedErr, actualErrs)
+			}
+			if tc.expectedErr != "" {
+				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)
+				}
+			}
+		})
+	}
+}
--- a/go.mod
+++ b/go.mod
@ -1,3 +1,3 @@
 module github.com/google/blueprint

-go 1.13
+go 1.18