Skip to content

Conversation

@harp-intel
Copy link
Contributor

This pull request refactors how CPU vendor and architecture constants are referenced throughout the codebase, centralizing them in the new perfspect/internal/cpus package. It also updates all usages to use the new constants and moves certain CPU-related helper functions from report to cpus, improving maintainability and consistency.

Key changes:

Centralization of CPU Vendor and Architecture Constants

  • Introduced the perfspect/internal/cpus package and replaced hardcoded strings like "GenuineIntel", "AuthenticAMD", "x86_64", and "aarch64" with constants such as cpus.IntelVendor, cpus.AMDVendor, cpus.X86Architecture, and cpus.ARMArchitecture across the codebase. [1] [2] [3]

Refactoring of CPU Helper Functions

  • Moved CPU lookup functions from the report package to the new cpus package, updating all usages (e.g., report.GetCPUByMicroArchitecturecpus.GetCPUByMicroArchitecture, report.GetCPUcpus.GetCPU). [1] [2] [3]

Script and Metadata Configuration Updates

Improved Maintainability

  • By centralizing these constants and helper functions, the codebase is now easier to maintain and extend for additional CPU vendors or architectures in the future. [1] [2] [3]

Minor Script Definition Adjustments

  • Adjusted certain script definitions to use MicroArchitectures instead of Models where appropriate, further improving clarity.

These changes collectively improve code clarity, reduce duplication, and make it easier to support new CPU types or architectures.

Signed-off-by: Harper, Jason M <jason.m.harper@intel.com>
@harp-intel harp-intel requested a review from Copilot October 22, 2025 22:11
Copy link
Contributor

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 refactors CPU vendor and architecture constant references throughout the codebase by introducing the perfspect/internal/cpus package. It centralizes CPU-related constants and helper functions, improving maintainability and consistency while adding support for Diamond Rapids (DMR) with family 19.

Key changes:

  • Introduced centralized CPU constants (vendors, architectures, families) in the new cpus package
  • Migrated CPU lookup functions from report to cpus package for better organization
  • Added DMR microarchitecture support (family 19, model 1)

Reviewed Changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
internal/cpus/cpu_defs.go New package with CPU constants, family validation helpers, and migrated CPU lookup functions; adds DMR CPU definition
internal/cpus/cpu_defs_test.go Package declaration updated from report to cpus and test function calls updated to use exported names
internal/script/script_defs.go Replaced hardcoded vendor/architecture strings with cpus constants; changed Families/Models fields to MicroArchitectures; added DMR to relevant script definitions
internal/script/script.go Updated script filtering logic to use MicroArchitectures field and cpus.GetCPU() for microarchitecture matching
internal/report/table_defs.go Replaced hardcoded strings with cpus constants; changed Families/Models fields to MicroArchitectures; added DMR support; updated family check to use cpus.IsIntelCPUFamilyStr()
internal/report/table_helpers.go Updated to use cpus constants and migrated functions; renamed local variable from cpus to cpuCount to avoid shadowing package name
internal/report/table_helpers_cache.go Updated function call to use migrated cpus.GetCPUByMicroArchitecture()
internal/report/table.go Updated table filtering logic to use MicroArchitectures field and cpus.GetCPU()
internal/common/targets.go Replaced hardcoded architecture strings with cpus constants
cmd/metrics/metrics.go Updated vendor comparisons and family checks to use cpus constants and helper functions
cmd/metrics/metadata.go Updated to use cpus constants in architecture switch statements and vendor comparisons; migrated to cpus.GetCPU()
cmd/metrics/summary.go Updated vendor comparison to use cpus.AMDVendor constant
cmd/config/set.go Replaced all hardcoded vendor strings with cpus.IntelVendor constant throughout configuration functions
cmd/config/config.go Updated vendor string to use cpus.IntelVendor constant

@harp-intel harp-intel merged commit 5b4e090 into main Oct 23, 2025
5 checks passed
@harp-intel harp-intel deleted the dmrprep branch October 23, 2025 17:12
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.

2 participants