Fix detecting cycles in parallelVisit when the first alphabetical
module is not part of the cycle, but depends on the cycle. Instead of
starting from the first alphabetical module, check every module in a
determinsitic order and return the first time a cycle is found.
Test: Test_parallelVisit
Change-Id: I03726f838ec42975251088ba75158103940115c2
Avoid reallocating module.forwardDeps and module.reverseDeps every
time through updateDependencies by resetting the slices without
reducing their capacity. Accumulate dependencies to visit directly
into module.forwardDeps. Use a loop instead of a map to check
for duplicates, average number of dependencies is measured to be
9.5, although there are a few outliers with up to 2108.
Reduces mean soong_build execution time on internal master from 87s
to 82.7s (5%).
Test: context_test.go
Change-Id: I58fcd5514e494bafa965443461851b21b7bce382
Variables, pools and rules each computed their full names every time
they were referenced, which required string concatenations. Since
every one is guaranteed to be accessed at least twice, once when the
definition is written into the ninja file and once for each reference,
precompute the full name. For local variables that can be done
during initialization, but for global variables add a pass to
PrepareBuildActions to compute the name for each live variable using
the final package names.
Test: ninja_writer_test.go
Change-Id: I2264b05e0409e36651db2fb5d463c16c698d4d5e
ninjaWriter repeatedly called io.WriteString() on its writer, which
does a type assertion every time. Replace its io.Writer with an
io.StringWriter and call WriteString on it directly.
Test: ninja_writer_test.go
Change-Id: Ie073d996a319190242bf6a00af07a13a60d078b5
* Invalidate module group cache if deps are modified
PreSingletons run after the blueprints have been parsed and can run
VisitAllModules; however, this seeds the cache of sorted modules which
may change after mutators have run.
This causes recomputation of the cache if any deps have been
modified since the last time it was run.
Change-Id: I79fc822dd630f84790f309ba4e6024588a8fe28e
In particular the formatting of dependencies with variants which lacked
braces that caused them to float together with the dependency names.
Also add some context to the ReplaceDependenciesIf panic message.
Test: m
Change-Id: Ibd03690317ca53f3ecbdd95033404d89d849b4dd
Providers are a new concept for Blueprint, based on providers in Bazel:
https://docs.bazel.build/versions/master/skylark/rules.html#providers
Providers aim to simplify the interaction between modules by replacing
type asserting to arbitrary interfaces with requesting optional data
objects from modules. This will also move Blueprint closer to supporting
incremental analysis by serializing the providers and only rerunning
the analysis phase on modules whose inputs have changed.
Change-Id: I39f5f78b372412a7dbf151ceccb3f917f6c874bf
The motivaion for this change is to allow writing code that uses the
newly added dependency module in the same mutator pass, for example to
add more dependencies. Like this:
for _, m := range ctx.AddVariationDependencies(nil, tag, deps...) {
if someModuleProperty(m); ok {
ctx.AddVariationDependencies(nil, tag, otherDep)
}
}
Note that there is no guarantee that the returned module has already
been processed by the current mutator.
The patch does not add runtime overhead on findng dependency modules,
as this has already been done previously.
Test: go test
Pass a channel to visitor functions called by parallelVisit that
allows them to pause the current visitor until a given visitor
has finished. This allows parallelVisit to work on a dependency
graph while it is being mutated.
Test: Test_parallelVisit
Change-Id: Id8b1542c22ac9914439310e31d992ae0d7318d69
Pull request #317 made the equality check in moduleMatchingVariant
more correct by comparing the variant maps instead of the names.
Using only the names effectively ignores any variation with an
empty name. Unfortuantely this broke a current user of
ReplaceDependenciesIf. Go back to the relaxed check for now.
Test: build on a branch with sdk modules
Change-Id: I11016c8df7bd91fd022d04fd3033756f54d7fa0b
AddFarVariationDependencies claims to take the first variant that
matches all the requested variations, but it did nothing of the sort.
It took the first variant that either matched or did not contain each
of the requested variations. A module with no variations was always
matched, and requesting variations that didn't apply to a module
still matched (for example, requesting an image variation for a
host module that was ignored by the image mutator).
Fix AddFarVariationDependencies by making subset actually check
for subsets.
Test: Test_findVariant
Change-Id: I10063fec342db2a1c0685a7db08e4a650d14bd4e
AddFarVariationDependencies takes the first matching variant. To
maintain sensible behavior on a module with aliases, the ordering
of aliases and module variants needs to be maintained so that
AddFarVariationDependencies can find an earlier matching alias
instead of a more specific variant.
Test: go test .
Change-Id: I78f4e96edd98159f3a62d94e240e5d652667bec4
Add tests for findVariant behavior that provides the matching
behaviors of AddVariationDependencies, AddFarVariationDependencies,
etc.
Test: Test_findVariant
Change-Id: I3494d57179c8b3d62f7d32e5a1b43c9b9672c2df
CreateAliasVariation creates a new variation for the current mutator
that is an alias to a variation that was created with CreateVarations
by the same mutator. It is similar to AliasVariation, except that
AliasVariation creates an alias from the pre-mutated variant to one
of the new variations, while CreateAliasVariation creates an alias
from a new post-mutated variation to one of the new variations.
Bug: 164216768
Test: TestCreateAliasVariation
Change-Id: I8847b8d6fc1d3248abc910b2fe2399881f9bdaee
Context.ModulePath is a duplicate of Context.BlueprintFile, and is
misleading because it returns the path to the Android.bp, and not
the path to the directory that contains the Android.bp file like
ModuleContext.ModuleDir.
Change-Id: I505136fce3c3d37595cd4b4e08de5e2691a2a0f6
Adds a CheckBlueprintSyntax(...) method to check the syntax of a
Blueprint file.
Changes processModuleDef and newModule from being method on *Context to
being standalone functions. That ensures that CheckBlueprintSyntax(...)
does not need to take a context and so there is no chance that it can
change its state.
Previously a LoadHook could not modify the name of a module because the
module was registered before the LoadHooks were run. That made it very
complicated (requiring mutators and auto generated names) to create a
module type whose name was determined by say the directory in which it
is defined.
This change moves the LoadHook execution slightly earlier so it runs
before registration of the module.
That caused one slight side problem which was that the
moduleInfo.Name() would fail when called in a LoadHook. That was
because that gets the name from group.name but was group was nil
because it is only set when the module is registered.
Modifying the moduleInfo.Name() method to get the name from the module
logicModule.Name() if group is nil fixed that. The reason for getting
the name from the group.name rather than the logicModule.Name() is that
the former tracks renames but the latter does not. However that is not
an issue in this case as there has been no opportunity for the module
to be renamed until after the LoadHook has returned.
Previously, WalkDeps() would record that a module was visited after the
first time it encountered the module irrespective of whether it recursed
into or not. This change moves the recording so it happens only after it
has been recursed into.
Added TestWalkDepsDuplicates_IgnoreFirstPath to test the change. Without
the change the test fails because it does not visit E.
Test refactoring:
* A depsMutator was added instead of relying on blueprintDepsMutator to
allow different tags to be used for different dependency types.
* Modified barModule and fooModule to support the new depsMutator and
add support for another type of dependency that is ignored by the
walking code.
* Extracted walkDependencyGraph() function to reuse common code.
Mutators were not propagating the results of ctx.AddNinjaFileDeps.
Test: examine out/soong/build.ninja.d
Fixes: 150689149
Change-Id: Ia1e69ebc9dfa94a05f4ecd9cc2a8691ee63c9dd5
There are 8935901 *ninjaString objects generated in an AOSP
aosp_blueline-userdebug build, and 7865180 of those are a literal
string with no ninja variables.
Each of those *ninjaString objects takes a minimum of 48 bytes for
2 slices, plus 8 bytes for the pointer to the ninjaString. For
the literal string case, one of those slices has a single element,
(costing another 16 bytes for the backing array), and the other
slice is empty, for a total of 72 bytes.
Replace *ninjaString with a ninjaString interface. This increases
the size of the reference from 8 bytes to 16 bytes, but using
a type alias of a string for the literal string implementation uses
only 16 bytes, saving 40 bytes per literal string or 314 MB.
Test: ninja_strings_test
Change-Id: Ic5fe16ed1f2a244fe6a8ccdf762919634d825cbe
The proptools functions took an inconsistent variety of
struct and *struct types. Some methods even took a struct
but returned a *struct. Make all the exported methods
take a *struct, with internal helpers for the ones that need
to take a struct.
Test: proptools tests
Change-Id: I60ce212606e96adcef66c531d57f69c39e1a1638
Add RegisterScopedModuleType to LoadHookContext that registers
a module type factory that will be visible for the remainder of
the file. Also add ModuleFactories that returns the globally
register module factories.
Test: all blueprint tests
Change-Id: If646a73befe3b8e45c4b0984531c6a39ddc8d066
Move LoadHooks from Blueprint and run them during ParseBlueprintsFiles.
This will allow them to add scoped module types that are visible to the
rest of the Blueprints file. Requires passing the config object to
ParseBlueprintsFiles.
Test: all blueprint tests
Change-Id: Ia2a2c9a0223d5458bfd48bd22ebed0fdbd0156c6
Adding a dependency on a module with variants can be problematic
if the code adding the dependency is not aware of every mutator
that has created variants. Add an AliasVariations to
BottomUpMutatorContext, which allows a mutator to alias the
original variant of a module to one of the new variants of the
module, which will allow future dependencies to be added using
the original list of variations. The aliases are transient,
and only exist until the next mutator that calls CreateVariations
when visiting the module without also calling AliasVariations.
Test: TestAlises
Change-Id: Ieaa04b5a6bdcb5a1ff5114b1e03460de795d4479
Modules are created with a nil variantionMap, which avoids allocating
an empty map if the module will never be split.
Context.addVariationDependencies constructs a variationMap to
compare against the variationMap of each variant, but constructed
an empty variationMap if no variations were specified. A nil
map is not the same as an empty map, so consistently use a nil
map and create it when populating the first entry.
Change-Id: I48b604659f9cdb23326b504a093cdfe5a3eb4f68
Use module groups instead of passing around the list of modules
extracted from a module group.
Test: blueprint tests
Change-Id: I02a79950c6377441c49129ebeb5f12be257df668
This fixes a bug that reverse dependency can't be made for modules
having local variations. Previously, when module A having local variants
calls AddReverseDependency to module B having local variants, the match
is tested between the non-local variants of module A against all
variants of module B, which can never be successful.
This change fixes it by using all variants of module A when
findMatchingVariants is called for AddReverseDependency.
Test: m
Change-Id: Ib289188a5dd58c72bd6ba07e3c0f825f8b1c6b1b
SetDefaultDependencyVariation sets the variation name that will be used
when a dangling dependency is found while a module is being split. A
dangling dependency can occur if a module is split to a variant that one
of its dependencies is not split into. When the default variation is not
set, such dangling dependency is a hard error. But with the new
function, the default variation can be set and subsequent calls to
CreateVariations and its variations on the same context uses the default
variation when necessary.
(If even the default variation does not exist for the dependent module,
it is an hard error)
Note that this is different from calling SetDependencyVariation("foo")
followed by CreateVariations("foo", "bar"). In that case, regardless of
whether a dependency of the current module has the variant 'bar' or not,
only the 'foo' variant is chosen.
With SetDefaultDependencyVariation("foo") followed by
CreateVariations("foo", "bar"), 'foo' variant is used only when the
'bar' variant of the current module depends on a module that does not
have 'bar' variant.
Bug: 138103882
Test: m
Change-Id: I4520ca87487994de024fdbacda3bef6636225f0d
Move most of the mutator context methods into BaseModuleContext
so they are available to both top down and bottom up mutators.
Only CreateModule is unique to TopDownMutatorContext, and the
dependency and variation adding methods are unique to the
BottomUpMutatorContext.
The dependency visiting methods are now available on
BottomUpMutatorContext, which requires delaying making newly
added dependencies visible to maintain the invariant that
the mutator has been called on the dependency before the
dependency can be visited by its parents.
Test: m checkbuild
Change-Id: Ie14afc02ac76d0b5a66b0e52de2aa9e17fd1bec0
The go comment for the blueprint.Context.ParseBlueprintsFiles was
actually place above the blueprint.Context.ParseFileList.
Bug: N/A
Test: N/A
Change-Id: I065d2f6ec034c614e40de745931f4c87cdb73422
Add Context.Singletons and Context.SingletonName to allow primary
builders to query Singletons returned by all registered
SingletonFactories.
Change-Id: Iee85643dd47f7472e6bb5ae8004374bd6ea0419e
1. Extract module type documentation.
2. Support primary builder customization of factory function to use for
documentation for each module type.
3. Change the ModuleType list order so that they are grouped by package.
This is basically minor refactoring + readability improvement done on
top of https://github.com/google/blueprint/pull/232.
Change-Id: If7413e5ac23486b85f18d02fb3ba288a38730c32
Writing the ninja file to a byte buffer causes a significant amount
of time to be spent in memmove when growing the byte slice. Write
the file directly to disk instead.
This also fixes some unhandled error warnings, which become more
likely when doing disk IO instead of byte buffer writes.
Change-Id: I5094e4c45cab4012713037f60c5a4fb00718f92e
Throw an error if run into a module without a name when generating
contexts.
Test: context_test.go
Change-Id: I3976d86d1f15b8ac10a7a38aa42ae277740a8f3b
Instead of sometimes re-running minibp/the primary builder during the
next phase, run bpglob earlier to check dependencies.
We've run into issues where the environment is slightly different
between bootstrapping phase and the main build phase. It's also a
problem because our primary builder (Soong) exports information used by
another tool (Kati) that runs in between the bootstrapping phases and
the main phase. When Soong would run in the main phase, it could get out
of sync, and would require the build to be run again.
To do this, add a "subninja" include a build-globs.ninja file to each
build.ninja file. The first time, this will be an empty file, but we'll
always run minibp / the primary builder anyway. When the builder runs,
in addition to writing a dependency file, write out the
build-globs.ninja file with the rules to run bpglob.
Since bpglob may need to be run very early, before it would normally be
built, build it with microfactory.
Change-Id: I89fcd849a8729e892f163d40060ab90b5d4dfa5d
WalkDeps was following every possible path through the dependency
tree, which can be enormous. Modify it to only call visit for
any particular (child, parent) pair once for each direct dependency
by not recursing into child if visitDown returns true but child
has already been visited.
Test: TestWalkDeps, TestWalkDepsDuplicates
Change-Id: Ieef28399bd10e744417cdeb661dfa04fbeb4ec60
Allow adding multiple dependencies on the same module. Adds a flag
to Context.walkDeps to determine whether dependencies should be
visited multiple times, and clarifies the behavior of
VisitDepsDepthFirst (continues to visit each module once, even if
there are multiple dependencies on it) and VisitDirectDeps (may
visit a module multiple times).
Test: TestWalkDepsDuplicates, TestVisit
Change-Id: I58afbe90490aca102d242d63e185386e1fe55d73
Running with the data race detector enabled hits a 8192 active
goroutine limit. Reduce the maximum number of active goroutines
by limiting parallelVisit to 1000 goroutines. Also rewrite
cloneModule in terms of parallelVisit.
Change-Id: Icdd63648e8e010557b624e12592156490b40adb9