From 4f5d2c1e97bbd20b61b346a31c62d996993a894d Mon Sep 17 00:00:00 2001 From: usta Date: Fri, 28 Oct 2022 23:32:01 -0400 Subject: [PATCH] include symlink metrics in bp2build_metrics.pb Bug: b/256212479 Test: Prior to thi CL bp2build.symlink_forest event was missing in bp2build_metrics.pb after a clean mixed build Change-Id: I53bfc4114a383c0d1f9c4c7945e7d4c69bc50b0c --- bp2build/bp2build.go | 4 +- bp2build/build_conversion.go | 8 +-- bp2build/conversion.go | 6 +- bp2build/conversion_test.go | 2 +- bp2build/metrics.go | 123 ++++++++++++++++++++--------------- cmd/soong_build/main.go | 25 ++++--- ui/build/upload.go | 5 +- 7 files changed, 98 insertions(+), 75 deletions(-) diff --git a/bp2build/bp2build.go b/bp2build/bp2build.go index 0e3d2a5f9..a75a84e13 100644 --- a/bp2build/bp2build.go +++ b/bp2build/bp2build.go @@ -26,7 +26,7 @@ import ( // Codegen is the backend of bp2build. The code generator is responsible for // writing .bzl files that are equivalent to Android.bp files that are capable // of being built with Bazel. -func Codegen(ctx *CodegenContext) CodegenMetrics { +func Codegen(ctx *CodegenContext) *CodegenMetrics { // This directory stores BUILD files that could be eventually checked-in. bp2buildDir := android.PathForOutput(ctx, "bp2build") if err := android.RemoveAllOutputDir(bp2buildDir); err != nil { @@ -48,7 +48,7 @@ func Codegen(ctx *CodegenContext) CodegenMetrics { soongInjectionDir := android.PathForOutput(ctx, bazel.SoongInjectionDirName) writeFiles(ctx, soongInjectionDir, CreateSoongInjectionFiles(ctx.Config(), res.metrics)) - return res.metrics + return &res.metrics } // Get the output directory and create it if it doesn't exist. diff --git a/bp2build/build_conversion.go b/bp2build/build_conversion.go index 82ce115f6..a06b89edb 100644 --- a/bp2build/build_conversion.go +++ b/bp2build/build_conversion.go @@ -28,7 +28,6 @@ import ( "android/soong/android" "android/soong/bazel" "android/soong/starlark_fmt" - "github.com/google/blueprint" "github.com/google/blueprint/proptools" ) @@ -245,12 +244,7 @@ func GenerateBazelTargets(ctx *CodegenContext, generateFilegroups bool) (convers buildFileToTargets := make(map[string]BazelTargets) // Simple metrics tracking for bp2build - metrics := CodegenMetrics{ - ruleClassCount: make(map[string]uint64), - convertedModulePathMap: make(map[string]string), - convertedModuleTypeCount: make(map[string]uint64), - totalModuleTypeCount: make(map[string]uint64), - } + metrics := CreateCodegenMetrics() dirs := make(map[string]bool) diff --git a/bp2build/conversion.go b/bp2build/conversion.go index 4d8b8a450..8ca13b8af 100644 --- a/bp2build/conversion.go +++ b/bp2build/conversion.go @@ -32,7 +32,7 @@ func CreateSoongInjectionFiles(cfg android.Config, metrics CodegenMetrics) []Baz files = append(files, newFile("apex_toolchain", GeneratedBuildFileName, "")) // Creates a //apex_toolchain package. files = append(files, newFile("apex_toolchain", "constants.bzl", apex.BazelApexToolchainVars())) - files = append(files, newFile("metrics", "converted_modules.txt", strings.Join(metrics.convertedModules, "\n"))) + files = append(files, newFile("metrics", "converted_modules.txt", strings.Join(metrics.Serialize().ConvertedModules, "\n"))) convertedModulePathMap, err := json.MarshalIndent(metrics.convertedModulePathMap, "", "\t") if err != nil { @@ -55,10 +55,6 @@ func CreateSoongInjectionFiles(cfg android.Config, metrics CodegenMetrics) []Baz return files } -func convertedModules(convertedModules []string) string { - return strings.Join(convertedModules, "\n") -} - func CreateBazelFiles( cfg android.Config, ruleShims map[string]RuleShim, diff --git a/bp2build/conversion_test.go b/bp2build/conversion_test.go index b696a9895..cfd612815 100644 --- a/bp2build/conversion_test.go +++ b/bp2build/conversion_test.go @@ -84,7 +84,7 @@ func TestCreateBazelFiles_QueryView_AddsTopLevelFiles(t *testing.T) { func TestCreateBazelFiles_Bp2Build_CreatesDefaultFiles(t *testing.T) { testConfig := android.TestConfig("", make(map[string]string), "", make(map[string][]byte)) - files := CreateSoongInjectionFiles(testConfig, CodegenMetrics{}) + files := CreateSoongInjectionFiles(testConfig, CreateCodegenMetrics()) expectedFilePaths := []bazelFilepath{ { diff --git a/bp2build/metrics.go b/bp2build/metrics.go index 0b459965c..bd216292a 100644 --- a/bp2build/metrics.go +++ b/bp2build/metrics.go @@ -9,25 +9,16 @@ import ( "android/soong/android" "android/soong/shared" "android/soong/ui/metrics/bp2build_metrics_proto" + "google.golang.org/protobuf/proto" "github.com/google/blueprint" ) -// Simple metrics struct to collect information about a Blueprint to BUILD +// CodegenMetrics represents information about the Blueprint-to-BUILD // conversion process. +// Use CreateCodegenMetrics() to get a properly initialized instance type CodegenMetrics struct { - // Total number of Soong modules converted to generated targets - generatedModuleCount uint64 - - // Total number of Soong modules converted to handcrafted targets - handCraftedModuleCount uint64 - - // Total number of unconverted Soong modules - unconvertedModuleCount uint64 - - // Counts of generated Bazel targets per Bazel rule class - ruleClassCount map[string]uint64 - + serialized *bp2build_metrics_proto.Bp2BuildMetrics // List of modules with unconverted deps // NOTE: NOT in the .proto moduleWithUnconvertedDepsMsgs []string @@ -36,40 +27,32 @@ type CodegenMetrics struct { // NOTE: NOT in the .proto moduleWithMissingDepsMsgs []string - // List of converted modules - convertedModules []string - // Map of converted modules and paths to call + // NOTE: NOT in the .proto convertedModulePathMap map[string]string +} - // Counts of converted modules by module type. - convertedModuleTypeCount map[string]uint64 - - // Counts of total modules by module type. - totalModuleTypeCount map[string]uint64 - - Events []*bp2build_metrics_proto.Event +func CreateCodegenMetrics() CodegenMetrics { + return CodegenMetrics{ + serialized: &bp2build_metrics_proto.Bp2BuildMetrics{ + RuleClassCount: make(map[string]uint64), + ConvertedModuleTypeCount: make(map[string]uint64), + TotalModuleTypeCount: make(map[string]uint64), + }, + convertedModulePathMap: make(map[string]string), + } } // Serialize returns the protoized version of CodegenMetrics: bp2build_metrics_proto.Bp2BuildMetrics -func (metrics *CodegenMetrics) Serialize() bp2build_metrics_proto.Bp2BuildMetrics { - return bp2build_metrics_proto.Bp2BuildMetrics{ - GeneratedModuleCount: metrics.generatedModuleCount, - HandCraftedModuleCount: metrics.handCraftedModuleCount, - UnconvertedModuleCount: metrics.unconvertedModuleCount, - RuleClassCount: metrics.ruleClassCount, - ConvertedModules: metrics.convertedModules, - ConvertedModuleTypeCount: metrics.convertedModuleTypeCount, - TotalModuleTypeCount: metrics.totalModuleTypeCount, - Events: metrics.Events, - } +func (metrics *CodegenMetrics) Serialize() *bp2build_metrics_proto.Bp2BuildMetrics { + return metrics.serialized } // Print the codegen metrics to stdout. func (metrics *CodegenMetrics) Print() { generatedTargetCount := uint64(0) - for _, ruleClass := range android.SortedStringKeys(metrics.ruleClassCount) { - count := metrics.ruleClassCount[ruleClass] + for _, ruleClass := range android.SortedStringKeys(metrics.serialized.RuleClassCount) { + count := metrics.serialized.RuleClassCount[ruleClass] fmt.Printf("[bp2build] %s: %d targets\n", ruleClass, count) generatedTargetCount += count } @@ -80,9 +63,9 @@ func (metrics *CodegenMetrics) Print() { %d converted modules have missing deps: %s `, - metrics.generatedModuleCount, + metrics.serialized.GeneratedModuleCount, generatedTargetCount, - metrics.handCraftedModuleCount, + metrics.serialized.HandCraftedModuleCount, metrics.TotalModuleCount(), len(metrics.moduleWithUnconvertedDepsMsgs), strings.Join(metrics.moduleWithUnconvertedDepsMsgs, "\n\t"), @@ -119,29 +102,67 @@ func (metrics *CodegenMetrics) Write(dir string) { fail(err, "Error outputting %s", metricsFile) } if _, err := os.Stat(metricsFile); err != nil { - fail(err, "MISSING BP2BUILD METRICS OUTPUT: Failed to `stat` %s", metricsFile) + if os.IsNotExist(err) { + fail(err, "MISSING BP2BUILD METRICS OUTPUT: %s", metricsFile) + } else { + fail(err, "FAILED TO `stat` BP2BUILD METRICS OUTPUT: %s", metricsFile) + } + } +} + +// ReadCodegenMetrics loads CodegenMetrics from `dir` +// returns a nil pointer if the file doesn't exist +func ReadCodegenMetrics(dir string) *CodegenMetrics { + metricsFile := filepath.Join(dir, bp2buildMetricsFilename) + if _, err := os.Stat(metricsFile); err != nil { + if os.IsNotExist(err) { + return nil + } else { + fail(err, "FAILED TO `stat` BP2BUILD METRICS OUTPUT: %s", metricsFile) + panic("unreachable after fail") + } + } + if buf, err := os.ReadFile(metricsFile); err != nil { + fail(err, "FAILED TO READ BP2BUILD METRICS OUTPUT: %s", metricsFile) + panic("unreachable after fail") + } else { + bp2BuildMetrics := bp2build_metrics_proto.Bp2BuildMetrics{ + RuleClassCount: make(map[string]uint64), + ConvertedModuleTypeCount: make(map[string]uint64), + TotalModuleTypeCount: make(map[string]uint64), + } + if err := proto.Unmarshal(buf, &bp2BuildMetrics); err != nil { + fail(err, "FAILED TO PARSE BP2BUILD METRICS OUTPUT: %s", metricsFile) + } + return &CodegenMetrics{ + serialized: &bp2BuildMetrics, + convertedModulePathMap: make(map[string]string), + } } } func (metrics *CodegenMetrics) IncrementRuleClassCount(ruleClass string) { - metrics.ruleClassCount[ruleClass] += 1 + metrics.serialized.RuleClassCount[ruleClass] += 1 } +func (metrics *CodegenMetrics) AddEvent(event *bp2build_metrics_proto.Event) { + metrics.serialized.Events = append(metrics.serialized.Events, event) +} func (metrics *CodegenMetrics) AddUnconvertedModule(moduleType string) { - metrics.unconvertedModuleCount += 1 - metrics.totalModuleTypeCount[moduleType] += 1 + metrics.serialized.UnconvertedModuleCount += 1 + metrics.serialized.TotalModuleTypeCount[moduleType] += 1 } func (metrics *CodegenMetrics) TotalModuleCount() uint64 { - return metrics.handCraftedModuleCount + - metrics.generatedModuleCount + - metrics.unconvertedModuleCount + return metrics.serialized.HandCraftedModuleCount + + metrics.serialized.GeneratedModuleCount + + metrics.serialized.UnconvertedModuleCount } // Dump serializes the metrics to the given filename func (metrics *CodegenMetrics) dump(filename string) (err error) { ser := metrics.Serialize() - return shared.Save(&ser, filename) + return shared.Save(ser, filename) } type ConversionType int @@ -154,14 +175,14 @@ const ( func (metrics *CodegenMetrics) AddConvertedModule(m blueprint.Module, moduleType string, dir string, conversionType ConversionType) { // Undo prebuilt_ module name prefix modifications moduleName := android.RemoveOptionalPrebuiltPrefix(m.Name()) - metrics.convertedModules = append(metrics.convertedModules, moduleName) + metrics.serialized.ConvertedModules = append(metrics.serialized.ConvertedModules, moduleName) metrics.convertedModulePathMap[moduleName] = "//" + dir - metrics.convertedModuleTypeCount[moduleType] += 1 - metrics.totalModuleTypeCount[moduleType] += 1 + metrics.serialized.ConvertedModuleTypeCount[moduleType] += 1 + metrics.serialized.TotalModuleTypeCount[moduleType] += 1 if conversionType == Handcrafted { - metrics.handCraftedModuleCount += 1 + metrics.serialized.HandCraftedModuleCount += 1 } else if conversionType == Generated { - metrics.generatedModuleCount += 1 + metrics.serialized.GeneratedModuleCount += 1 } } diff --git a/cmd/soong_build/main.go b/cmd/soong_build/main.go index 1f3507db6..1e356e5b2 100644 --- a/cmd/soong_build/main.go +++ b/cmd/soong_build/main.go @@ -651,13 +651,23 @@ func runSymlinkForestCreation(configuration android.Config, extraNinjaDeps []str writeDepFile(symlinkForestMarker, eventHandler, ninjaDeps) touch(shared.JoinPath(topDir, symlinkForestMarker)) + metricsDir := configuration.Getenv("LOG_DIR") + codegenMetrics := bp2build.ReadCodegenMetrics(metricsDir) + if codegenMetrics == nil { + m := bp2build.CreateCodegenMetrics() + codegenMetrics = &m + } else { + //TODO (usta) we cannot determine if we loaded a stale file, i.e. from an unrelated prior + //invocation of codegen. We should simply use a separate .pb file + } + writeBp2BuildMetrics(codegenMetrics, configuration, eventHandler) } // Run Soong in the bp2build mode. This creates a standalone context that registers // an alternate pipeline of mutators and singletons specifically for generating // Bazel BUILD files instead of Ninja files. func runBp2Build(configuration android.Config, extraNinjaDeps []string) { - var codegenMetrics bp2build.CodegenMetrics + var codegenMetrics *bp2build.CodegenMetrics eventHandler := metrics.EventHandler{} eventHandler.Do("bp2build", func() { @@ -706,19 +716,18 @@ func runBp2Build(configuration android.Config, extraNinjaDeps []string) { if configuration.IsEnvTrue("BP2BUILD_VERBOSE") { codegenMetrics.Print() } - writeBp2BuildMetrics(&codegenMetrics, configuration, eventHandler) + writeBp2BuildMetrics(codegenMetrics, configuration, eventHandler) } // Write Bp2Build metrics into $LOG_DIR func writeBp2BuildMetrics(codegenMetrics *bp2build.CodegenMetrics, configuration android.Config, eventHandler metrics.EventHandler) { for _, event := range eventHandler.CompletedEvents() { - codegenMetrics.Events = append(codegenMetrics.Events, - &bp2build_metrics_proto.Event{ - Name: event.Id, - StartTime: uint64(event.Start.UnixNano()), - RealTime: event.RuntimeNanoseconds(), - }) + codegenMetrics.AddEvent(&bp2build_metrics_proto.Event{ + Name: event.Id, + StartTime: uint64(event.Start.UnixNano()), + RealTime: event.RuntimeNanoseconds(), + }) } metricsDir := configuration.Getenv("LOG_DIR") if len(metricsDir) < 1 { diff --git a/ui/build/upload.go b/ui/build/upload.go index 687f51977..9f14bdd7c 100644 --- a/ui/build/upload.go +++ b/ui/build/upload.go @@ -18,6 +18,7 @@ package build // another. import ( + "fmt" "io/ioutil" "os" "path/filepath" @@ -56,7 +57,9 @@ func pruneMetricsFiles(paths []string) []string { } if fi.IsDir() { - if l, err := ioutil.ReadDir(p); err == nil { + if l, err := ioutil.ReadDir(p); err != nil { + _, _ = fmt.Fprintf(os.Stderr, "Failed to find files under %s\n", p) + } else { files := make([]string, 0, len(l)) for _, fi := range l { files = append(files, filepath.Join(p, fi.Name()))