diff --git a/aidl_library/aidl_library.go b/aidl_library/aidl_library.go index 598510386..7449d678c 100644 --- a/aidl_library/aidl_library.go +++ b/aidl_library/aidl_library.go @@ -108,17 +108,17 @@ type AidlLibraryInfo struct { // The direct aidl files of the module Srcs android.Paths // The include dirs to the direct aidl files and those provided from transitive aidl_library deps - IncludeDirs android.DepSet + IncludeDirs android.DepSet[android.Path] // The direct hdrs and hdrs from transitive deps - Hdrs android.DepSet + Hdrs android.DepSet[android.Path] } // AidlLibraryProvider provides the srcs and the transitive include dirs var AidlLibraryProvider = blueprint.NewProvider(AidlLibraryInfo{}) func (lib *AidlLibrary) GenerateAndroidBuildActions(ctx android.ModuleContext) { - includeDirsDepSetBuilder := android.NewDepSetBuilder(android.PREORDER) - hdrsDepSetBuilder := android.NewDepSetBuilder(android.PREORDER) + includeDirsDepSetBuilder := android.NewDepSetBuilder[android.Path](android.PREORDER) + hdrsDepSetBuilder := android.NewDepSetBuilder[android.Path](android.PREORDER) if len(lib.properties.Srcs) == 0 && len(lib.properties.Hdrs) == 0 { ctx.ModuleErrorf("at least srcs or hdrs prop must be non-empty") diff --git a/aidl_library/aidl_library_test.go b/aidl_library/aidl_library_test.go index d9dd24515..020562904 100644 --- a/aidl_library/aidl_library_test.go +++ b/aidl_library/aidl_library_test.go @@ -52,7 +52,7 @@ func TestAidlLibrary(t *testing.T) { t, "aidl include dirs", []string{"package_foo/a", "package_bar/x"}, - actualInfo.IncludeDirs.ToList().Strings(), + android.Paths(actualInfo.IncludeDirs.ToList()).Strings(), ) android.AssertPathsRelativeToTopEquals( @@ -101,7 +101,7 @@ func TestAidlLibraryWithoutStripImportPrefix(t *testing.T) { t, "aidl include dirs", []string{"package_foo", "package_bar"}, - actualInfo.IncludeDirs.ToList().Strings(), + android.Paths(actualInfo.IncludeDirs.ToList()).Strings(), ) android.AssertPathsRelativeToTopEquals( diff --git a/android/Android.bp b/android/Android.bp index 94d2c04f6..0ebc29b60 100644 --- a/android/Android.bp +++ b/android/Android.bp @@ -48,7 +48,6 @@ bootstrap_go_package { "defaults.go", "defs.go", "depset_generic.go", - "depset_paths.go", "deptag.go", "expand.go", "filegroup.go", diff --git a/android/depset_generic.go b/android/depset_generic.go index f00e462c2..ae14d3271 100644 --- a/android/depset_generic.go +++ b/android/depset_generic.go @@ -16,10 +16,9 @@ package android import ( "fmt" - "reflect" ) -// depSet is designed to be conceptually compatible with Bazel's depsets: +// DepSet is designed to be conceptually compatible with Bazel's depsets: // https://docs.bazel.build/versions/master/skylark/depsets.html type DepSetOrder int @@ -43,142 +42,114 @@ func (o DepSetOrder) String() string { } } -// A depSet efficiently stores a slice of an arbitrary type from transitive dependencies without -// copying. It is stored as a DAG of depSet nodes, each of which has some direct contents and a list -// of dependency depSet nodes. +type depSettableType comparable + +// A DepSet efficiently stores a slice of an arbitrary type from transitive dependencies without +// copying. It is stored as a DAG of DepSet nodes, each of which has some direct contents and a list +// of dependency DepSet nodes. // -// A depSet has an order that will be used to walk the DAG when ToList() is called. The order +// A DepSet has an order that will be used to walk the DAG when ToList() is called. The order // can be POSTORDER, PREORDER, or TOPOLOGICAL. POSTORDER and PREORDER orders return a postordered // or preordered left to right flattened list. TOPOLOGICAL returns a list that guarantees that // elements of children are listed after all of their parents (unless there are duplicate direct -// elements in the depSet or any of its transitive dependencies, in which case the ordering of the +// elements in the DepSet or any of its transitive dependencies, in which case the ordering of the // duplicated element is not guaranteed). // -// A depSet is created by newDepSet or newDepSetBuilder.Build from the slice for direct contents -// and the *depSets of dependencies. A depSet is immutable once created. -// -// This object uses reflection to remain agnostic to the type it contains. It should be replaced -// with generics once those exist in Go. Callers should generally use a thin wrapper around depSet -// that provides type-safe methods like DepSet for Paths. -type depSet struct { +// A DepSet is created by NewDepSet or NewDepSetBuilder.Build from the slice for direct contents +// and the *DepSets of dependencies. A DepSet is immutable once created. +type DepSet[T depSettableType] struct { preorder bool reverse bool order DepSetOrder - direct interface{} - transitive []*depSet + direct []T + transitive []*DepSet[T] } -type depSetInterface interface { - embeddedDepSet() *depSet -} - -func (d *depSet) embeddedDepSet() *depSet { - return d -} - -var _ depSetInterface = (*depSet)(nil) - -// newDepSet returns an immutable depSet with the given order, direct and transitive contents. -// direct must be a slice, but is not type-safe due to the lack of generics in Go. It can be a -// nil slice, but not a nil interface{}, i.e. []string(nil) but not nil. -func newDepSet(order DepSetOrder, direct interface{}, transitive interface{}) *depSet { - var directCopy interface{} - transitiveDepSet := sliceToDepSets(transitive, order) - - if order == TOPOLOGICAL { - directCopy = reverseSlice(direct) - reverseSliceInPlace(transitiveDepSet) - } else { - directCopy = copySlice(direct) +// NewDepSet returns an immutable DepSet with the given order, direct and transitive contents. +func NewDepSet[T depSettableType](order DepSetOrder, direct []T, transitive []*DepSet[T]) *DepSet[T] { + var directCopy []T + var transitiveCopy []*DepSet[T] + for _, t := range transitive { + if t.order != order { + panic(fmt.Errorf("incompatible order, new DepSet is %s but transitive DepSet is %s", + order, t.order)) + } } - return &depSet{ + if order == TOPOLOGICAL { + // TOPOLOGICAL is implemented as a postorder traversal followed by reversing the output. + // Pre-reverse the inputs here so their order is maintained in the output. + directCopy = reverseSlice(direct) + transitiveCopy = reverseSlice(transitive) + } else { + directCopy = append([]T(nil), direct...) + transitiveCopy = append([]*DepSet[T](nil), transitive...) + } + + return &DepSet[T]{ preorder: order == PREORDER, reverse: order == TOPOLOGICAL, order: order, direct: directCopy, - transitive: transitiveDepSet, + transitive: transitiveCopy, } } -// depSetBuilder is used to create an immutable depSet. -type depSetBuilder struct { +// DepSetBuilder is used to create an immutable DepSet. +type DepSetBuilder[T depSettableType] struct { order DepSetOrder - direct reflect.Value - transitive []*depSet + direct []T + transitive []*DepSet[T] } -// newDepSetBuilder returns a depSetBuilder to create an immutable depSet with the given order and -// type, represented by a slice of type that will be in the depSet. -func newDepSetBuilder(order DepSetOrder, typ interface{}) *depSetBuilder { - empty := reflect.Zero(reflect.TypeOf(typ)) - return &depSetBuilder{ - order: order, - direct: empty, +// NewDepSetBuilder returns a DepSetBuilder to create an immutable DepSet with the given order and +// type, represented by a slice of type that will be in the DepSet. +func NewDepSetBuilder[T depSettableType](order DepSetOrder) *DepSetBuilder[T] { + return &DepSetBuilder[T]{ + order: order, } } -// sliceToDepSets converts a slice of any type that implements depSetInterface (by having a depSet -// embedded in it) into a []*depSet. -func sliceToDepSets(in interface{}, order DepSetOrder) []*depSet { - slice := reflect.ValueOf(in) - length := slice.Len() - out := make([]*depSet, length) - for i := 0; i < length; i++ { - vi := slice.Index(i) - depSetIntf, ok := vi.Interface().(depSetInterface) - if !ok { - panic(fmt.Errorf("element %d is a %s, not a depSetInterface", i, vi.Type())) - } - depSet := depSetIntf.embeddedDepSet() - if depSet.order != order { - panic(fmt.Errorf("incompatible order, new depSet is %s but transitive depSet is %s", - order, depSet.order)) - } - out[i] = depSet - } - return out -} - -// DirectSlice adds direct contents to the depSet being built by a depSetBuilder. Newly added direct -// contents are to the right of any existing direct contents. The argument must be a slice, but -// is not type-safe due to the lack of generics in Go. -func (b *depSetBuilder) DirectSlice(direct interface{}) *depSetBuilder { - b.direct = reflect.AppendSlice(b.direct, reflect.ValueOf(direct)) +// DirectSlice adds direct contents to the DepSet being built by a DepSetBuilder. Newly added direct +// contents are to the right of any existing direct contents. +func (b *DepSetBuilder[T]) DirectSlice(direct []T) *DepSetBuilder[T] { + b.direct = append(b.direct, direct...) return b } -// Direct adds direct contents to the depSet being built by a depSetBuilder. Newly added direct -// contents are to the right of any existing direct contents. The argument must be the same type -// as the element of the slice passed to newDepSetBuilder, but is not type-safe due to the lack of -// generics in Go. -func (b *depSetBuilder) Direct(direct interface{}) *depSetBuilder { - b.direct = reflect.Append(b.direct, reflect.ValueOf(direct)) +// Direct adds direct contents to the DepSet being built by a DepSetBuilder. Newly added direct +// contents are to the right of any existing direct contents. +func (b *DepSetBuilder[T]) Direct(direct ...T) *DepSetBuilder[T] { + b.direct = append(b.direct, direct...) return b } // Transitive adds transitive contents to the DepSet being built by a DepSetBuilder. Newly added -// transitive contents are to the right of any existing transitive contents. The argument can -// be any slice of type that has depSet embedded in it. -func (b *depSetBuilder) Transitive(transitive interface{}) *depSetBuilder { - depSets := sliceToDepSets(transitive, b.order) - b.transitive = append(b.transitive, depSets...) +// transitive contents are to the right of any existing transitive contents. +func (b *DepSetBuilder[T]) Transitive(transitive ...*DepSet[T]) *DepSetBuilder[T] { + for _, t := range transitive { + if t.order != b.order { + panic(fmt.Errorf("incompatible order, new DepSet is %s but transitive DepSet is %s", + b.order, t.order)) + } + } + b.transitive = append(b.transitive, transitive...) return b } -// Returns the depSet being built by this depSetBuilder. The depSetBuilder retains its contents +// Returns the DepSet being built by this DepSetBuilder. The DepSetBuilder retains its contents // for creating more depSets. -func (b *depSetBuilder) Build() *depSet { - return newDepSet(b.order, b.direct.Interface(), b.transitive) +func (b *DepSetBuilder[T]) Build() *DepSet[T] { + return NewDepSet(b.order, b.direct, b.transitive) } // walk calls the visit method in depth-first order on a DepSet, preordered if d.preorder is set, // otherwise postordered. -func (d *depSet) walk(visit func(interface{})) { - visited := make(map[*depSet]bool) +func (d *DepSet[T]) walk(visit func([]T)) { + visited := make(map[*DepSet[T]]bool) - var dfs func(d *depSet) - dfs = func(d *depSet) { + var dfs func(d *DepSet[T]) + dfs = func(d *DepSet[T]) { visited[d] = true if d.preorder { visit(d.direct) @@ -197,155 +168,33 @@ func (d *depSet) walk(visit func(interface{})) { dfs(d) } -// ToList returns the depSet flattened to a list. The order in the list is based on the order -// of the depSet. POSTORDER and PREORDER orders return a postordered or preordered left to right +// ToList returns the DepSet flattened to a list. The order in the list is based on the order +// of the DepSet. POSTORDER and PREORDER orders return a postordered or preordered left to right // flattened list. TOPOLOGICAL returns a list that guarantees that elements of children are listed // after all of their parents (unless there are duplicate direct elements in the DepSet or any of // its transitive dependencies, in which case the ordering of the duplicated element is not // guaranteed). -// -// This method uses a reflection-based implementation to find the unique elements in slice, which -// is around 3x slower than a concrete implementation. Type-safe wrappers around depSet can -// provide their own implementation of ToList that calls depSet.toList with a method that -// uses a concrete implementation. -func (d *depSet) ToList() interface{} { - return d.toList(firstUnique) +func (d *DepSet[T]) ToList() []T { + return d.toList(firstUnique[T]) } -// toList returns the depSet flattened to a list. The order in the list is based on the order -// of the depSet. POSTORDER and PREORDER orders return a postordered or preordered left to right +// toList returns the DepSet flattened to a list. The order in the list is based on the order +// of the DepSet. POSTORDER and PREORDER orders return a postordered or preordered left to right // flattened list. TOPOLOGICAL returns a list that guarantees that elements of children are listed // after all of their parents (unless there are duplicate direct elements in the DepSet or any of // its transitive dependencies, in which case the ordering of the duplicated element is not // guaranteed). The firstUniqueFunc is used to remove duplicates from the list. -func (d *depSet) toList(firstUniqueFunc func(interface{}) interface{}) interface{} { +func (d *DepSet[T]) toList(firstUniqueFunc func([]T) []T) []T { if d == nil { return nil } - slice := reflect.Zero(reflect.TypeOf(d.direct)) - d.walk(func(paths interface{}) { - slice = reflect.AppendSlice(slice, reflect.ValueOf(paths)) + var list []T + d.walk(func(paths []T) { + list = append(list, paths...) }) - list := slice.Interface() list = firstUniqueFunc(list) if d.reverse { reverseSliceInPlace(list) } return list } - -// firstUnique returns all unique elements of a slice, keeping the first copy of each. It -// modifies the slice contents in place, and returns a subslice of the original slice. The -// argument must be a slice, but is not type-safe due to the lack of reflection in Go. -// -// Performance of the reflection-based firstUnique is up to 3x slower than a concrete type -// version such as FirstUniqueStrings. -func firstUnique(slice interface{}) interface{} { - // 4 was chosen based on Benchmark_firstUnique results. - if reflect.ValueOf(slice).Len() > 4 { - return firstUniqueMap(slice) - } - return firstUniqueList(slice) -} - -// firstUniqueList is an implementation of firstUnique using an O(N^2) list comparison to look for -// duplicates. -func firstUniqueList(in interface{}) interface{} { - writeIndex := 0 - slice := reflect.ValueOf(in) - length := slice.Len() -outer: - for readIndex := 0; readIndex < length; readIndex++ { - readValue := slice.Index(readIndex) - for compareIndex := 0; compareIndex < writeIndex; compareIndex++ { - compareValue := slice.Index(compareIndex) - // These two Interface() calls seem to cause an allocation and significantly - // slow down this list-based implementation. The map implementation below doesn't - // have this issue because reflect.Value.MapIndex takes a Value and appears to be - // able to do the map lookup without an allocation. - if readValue.Interface() == compareValue.Interface() { - // The value at readIndex already exists somewhere in the output region - // of the slice before writeIndex, skip it. - continue outer - } - } - if readIndex != writeIndex { - writeValue := slice.Index(writeIndex) - writeValue.Set(readValue) - } - writeIndex++ - } - return slice.Slice(0, writeIndex).Interface() -} - -var trueValue = reflect.ValueOf(true) - -// firstUniqueList is an implementation of firstUnique using an O(N) hash set lookup to look for -// duplicates. -func firstUniqueMap(in interface{}) interface{} { - writeIndex := 0 - slice := reflect.ValueOf(in) - length := slice.Len() - seen := reflect.MakeMapWithSize(reflect.MapOf(slice.Type().Elem(), trueValue.Type()), slice.Len()) - for readIndex := 0; readIndex < length; readIndex++ { - readValue := slice.Index(readIndex) - if seen.MapIndex(readValue).IsValid() { - continue - } - seen.SetMapIndex(readValue, trueValue) - if readIndex != writeIndex { - writeValue := slice.Index(writeIndex) - writeValue.Set(readValue) - } - writeIndex++ - } - return slice.Slice(0, writeIndex).Interface() -} - -// reverseSliceInPlace reverses the elements of a slice in place. The argument must be a slice, but -// is not type-safe due to the lack of reflection in Go. -func reverseSliceInPlace(in interface{}) { - swapper := reflect.Swapper(in) - slice := reflect.ValueOf(in) - length := slice.Len() - for i, j := 0, length-1; i < j; i, j = i+1, j-1 { - swapper(i, j) - } -} - -// reverseSlice returns a copy of a slice in reverse order. The argument must be a slice, but is -// not type-safe due to the lack of reflection in Go. -func reverseSlice(in interface{}) interface{} { - slice := reflect.ValueOf(in) - if !slice.IsValid() || slice.IsNil() { - return in - } - if slice.Kind() != reflect.Slice { - panic(fmt.Errorf("%t is not a slice", in)) - } - length := slice.Len() - if length == 0 { - return in - } - out := reflect.MakeSlice(slice.Type(), length, length) - for i := 0; i < length; i++ { - out.Index(i).Set(slice.Index(length - 1 - i)) - } - return out.Interface() -} - -// copySlice returns a copy of a slice. The argument must be a slice, but is not type-safe due to -// the lack of reflection in Go. -func copySlice(in interface{}) interface{} { - slice := reflect.ValueOf(in) - if !slice.IsValid() || slice.IsNil() { - return in - } - length := slice.Len() - if length == 0 { - return in - } - out := reflect.MakeSlice(slice.Type(), length, length) - reflect.Copy(out, slice) - return out.Interface() -} diff --git a/android/depset_paths.go b/android/depset_paths.go deleted file mode 100644 index ed561ba24..000000000 --- a/android/depset_paths.go +++ /dev/null @@ -1,94 +0,0 @@ -// Copyright 2020 Google Inc. All rights reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package android - -// This file implements DepSet, a thin type-safe wrapper around depSet that contains Paths. - -// A DepSet efficiently stores Paths from transitive dependencies without copying. It is stored -// as a DAG of DepSet nodes, each of which has some direct contents and a list of dependency -// DepSet nodes. -// -// A DepSet has an order that will be used to walk the DAG when ToList() is called. The order -// can be POSTORDER, PREORDER, or TOPOLOGICAL. POSTORDER and PREORDER orders return a postordered -// or preordered left to right flattened list. TOPOLOGICAL returns a list that guarantees that -// elements of children are listed after all of their parents (unless there are duplicate direct -// elements in the DepSet or any of its transitive dependencies, in which case the ordering of the -// duplicated element is not guaranteed). -// -// A DepSet is created by NewDepSet or NewDepSetBuilder.Build from the Paths for direct contents -// and the *DepSets of dependencies. A DepSet is immutable once created. -type DepSet struct { - depSet -} - -// DepSetBuilder is used to create an immutable DepSet. -type DepSetBuilder struct { - depSetBuilder -} - -// NewDepSet returns an immutable DepSet with the given order, direct and transitive contents. -func NewDepSet(order DepSetOrder, direct Paths, transitive []*DepSet) *DepSet { - return &DepSet{*newDepSet(order, direct, transitive)} -} - -// NewDepSetBuilder returns a DepSetBuilder to create an immutable DepSet with the given order. -func NewDepSetBuilder(order DepSetOrder) *DepSetBuilder { - return &DepSetBuilder{*newDepSetBuilder(order, Paths(nil))} -} - -// Direct adds direct contents to the DepSet being built by a DepSetBuilder. Newly added direct -// contents are to the right of any existing direct contents. -func (b *DepSetBuilder) Direct(direct ...Path) *DepSetBuilder { - b.depSetBuilder.DirectSlice(direct) - return b -} - -// Transitive adds transitive contents to the DepSet being built by a DepSetBuilder. Newly added -// transitive contents are to the right of any existing transitive contents. -func (b *DepSetBuilder) Transitive(transitive ...*DepSet) *DepSetBuilder { - b.depSetBuilder.Transitive(transitive) - return b -} - -// Returns the DepSet being built by this DepSetBuilder. The DepSetBuilder retains its contents -// for creating more DepSets. -func (b *DepSetBuilder) Build() *DepSet { - return &DepSet{*b.depSetBuilder.Build()} -} - -// ToList returns the DepSet flattened to a list. The order in the list is based on the order -// of the DepSet. POSTORDER and PREORDER orders return a postordered or preordered left to right -// flattened list. TOPOLOGICAL returns a list that guarantees that elements of children are listed -// after all of their parents (unless there are duplicate direct elements in the DepSet or any of -// its transitive dependencies, in which case the ordering of the duplicated element is not -// guaranteed). -func (d *DepSet) ToList() Paths { - if d == nil { - return nil - } - return d.toList(func(paths interface{}) interface{} { - return FirstUniquePaths(paths.(Paths)) - }).(Paths) -} - -// ToSortedList returns the direct and transitive contents of a DepSet in lexically sorted order -// with duplicates removed. -func (d *DepSet) ToSortedList() Paths { - if d == nil { - return nil - } - paths := d.ToList() - return SortedUniquePaths(paths) -} diff --git a/android/depset_test.go b/android/depset_test.go index 955ccb001..376dffad1 100644 --- a/android/depset_test.go +++ b/android/depset_test.go @@ -17,51 +17,40 @@ package android import ( "fmt" "reflect" - "strconv" "strings" "testing" ) func ExampleDepSet_ToList_postordered() { - a := NewDepSetBuilder(POSTORDER).Direct(PathForTesting("a")).Build() - b := NewDepSetBuilder(POSTORDER).Direct(PathForTesting("b")).Transitive(a).Build() - c := NewDepSetBuilder(POSTORDER).Direct(PathForTesting("c")).Transitive(a).Build() - d := NewDepSetBuilder(POSTORDER).Direct(PathForTesting("d")).Transitive(b, c).Build() + a := NewDepSetBuilder[Path](POSTORDER).Direct(PathForTesting("a")).Build() + b := NewDepSetBuilder[Path](POSTORDER).Direct(PathForTesting("b")).Transitive(a).Build() + c := NewDepSetBuilder[Path](POSTORDER).Direct(PathForTesting("c")).Transitive(a).Build() + d := NewDepSetBuilder[Path](POSTORDER).Direct(PathForTesting("d")).Transitive(b, c).Build() - fmt.Println(d.ToList().Strings()) + fmt.Println(Paths(d.ToList()).Strings()) // Output: [a b c d] } func ExampleDepSet_ToList_preordered() { - a := NewDepSetBuilder(PREORDER).Direct(PathForTesting("a")).Build() - b := NewDepSetBuilder(PREORDER).Direct(PathForTesting("b")).Transitive(a).Build() - c := NewDepSetBuilder(PREORDER).Direct(PathForTesting("c")).Transitive(a).Build() - d := NewDepSetBuilder(PREORDER).Direct(PathForTesting("d")).Transitive(b, c).Build() + a := NewDepSetBuilder[Path](PREORDER).Direct(PathForTesting("a")).Build() + b := NewDepSetBuilder[Path](PREORDER).Direct(PathForTesting("b")).Transitive(a).Build() + c := NewDepSetBuilder[Path](PREORDER).Direct(PathForTesting("c")).Transitive(a).Build() + d := NewDepSetBuilder[Path](PREORDER).Direct(PathForTesting("d")).Transitive(b, c).Build() - fmt.Println(d.ToList().Strings()) + fmt.Println(Paths(d.ToList()).Strings()) // Output: [d b a c] } func ExampleDepSet_ToList_topological() { - a := NewDepSetBuilder(TOPOLOGICAL).Direct(PathForTesting("a")).Build() - b := NewDepSetBuilder(TOPOLOGICAL).Direct(PathForTesting("b")).Transitive(a).Build() - c := NewDepSetBuilder(TOPOLOGICAL).Direct(PathForTesting("c")).Transitive(a).Build() - d := NewDepSetBuilder(TOPOLOGICAL).Direct(PathForTesting("d")).Transitive(b, c).Build() + a := NewDepSetBuilder[Path](TOPOLOGICAL).Direct(PathForTesting("a")).Build() + b := NewDepSetBuilder[Path](TOPOLOGICAL).Direct(PathForTesting("b")).Transitive(a).Build() + c := NewDepSetBuilder[Path](TOPOLOGICAL).Direct(PathForTesting("c")).Transitive(a).Build() + d := NewDepSetBuilder[Path](TOPOLOGICAL).Direct(PathForTesting("d")).Transitive(b, c).Build() - fmt.Println(d.ToList().Strings()) + fmt.Println(Paths(d.ToList()).Strings()) // Output: [d b c a] } -func ExampleDepSet_ToSortedList() { - a := NewDepSetBuilder(POSTORDER).Direct(PathForTesting("a")).Build() - b := NewDepSetBuilder(POSTORDER).Direct(PathForTesting("b")).Transitive(a).Build() - c := NewDepSetBuilder(POSTORDER).Direct(PathForTesting("c")).Transitive(a).Build() - d := NewDepSetBuilder(POSTORDER).Direct(PathForTesting("d")).Transitive(b, c).Build() - - fmt.Println(d.ToSortedList().Strings()) - // Output: [a b c d] -} - // Tests based on Bazel's ExpanderTestBase.java to ensure compatibility // https://github.com/bazelbuild/bazel/blob/master/src/test/java/com/google/devtools/build/lib/collect/nestedset/ExpanderTestBase.java func TestDepSet(t *testing.T) { @@ -74,13 +63,13 @@ func TestDepSet(t *testing.T) { tests := []struct { name string - depSet func(t *testing.T, order DepSetOrder) *DepSet + depSet func(t *testing.T, order DepSetOrder) *DepSet[Path] postorder, preorder, topological []string }{ { name: "simple", - depSet: func(t *testing.T, order DepSetOrder) *DepSet { - return NewDepSet(order, Paths{c, a, b}, nil) + depSet: func(t *testing.T, order DepSetOrder) *DepSet[Path] { + return NewDepSet[Path](order, Paths{c, a, b}, nil) }, postorder: []string{"c", "a", "b"}, preorder: []string{"c", "a", "b"}, @@ -88,8 +77,8 @@ func TestDepSet(t *testing.T) { }, { name: "simpleNoDuplicates", - depSet: func(t *testing.T, order DepSetOrder) *DepSet { - return NewDepSet(order, Paths{c, a, a, a, b}, nil) + depSet: func(t *testing.T, order DepSetOrder) *DepSet[Path] { + return NewDepSet[Path](order, Paths{c, a, a, a, b}, nil) }, postorder: []string{"c", "a", "b"}, preorder: []string{"c", "a", "b"}, @@ -97,9 +86,9 @@ func TestDepSet(t *testing.T) { }, { name: "nesting", - depSet: func(t *testing.T, order DepSetOrder) *DepSet { - subset := NewDepSet(order, Paths{c, a, e}, nil) - return NewDepSet(order, Paths{b, d}, []*DepSet{subset}) + depSet: func(t *testing.T, order DepSetOrder) *DepSet[Path] { + subset := NewDepSet[Path](order, Paths{c, a, e}, nil) + return NewDepSet[Path](order, Paths{b, d}, []*DepSet[Path]{subset}) }, postorder: []string{"c", "a", "e", "b", "d"}, preorder: []string{"b", "d", "c", "a", "e"}, @@ -107,14 +96,14 @@ func TestDepSet(t *testing.T) { }, { name: "builderReuse", - depSet: func(t *testing.T, order DepSetOrder) *DepSet { + depSet: func(t *testing.T, order DepSetOrder) *DepSet[Path] { assertEquals := func(t *testing.T, w, g Paths) { t.Helper() if !reflect.DeepEqual(w, g) { t.Errorf("want %q, got %q", w, g) } } - builder := NewDepSetBuilder(order) + builder := NewDepSetBuilder[Path](order) assertEquals(t, nil, builder.Build().ToList()) builder.Direct(b) @@ -123,7 +112,7 @@ func TestDepSet(t *testing.T) { builder.Direct(d) assertEquals(t, Paths{b, d}, builder.Build().ToList()) - child := NewDepSetBuilder(order).Direct(c, a, e).Build() + child := NewDepSetBuilder[Path](order).Direct(c, a, e).Build() builder.Transitive(child) return builder.Build() }, @@ -133,9 +122,9 @@ func TestDepSet(t *testing.T) { }, { name: "builderChaining", - depSet: func(t *testing.T, order DepSetOrder) *DepSet { - return NewDepSetBuilder(order).Direct(b).Direct(d). - Transitive(NewDepSetBuilder(order).Direct(c, a, e).Build()).Build() + depSet: func(t *testing.T, order DepSetOrder) *DepSet[Path] { + return NewDepSetBuilder[Path](order).Direct(b).Direct(d). + Transitive(NewDepSetBuilder[Path](order).Direct(c, a, e).Build()).Build() }, postorder: []string{"c", "a", "e", "b", "d"}, preorder: []string{"b", "d", "c", "a", "e"}, @@ -143,9 +132,9 @@ func TestDepSet(t *testing.T) { }, { name: "transitiveDepsHandledSeparately", - depSet: func(t *testing.T, order DepSetOrder) *DepSet { - subset := NewDepSetBuilder(order).Direct(c, a, e).Build() - builder := NewDepSetBuilder(order) + depSet: func(t *testing.T, order DepSetOrder) *DepSet[Path] { + subset := NewDepSetBuilder[Path](order).Direct(c, a, e).Build() + builder := NewDepSetBuilder[Path](order) // The fact that we add the transitive subset between the Direct(b) and Direct(d) // calls should not change the result. builder.Direct(b) @@ -159,9 +148,9 @@ func TestDepSet(t *testing.T) { }, { name: "nestingNoDuplicates", - depSet: func(t *testing.T, order DepSetOrder) *DepSet { - subset := NewDepSetBuilder(order).Direct(c, a, e).Build() - return NewDepSetBuilder(order).Direct(b, d, e).Transitive(subset).Build() + depSet: func(t *testing.T, order DepSetOrder) *DepSet[Path] { + subset := NewDepSetBuilder[Path](order).Direct(c, a, e).Build() + return NewDepSetBuilder[Path](order).Direct(b, d, e).Transitive(subset).Build() }, postorder: []string{"c", "a", "e", "b", "d"}, preorder: []string{"b", "d", "e", "c", "a"}, @@ -169,10 +158,10 @@ func TestDepSet(t *testing.T) { }, { name: "chain", - depSet: func(t *testing.T, order DepSetOrder) *DepSet { - c := NewDepSetBuilder(order).Direct(c).Build() - b := NewDepSetBuilder(order).Direct(b).Transitive(c).Build() - a := NewDepSetBuilder(order).Direct(a).Transitive(b).Build() + depSet: func(t *testing.T, order DepSetOrder) *DepSet[Path] { + c := NewDepSetBuilder[Path](order).Direct(c).Build() + b := NewDepSetBuilder[Path](order).Direct(b).Transitive(c).Build() + a := NewDepSetBuilder[Path](order).Direct(a).Transitive(b).Build() return a }, @@ -182,11 +171,11 @@ func TestDepSet(t *testing.T) { }, { name: "diamond", - depSet: func(t *testing.T, order DepSetOrder) *DepSet { - d := NewDepSetBuilder(order).Direct(d).Build() - c := NewDepSetBuilder(order).Direct(c).Transitive(d).Build() - b := NewDepSetBuilder(order).Direct(b).Transitive(d).Build() - a := NewDepSetBuilder(order).Direct(a).Transitive(b).Transitive(c).Build() + depSet: func(t *testing.T, order DepSetOrder) *DepSet[Path] { + d := NewDepSetBuilder[Path](order).Direct(d).Build() + c := NewDepSetBuilder[Path](order).Direct(c).Transitive(d).Build() + b := NewDepSetBuilder[Path](order).Direct(b).Transitive(d).Build() + a := NewDepSetBuilder[Path](order).Direct(a).Transitive(b).Transitive(c).Build() return a }, @@ -196,12 +185,12 @@ func TestDepSet(t *testing.T) { }, { name: "extendedDiamond", - depSet: func(t *testing.T, order DepSetOrder) *DepSet { - d := NewDepSetBuilder(order).Direct(d).Build() - e := NewDepSetBuilder(order).Direct(e).Build() - b := NewDepSetBuilder(order).Direct(b).Transitive(d).Transitive(e).Build() - c := NewDepSetBuilder(order).Direct(c).Transitive(e).Transitive(d).Build() - a := NewDepSetBuilder(order).Direct(a).Transitive(b).Transitive(c).Build() + depSet: func(t *testing.T, order DepSetOrder) *DepSet[Path] { + d := NewDepSetBuilder[Path](order).Direct(d).Build() + e := NewDepSetBuilder[Path](order).Direct(e).Build() + b := NewDepSetBuilder[Path](order).Direct(b).Transitive(d).Transitive(e).Build() + c := NewDepSetBuilder[Path](order).Direct(c).Transitive(e).Transitive(d).Build() + a := NewDepSetBuilder[Path](order).Direct(a).Transitive(b).Transitive(c).Build() return a }, postorder: []string{"d", "e", "b", "c", "a"}, @@ -210,13 +199,13 @@ func TestDepSet(t *testing.T) { }, { name: "extendedDiamondRightArm", - depSet: func(t *testing.T, order DepSetOrder) *DepSet { - d := NewDepSetBuilder(order).Direct(d).Build() - e := NewDepSetBuilder(order).Direct(e).Build() - b := NewDepSetBuilder(order).Direct(b).Transitive(d).Transitive(e).Build() - c2 := NewDepSetBuilder(order).Direct(c2).Transitive(e).Transitive(d).Build() - c := NewDepSetBuilder(order).Direct(c).Transitive(c2).Build() - a := NewDepSetBuilder(order).Direct(a).Transitive(b).Transitive(c).Build() + depSet: func(t *testing.T, order DepSetOrder) *DepSet[Path] { + d := NewDepSetBuilder[Path](order).Direct(d).Build() + e := NewDepSetBuilder[Path](order).Direct(e).Build() + b := NewDepSetBuilder[Path](order).Direct(b).Transitive(d).Transitive(e).Build() + c2 := NewDepSetBuilder[Path](order).Direct(c2).Transitive(e).Transitive(d).Build() + c := NewDepSetBuilder[Path](order).Direct(c).Transitive(c2).Build() + a := NewDepSetBuilder[Path](order).Direct(a).Transitive(b).Transitive(c).Build() return a }, postorder: []string{"d", "e", "b", "c2", "c", "a"}, @@ -225,10 +214,10 @@ func TestDepSet(t *testing.T) { }, { name: "orderConflict", - depSet: func(t *testing.T, order DepSetOrder) *DepSet { - child1 := NewDepSetBuilder(order).Direct(a, b).Build() - child2 := NewDepSetBuilder(order).Direct(b, a).Build() - parent := NewDepSetBuilder(order).Transitive(child1).Transitive(child2).Build() + depSet: func(t *testing.T, order DepSetOrder) *DepSet[Path] { + child1 := NewDepSetBuilder[Path](order).Direct(a, b).Build() + child2 := NewDepSetBuilder[Path](order).Direct(b, a).Build() + parent := NewDepSetBuilder[Path](order).Transitive(child1).Transitive(child2).Build() return parent }, postorder: []string{"a", "b"}, @@ -237,12 +226,12 @@ func TestDepSet(t *testing.T) { }, { name: "orderConflictNested", - depSet: func(t *testing.T, order DepSetOrder) *DepSet { - a := NewDepSetBuilder(order).Direct(a).Build() - b := NewDepSetBuilder(order).Direct(b).Build() - child1 := NewDepSetBuilder(order).Transitive(a).Transitive(b).Build() - child2 := NewDepSetBuilder(order).Transitive(b).Transitive(a).Build() - parent := NewDepSetBuilder(order).Transitive(child1).Transitive(child2).Build() + depSet: func(t *testing.T, order DepSetOrder) *DepSet[Path] { + a := NewDepSetBuilder[Path](order).Direct(a).Build() + b := NewDepSetBuilder[Path](order).Direct(b).Build() + child1 := NewDepSetBuilder[Path](order).Transitive(a).Transitive(b).Build() + child2 := NewDepSetBuilder[Path](order).Transitive(b).Transitive(a).Build() + parent := NewDepSetBuilder[Path](order).Transitive(child1).Transitive(child2).Build() return parent }, postorder: []string{"a", "b"}, @@ -255,19 +244,19 @@ func TestDepSet(t *testing.T) { t.Run(tt.name, func(t *testing.T) { t.Run("postorder", func(t *testing.T) { depSet := tt.depSet(t, POSTORDER) - if g, w := depSet.ToList().Strings(), tt.postorder; !reflect.DeepEqual(g, w) { + if g, w := Paths(depSet.ToList()).Strings(), tt.postorder; !reflect.DeepEqual(g, w) { t.Errorf("expected ToList() = %q, got %q", w, g) } }) t.Run("preorder", func(t *testing.T) { depSet := tt.depSet(t, PREORDER) - if g, w := depSet.ToList().Strings(), tt.preorder; !reflect.DeepEqual(g, w) { + if g, w := Paths(depSet.ToList()).Strings(), tt.preorder; !reflect.DeepEqual(g, w) { t.Errorf("expected ToList() = %q, got %q", w, g) } }) t.Run("topological", func(t *testing.T) { depSet := tt.depSet(t, TOPOLOGICAL) - if g, w := depSet.ToList().Strings(), tt.topological; !reflect.DeepEqual(g, w) { + if g, w := Paths(depSet.ToList()).Strings(), tt.topological; !reflect.DeepEqual(g, w) { t.Errorf("expected ToList() = %q, got %q", w, g) } }) @@ -288,7 +277,7 @@ func TestDepSetInvalidOrder(t *testing.T) { } } }() - NewDepSet(order1, nil, []*DepSet{NewDepSet(order2, nil, nil)}) + NewDepSet(order1, nil, []*DepSet[Path]{NewDepSet[Path](order2, nil, nil)}) t.Fatal("expected panic") } @@ -304,87 +293,3 @@ func TestDepSetInvalidOrder(t *testing.T) { }) } } - -func Test_firstUnique(t *testing.T) { - f := func(t *testing.T, imp func([]string) []string, in, want []string) { - t.Helper() - out := imp(in) - if !reflect.DeepEqual(out, want) { - t.Errorf("incorrect output:") - t.Errorf(" input: %#v", in) - t.Errorf(" expected: %#v", want) - t.Errorf(" got: %#v", out) - } - } - - for _, testCase := range firstUniqueStringsTestCases { - t.Run("list", func(t *testing.T) { - f(t, func(s []string) []string { - return firstUniqueList(s).([]string) - }, testCase.in, testCase.out) - }) - t.Run("map", func(t *testing.T) { - f(t, func(s []string) []string { - return firstUniqueMap(s).([]string) - }, testCase.in, testCase.out) - }) - } -} - -func Benchmark_firstUnique(b *testing.B) { - implementations := []struct { - name string - f func([]string) []string - }{ - { - name: "list", - f: func(slice []string) []string { - return firstUniqueList(slice).([]string) - }, - }, - { - name: "map", - f: func(slice []string) []string { - return firstUniqueMap(slice).([]string) - }, - }, - { - name: "optimal", - f: func(slice []string) []string { - return firstUnique(slice).([]string) - }, - }, - } - const maxSize = 1024 - uniqueStrings := make([]string, maxSize) - for i := range uniqueStrings { - uniqueStrings[i] = strconv.Itoa(i) - } - sameString := make([]string, maxSize) - for i := range sameString { - sameString[i] = uniqueStrings[0] - } - - f := func(b *testing.B, imp func([]string) []string, s []string) { - for i := 0; i < b.N; i++ { - b.ReportAllocs() - s = append([]string(nil), s...) - imp(s) - } - } - - for n := 1; n <= maxSize; n <<= 1 { - b.Run(strconv.Itoa(n), func(b *testing.B) { - for _, implementation := range implementations { - b.Run(implementation.name, func(b *testing.B) { - b.Run("same", func(b *testing.B) { - f(b, implementation.f, sameString[:n]) - }) - b.Run("unique", func(b *testing.B) { - f(b, implementation.f, uniqueStrings[:n]) - }) - }) - } - }) - } -} diff --git a/android/license_metadata.go b/android/license_metadata.go index 73000a9fa..8933bd511 100644 --- a/android/license_metadata.go +++ b/android/license_metadata.go @@ -55,7 +55,7 @@ func buildLicenseMetadata(ctx ModuleContext, licenseMetadataFile WritablePath) { var allDepMetadataFiles Paths var allDepMetadataArgs []string var allDepOutputFiles Paths - var allDepMetadataDepSets []*PathsDepSet + var allDepMetadataDepSets []*DepSet[Path] ctx.VisitDirectDepsBlueprint(func(bpdep blueprint.Module) { dep, _ := bpdep.(Module) @@ -127,7 +127,7 @@ func buildLicenseMetadata(ctx ModuleContext, licenseMetadataFile WritablePath) { JoinWithPrefix(proptools.NinjaAndShellEscapeListIncludingSpaces(base.commonProperties.Effective_license_text.Strings()), "-n ")) if isContainer { - transitiveDeps := newPathsDepSet(nil, allDepMetadataDepSets).ToList() + transitiveDeps := Paths(NewDepSet[Path](TOPOLOGICAL, nil, allDepMetadataDepSets).ToList()) args = append(args, JoinWithPrefix(proptools.NinjaAndShellEscapeListIncludingSpaces(transitiveDeps.Strings()), "-d ")) orderOnlyDeps = append(orderOnlyDeps, transitiveDeps...) @@ -170,7 +170,7 @@ func buildLicenseMetadata(ctx ModuleContext, licenseMetadataFile WritablePath) { ctx.SetProvider(LicenseMetadataProvider, &LicenseMetadataInfo{ LicenseMetadataPath: licenseMetadataFile, - LicenseMetadataDepSet: newPathsDepSet(Paths{licenseMetadataFile}, allDepMetadataDepSets), + LicenseMetadataDepSet: NewDepSet(TOPOLOGICAL, Paths{licenseMetadataFile}, allDepMetadataDepSets), }) } @@ -198,7 +198,7 @@ var LicenseMetadataProvider = blueprint.NewProvider(&LicenseMetadataInfo{}) // LicenseMetadataInfo stores the license metadata path for a module. type LicenseMetadataInfo struct { LicenseMetadataPath Path - LicenseMetadataDepSet *PathsDepSet + LicenseMetadataDepSet *DepSet[Path] } // licenseAnnotationsFromTag returns the LicenseAnnotations for a tag (if any) converted into diff --git a/android/module.go b/android/module.go index ba327108f..c1ba67a37 100644 --- a/android/module.go +++ b/android/module.go @@ -1517,10 +1517,10 @@ type ModuleBase struct { noAddressSanitizer bool installFiles InstallPaths - installFilesDepSet *installPathsDepSet + installFilesDepSet *DepSet[InstallPath] checkbuildFiles Paths packagingSpecs []PackagingSpec - packagingSpecsDepSet *packagingSpecsDepSet + packagingSpecsDepSet *DepSet[PackagingSpec] // katiInstalls tracks the install rules that were created by Soong but are being exported // to Make to convert to ninja rules so that Make can add additional dependencies. katiInstalls katiInstalls @@ -2083,9 +2083,9 @@ func (m *ModuleBase) EffectiveLicenseFiles() Paths { // computeInstallDeps finds the installed paths of all dependencies that have a dependency // tag that is annotated as needing installation via the isInstallDepNeeded method. -func (m *ModuleBase) computeInstallDeps(ctx ModuleContext) ([]*installPathsDepSet, []*packagingSpecsDepSet) { - var installDeps []*installPathsDepSet - var packagingSpecs []*packagingSpecsDepSet +func (m *ModuleBase) computeInstallDeps(ctx ModuleContext) ([]*DepSet[InstallPath], []*DepSet[PackagingSpec]) { + var installDeps []*DepSet[InstallPath] + var packagingSpecs []*DepSet[PackagingSpec] ctx.VisitDirectDeps(func(dep Module) { if isInstallDepNeeded(dep, ctx.OtherModuleDependencyTag(dep)) { // Installation is still handled by Make, so anything hidden from Make is not @@ -2380,7 +2380,7 @@ func (m *ModuleBase) GenerateBuildActions(blueprintCtx blueprint.ModuleContext) // set m.installFilesDepSet to only the transitive dependencies to be used as the dependencies // of installed files of this module. It will be replaced by a depset including the installed // files of this module at the end for use by modules that depend on this one. - m.installFilesDepSet = newInstallPathsDepSet(nil, dependencyInstallFiles) + m.installFilesDepSet = NewDepSet[InstallPath](TOPOLOGICAL, nil, dependencyInstallFiles) // Temporarily continue to call blueprintCtx.GetMissingDependencies() to maintain the previous behavior of never // reporting missing dependency errors in Blueprint when AllowMissingDependencies == true. @@ -2487,8 +2487,8 @@ func (m *ModuleBase) GenerateBuildActions(blueprintCtx blueprint.ModuleContext) } } - m.installFilesDepSet = newInstallPathsDepSet(m.installFiles, dependencyInstallFiles) - m.packagingSpecsDepSet = newPackagingSpecsDepSet(m.packagingSpecs, dependencyPackagingSpecs) + m.installFilesDepSet = NewDepSet[InstallPath](TOPOLOGICAL, m.installFiles, dependencyInstallFiles) + m.packagingSpecsDepSet = NewDepSet[PackagingSpec](TOPOLOGICAL, m.packagingSpecs, dependencyPackagingSpecs) buildLicenseMetadata(ctx, m.licenseMetadataFile) @@ -3369,7 +3369,7 @@ func (m *moduleContext) installFile(installPath InstallPath, name string, srcPat m.module.base().hooks.runInstallHooks(m, srcPath, fullInstallPath, false) if !m.skipInstall() { - deps = append(deps, m.module.base().installFilesDepSet.ToList().Paths()...) + deps = append(deps, InstallPaths(m.module.base().installFilesDepSet.ToList()).Paths()...) var implicitDeps, orderOnlyDeps Paths @@ -3944,26 +3944,6 @@ func CheckBlueprintSyntax(ctx BaseModuleContext, filename string, contents strin return blueprint.CheckBlueprintSyntax(bpctx.ModuleFactories(), filename, contents) } -// installPathsDepSet is a thin type-safe wrapper around the generic depSet. It always uses -// topological order. -type installPathsDepSet struct { - depSet -} - -// newInstallPathsDepSet returns an immutable packagingSpecsDepSet with the given direct and -// transitive contents. -func newInstallPathsDepSet(direct InstallPaths, transitive []*installPathsDepSet) *installPathsDepSet { - return &installPathsDepSet{*newDepSet(TOPOLOGICAL, direct, transitive)} -} - -// ToList returns the installPathsDepSet flattened to a list in topological order. -func (d *installPathsDepSet) ToList() InstallPaths { - if d == nil { - return nil - } - return d.depSet.ToList().(InstallPaths) -} - func registerSoongConfigTraceMutator(ctx RegisterMutatorsContext) { ctx.BottomUp("soongconfigtrace", soongConfigTraceMutator).Parallel() } diff --git a/android/packaging.go b/android/packaging.go index c764a6df5..503bb97e0 100644 --- a/android/packaging.go +++ b/android/packaging.go @@ -282,23 +282,3 @@ func (p *PackagingBase) CopyDepsToZip(ctx ModuleContext, specs map[string]Packag builder.Build("zip_deps", fmt.Sprintf("Zipping deps for %s", ctx.ModuleName())) return entries } - -// packagingSpecsDepSet is a thin type-safe wrapper around the generic depSet. It always uses -// topological order. -type packagingSpecsDepSet struct { - depSet -} - -// newPackagingSpecsDepSet returns an immutable packagingSpecsDepSet with the given direct and -// transitive contents. -func newPackagingSpecsDepSet(direct []PackagingSpec, transitive []*packagingSpecsDepSet) *packagingSpecsDepSet { - return &packagingSpecsDepSet{*newDepSet(TOPOLOGICAL, direct, transitive)} -} - -// ToList returns the packagingSpecsDepSet flattened to a list in topological order. -func (d *packagingSpecsDepSet) ToList() []PackagingSpec { - if d == nil { - return nil - } - return d.depSet.ToList().([]PackagingSpec) -} diff --git a/android/paths.go b/android/paths.go index 0f3d97232..b8a119b41 100644 --- a/android/paths.go +++ b/android/paths.go @@ -2199,23 +2199,3 @@ func IsThirdPartyPath(path string) bool { } return false } - -// PathsDepSet is a thin type-safe wrapper around the generic depSet. It always uses -// topological order. -type PathsDepSet struct { - depSet -} - -// newPathsDepSet returns an immutable PathsDepSet with the given direct and -// transitive contents. -func newPathsDepSet(direct Paths, transitive []*PathsDepSet) *PathsDepSet { - return &PathsDepSet{*newDepSet(TOPOLOGICAL, direct, transitive)} -} - -// ToList returns the PathsDepSet flattened to a list in topological order. -func (d *PathsDepSet) ToList() Paths { - if d == nil { - return nil - } - return d.depSet.ToList().(Paths) -} diff --git a/android/util.go b/android/util.go index 08a3521a5..c4ce71a5c 100644 --- a/android/util.go +++ b/android/util.go @@ -284,38 +284,74 @@ func FirstUniqueStrings(list []string) []string { list = CopyOf(list) // 128 was chosen based on BenchmarkFirstUniqueStrings results. if len(list) > 128 { - return firstUniqueStringsMap(list) + return firstUnique(list) } - return firstUniqueStringsList(list) + return firstUnique(list) } -func firstUniqueStringsList(list []string) []string { - k := 0 +// firstUnique returns all unique elements of a slice, keeping the first copy of each. It +// modifies the slice contents in place, and returns a subslice of the original slice. +func firstUnique[T comparable](slice []T) []T { + // 4 was chosen based on Benchmark_firstUnique results. + if len(slice) > 4 { + return firstUniqueMap(slice) + } + return firstUniqueList(slice) +} + +// firstUniqueList is an implementation of firstUnique using an O(N^2) list comparison to look for +// duplicates. +func firstUniqueList[T any](in []T) []T { + writeIndex := 0 outer: - for i := 0; i < len(list); i++ { - for j := 0; j < k; j++ { - if list[i] == list[j] { + for readIndex := 0; readIndex < len(in); readIndex++ { + for compareIndex := 0; compareIndex < writeIndex; compareIndex++ { + if interface{}(in[readIndex]) == interface{}(in[compareIndex]) { + // The value at readIndex already exists somewhere in the output region + // of the slice before writeIndex, skip it. continue outer } } - list[k] = list[i] - k++ + if readIndex != writeIndex { + in[writeIndex] = in[readIndex] + } + writeIndex++ } - return list[:k] + return in[0:writeIndex] } -func firstUniqueStringsMap(list []string) []string { - k := 0 - seen := make(map[string]bool, len(list)) - for i := 0; i < len(list); i++ { - if seen[list[i]] { +// firstUniqueMap is an implementation of firstUnique using an O(N) hash set lookup to look for +// duplicates. +func firstUniqueMap[T comparable](in []T) []T { + writeIndex := 0 + seen := make(map[T]bool, len(in)) + for readIndex := 0; readIndex < len(in); readIndex++ { + if _, exists := seen[in[readIndex]]; exists { continue } - seen[list[i]] = true - list[k] = list[i] - k++ + seen[in[readIndex]] = true + if readIndex != writeIndex { + in[writeIndex] = in[readIndex] + } + writeIndex++ } - return list[:k] + return in[0:writeIndex] +} + +// reverseSliceInPlace reverses the elements of a slice in place. +func reverseSliceInPlace[T any](in []T) { + for i, j := 0, len(in)-1; i < j; i, j = i+1, j-1 { + in[i], in[j] = in[j], in[i] + } +} + +// reverseSlice returns a copy of a slice in reverse order. +func reverseSlice[T any](in []T) []T { + out := make([]T, len(in)) + for i := 0; i < len(in); i++ { + out[i] = in[len(in)-1-i] + } + return out } // LastUniqueStrings returns all unique elements of a slice of strings, keeping the last copy of diff --git a/android/util_test.go b/android/util_test.go index a2ef58958..bee31a9fd 100644 --- a/android/util_test.go +++ b/android/util_test.go @@ -74,10 +74,10 @@ func TestFirstUniqueStrings(t *testing.T) { for _, testCase := range firstUniqueStringsTestCases { t.Run("list", func(t *testing.T) { - f(t, firstUniqueStringsList, testCase.in, testCase.out) + f(t, firstUniqueList[string], testCase.in, testCase.out) }) t.Run("map", func(t *testing.T) { - f(t, firstUniqueStringsMap, testCase.in, testCase.out) + f(t, firstUniqueMap[string], testCase.in, testCase.out) }) } } @@ -604,11 +604,11 @@ func BenchmarkFirstUniqueStrings(b *testing.B) { }{ { name: "list", - f: firstUniqueStringsList, + f: firstUniqueList[string], }, { name: "map", - f: firstUniqueStringsMap, + f: firstUniqueMap[string], }, { name: "optimal", diff --git a/cc/cc.go b/cc/cc.go index 348400026..a7fda2d18 100644 --- a/cc/cc.go +++ b/cc/cc.go @@ -150,7 +150,7 @@ type PathDeps struct { StaticLibs, LateStaticLibs, WholeStaticLibs android.Paths // Transitive static library dependencies of static libraries for use in ordering. - TranstiveStaticLibrariesForOrdering *android.DepSet + TranstiveStaticLibrariesForOrdering *android.DepSet[android.Path] // Paths to .o files Objs Objects @@ -3549,8 +3549,8 @@ func ChooseStubOrImpl(ctx android.ModuleContext, dep android.Module) (SharedLibr // to match the topological order of the dependency tree, including any static analogues of // direct shared libraries. It returns the ordered static dependencies, and an android.DepSet // of the transitive dependencies. -func orderStaticModuleDeps(staticDeps []StaticLibraryInfo, sharedDeps []SharedLibraryInfo) (ordered android.Paths, transitive *android.DepSet) { - transitiveStaticLibsBuilder := android.NewDepSetBuilder(android.TOPOLOGICAL) +func orderStaticModuleDeps(staticDeps []StaticLibraryInfo, sharedDeps []SharedLibraryInfo) (ordered android.Paths, transitive *android.DepSet[android.Path]) { + transitiveStaticLibsBuilder := android.NewDepSetBuilder[android.Path](android.TOPOLOGICAL) var staticPaths android.Paths for _, staticDep := range staticDeps { staticPaths = append(staticPaths, staticDep.StaticLibrary) diff --git a/cc/cc_test.go b/cc/cc_test.go index 701c3bb74..2635b8fd5 100644 --- a/cc/cc_test.go +++ b/cc/cc_test.go @@ -2700,8 +2700,8 @@ func TestStaticLibDepReordering(t *testing.T) { variant := "android_arm64_armv8-a_static" moduleA := ctx.ModuleForTests("a", variant).Module().(*Module) - actual := ctx.ModuleProvider(moduleA, StaticLibraryInfoProvider).(StaticLibraryInfo). - TransitiveStaticLibrariesForOrdering.ToList().RelativeToTop() + actual := android.Paths(ctx.ModuleProvider(moduleA, StaticLibraryInfoProvider).(StaticLibraryInfo). + TransitiveStaticLibrariesForOrdering.ToList()).RelativeToTop() expected := GetOutputPaths(ctx, variant, []string{"a", "c", "b", "d"}) if !reflect.DeepEqual(actual, expected) { @@ -2736,8 +2736,8 @@ func TestStaticLibDepReorderingWithShared(t *testing.T) { variant := "android_arm64_armv8-a_static" moduleA := ctx.ModuleForTests("a", variant).Module().(*Module) - actual := ctx.ModuleProvider(moduleA, StaticLibraryInfoProvider).(StaticLibraryInfo). - TransitiveStaticLibrariesForOrdering.ToList().RelativeToTop() + actual := android.Paths(ctx.ModuleProvider(moduleA, StaticLibraryInfoProvider).(StaticLibraryInfo). + TransitiveStaticLibrariesForOrdering.ToList()).RelativeToTop() expected := GetOutputPaths(ctx, variant, []string{"a", "c", "b"}) if !reflect.DeepEqual(actual, expected) { diff --git a/cc/library.go b/cc/library.go index 47df53e4a..aec6433d8 100644 --- a/cc/library.go +++ b/cc/library.go @@ -892,7 +892,7 @@ func (handler *ccLibraryBazelHandler) generateStaticBazelBuildActions(ctx androi // TODO(b/190524881): Include transitive static libraries in this provider to support // static libraries with deps. - TransitiveStaticLibrariesForOrdering: android.NewDepSetBuilder(android.TOPOLOGICAL). + TransitiveStaticLibrariesForOrdering: android.NewDepSetBuilder[android.Path](android.TOPOLOGICAL). Direct(outputFilePath). Build(), }) @@ -1649,7 +1649,7 @@ func (library *libraryDecorator) linkStatic(ctx ModuleContext, Objects: library.objects, WholeStaticLibsFromPrebuilts: library.wholeStaticLibsFromPrebuilts, - TransitiveStaticLibrariesForOrdering: android.NewDepSetBuilder(android.TOPOLOGICAL). + TransitiveStaticLibrariesForOrdering: android.NewDepSetBuilder[android.Path](android.TOPOLOGICAL). Direct(outputFile). Transitive(deps.TranstiveStaticLibrariesForOrdering). Build(), @@ -1794,7 +1794,7 @@ func (library *libraryDecorator) linkShared(ctx ModuleContext, library.coverageOutputFile = transformCoverageFilesToZip(ctx, objs, library.getLibName(ctx)) library.linkSAbiDumpFiles(ctx, objs, fileName, unstrippedOutputFile) - var transitiveStaticLibrariesForOrdering *android.DepSet + var transitiveStaticLibrariesForOrdering *android.DepSet[android.Path] if static := ctx.GetDirectDepsWithTag(staticVariantTag); len(static) > 0 { s := ctx.OtherModuleProvider(static[0], StaticLibraryInfoProvider).(StaticLibraryInfo) transitiveStaticLibrariesForOrdering = s.TransitiveStaticLibrariesForOrdering diff --git a/cc/linkable.go b/cc/linkable.go index 976a38201..19e6501de 100644 --- a/cc/linkable.go +++ b/cc/linkable.go @@ -345,7 +345,7 @@ type SharedLibraryInfo struct { TableOfContents android.OptionalPath // should be obtained from static analogue - TransitiveStaticLibrariesForOrdering *android.DepSet + TransitiveStaticLibrariesForOrdering *android.DepSet[android.Path] } var SharedLibraryInfoProvider = blueprint.NewProvider(SharedLibraryInfo{}) @@ -387,7 +387,7 @@ type StaticLibraryInfo struct { // This isn't the actual transitive DepSet, shared library dependencies have been // converted into static library analogues. It is only used to order the static // library dependencies that were specified for the current module. - TransitiveStaticLibrariesForOrdering *android.DepSet + TransitiveStaticLibrariesForOrdering *android.DepSet[android.Path] } var StaticLibraryInfoProvider = blueprint.NewProvider(StaticLibraryInfo{}) diff --git a/cc/ndk_prebuilt.go b/cc/ndk_prebuilt.go index 1d15cf83f..d3a0a002e 100644 --- a/cc/ndk_prebuilt.go +++ b/cc/ndk_prebuilt.go @@ -113,7 +113,7 @@ func (ndk *ndkPrebuiltStlLinker) link(ctx ModuleContext, flags Flags, ndk.libraryDecorator.flagExporter.setProvider(ctx) if ndk.static() { - depSet := android.NewDepSetBuilder(android.TOPOLOGICAL).Direct(lib).Build() + depSet := android.NewDepSetBuilder[android.Path](android.TOPOLOGICAL).Direct(lib).Build() ctx.SetProvider(StaticLibraryInfoProvider, StaticLibraryInfo{ StaticLibrary: lib, diff --git a/cc/prebuilt.go b/cc/prebuilt.go index 44cd0d73f..a4ca59050 100644 --- a/cc/prebuilt.go +++ b/cc/prebuilt.go @@ -140,7 +140,7 @@ func (p *prebuiltLibraryLinker) link(ctx ModuleContext, } if p.static() { - depSet := android.NewDepSetBuilder(android.TOPOLOGICAL).Direct(in).Build() + depSet := android.NewDepSetBuilder[android.Path](android.TOPOLOGICAL).Direct(in).Build() ctx.SetProvider(StaticLibraryInfoProvider, StaticLibraryInfo{ StaticLibrary: in, @@ -508,7 +508,7 @@ func (h *prebuiltLibraryBazelHandler) processStaticBazelQueryResponse(ctx androi h.module.outputFile = android.OptionalPathForPath(outputPath) - depSet := android.NewDepSetBuilder(android.TOPOLOGICAL).Direct(outputPath).Build() + depSet := android.NewDepSetBuilder[android.Path](android.TOPOLOGICAL).Direct(outputPath).Build() ctx.SetProvider(StaticLibraryInfoProvider, StaticLibraryInfo{ StaticLibrary: outputPath, TransitiveStaticLibrariesForOrdering: depSet, diff --git a/cc/snapshot_prebuilt.go b/cc/snapshot_prebuilt.go index bb6e257e5..a5729dfc0 100644 --- a/cc/snapshot_prebuilt.go +++ b/cc/snapshot_prebuilt.go @@ -499,7 +499,7 @@ func (p *snapshotLibraryDecorator) link(ctx ModuleContext, flags Flags, deps Pat } if p.static() { - depSet := android.NewDepSetBuilder(android.TOPOLOGICAL).Direct(in).Build() + depSet := android.NewDepSetBuilder[android.Path](android.TOPOLOGICAL).Direct(in).Build() ctx.SetProvider(StaticLibraryInfoProvider, StaticLibraryInfo{ StaticLibrary: in, diff --git a/dexpreopt/config.go b/dexpreopt/config.go index e61ebe624..bb83dc842 100644 --- a/dexpreopt/config.go +++ b/dexpreopt/config.go @@ -321,7 +321,7 @@ func getGlobalConfigRaw(ctx android.PathContext) globalConfigAndRaw { missingDepsCtx.AddMissingDependencies([]string{err.Error()}) } } else { - android.ReportPathErrorf(ctx, "%w", err) + android.ReportPathErrorf(ctx, "%s", err) } } diff --git a/go.mod b/go.mod index 4a511c528..0a11bd2db 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module android/soong -go 1.19 +go 1.20 require ( github.com/google/blueprint v0.0.0 diff --git a/java/base.go b/java/base.go index 677928320..3fd5a4507 100644 --- a/java/base.go +++ b/java/base.go @@ -1747,24 +1747,24 @@ func (j *Module) instrument(ctx android.ModuleContext, flags javaBuilderFlags, type providesTransitiveHeaderJars struct { // set of header jars for all transitive libs deps - transitiveLibsHeaderJars *android.DepSet + transitiveLibsHeaderJars *android.DepSet[android.Path] // set of header jars for all transitive static libs deps - transitiveStaticLibsHeaderJars *android.DepSet + transitiveStaticLibsHeaderJars *android.DepSet[android.Path] } -func (j *providesTransitiveHeaderJars) TransitiveLibsHeaderJars() *android.DepSet { +func (j *providesTransitiveHeaderJars) TransitiveLibsHeaderJars() *android.DepSet[android.Path] { return j.transitiveLibsHeaderJars } -func (j *providesTransitiveHeaderJars) TransitiveStaticLibsHeaderJars() *android.DepSet { +func (j *providesTransitiveHeaderJars) TransitiveStaticLibsHeaderJars() *android.DepSet[android.Path] { return j.transitiveStaticLibsHeaderJars } func (j *providesTransitiveHeaderJars) collectTransitiveHeaderJars(ctx android.ModuleContext) { directLibs := android.Paths{} directStaticLibs := android.Paths{} - transitiveLibs := []*android.DepSet{} - transitiveStaticLibs := []*android.DepSet{} + transitiveLibs := []*android.DepSet[android.Path]{} + transitiveStaticLibs := []*android.DepSet[android.Path]{} ctx.VisitDirectDeps(func(module android.Module) { // don't add deps of the prebuilt version of the same library if ctx.ModuleName() == android.RemoveOptionalPrebuiltPrefix(module.Name()) { diff --git a/java/java.go b/java/java.go index a026610c2..3e6b96b26 100644 --- a/java/java.go +++ b/java/java.go @@ -231,10 +231,10 @@ type JavaInfo struct { HeaderJars android.Paths // set of header jars for all transitive libs deps - TransitiveLibsHeaderJars *android.DepSet + TransitiveLibsHeaderJars *android.DepSet[android.Path] // set of header jars for all transitive static libs deps - TransitiveStaticLibsHeaderJars *android.DepSet + TransitiveStaticLibsHeaderJars *android.DepSet[android.Path] // ImplementationAndResourceJars is a list of jars that contain the implementations of classes // in the module as well as any resources included in the module. diff --git a/java/lint.go b/java/lint.go index a0f99707a..f84f1c065 100644 --- a/java/lint.go +++ b/java/lint.go @@ -117,18 +117,18 @@ type LintDepSetsIntf interface { } type LintDepSets struct { - HTML, Text, XML *android.DepSet + HTML, Text, XML *android.DepSet[android.Path] } type LintDepSetsBuilder struct { - HTML, Text, XML *android.DepSetBuilder + HTML, Text, XML *android.DepSetBuilder[android.Path] } func NewLintDepSetBuilder() LintDepSetsBuilder { return LintDepSetsBuilder{ - HTML: android.NewDepSetBuilder(android.POSTORDER), - Text: android.NewDepSetBuilder(android.POSTORDER), - XML: android.NewDepSetBuilder(android.POSTORDER), + HTML: android.NewDepSetBuilder[android.Path](android.POSTORDER), + Text: android.NewDepSetBuilder[android.Path](android.POSTORDER), + XML: android.NewDepSetBuilder[android.Path](android.POSTORDER), } } @@ -553,9 +553,9 @@ func (l *linter) lint(ctx android.ModuleContext) { } func BuildModuleLintReportZips(ctx android.ModuleContext, depSets LintDepSets) android.Paths { - htmlList := depSets.HTML.ToSortedList() - textList := depSets.Text.ToSortedList() - xmlList := depSets.XML.ToSortedList() + htmlList := android.SortedUniquePaths(depSets.HTML.ToList()) + textList := android.SortedUniquePaths(depSets.Text.ToList()) + xmlList := android.SortedUniquePaths(depSets.XML.ToList()) if len(htmlList) == 0 && len(textList) == 0 && len(xmlList) == 0 { return nil diff --git a/rust/library.go b/rust/library.go index a3a567281..5e445a92e 100644 --- a/rust/library.go +++ b/rust/library.go @@ -567,7 +567,7 @@ func (library *libraryDecorator) compile(ctx ModuleContext, flags Flags, deps Pa } if library.static() { - depSet := android.NewDepSetBuilder(android.TOPOLOGICAL).Direct(outputFile).Build() + depSet := android.NewDepSetBuilder[android.Path](android.TOPOLOGICAL).Direct(outputFile).Build() ctx.SetProvider(cc.StaticLibraryInfoProvider, cc.StaticLibraryInfo{ StaticLibrary: outputFile, diff --git a/ui/metrics/metrics.go b/ui/metrics/metrics.go index 6d6031649..d68ced859 100644 --- a/ui/metrics/metrics.go +++ b/ui/metrics/metrics.go @@ -230,7 +230,7 @@ func (m *Metrics) SetBuildDateTime(buildTimestamp time.Time) { func (m *Metrics) UpdateTotalRealTimeAndNonZeroExit(data []byte, bazelExitCode int32) error { if err := proto.Unmarshal(data, &m.metrics); err != nil { - return fmt.Errorf("Failed to unmarshal proto", err) + return fmt.Errorf("Failed to unmarshal proto: %w", err) } startTime := *m.metrics.Total.StartTime endTime := uint64(time.Now().UnixNano())