-
Notifications
You must be signed in to change notification settings - Fork 45
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
Conversation
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]>
Signed-off-by: Harper, Jason M <[email protected]>
Signed-off-by: Harper, Jason M <[email protected]>
Signed-off-by: Harper, Jason M <[email protected]>
…ric expressions Signed-off-by: Harper, Jason M <[email protected]>
There was a problem hiding this 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
andperfmonevents2perfspect.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 likereBracketedNumber
to clarify its purpose.
reConstantInt := regexp.MustCompile(`\[(\d+)\]`)
scripts/perfmonmetrics2perfspect.py:76
- Indentation for the
else
block is inconsistent with theif
branch above, causing a Python syntax error. Align this line to match the 8-space indent used in theif
branch.
metric["name"] = m["LegacyName"]
No description provided.