From 70265ab3899b992c4842de1d2f52a4ee2fcf9dd4 Mon Sep 17 00:00:00 2001 From: "Harper, Jason M" Date: Thu, 19 Feb 2026 14:55:37 -0800 Subject: [PATCH 1/2] fix: address Ampere1 event processing issues Signed-off-by: Harper, Jason M --- cmd/metrics/loader_component.go | 76 +++++++++++++++++++++++++++++++-- cmd/metrics/metadata.go | 1 + cmd/metrics/metrics.go | 2 + internal/cpus/cpus.go | 17 ++++---- 4 files changed, 84 insertions(+), 12 deletions(-) diff --git a/cmd/metrics/loader_component.go b/cmd/metrics/loader_component.go index a53ad8ce..a829af33 100644 --- a/cmd/metrics/loader_component.go +++ b/cmd/metrics/loader_component.go @@ -13,6 +13,7 @@ import ( "perfspect/internal/util" "regexp" "slices" + "strconv" "strings" "github.com/casbin/govaluate" @@ -56,6 +57,10 @@ func (cm *ComponentMetric) getLegacyName() string { type ComponentEvent struct { ArchStdEvent string `json:"ArchStdEvent"` PublicDescription string `json:"PublicDescription"` + EventCode string `json:"EventCode"` + EventName string `json:"EventName"` + BriefDescription string `json:"BriefDescription"` + Errata string `json:"Errata"` } func (l *ComponentLoader) loadMetricDefinitions(selectedMetrics []string, metadata Metadata) ([]MetricDefinition, error) { @@ -196,7 +201,11 @@ func (l *ComponentLoader) formEventGroups(metrics []MetricDefinition, events []C numGPCounters := metadata.NumGeneralPurposeCounters // groups can have at most this many events (plus fixed counters) eventNames := make(map[string]bool) for _, event := range events { - eventNames[event.ArchStdEvent] = true + if event.EventName != "" { + eventNames[event.EventName] = true + } else if event.ArchStdEvent != "" { + eventNames[event.ArchStdEvent] = true + } } for _, metric := range metrics { @@ -226,7 +235,28 @@ func (l *ComponentLoader) formEventGroups(metrics []MetricDefinition, events []C continue } // Add the event to the current group - currentGroup = append(currentGroup, EventDefinition{Name: variable, Raw: variable, Device: "cpu"}) + var perfRaw string + event := findEventByName(events, variable) + if event == nil { + slog.Warn("Could not find event definition for metric variable, skipping variable", slog.String("metric", metric.Name), slog.String("variable", variable)) + continue + } + if event.ArchStdEvent != "" { + perfRaw = event.ArchStdEvent + } else if event.EventName != "" { + if strings.Contains(metadata.PerfSupportedEvents, event.EventName) { + perfRaw = event.EventName + } else if event.EventCode != "" { + perfRaw = fmt.Sprintf("armv8_pmuv3_0/config=%s,name=%s/", event.EventCode, variable) + } else { + slog.Warn("Event definition for metric variable does not have ArchStdEvent, EventName supported by perf, or EventCode, skipping variable", slog.String("metric", metric.Name), slog.String("variable", variable)) + continue + } + } else { + slog.Warn("Event definition for metric variable does not have EventName or ArchStdEvent, skipping variable", slog.String("metric", metric.Name), slog.String("variable", variable)) + continue + } + currentGroup = append(currentGroup, EventDefinition{Name: variable, Raw: perfRaw, Device: "cpu"}) // Only increment the GP counter count if this isn't a fixed counter if variable != "CPU_CYCLES" { @@ -256,6 +286,16 @@ func (l *ComponentLoader) formEventGroups(metrics []MetricDefinition, events []C return groups, nil } +// findEventByName searches for an event by name in the list of events +func findEventByName(events []ComponentEvent, name string) *ComponentEvent { + for _, event := range events { + if event.EventName == name || event.ArchStdEvent == name { + return &event + } + } + return nil +} + // mergeSmallGroups merges groups that have few events, ensuring that the merged group does not exceed numGPCounters // CPU_CYCLES is a fixed counter and does not count against the numGPCounters limit // events in a group are unique (no duplicates) @@ -385,7 +425,8 @@ func initializeComponentMetricVariables(expression string) map[string]int { } constants := map[string]bool{ - "#slots": true, + "#slots": true, + "duration_time": true, } functions := map[string]bool{ @@ -406,7 +447,7 @@ func initializeComponentMetricVariables(expression string) map[string]int { } // Skip some tokens - if isInteger(token) || isHex(token) || operators[token] || constants[token] || functions[token] { + if isInteger(token) || isHex(token) || isExp(token) || operators[token] || constants[token] || functions[token] { continue } @@ -417,6 +458,19 @@ func initializeComponentMetricVariables(expression string) map[string]int { return variables } +func isExp(s string) bool { + // Check if the string is in the format 1eN where N is a positive integer + if len(s) < 3 || s[0] != '1' || s[1] != 'e' { + return false + } + for _, c := range s[2:] { + if c < '0' || c > '9' { + return false + } + } + return true +} + func isHex(s string) bool { // Check if the string starts with "0x" or "0X" if len(s) < 3 || !(strings.HasPrefix(s, "0x") || strings.HasPrefix(s, "0X")) { @@ -445,6 +499,8 @@ func isInteger(s string) bool { func initializeComponentMetricEvaluable(expression string, evaluatorFunctions map[string]govaluate.ExpressionFunction, metadata Metadata) *govaluate.EvaluableExpression { // replace #slots with metadata.ARMSlots transformedExpression := strings.ReplaceAll(expression, "#slots", fmt.Sprintf("%d", metadata.ARMSlots)) + // replace duration_time with metadata.CollectionInterval.Seconds() + transformedExpression = strings.ReplaceAll(transformedExpression, "duration_time", fmt.Sprintf("%f", metadata.CollectionInterval.Seconds())) // replace if else with ?: transformedExpression, err := transformExpression(transformedExpression) if err != nil { @@ -469,6 +525,18 @@ func initializeComponentMetricEvaluable(expression string, evaluatorFunctions ma } return match // Should not happen, but return the original match if extraction fails }) + // govaluate doesn't like numeric constances in the format 1eN where N is a positive integer, so we replace them with the equivalent integer value + rxScientific := regexp.MustCompile(`1e(\d+)`) + if rxScientific.MatchString(transformedExpression) { + transformedExpression = rxScientific.ReplaceAllStringFunc(transformedExpression, func(match string) string { + exp, _ := strconv.Atoi(match[2:]) + result := 1 + for range exp { + result *= 10 + } + return fmt.Sprintf("%d", result) + }) + } if transformedExpression != expression { slog.Debug("Transformed metric expression", slog.String("original", expression), slog.String("transformed", transformedExpression)) diff --git a/cmd/metrics/metadata.go b/cmd/metrics/metadata.go index 4b7c7017..87ac4461 100644 --- a/cmd/metrics/metadata.go +++ b/cmd/metrics/metadata.go @@ -99,6 +99,7 @@ type Metadata struct { CollectionStartTime time.Time PerfSpectVersion string WithWorkload bool // true if metrics were collected with a user-provided workload application + CollectionInterval time.Duration } // MetadataCollector defines the interface for architecture-specific metadata collection. diff --git a/cmd/metrics/metrics.go b/cmd/metrics/metrics.go index 2c7bdf11..4b8c62ba 100644 --- a/cmd/metrics/metrics.go +++ b/cmd/metrics/metrics.go @@ -1230,6 +1230,8 @@ func prepareMetrics(targetContext *targetContext, localTempDir string, channelEr channelError <- targetError{target: myTarget, err: err} return } + // set the collection interval before loading metrics so that metric expressions using duration_time are compiled with the correct value + targetContext.metadata.CollectionInterval = time.Duration(flagPerfPrintInterval) * time.Second slog.Debug("metadata: " + targetContext.metadata.String()) if !targetContext.metadata.SupportsInstructions { slog.Error("Target does not support instructions event collection", slog.String("target", myTarget.GetName())) diff --git a/internal/cpus/cpus.go b/internal/cpus/cpus.go index b548a257..0ce48437 100644 --- a/internal/cpus/cpus.go +++ b/internal/cpus/cpus.go @@ -206,14 +206,15 @@ var cpuIdentifiersARM = []struct { Identifier CPUIdentifierARM MicroArchitecture string }{ - {CPUIdentifierARM{Implementer: "0x41", Part: "0xd0c", DmidecodePart: "AWS Graviton2"}, UarchGraviton2}, // AWS Graviton 2 ([m|c|r]6g) Neoverse-N1 - {CPUIdentifierARM{Implementer: "0x41", Part: "0xd40", DmidecodePart: "AWS Graviton3"}, UarchGraviton3}, // AWS Graviton 3 ([m|c|r]7g) Neoverse-V1 - {CPUIdentifierARM{Implementer: "0x41", Part: "0xd4f", DmidecodePart: "AWS Graviton4"}, UarchGraviton4}, // AWS Graviton 4 ([m|c|r]8g) Neoverse-V2 - {CPUIdentifierARM{Implementer: "0x41", Part: "0xd4f", DmidecodePart: "Not Specified"}, UarchAxion}, // GCP Axion (c4a) Neoverse-V2 - {CPUIdentifierARM{Implementer: "0x41", Part: "0xd0c", DmidecodePart: "Not Specified"}, UarchAltraFamily}, // Ampere Altra - {CPUIdentifierARM{Implementer: "0xc0", Part: "0xac3", DmidecodePart: ""}, UarchAmpereOneAC03}, // AmpereOne AC03 - {CPUIdentifierARM{Implementer: "0xc0", Part: "0xac4", DmidecodePart: "X"}, UarchAmpereOneAC04}, // AmpereOne AC04 - {CPUIdentifierARM{Implementer: "0xc0", Part: "0xac4", DmidecodePart: "M"}, UarchAmpereOneAC04_1}, // AmpereOne AC04_1 + {CPUIdentifierARM{Implementer: "0x41", Part: "0xd0c", DmidecodePart: "AWS Graviton2"}, UarchGraviton2}, // AWS Graviton 2 ([m|c|r]6g) Neoverse-N1 + {CPUIdentifierARM{Implementer: "0x41", Part: "0xd40", DmidecodePart: "AWS Graviton3"}, UarchGraviton3}, // AWS Graviton 3 ([m|c|r]7g) Neoverse-V1 + {CPUIdentifierARM{Implementer: "0x41", Part: "0xd4f", DmidecodePart: "AWS Graviton4"}, UarchGraviton4}, // AWS Graviton 4 ([m|c|r]8g) Neoverse-V2 + {CPUIdentifierARM{Implementer: "0x41", Part: "0xd4f", DmidecodePart: "Not Specified"}, UarchAxion}, // GCP Axion (c4a) Neoverse-V2 + {CPUIdentifierARM{Implementer: "0x41", Part: "0xd0c", DmidecodePart: "Not Specified"}, UarchAltraFamily}, // Ampere Altra + {CPUIdentifierARM{Implementer: "0xc0", Part: "0xac3", DmidecodePart: ""}, UarchAmpereOneAC03}, // AmpereOne AC03 + {CPUIdentifierARM{Implementer: "0xc0", Part: "0xac4", DmidecodePart: "X"}, UarchAmpereOneAC04}, // AmpereOne AC04 + {CPUIdentifierARM{Implementer: "0xc0", Part: "0xac4", DmidecodePart: "M"}, UarchAmpereOneAC04_1}, // AmpereOne AC04_1 + {CPUIdentifierARM{Implementer: "0xc0", Part: "0xac4", DmidecodePart: "Not Specified"}, UarchAmpereOneAC04_1}, // Ampere-1a (VM.Standard.A4.Flex on OCI) } // NewCPUIdentifier creates a CPUIdentifier with all data elements From c1b84d25ba0f407d5d9147b20885350b771e60eb Mon Sep 17 00:00:00 2001 From: "Harper, Jason M" Date: Fri, 20 Feb 2026 15:46:08 -0800 Subject: [PATCH 2/2] address review comments Signed-off-by: Harper, Jason M --- cmd/metrics/loader_component.go | 24 +++++++----------------- 1 file changed, 7 insertions(+), 17 deletions(-) diff --git a/cmd/metrics/loader_component.go b/cmd/metrics/loader_component.go index a829af33..ed0d890d 100644 --- a/cmd/metrics/loader_component.go +++ b/cmd/metrics/loader_component.go @@ -199,14 +199,6 @@ func (l *ComponentLoader) loadEventDefinitions(metadata Metadata) (events []Comp func (l *ComponentLoader) formEventGroups(metrics []MetricDefinition, events []ComponentEvent, metadata Metadata) (groups []GroupDefinition, err error) { numGPCounters := metadata.NumGeneralPurposeCounters // groups can have at most this many events (plus fixed counters) - eventNames := make(map[string]bool) - for _, event := range events { - if event.EventName != "" { - eventNames[event.EventName] = true - } else if event.ArchStdEvent != "" { - eventNames[event.ArchStdEvent] = true - } - } for _, metric := range metrics { var metricGroups []GroupDefinition @@ -229,11 +221,6 @@ func (l *ComponentLoader) formEventGroups(metrics []MetricDefinition, events []C slices.Sort(variables) for _, variable := range variables { - // confirm variable is a valid event - if _, exists := eventNames[variable]; !exists { - slog.Warn("Metric variable does not correspond to a known event, skipping variable", slog.String("metric", metric.Name), slog.String("variable", variable)) - continue - } // Add the event to the current group var perfRaw string event := findEventByName(events, variable) @@ -244,6 +231,7 @@ func (l *ComponentLoader) formEventGroups(metrics []MetricDefinition, events []C if event.ArchStdEvent != "" { perfRaw = event.ArchStdEvent } else if event.EventName != "" { + // if the event name is supported by perf, use that. Otherwise, fall back to using the event code with the armv8_pmuv3_0/config= raw event format if strings.Contains(metadata.PerfSupportedEvents, event.EventName) { perfRaw = event.EventName } else if event.EventCode != "" { @@ -253,6 +241,7 @@ func (l *ComponentLoader) formEventGroups(metrics []MetricDefinition, events []C continue } } else { + // this shouldn't happen since the variable should match either ArchStdEvent or EventName, but log just in case slog.Warn("Event definition for metric variable does not have EventName or ArchStdEvent, skipping variable", slog.String("metric", metric.Name), slog.String("variable", variable)) continue } @@ -287,10 +276,11 @@ func (l *ComponentLoader) formEventGroups(metrics []MetricDefinition, events []C } // findEventByName searches for an event by name in the list of events +// it checks both EventName and ArchStdEvent fields for a match func findEventByName(events []ComponentEvent, name string) *ComponentEvent { - for _, event := range events { - if event.EventName == name || event.ArchStdEvent == name { - return &event + for i := range events { + if events[i].EventName == name || events[i].ArchStdEvent == name { + return &events[i] } } return nil @@ -525,7 +515,7 @@ func initializeComponentMetricEvaluable(expression string, evaluatorFunctions ma } return match // Should not happen, but return the original match if extraction fails }) - // govaluate doesn't like numeric constances in the format 1eN where N is a positive integer, so we replace them with the equivalent integer value + // govaluate doesn't like numeric constants in the format 1eN where N is a positive integer, so we replace them with the equivalent integer value rxScientific := regexp.MustCompile(`1e(\d+)`) if rxScientific.MatchString(transformedExpression) { transformedExpression = rxScientific.ReplaceAllStringFunc(transformedExpression, func(match string) string {