Skip to content

Commit a6c02f6

Browse files
charles-typmeta-codesync[bot]
authored andcommitted
Fix topdown parameter and result parsing due to external changes
Summary: More context: https://fb.workplace.com/groups/1943855219377055/permalink/2289387328157174/ **Problem:** ARM's topdown-tool changed its interface: * **Old behavior:** We use flags "-a, -i, --csv", wrote results directly to a CSV file * **New behavior:** Flags are removed, new flags are added "--csv-output-path", "--cpu-generate-csv". Creates a directory structure with timestamp subdirectories containing the CSV file at `<output_dir>/<timestamp>/cpu/<dynamic_filename>_metrics.csv` **Changes Made:** ### 1. Updated `run()` method (lines 469-485) Modified the topdown-tool command-line arguments: * Removed: `-a` flag * Changed: `-i` → `-I` (capital I for interval) * Changed: `--csv` → `--csv-output-path` * Added: `--cpu-generate-csv` and `"metrics"` positional argument ### 2. Updated `write_csv()` method (lines 488-598) Implemented comprehensive directory cleanup and CSV extraction: * **Finds latest timestamp:** Searches for timestamp directories (e.g., `2025_11_17_22_34_34`) and selects the most recent * **Dynamic CSV detection:** Searches for files matching `*core_aggregate*_metrics.csv` pattern instead of using hardcoded filenames (handles different core configurations like `neoverse_v2_core_aggregate_(0,25-71)_metrics.csv`) * **Directory cleanup:** 1. Reads the CSV data from `<csvpath>/<timestamp>/cpu/<dynamic_csv_name>` 2. Copies CSV to temporary location 3. Removes entire directory structure using `shutil.rmtree()` 4. Moves CSV file back to `self.csvpath` location (now as a file, not a directory) * **Error handling:** Added comprehensive error handling with logging for all operations * **Maintains existing functionality:** Still processes and writes transposed CSV as before **Result:** The tool now correctly handles the new ARM topdown-tool output format while ensuring only a CSV file remains at the expected location instead of a directory structure. Reviewed By: YifanYuan3 Differential Revision: D87276904 fbshipit-source-id: 4efcee755d5e970cbb2ce31647879cd2816ba5fb
1 parent 1a08cca commit a6c02f6

File tree

1 file changed

+95
-5
lines changed

1 file changed

+95
-5
lines changed

benchpress/plugins/hooks/perf_monitors/topdown.py

Lines changed: 95 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -473,10 +473,11 @@ def run(self):
473473
self.proc = subprocess.Popen(
474474
[
475475
"topdown-tool",
476-
"-a",
477-
"-i",
476+
"--cpu-generate-csv",
477+
"metrics",
478+
"-I",
478479
str(self.interval * 1000),
479-
"--csv",
480+
"--csv-output-path",
480481
self.csvpath,
481482
],
482483
stdout=subprocess.PIPE,
@@ -488,12 +489,101 @@ def run(self):
488489
def write_csv(self):
489490
"""
490491
Override the original write_csv() method to write a transposed version
491-
of CSV table based on what topdown-tool generates
492+
of CSV table based on what topdown-tool generates.
493+
494+
The topdown-tool now writes results to a directory structure:
495+
<output_dir>/<timestamp>/cpu/neoverse_v2_metrics.csv
496+
497+
This method finds the latest timestamp, copies the CSV to the expected
498+
location, and cleans up the directory structure.
492499
"""
500+
import shutil
501+
502+
# Check if csvpath exists and is a directory
493503
if not os.path.exists(self.csvpath):
504+
logger.warning(f"Output path {self.csvpath} does not exist")
505+
return
506+
507+
if not os.path.isdir(self.csvpath):
508+
logger.warning(f"Output path {self.csvpath} is not a directory")
494509
return
510+
511+
# Find the timestamp directory (most recent if multiple exist)
512+
timestamp_dirs = []
513+
try:
514+
for item in os.listdir(self.csvpath):
515+
item_path = os.path.join(self.csvpath, item)
516+
if os.path.isdir(item_path):
517+
timestamp_dirs.append(item)
518+
except OSError as e:
519+
logger.warning(f"Error listing directory {self.csvpath}: {e}")
520+
return
521+
522+
if not timestamp_dirs:
523+
logger.warning(f"No timestamp directories found in {self.csvpath}")
524+
return
525+
526+
# Sort to get the most recent timestamp directory
527+
timestamp_dirs.sort(reverse=True)
528+
latest_timestamp = timestamp_dirs[0]
529+
530+
# Find the CSV file with core_aggregate pattern in the cpu directory
531+
cpu_dir = os.path.join(self.csvpath, latest_timestamp, "cpu")
532+
if not os.path.exists(cpu_dir):
533+
logger.warning(f"CPU directory not found at {cpu_dir}")
534+
return
535+
536+
# Look for files matching the pattern *core_aggregate*_metrics.csv
537+
csv_file_path = None
538+
try:
539+
for filename in os.listdir(cpu_dir):
540+
if "core_aggregate" in filename and filename.endswith("_metrics.csv"):
541+
csv_file_path = os.path.join(cpu_dir, filename)
542+
break
543+
except OSError as e:
544+
logger.warning(f"Error listing CPU directory {cpu_dir}: {e}")
545+
return
546+
547+
if csv_file_path is None or not os.path.exists(csv_file_path):
548+
logger.warning(
549+
f"CSV file with 'core_aggregate' pattern not found in {cpu_dir}"
550+
)
551+
return
552+
553+
# Read the CSV data before cleaning up
554+
try:
555+
df = pd.read_csv(csv_file_path)
556+
except Exception as e:
557+
logger.warning(f"Error reading CSV file {csv_file_path}: {e}")
558+
return
559+
560+
# Copy the CSV to a temporary location
561+
temp_csv_path = self.csvpath + ".tmp"
562+
try:
563+
shutil.copy2(csv_file_path, temp_csv_path)
564+
except Exception as e:
565+
logger.warning(f"Error copying CSV to temp location: {e}")
566+
return
567+
568+
# Clean up the directory structure
569+
try:
570+
shutil.rmtree(self.csvpath)
571+
except Exception as e:
572+
logger.warning(f"Error removing directory {self.csvpath}: {e}")
573+
# Clean up temp file if directory removal failed
574+
if os.path.exists(temp_csv_path):
575+
os.remove(temp_csv_path)
576+
return
577+
578+
# Move the CSV to the expected location (now as a file, not directory)
579+
try:
580+
shutil.move(temp_csv_path, self.csvpath)
581+
except Exception as e:
582+
logger.warning(f"Error moving temp CSV to {self.csvpath}: {e}")
583+
return
584+
585+
# Now process and write the transposed CSV
495586
t_csv_path = self.gen_path(f"{self.name}-transposed.csv")
496-
df = pd.read_csv(self.csvpath)
497587
t_rows = []
498588
for i in range(len(df)):
499589
metric_key = df.iloc[i]["group"] + "/" + df.iloc[i]["metric"]

0 commit comments

Comments
 (0)