feat(rocprofiler-compute): Ingest code-object ISA into analysis DB#8395
Open
vedithal-amd wants to merge 8 commits into
Conversation
…sampling Relocate pc_sampling_analysis.py from src/utils to the src/pc_sampling package so PC-sampling analysis code lives alongside pc_sampling_profile. Update importers and CMake test wiring; no behavior change. Co-Authored-By: Claude Opus 4
Backfill the analysis DB instruction_line table with the full per-object disassembly from <pid>_code_obj_info.json, including instructions that were never sampled. Reconcile disassembly instructions to ps-space offsets via virtual address (code_object_offset = virtual_address - load_base) and skip any offset already sampled so no duplicate ISA is inserted. - New src/pc_sampling/code_object_analysis.py: pure parser plus a thin discovery loader for the code-object artifacts. - Un-sampled lines are stored with kernel_uuid NULL; per-kernel attribution of code-object ISA is deferred to the native kernel-mapping work. - Code objects with no known load_base are skipped (no ps-space offset). - No schema change and no user-visible output change (ISA lands only in the raw table, not the pc_sampling view or CLI). Co-Authored-By: Claude Opus 4
Refresh the PC-sampling fixture with a matched ps_file + code_obj_info pair from one run, with enough samples to exercise both sampled-row population and full-ISA backfill. Rename the arch dir to MI300X_A1 to match the captured hardware. - Enhance the db-output analyze test to assert backfilled ISA lines are un-attributed (kernel NULL) with no sample state, sampled lines stay kernel-attributed, and no (code object, offset) is duplicated. - Rewrite the code_object_analysis parser tests to build inline mock payloads instead of reading a workload artifact. Co-Authored-By: Claude Opus 4
…ed tests The ENABLE_COVERAGE report's ALL_TEST_NAMES omitted 18 registered Python tests, so generate_coverage_report did not depend on them and could run before they finished. Add the missing entries (analysis DB/ORM, code-object and PC-sampling analysis, profile multiplexing/attach/multi-rank, tty, and others) so the report depends on the full Python test suite. Conditionally-registered tests (test_import_guard, test_profile_multi_rank) are listed as plain DEPENDS entries; CTest ignores dependency names that are not registered in the current configuration. Co-Authored-By: Claude Opus 4
Reformat the fixture ps_file_results.json and code_obj_info.json to indented multi-line so diffs and manual inspection are readable. Co-Authored-By: Claude Opus 4
Drop the CodeObjectStoreCatalog handoff: add_code_object_isa is now self-sufficient, deriving existing stores and offsets from the workload's in-memory ORM graph instead of a state object passed from add_pc_sampling_data. - add_pc_sampling_data returns nothing and knows nothing about ISA ingestion. - Share instruction-line construction; split the sample-state and its child counts into _add_sample_state. - Switch the code-object tests to common.get_output_dir/clean_output_dir with try/finally so cleanup runs on failure. Co-Authored-By: Claude Opus 4
…naming Fold _add_sample_state and _add_disassembly_lines into their callers so the sampled-row and ISA passes read top to bottom without indirection. Decouple the ISA pass from PC-sampling: it derives existing code objects and offsets from the workload's in-memory ORM graph, no state object passed between passes. - Name the reused-store map existing_code_objects; drop the "backfill" and "catalog" terminology from code and tests. - Explain why an instruction is skipped when its code object has no load_base. Co-Authored-By: Claude Opus 4
vedithal-amd
changed the title
Contributor
There was a problem hiding this comment.
Pull request overview
This PR extends rocprofiler-compute’s PC-sampling DB ingestion to also store the full per-code-object disassembly (ISA) into the analysis database, so the DB contains complete instruction context beyond just sampled PCs. It introduces a code-object disassembly loader/parser, integrates ISA ingestion into the DB pipeline, relocates PC-sampling analysis utilities into the pc_sampling package, and updates/expands tests plus associated workload fixtures.
Changes:
- Add a code-object disassembly parser/loader and ingest ISA-only instruction lines into the analysis DB (kernel attribution deferred; ISA-only lines have
kernel_uuidNULL). - Move PC-sampling analysis utilities into
src/pc_sampling/pc_sampling_analysis.pyand update imports/callers. - Update workloads and tests (new unit tests, refreshed PC-sampling-only workload artifacts, and coverage test list fixes).
Reviewed changes
Copilot reviewed 15 out of 19 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| projects/rocprofiler-compute/tests/workloads/vcopy_pc_sampling_only/MI300X_A1/sysinfo.csv | Adds updated MI300X_A1 workload sysinfo fixture for PC-sampling-only testing. |
| projects/rocprofiler-compute/tests/workloads/vcopy_pc_sampling_only/MI300X_A1/profiling_config.yaml | Updates workload config to reflect PC-sampling-only run settings used by refreshed fixtures. |
| projects/rocprofiler-compute/tests/workloads/vcopy_pc_sampling_only/MI300X_A1/log.txt | Adds updated run log fixture showing code object info artifact emission. |
| projects/rocprofiler-compute/tests/workloads/vcopy_pc_sampling_only/MI300A_A1/sysinfo.csv | Removes older MI300A_A1 sysinfo fixture as part of workload refresh. |
| projects/rocprofiler-compute/tests/workloads/vcopy_pc_sampling_only/MI300A_A1/ps_file_results.json | Removes older MI300A_A1 PC-sampling results fixture. |
| projects/rocprofiler-compute/tests/workloads/vcopy_pc_sampling_only/MI300A_A1/pmc_dispatch_info.csv | Removes older MI300A_A1 dispatch info fixture. |
| projects/rocprofiler-compute/tests/workloads/vcopy_pc_sampling_only/MI300A_A1/log.txt | Removes older MI300A_A1 run log fixture. |
| projects/rocprofiler-compute/tests/test_pc_sampling_analysis.py | Updates imports for relocated module and extends DB E2E assertions to cover ISA-only lines and de-duplication. |
| projects/rocprofiler-compute/tests/test_code_object_analysis.py | Adds unit tests for code-object disassembly parsing and recursive discovery/loader behavior. |
| projects/rocprofiler-compute/tests/test_categories.yaml | Registers the new test in test category groupings (including coverage-enabled sets). |
| projects/rocprofiler-compute/tests/test_analyze_commands.py | Updates imports to the relocated PC-sampling analysis module. |
| projects/rocprofiler-compute/tests/test_analysis_db.py | Adds DB unit tests for ISA ingestion behavior (unsampled lines, dedup, store creation, skip-on-null-load_base). |
| projects/rocprofiler-compute/src/utils/parser.py | Switches PC-sampling imports to the relocated pc_sampling.pc_sampling_analysis module. |
| projects/rocprofiler-compute/src/rocprof_compute_analyze/analysis_db.py | Integrates ISA ingestion (add_code_object_isa) into DB analysis and uses the new loader/parser. |
| projects/rocprofiler-compute/src/pc_sampling/pc_sampling_analysis.py | Adds/relocates PC-sampling normalization/aggregation/enrichment helpers used by DB ingestion and tests. |
| projects/rocprofiler-compute/src/pc_sampling/code_object_analysis.py | Introduces code-object ISA parsing and recursive discovery loader for *_code_obj_info.json artifacts. |
| projects/rocprofiler-compute/CMakeLists.txt | Adds the new test target and reconciles the ENABLE_COVERAGE test dependency list. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Document that ISA offsets for non-surviving pids are wrong because load_base comes only from the single clobbered ps_file, pending the multi-process results-file fix. Co-Authored-By: Claude Opus 4
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
Add the full per-code-object disassembly emitted by the native PC sampling collector to the analysis database, including instructions that were never PC-sampled. This gives the DB complete ISA context alongside the sampled counts landed in the normalized schema PR. Stacked on #8334; no schema change and no user-visible output change.
Technical Details
src/pc_sampling/code_object_analysis.py: a pure parser (code-object disassembly dict -> per-object instruction records) plus a thin discovery loader that finds the per-process disassembly artifacts under a workload.code_object_offset = virtual_address - load_base(the disassembly's own offset is into the ELF file, a different axis).kernel_uuidNULL. Per-kernel attribution of code-object ISA is deferred to the native kernel-mapping work.load_baseare skipped (the offset cannot be derived).pc_sampling_analysis.pyfromsrc/utilsto thesrc/pc_samplingpackage (mechanical, no behavior change).JIRA ID
JIRA ID: AIPROFCOMP-540
Test Plan
Test Result
Python unit tests pass; ruff clean.
Submission Checklist