Skip to content

Commit de4bd6c

Browse files
feat: Speed up bob graph generation hot paths
Batch dependency-sort reachability checks, avoid reflection and channel overhead in feature and flag hot paths, and skip template parsing for plain strings. In the codex-bob-trace-final build directory this reduces build.ninja generation from the post-struct-cache ~28s range to ~9s in repeated measurements. (cherry picked from commit 06e2afbbfaf0027e764ca96762b4dcb005808c8e) Change-Id: Ib74ce22b4f7a9d155044bdf367ddee17b950d316
1 parent d0f6d18 commit de4bd6c

7 files changed

Lines changed: 116 additions & 34 deletions

File tree

‎core/dep_sorter.go‎

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -66,14 +66,17 @@ func (handler *graphMutatorHandler) ResolveDependencySortMutator(ctx blueprint.B
6666

6767
temporaryPaths := map[string][]string{} // For preserving order in declaration
6868

69-
for i, previous := range mainBuild.Static_libs {
70-
for j := i + 1; j < len(mainBuild.Static_libs); j++ {
71-
lib := mainBuild.Static_libs[j]
72-
if !g.IsReachable(lib, previous) {
73-
if g.AddEdge(previous, lib) {
74-
temporaryPaths[previous] = append(temporaryPaths[previous], lib)
75-
g.SetEdgeColor(previous, lib, "pink")
76-
}
69+
for j := 1; j < len(mainBuild.Static_libs); j++ {
70+
lib := mainBuild.Static_libs[j]
71+
reachableFromLib := graph.GetSubgraphNodeSet(g, lib)
72+
for i := 0; i < j; i++ {
73+
previous := mainBuild.Static_libs[i]
74+
if reachableFromLib[previous] {
75+
continue
76+
}
77+
if g.AddEdge(previous, lib) {
78+
temporaryPaths[previous] = append(temporaryPaths[previous], lib)
79+
g.SetEdgeColor(previous, lib, "pink")
7780
}
7881
}
7982
}

‎core/feature.go‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -229,14 +229,20 @@ func coalesceTypes(list ...reflect.Type) reflect.Type {
229229
func (f *Features) AppendProps(dst []interface{}, properties *config.Properties) error {
230230
// featuresData is struct created in Features.Init function
231231
featuresData := reflect.ValueOf(f.BlueprintEmbed).Elem()
232+
featuresType := featuresData.Type()
232233

233-
for _, featureKey := range properties.FeatureList {
234+
for i, featureKey := range properties.FeatureList {
234235
if properties.Features[featureKey] { // Check the feature is enabled
235-
// Features are matched like "Feature_name" - feature structure
236-
featureFieldName := featurePropertyName(featureKey)
237-
featureStruct := featuresData.FieldByName(featureFieldName)
238-
if !featureStruct.IsValid() {
239-
utils.Die("Field returned for property %s isn't valid\n", featureFieldName)
236+
if i >= featuresData.NumField() {
237+
utils.Die("Field returned for property %s isn't valid\n", featureKey)
238+
}
239+
240+
// featureStructType creates fields in FeatureList order. Indexing avoids
241+
// a reflective name lookup for every enabled feature on every module.
242+
featureStruct := featuresData.Field(i)
243+
expectedFieldName := featurePropertyName(featureKey)
244+
if featuresType.Field(i).Name != expectedFieldName {
245+
utils.Die("Field returned for property %s isn't valid\n", expectedFieldName)
240246
}
241247

242248
// If featureProps is nil then we've determined that we can skip this,

‎core/flag/flag_collection.go‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,9 @@ package flag
55
type Flags []Flag
66

77
func (fs Flags) Contains(query Flag) bool {
8+
queryString := query.ToString()
89
for _, f := range fs {
9-
if f.ToString() == query.ToString() {
10+
if f.ToString() == queryString {
1011
return true
1112
}
1213
}
@@ -83,14 +84,16 @@ func (fs Flags) IteratePredicate(predicate func(Flag) bool) <-chan Flag {
8384
}
8485

8586
func (fs Flags) ForEach(functor func(Flag)) {
86-
for f := range fs.Iterate() {
87+
for _, f := range fs {
8788
functor(f)
8889
}
8990
}
9091

9192
func (fs Flags) ForEachIf(predicate func(Flag) bool, functor func(Flag)) {
92-
for f := range fs.IteratePredicate(predicate) {
93-
functor(f)
93+
for _, f := range fs {
94+
if predicate(f) {
95+
functor(f)
96+
}
9497
}
9598
}
9699

‎core/flag/flag_consumer.go‎

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,15 @@ func ReferenceFlagsInTransitive(ctx blueprint.BaseModuleContext) (ret Flags) {
2222
// * Transitive exports. These make `reexport_libs` obsolete.
2323

2424
visited := map[string]bool{}
25+
addedFlags := map[string]bool{}
26+
appendIfUnique := func(f Flag) {
27+
flagString := f.ToString()
28+
if addedFlags[flagString] {
29+
return
30+
}
31+
addedFlags[flagString] = true
32+
ret = append(ret, f)
33+
}
2534

2635
ctx.VisitDirectDeps(func(child blueprint.Module) {
2736
if visited[child.Name()] {
@@ -36,9 +45,7 @@ func ReferenceFlagsInTransitive(ctx blueprint.BaseModuleContext) (ret Flags) {
3645
},
3746
)
3847

39-
flags.ForEach(func(f Flag) {
40-
ret = ret.AppendIfUnique(f)
41-
})
48+
flags.ForEach(appendIfUnique)
4249
}
4350
})
4451

@@ -55,9 +62,7 @@ func ReferenceFlagsInTransitive(ctx blueprint.BaseModuleContext) (ret Flags) {
5562
},
5663
)
5764

58-
flags.ForEach(func(f Flag) {
59-
ret = ret.AppendIfUnique(f)
60-
})
65+
flags.ForEach(appendIfUnique)
6166
}
6267

6368
return true

‎core/flag/flag_property_parser.go‎

Lines changed: 53 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package flag
22

33
import (
44
"reflect"
5+
"sync"
56

67
"github.com/google/blueprint"
78
)
@@ -14,13 +15,61 @@ type FlagParserTableEntry struct {
1415

1516
type FlagParserTable []FlagParserTableEntry
1617

18+
var fieldIndexCache = struct {
19+
sync.RWMutex
20+
indexes map[fieldIndexCacheKey][]int
21+
}{
22+
indexes: make(map[fieldIndexCacheKey][]int),
23+
}
24+
25+
type fieldIndexCacheKey struct {
26+
typ reflect.Type
27+
name string
28+
}
29+
30+
func concreteValue(v reflect.Value) reflect.Value {
31+
for v.Kind() == reflect.Interface || v.Kind() == reflect.Ptr {
32+
v = v.Elem()
33+
}
34+
return v
35+
}
36+
37+
func cachedFieldByName(v reflect.Value, name string) reflect.Value {
38+
v = concreteValue(v)
39+
key := fieldIndexCacheKey{
40+
typ: v.Type(),
41+
name: name,
42+
}
43+
44+
fieldIndexCache.RLock()
45+
index, ok := fieldIndexCache.indexes[key]
46+
fieldIndexCache.RUnlock()
47+
if !ok {
48+
field, found := key.typ.FieldByName(name)
49+
if !found {
50+
panic("missing flag property field: " + name)
51+
}
52+
index = append([]int(nil), field.Index...)
53+
54+
fieldIndexCache.Lock()
55+
if cached, ok := fieldIndexCache.indexes[key]; ok {
56+
index = cached
57+
} else {
58+
fieldIndexCache.indexes[key] = index
59+
}
60+
fieldIndexCache.Unlock()
61+
}
62+
63+
return v.FieldByIndex(index)
64+
}
65+
1766
// Helper method to scrape many properties from a module struct.
1867
func ParseFromProperties(owner blueprint.Module, luts FlagParserTable, s interface{}) (ret Flags) {
68+
v := reflect.ValueOf(s)
1969
for _, entry := range luts {
20-
s := reflect.Indirect(reflect.ValueOf(&s))
21-
v := s.Elem().FieldByName(entry.PropertyName)
22-
for _, s := range v.Interface().([]string) {
23-
ret = append(ret, entry.Factory(s, owner, entry.Tag))
70+
values := cachedFieldByName(v, entry.PropertyName)
71+
for i := 0; i < values.Len(); i++ {
72+
ret = append(ret, entry.Factory(values.Index(i).String(), owner, entry.Tag))
2473
}
2574
}
2675
return

‎core/template.go‎

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -53,18 +53,23 @@ func applyTemplateString(elem reflect.Value, stringvalues map[string]string, fun
5353
utils.Die("elem is not a string")
5454
}
5555

56+
input := elem.String()
57+
if !strings.Contains(input, "{{") {
58+
return
59+
}
60+
5661
t := template.New("TemplateProps")
5762
t.Option("missingkey=error")
5863
t.Funcs(funcmap)
5964

60-
tmpl, err := t.Parse(elem.String())
65+
tmpl, err := t.Parse(input)
6166
if err != nil {
62-
utils.Die("Error parsing string '%s': %s", elem.String(), err.Error())
67+
utils.Die("Error parsing string '%s': %s", input, err.Error())
6368
}
6469
buf := new(bytes.Buffer)
6570
err = tmpl.Execute(buf, stringvalues)
6671
if err != nil {
67-
utils.Die("Error executing string '%s': %s", elem.String(), err.Error())
72+
utils.Die("Error executing string '%s': %s", input, err.Error())
6873
}
6974
elem.SetString(buf.String())
7075
}
@@ -86,10 +91,15 @@ func shlexExpand(field reflect.Value, stringvalues map[string]string) {
8691
newSlice := make([]string, 0, field.Len())
8792
for j := 0; j < field.Len(); j++ {
8893
elem := field.Index(j)
94+
input := elem.String()
95+
if !strings.Contains(input, "{{") {
96+
newSlice = append(newSlice, input)
97+
continue
98+
}
8999

90-
captures := shlexRegexpr.FindStringSubmatch(elem.String())
100+
captures := shlexRegexpr.FindStringSubmatch(input)
91101
if len(captures) == 0 {
92-
newSlice = append(newSlice, elem.String())
102+
newSlice = append(newSlice, input)
93103
continue
94104
}
95105

‎internal/graph/graph.go‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -656,13 +656,19 @@ func walkDownNoCopy(graph Graph, nodeID string, visited map[string]bool,
656656

657657
// A faster alternative to GetSubgraph(graph, start).GetNodeCount()
658658
func GetSubgraphNodeCount(graph Graph, start string) int {
659+
return len(GetSubgraphNodeSet(graph, start))
660+
}
661+
662+
// A faster alternative to GetSubgraph(graph, start).GetNodes() when only
663+
// membership queries are needed.
664+
func GetSubgraphNodeSet(graph Graph, start string) map[string]bool {
659665
visited := make(map[string]bool)
660666

661667
walkDownNoCopy(graph, start, visited,
662668
func(Graph, string, map[string]bool) bool { return false },
663669
func(Graph, string, string, map[string]bool) bool { return false })
664670

665-
return len(visited)
671+
return visited
666672
}
667673

668674
// A faster alternative to GetSubgraph(graph, start).HasNode(target)

0 commit comments

Comments
 (0)