Skip to content

Upgrade TMA metrics for GNR #388

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 21 commits into from
Jun 23, 2025
Merged

Upgrade TMA metrics for GNR #388

merged 21 commits into from
Jun 23, 2025

Conversation

harp-intel
Copy link
Contributor

No description provided.

Signed-off-by: Harper, Jason M <[email protected]>
Signed-off-by: Harper, Jason M <[email protected]>
Signed-off-by: Harper, Jason M <[email protected]>
Signed-off-by: Harper, Jason M <[email protected]>
Signed-off-by: Harper, Jason M <[email protected]>
Signed-off-by: Harper, Jason M <[email protected]>
Signed-off-by: Harper, Jason M <[email protected]>
Signed-off-by: Harper, Jason M <[email protected]>
Signed-off-by: Harper, Jason M <[email protected]>
Signed-off-by: Harper, Jason M <[email protected]>
Signed-off-by: Harper, Jason M <[email protected]>
Signed-off-by: Harper, Jason M <[email protected]>
Signed-off-by: Harper, Jason M <[email protected]>
Signed-off-by: Harper, Jason M <[email protected]>
Signed-off-by: Harper, Jason M <[email protected]>
Signed-off-by: Harper, Jason M <[email protected]>
@harp-intel harp-intel marked this pull request as draft June 20, 2025 13:48
@harp-intel harp-intel marked this pull request as ready for review June 20, 2025 14:00
@harp-intel harp-intel requested a review from Copilot June 23, 2025 16:44
Copilot

This comment was marked as outdated.

@harp-intel harp-intel requested a review from Copilot June 23, 2025 17:08
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR upgrades TMA metrics for the Granite Rapids (GNR) platform by adding retire-latency support and refreshing scripts and definitions for event/metric translation and formatting.

  • Add and load *_retire_latency.json data and replace [event:retire_latency] constants in expressions
  • Enhance perfmonmetrics2perfspect.py and perfmonevents2perfspect.py with prefix stripping, parentheses checks, and improved CLI handling
  • Update GNR metric JSON (gnr.json), retire-latency JSON, and event list TXT (gnr.txt) with new and revised metrics/events
  • Inject inline-comment stripping in event_defs.go and wire TXN as an uncollectable event

Reviewed Changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
scripts/perfmonmetrics2perfspect.py Strip metric_ prefix, validate parentheses, and skip ill-formed metrics
scripts/perfmonevents2perfspect.py Refactor format_event, inline usage message update, and optional file logic
scripts/check_events.py Add "TXN" to core-event filter
cmd/metrics/metric_defs.go Load retire latencies, replace [n] constants with regex, adjust transforms
cmd/metrics/event_defs.go Strip end-of-line comments before parsing event definitions
Comments suppressed due to low confidence (2)

cmd/metrics/metric_defs.go:108

  • [nitpick] The variable name reConstantInt is ambiguous; consider renaming it to something more descriptive like reBracketedNumber to clarify its purpose.
	reConstantInt := regexp.MustCompile(`\[(\d+)\]`)

scripts/perfmonmetrics2perfspect.py:76

  • Indentation for the else block is inconsistent with the if branch above, causing a Python syntax error. Align this line to match the 8-space indent used in the if branch.
            metric["name"] = m["LegacyName"]

@harp-intel harp-intel merged commit 92e4586 into main Jun 23, 2025
4 checks passed
@harp-intel harp-intel deleted the gnrtma branch June 26, 2025 16:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant