From 2087001db2205cef7bdde70c18649939cc514986 Mon Sep 17 00:00:00 2001 From: "Wimalasuriyan, Sajeeth" Date: Wed, 16 Sep 2026 14:02:49 -0400 Subject: [PATCH] Refresh the agent guides against the current source The .agents/ guides had drifted far enough to mislead. The Ask Optiq assistant that landed in #1029 was undocumented, and several symbols the guides sent a reader looking for - the controller plot layer, ComputeDataProvider, COMPUTE_TABLE_DEFINITIONS, ResolveProfilerType - do not exist in the tree, so recipes built on them were unfollowable. Audited each guide against the code it describes and corrected what it claims. UI.md gains the Ask Optiq section: the tool set and which handler table owns each body, the turn phases, the timeouts and why they differ, and the private DataProvider client id that keeps assistant queries out of the tabs' tables. CONTROLLER.md and DATABASE.md drop the deleted classes and fix signatures that moved - bind_trace_to_database takes a config path, PackedTable's merge and sort helpers take a QueryManager, and the table machinery overrides live on QueryManager rather than ProfileDatabase. SCRIPTING.md no longer reads as though nothing shipped, and PYTHON.md describes the details-panel Script tab that exists rather than a toolbar button that does not. Where a name survives in the guides only because an earlier revision claimed it, the text now says so outright, so the next reader does not spend the search finding out it was never there. Co-authored-by: Cursor --- .agents/CONTROLLER.md | 318 ++++++++++++------ .agents/DATABASE.md | 284 ++++++++++++---- .agents/SCRIPTING.md | 167 +++++++--- .agents/UI.md | 752 ++++++++++++++++++++++++++++++++++-------- AGENTS.md | 20 +- PYTHON.md | 8 +- src/model/README.md | 23 +- 7 files changed, 1207 insertions(+), 365 deletions(-) diff --git a/.agents/CONTROLLER.md b/.agents/CONTROLLER.md index 636a7a128..356fd47a1 100644 --- a/.agents/CONTROLLER.md +++ b/.agents/CONTROLLER.md @@ -43,21 +43,37 @@ source wins; please update this file in the same change. - **Does NOT own:** SQLite I/O (that is `src/model/`), ImGui rendering (that is `src/view/`). - **Linked by:** the View through `DataProvider` - (`src/view/src/rocprofvis_data_provider.h`). Python CFFI binds the model - library directly and does not use this controller ABI. + (`src/view/src/rocprofvis_data_provider.h`). The CFFI bindings under + `src/model/python/` bind the model library directly and do not use + this controller ABI - but note that `src/controller/src/python/` is a + *different* thing: in-app analysis scripts, which do go through this + ABI from inside the controller. - **Build flags:** - `BUILD_TESTING` - builds `roc-optiq-controller-system-tests` and `roc-optiq-controller-compute-tests` (Catch2). Tests are wired - against fixture traces under `sample/`. With - `ROCPROFVIS_ENABLE_SCRIPTING=ON`, also builds - `roc-optiq-controller-script-tests`. - -The View must never include controller `src/` headers - only `inc/`. -The controller must never include View headers. + against fixture traces under `sample/`. + - `ROCPROFVIS_ENABLE_SCRIPTING` - compiles + `rocprofvis_controller_script.cpp` and + `python/rocprofvis_controller_python.cpp`, links `roc-optiq-python`, + and adds `roc-optiq-controller-script-tests`. + - `ROCPROFVIS_ENABLE_PROFILER` - compiles `src/controller/src/profiler/` + and adds `roc-optiq-controller-profiler-tests`. + - `ROCPROFVIS_ENABLE_REMOTE` - compiles `src/controller/src/remote/`. + + All four feature options default to `OFF`. + +The controller must never include View headers. The reverse rule - +"the View includes only `inc/`" - is the intent, but it is **not +currently enforced and not currently true**: +`src/controller/CMakeLists.txt` exports `src/` as a `PUBLIC` include +directory, and several View files include +`rocprofvis_controller_analysis.h`, which lives in `src/` despite being +an `extern "C"` surface. Treat new `src/` includes from the View as +something to avoid rather than as precedent. ## 2. Public C ABI Surface (`src/controller/inc/`) -Three headers form the entire public contract: +`inc/` holds five headers. These three are the core contract: - `rocprofvis_controller.h` - all functions. - `rocprofvis_controller_types.h` - opaque handle typedefs and the full @@ -68,6 +84,15 @@ Three headers form the entire public contract: `rocprofvis_controller_primitive_type_t`, sort orders, the property banks for events / samples / tracks / tables / summary / etc. +Two more are feature surfaces: + +- `rocprofvis_controller_script.h` - the three `rocprofvis_script_*` + functions. See `.agents/SCRIPTING.md` §4. +- `rocprofvis_profiler.h` - in-app profiler launch. + +A sixth `extern "C"` surface, `rocprofvis_controller_analysis.h`, lives +in `src/` rather than `inc/`. That is an inconsistency, not a rule. + ### 2.1 Handle types All controller objects are opaque `rocprofvis_handle_t*` from the @@ -98,7 +123,6 @@ typedef rocprofvis_handle_t rocprofvis_controller_counter_t; typedef rocprofvis_handle_t rocprofvis_controller_summary_t; typedef rocprofvis_handle_t rocprofvis_controller_summary_metrics_t; typedef rocprofvis_handle_t rocprofvis_controller_topology_node_t; -typedef rocprofvis_handle_t rocprofvis_controller_plot_t; typedef rocprofvis_handle_t rocprofvis_controller_workload_t; typedef rocprofvis_handle_t rocprofvis_controller_kernel_t; typedef rocprofvis_handle_t rocprofvis_controller_metrics_container_t; @@ -111,12 +135,18 @@ You can always recover the runtime kind via ### 2.2 Lifecycle: alloc / load / free ```c -rocprofvis_controller_t* rocprofvis_controller_alloc(const char* filename); +rocprofvis_controller_t* rocprofvis_controller_alloc(char const* const filename, + char const* const config_path); +rocprofvis_controller_t* rocprofvis_controller_alloc_compare( + char const* const* filenames, uint64_t count); rocprofvis_result_t rocprofvis_controller_load_async( rocprofvis_controller_t*, rocprofvis_controller_future_t*); void rocprofvis_controller_free(rocprofvis_controller_t*); ``` +`rocprofvis_controller_alloc_compare` is the entry point behind the +Compare feature: it builds one `SystemTrace` over several files. + `rocprofvis_controller_alloc` sniffs the file with `rocprofvis_db_identify_type` (from `src/model/`). It returns either a `SystemTrace*` (rocpd / rocprof / multinode SQLite) or a @@ -316,7 +346,13 @@ See [`.agents/SCRIPTING.md`](./SCRIPTING.md). state with `std::mutex` / `std::shared_mutex`). 2. **Never** kick off a `std::thread` from controller code outside `JobSystem` or `MemoryManager`. New asynchronous work goes through - `JobSystem::Get().IssueJob(...)`. + `JobSystem::Get().IssueJob(...)`. The one sanctioned exception is + `roc-optiq-python`, which the controller links when scripting is on: + it owns a dedicated interpreter thread and a watchdog thread, both + started inside the runtime rather than by controller code, because + CPython requires every call to come from the thread that holds the + GIL. A script's `Job` is completed from that thread via + `Job::Complete` instead of being run by a worker. 3. The `Future` is the only legitimate cross-thread fence between the View and a controller job. It exposes `Wait`, `Cancel`, `IsCancelled`, plus progress data via the property API. @@ -359,9 +395,8 @@ Every public object type (`SystemTrace`, `Track`, `Graph`, `Event`, `Sample`, `SampleLOD`, `FlowControl`, `CallStack`, `ExtData`, `Future`, `Array`, `Arguments`, `Table`, `Summary`, `SummaryMetrics`, `TopologyNode` and friends, plus the compute-only `ComputeTrace`, -`Workload`, `Kernel`, `PcSampling`, `Roofline`, `MetricsContainer`, -`ComputeTable`, `ComputePivotTable`, `Plot`, `ComputePlot`, -`PlotSeries`) inherits from `Handle`. +`Workload`, `Kernel`, `PcSampling`, `Roofline`, `MetricsContainer`, and +`ComputePivotTable`) inherits from `Handle`. `m_first_prop_index` / `m_last_prop_index` form a guard band so an unhandled getter falls back to `UnhandledProperty(property)`. A property @@ -437,8 +472,12 @@ class Job { public: Job(JobFunction function, Future* future); + ~Job(); void Execute(); void Cancel(); + // Complete a job that is not run by JobSystem (e.g. the Python + // interpreter thread). No-op if the job already left Pending. + void Complete(rocprofvis_result_t result); rocprofvis_result_t GetResult() const; rocprofvis_result_t Wait(float timeout); }; @@ -453,13 +492,27 @@ public: ``` A `Job` owns a `JobFunction` and the `Future*` it should signal. The -worker pool drains `m_jobs` under `m_queue_mutex`. Cancellation flips -the `Future`'s cancel flag and the `JobFunction` is expected to check -it cooperatively. +worker pool drains `m_jobs` under `m_queue_mutex`; the pool is sized by +`std::thread::hardware_concurrency()`. Cancellation flips the `Future`'s +cancel flag and the `JobFunction` is expected to check it +cooperatively. + +`JobSystem::Get()` returns a **static member object** +(`JobSystem JobSystem::s_self;`), not a function-local static. That is +worth knowing for initialization order, and it is the reason section 10's +"no globals" rule carries an exception for it. + +**`Job::Complete` is the escape hatch for work the pool does not run.** +A script executes on the Python interpreter thread, so its job is never +handed to a worker; `ScriptEngine` issues the job to own the `Future`, +then calls `Complete(result)` from the interpreter's `done` callback. It +is a no-op if the job already left `Pending`, so a cancel that beat the +interpreter cannot be overwritten. Use it only when something outside +`JobSystem` genuinely owns the execution. **Rule:** any new async fetcher writes its body as a `JobFunction` lambda capturing the request inputs by value, calls into -`Trace::*Fetch*` / `Table::Fetch` / `Plot::Fetch`, writes results into +`Trace::*Fetch*` or `Table::Fetch`, writes results into the caller's `Array` or `MetricsContainer`, then returns its `rocprofvis_result_t`. Issue it with `JobSystem::Get().IssueJob(...)`. @@ -544,9 +597,14 @@ protected: }; ``` -Subclasses are `SystemTable` (events/samples/search-results/kernel -instances), `ComputeTable` (catalog of pre-baked compute CSV tables), -and `ComputePivotTable` (dynamic pivoted metric matrix). +Direct subclasses are `SystemTable` +(`system/rocprofvis_controller_table_system.h`, events / samples / +kernel instances) and `ComputePivotTable` +(`compute/rocprofvis_controller_table_compute_pivot.h`, the dynamic +pivoted metric matrix). `SystemTable` is itself subclassed twice: +`EventSearchTable` +(`system/rocprofvis_controller_table_system_search.h`) and +`Analysis::EventsTable` (`rocprofvis_controller_analysis.h:180`). ### 4.11 `Trace` base class File: `rocprofvis_controller_trace.{h,cpp}`. @@ -843,13 +901,14 @@ are always compiled. The compute objects all share the same `Handle` base, the same `Reference<>` validation, the same `JobSystem` / `Future` plumbing. -Build note: `src/controller/CMakeLists.txt` currently compiles the -active compute controller set (`trace_compute`, `workload`, `kernel`, -`metrics_container`, `roofline`, and `table_compute_pivot`). Older / -experimental compute table and plot sources also exist in -`src/controller/src/compute/`; keep them documented for discoverability, -but check CMake before assuming a class is linked into -`roc-optiq-controller`. +Build note: `src/controller/src/compute/` holds exactly seven +`.h`/`.cpp` pairs - `trace_compute`, `workload`, `kernel`, +`metrics_container`, `roofline`, `pc_sampling`, and +`table_compute_pivot` - and `src/controller/CMakeLists.txt` compiles all +of them. There is no unbuilt or experimental compute source here any +more; earlier revisions of this guide described `table_compute`, +`plot`, `plot_compute`, `plot_series`, and `compute_metrics.h`, all of +which have since been deleted. ### 6.1 `ComputeTrace` (`rocprofvis_controller_trace_compute.{h,cpp}`) @@ -955,25 +1014,14 @@ struct Metric { Property bank: `rocprofvis_controller_metrics_container_properties_t`. -### 6.6 `ComputeTable` and `ComputePivotTable` -Files: `rocprofvis_controller_table_compute.{h,cpp}`, -`rocprofvis_controller_table_compute_pivot.{h,cpp}`, -`rocprofvis_controller_compute_metrics.h`. - -`ComputePivotTable` is part of the active controller target. -`ComputeTable` source exists but is not currently listed in -`src/controller/CMakeLists.txt`; treat it as older / auxiliary code -unless you wire it into the build. - -`ComputeTable` is the catalog wrapper for the pre-baked compute CSV -tables. The mapping from CSV filename to logical table type lives in -`COMPUTE_TABLE_DEFINITIONS` inside -`rocprofvis_controller_compute_metrics.h` (top-kernels, sysinfo, -speed-of-light, memory chart, command processor, workgroup manager, -wavefront launch / runtime, instruction mixes, compute units, LDS, -caches, fabric, etc.). `Setup()` loads the CSV into `m_metrics_map`; -`Fetch(index, count, ...)` returns rows; `GetMetric(key, &out)` and -`GetMetricFuzzy(key, &out)` look metrics up by name. +### 6.6 `ComputePivotTable` +File: `rocprofvis_controller_table_compute_pivot.{h,cpp}`. + +The only compute table class. A `ComputeTable` catalog wrapper backed by +`COMPUTE_TABLE_DEFINITIONS` in a `rocprofvis_controller_compute_metrics.h` +used to sit beside it; **both are gone**, along with `GetMetric` / +`GetMetricFuzzy` and `m_metrics_map`. Metric lookup now lives in +`MetricsContainer` (section 6.5). `ComputePivotTable` is the dynamic pivot used by the "Add Metric" workflow in the View. `Setup()` accepts: @@ -1023,24 +1071,19 @@ nibbles: source `0x10000000`, ISA `0x20000000`, and stalls `0x30000000`. Instruction/source correlation properties include both source-line and owning source-file UUIDs. -### 6.8 `ComputePlot`, `Plot`, `PlotSeries` -Files: `rocprofvis_controller_plot.{h,cpp}`, -`rocprofvis_controller_plot_compute.{h,cpp}`, -`rocprofvis_controller_plot_series.{h,cpp}`. - -These plot classes exist in source but are not currently listed in -`src/controller/CMakeLists.txt`; the current View-side compute roofline -path does not depend on these controller plot classes being linked. - -`Plot` is the abstract base for any data plot (axes + named series). -`ComputePlot : Plot` consumes one or more `ComputeTable`s and -populates the `m_series` map keyed on series name. `PlotSeries` is the -concrete `(x, y)` value vector exposed to callers via the property -API. The static catalog of built-in compute plots is -`COMPUTE_PLOT_DEFINITIONS` in -`rocprofvis_controller_compute_metrics.h` (kernel duration pie, -SOL plots, instruction mix plots, cache stalls, etc.). Roofline plots -are configured by `ROOFLINE_DEFINITION` in the same header. +### 6.8 There is no controller-side plot layer + +**Removed.** `Plot`, `ComputePlot`, and `PlotSeries` - and with them +`COMPUTE_PLOT_DEFINITIONS` and `ROOFLINE_DEFINITION` - no longer exist +in the tree. There is also no `rocprofvis_controller_plot_t` handle +typedef, despite one appearing in older copies of section 2.1. + +Plotting is a View concern. The compute roofline is built in +`src/view/src/compute/rocprofvis_compute_roofline.{h,cpp}` on top of +`Roofline` (section 6.4), and the other charts read metrics through +`MetricsContainer` and `ComputePivotTable`. **Do not reintroduce a +controller plot class** without a reason the View layer cannot serve; +the previous one was deleted rather than fixed. ## 7. Memory Manager & Segment Timeline @@ -1082,11 +1125,12 @@ process-wide LRU eviction policy. Constructor: ```cpp void Init(size_t num_objects); void Configure(double weight); +bool IsShuttingDown(); void AddLRUReference(SegmentTimeline* owner, Segment* reference, - uint32_t lod, void* array_ptr); -rocprofvis_result_t EnterArrayOwnership(void* array_ptr, + uint64_t array_id); +rocprofvis_result_t EnterArrayOwnership(uint64_t array_id, rocprofvis_owner_type_t type); -rocprofvis_result_t CancelArrayOwnership(void* array_ptr, +rocprofvis_result_t CancelArrayOwnership(uint64_t array_id, rocprofvis_owner_type_t type); void Delete(Handle* handle, SegmentTimeline* owner); Event* NewEvent(uint64_t id, double s, double e, SegmentTimeline*); @@ -1106,9 +1150,11 @@ Internals worth knowing: `ManageLRU()` on a condition variable. When total `m_lru_storage_memory_used` exceeds `m_lru_size_limit` (computed from `s_physical_memory_avail`, `kUseVailMemoryPercent`, and per-trace weight) it walks `m_lru_array` - (sorted by oldest timestamp) and evicts segments by removing their - `array_ptr`s, deleting the resident `Event*` / `Sample*` / `SampleLOD*`s - through the pools, and clearing the `valid` bit on the segment. + - a `std::map>`, + with each member holding an `std::unordered_set m_array_ids` + - and evicts segments by dropping those array ids, deleting the + resident `Event*` / `Sample*` / `SampleLOD*`s through the pools, and + clearing the `valid` bit on the segment. - Static `s_memory_manager_instances` and `Configure(weight)` let multiple traces share the global memory budget proportionally. - `kShortTracksMemoryPoolIdentifier = 1` partitions short, dense @@ -1121,16 +1167,31 @@ Internals worth knowing: ### 7.3 Array ownership and free-time eviction -When a `Graph` fills an `Array` with pooled events, it calls -`MemoryManager::EnterArrayOwnership(&array_vector, kRocProfVisOwnerTypeGraph)` -to mark every reachable segment as in-use so the LRU thread will not -evict them while the View holds the data. When the View later calls -`rocprofvis_controller_array_free`, the controller-side free path -checks whether the array was created by a `Trace` and, if so, calls -`CancelArrayOwnership(...)` to release the in-use marker. This is why -graph-output arrays carry a `Trace*` context and ordinary arrays do -not. **Do not skip this step** when adding a new fetch path that -returns pooled objects. +Ownership is keyed on the array's id, not its address: +`Array::GetArrayId()` is what the manager records. + +Two fetch paths take ownership, under two different owner types: + +- `Graph::...` calls + `mgr->EnterArrayOwnership(array.GetArrayId(), kRocProfVisOwnerTypeGraph)` + (`system/rocprofvis_controller_graph.cpp:696`). +- `Track::...` calls + `mgr->EnterArrayOwnership(array.GetArrayId(), kRocProfVisOwnerTypeTrack)` + and then `m_segments.AddActiveArray(array.GetArrayId())` + (`system/rocprofvis_controller_track.cpp:474-476`). + +Either marks every reachable segment as in-use so the LRU thread will +not evict it while the View holds the data. When the View later calls +`rocprofvis_controller_array_free`, the free path cancels **both** owner +types unconditionally (`rocprofvis_controller.cpp:777-778`), so it does +not need to know which path produced the array. Both graph-output *and* +raw-track arrays carry a `Trace*` context, which is how the free path +finds the manager at all. + +**Do not skip this step** when adding a new fetch path that returns +pooled objects: without it the events are evicted under the View the +moment the LRU thread runs. `SegmentTimeline::AddActiveArray` / +`RemoveActiveArray` are the related pin for a fetch still in flight. ## 8. Request Lifecycle: View Call -> Future -> Pixels @@ -1196,6 +1257,10 @@ Property bank starting points (`uint32_t` enum bases): |---------------------------------------|--------------| | System controller | `0x00000000` | | Timeline | `0x10000000` | +| Summary aggregation level | `0x11000000` | +| Summary arguments | `0x12000000` | +| **Script result** | `0x14000000` | +| **Script context** | `0x15000000` | | View | `0x20000000` | | Track | `0x30000000` | | Sample | `0x40000000` | @@ -1210,8 +1275,19 @@ Property bank starting points (`uint32_t` enum bases): | Ext Data | `0xD0000000` | | Table Arguments / Summary | `0xE0000000` | | Summary Metrics | `0xF0000000` | +| Remote | `0xF5000000` | | Common (memory usage, etc.) | `0xFFFF0000` | +**A bank is not one per high nibble**, despite how the first column +reads. Four banks share nibble `0x1` with Timeline (summary aggregation +level, summary arguments, and the two script banks), and Remote sits +inside nibble `0xF` with Summary Metrics. Allocate a new bank by +picking an unused *base* from this table, not by taking the next +nibble. The script banks are +`rocprofvis_controller_script_result_properties_t` and +`rocprofvis_controller_script_context_properties_t`, both in +`rocprofvis_controller_enums.h`; see `.agents/SCRIPTING.md` §4. + Most compute-side banks start at the `__kRPVControllerComputePropertiesFirst` family. The auto-incrementing `__first / __last` brackets in each enum are an extension hint - if @@ -1264,13 +1340,16 @@ These supplement `CODING.md`. When the two disagree, `CODING.md` wins. go through `MemoryManager::New*`. Free with `MemoryManager::Delete`. Do not call `new`/`delete` on these classes directly. - **Threading:** schedule async work via `JobSystem::Get().IssueJob`. - The only sanctioned long-lived thread outside `JobSystem` is - `MemoryManager::m_lru_thread`; if you must add another, follow the - same shutdown / atomic-flag pattern. + The sanctioned long-lived threads outside `JobSystem` are + `MemoryManager::m_lru_thread` and, with scripting on, the interpreter + and watchdog threads inside `roc-optiq-python`; if you must add + another, follow the same shutdown / atomic-flag pattern. - **String interning:** any high-cardinality string (event names, metric names, file paths in callstacks) goes through `StringTable`. -- **No globals.** Use `static T& Get()` singletons (matches - `JobSystem`, `StringTable`, `Analysis`). +- **No globals.** Use a `Get()` accessor rather than a bare global + (`JobSystem`, `StringTable`; `Analysis` uses `GetInstance()`). Note + `JobSystem` backs its accessor with a static member object + (`s_self`), not a function-local static. ## 11. Reuse Catalog (controller edition) @@ -1286,8 +1365,8 @@ These supplement `CODING.md`. When the two disagree, `CODING.md` wins. | Return a primitive cell | `Data` tagged union | | Hold an interned string | `StringTable::Get().AddString(s, store)` | | Allocate an `Event` / `Sample` / `SampleLOD` | `MemoryManager::NewEvent` / `NewSample` / `NewSampleLOD` | -| Mark an array as in-use so segments survive eviction | `MemoryManager::EnterArrayOwnership(arr, kRocProfVisOwnerTypeGraph)` | -| Release an array's in-use grip | `MemoryManager::CancelArrayOwnership(arr, type)` (called by `array_free`) | +| Mark an array as in-use so segments survive eviction | `MemoryManager::EnterArrayOwnership(array.GetArrayId(), kRocProfVisOwnerTypeGraph \| ...Track)` | +| Release an array's in-use grip | `MemoryManager::CancelArrayOwnership(array_id, type)` (called by `array_free` for both types) | | Walk segments inside `[start, end]` | `SegmentTimeline::FetchSegments(start, end, user_ptr, future, func)` | | Populate missing raw-track segments | `Track::FetchSegments(...)` / `Track::Fetch(...)` | | Read controller-internal track state | `Track` typed getters; reserve generic properties for C ABI dispatch | @@ -1297,8 +1376,8 @@ These supplement `CODING.md`. When the two disagree, `CODING.md` wins. | RAII-wrap a `rocprofvis_db_future_t` | `DataModelFuturePtr` (file-scope in `rocprofvis_controller_trace_system.cpp`) | | Implement a new table | Subclass `Table`, override `Setup` / `Fetch` / `ExportCSV` | | Implement a new system table use case | Add to `rocprofvis_dm_table_use_case_enum_t` and switch in `SystemTable` | -| Implement a new compute pre-baked table | Add a `ComputeTableDefinition` row in `COMPUTE_TABLE_DEFINITIONS` | -| Implement a new compute plot | Add a `ComputeTablePlotDefinition` row in `COMPUTE_PLOT_DEFINITIONS` | +| Pivot compute metrics into a kernel x metric matrix | `ComputePivotTable` - there is no other compute table class | +| Plot compute data | Do it in the View; the controller has no plot layer (section 6.8) | | Fetch one PC-sampling layer | Use the matching `ComputeTrace::AsyncFetchPcSampling*` method and the kernel-owned `PcSampling` handle | | Implement a new analysis function | Extend `Analysis` and add a free function in `rocprofvis_controller_analysis.h` | | Add a new object type | See section 9 (six-step recipe) | @@ -1361,17 +1440,24 @@ Catch2 tests live in `src/controller/tests/`: `sample/rocprof_compute_23ed6f36.db`. Tests the compute load, workload + kernel + roofline + metric-fetch + pivot-table flows. - `rocprofvis_controller_script_tests.cpp` - built only with - `ROCPROFVIS_ENABLE_SCRIPTING=ON`. Phase 0: execute a source string - and read `optiq.result.text`. Phase 1: load - `sample/trace_70b_1024_32.rpd`, fetch events / a private query - table from Python, and confirm `table_alloc` is not the UI Event - Table singleton. Accepts `--input_file`. + `ROCPROFVIS_ENABLE_SCRIPTING=ON`, 22 cases against + `sample/trace_70b_1024_32.rpd`. Beyond the basics (execute a source + string, read `optiq.result.text`, fetch events and a private query + table, confirm `table_alloc` is not the UI Event Table singleton) it + covers the parts that are easy to break silently: interpreter-thread + identity, the wall-clock deadline and its retry behaviour, recovery + after a timed-out run, cancel-while-queued, traceback content, + `print`, the dangerous-builtin refusals, the parse-tree source screen + and its syntax-error path, the run-generation guard, and the + selection context. Accepts `--input_file`. +- `rocprofvis_controller_profiler_tests.cpp` - built only with + `ROCPROFVIS_ENABLE_PROFILER=ON`. The compute controller test currently does not exercise the PC-sampling ABI. Changes to the three PC-sampling fetchers or their property bank should add coverage for the matching schema-2.2 fixture. -Both binaries accept `--input_file ` (parsed by Catch2 + Clara). +The binaries accept `--input_file ` (parsed by Catch2 + Clara). Logs land in `Testing/Temporary/rocprofvis_controller_*_tests/`. When you add a new public API or a new domain class, add a @@ -1390,6 +1476,13 @@ free" sequence. `rocprofvis_controller_object_type_t`, `rocprofvis_controller_primitive_type_t`, sort orders, table types, table arguments, and PC-sampling property groups/arguments. +- `rocprofvis_controller_script.h` -> `rocprofvis_script_execute_async`, + `rocprofvis_script_cancel`, `rocprofvis_script_result_free`. +- `rocprofvis_profiler.h` -> in-app profiler launch + (`ROCPROFVIS_ENABLE_PROFILER`). + +Plus `src/rocprofvis_controller_analysis.h`, which is `extern "C"` but +not in `inc/`. ### Core building blocks (`src/controller/src/`) @@ -1425,6 +1518,9 @@ free" sequence. - `rocprofvis_controller_graph.{h,cpp}` -> `Graph` (LOD per track). - `rocprofvis_controller_timeline.{h,cpp}` -> `Timeline`. - `rocprofvis_controller_table_system.{h,cpp}` -> `SystemTable`. +- `rocprofvis_controller_table_system_search.{h,cpp}` -> + `EventSearchTable : SystemTable`, behind + `rocprofvis_controller_search_table_alloc`. - `rocprofvis_controller_summary.{h,cpp}` -> `Summary`. - `rocprofvis_controller_summary_metrics.{h,cpp}` -> `SummaryMetrics`. - `rocprofvis_controller_topology.{h,cpp}` -> `TopologyNode`, @@ -1443,23 +1539,29 @@ free" sequence. - `rocprofvis_controller_pc_sampling.{h,cpp}` -> `PcSampling` (three-layer PC sampling data; owned by `Kernel`). - `rocprofvis_controller_roofline.{h,cpp}` -> `Roofline`. - `rocprofvis_controller_metrics_container.{h,cpp}` -> `MetricsContainer`. -- `rocprofvis_controller_table_compute.{h,cpp}` -> `ComputeTable` - (source present; not currently compiled by `src/controller/CMakeLists.txt`). - `rocprofvis_controller_table_compute_pivot.{h,cpp}` -> `ComputePivotTable`. -- `rocprofvis_controller_plot.{h,cpp}` -> `Plot` base (source present; - not currently compiled by `src/controller/CMakeLists.txt`). -- `rocprofvis_controller_plot_compute.{h,cpp}` -> `ComputePlot` - (source present; not currently compiled by `src/controller/CMakeLists.txt`). -- `rocprofvis_controller_plot_series.{h,cpp}` -> `PlotSeries` - (source present; not currently compiled by `src/controller/CMakeLists.txt`). -- `rocprofvis_controller_compute_metrics.h` -> static catalog - (`COMPUTE_TABLE_DEFINITIONS`, `COMPUTE_PLOT_DEFINITIONS`, - `COMPUTE_METRIC_DEFINITIONS`, `ROOFLINE_DEFINITION`). +- `rocprofvis_controller_pc_sampling.{h,cpp}` -> `PcSampling`. + +That is the complete list: seven `.h`/`.cpp` pairs, all compiled. If you +are looking for `ComputeTable`, `Plot`, `ComputePlot`, `PlotSeries`, or +`rocprofvis_controller_compute_metrics.h`, they were deleted. + +### Feature directories + +- `src/controller/src/profiler/` -> in-app profiler launch, compiled + only with `ROCPROFVIS_ENABLE_PROFILER`. +- `src/controller/src/remote/` -> remote / SSH support, compiled only + with `ROCPROFVIS_ENABLE_REMOTE`. +- `src/controller/src/python/rocprofvis_controller_python.{h,cpp}` -> + the `optiq` Python bindings, compiled only with + `ROCPROFVIS_ENABLE_SCRIPTING`. See `.agents/SCRIPTING.md` §5. ### Tests (`src/controller/tests/`) - `rocprofvis_controller_system_tests.cpp` - `rocprofvis_controller_compute_tests.cpp` +- `rocprofvis_controller_script_tests.cpp` (`ROCPROFVIS_ENABLE_SCRIPTING`) +- `rocprofvis_controller_profiler_tests.cpp` (`ROCPROFVIS_ENABLE_PROFILER`) --- diff --git a/.agents/DATABASE.md b/.agents/DATABASE.md index b5d17cf75..e6d8c7970 100644 --- a/.agents/DATABASE.md +++ b/.agents/DATABASE.md @@ -198,22 +198,44 @@ rocprofvis_dm_result_t rocprofvis_db_trim_save_async( The View / controller never hand-build SQL. They request a query string from the model layer: +There are **three** builders, and the string-filter arguments belong to +only one of them: + ```c rocprofvis_dm_result_t rocprofvis_db_build_table_query( database, use_case, start, end, num_tracks, tracks, where, filter, group, group_cols, sort_column, sort_order, - num_string_table_filters, string_table_filters, max_count, offset, count_only, char** out_query); // caller frees +rocprofvis_dm_result_t rocprofvis_db_build_event_search_query( + database, start, end, num, ops, where, + num_string_table_filters, string_table_filters, + bool include_substring, bool include_category, bool partial_matching, + sort_column, sort_order, max_count, offset, count_only, + char** out_query); // caller frees + rocprofvis_dm_result_t rocprofvis_db_build_compute_query( database, compute_use_case, num_params, params, char** out_query); ``` +**`rocprofvis_db_build_table_query` takes no string-table filters.** +Earlier revisions of this guide showed `num_string_table_filters`, +`string_table_filters`, and `include_substring` on it; those three +belong to `rocprofvis_db_build_event_search_query`, which is the name +search path. Two further flags on that function decide matching +semantics and are easy to get backwards: + +- `include_category` - also match against the event category, not just + the name. +- `partial_matching` - true means a string matches if it matches **any** + filter; false means it must match **all** of them. + `rocprofvis_dm_table_use_case_enum_t` covers the three system table shapes (`kRPVDMTableUseCaseEventTrackTable`, `kRPVDMTableUseCaseSampleTrackTable`, -`kRPVDMTableUseCaseEventSearch`). +`kRPVDMTableUseCaseEventSearch`), plus the `kRPVDMTableNumUsecases` +sentinel - **three, not four**. `rocprofvis_db_compute_use_case_enum_t` covers all the compute query shapes (workload list, top kernels, kernels list, metric definitions, roofline ceilings, kernel intensities, metric values, kernel metric @@ -223,12 +245,15 @@ matrix, and the schema-2.2 PC-sampling tables). ```c rocprofvis_dm_trace_t rocprofvis_dm_create_trace(void); -rocprofvis_dm_result_t rocprofvis_dm_bind_trace_to_database(trace, database); +rocprofvis_dm_result_t rocprofvis_dm_bind_trace_to_database(trace, database, config_path); rocprofvis_dm_result_t rocprofvis_dm_delete_trace(trace); ``` `bind_trace_to_database` plugs the trace's binding callbacks (section 6) into the database so subsequent async DB calls can populate the trace. +`config_path` is optional (`nullptr` in every in-tree C++ caller). It is +stored on the bind struct and read by the Google / Perfetto path as an +application config directory. ### 2.7 Trace deletion / GC helpers @@ -326,14 +351,16 @@ query API can refer to "track X for operation Y" with one integer. +-----------------+----------------+ | v - +----------------------------------+ - | Database (abstract) | - | +-- SqliteDatabase (abstract) | - | +-- ProfileDatabase | rocpd / rocprof - | | +-- RocpdDatabase | legacy schema - | | +-- RocprofDatabase | modern + multinode - | +-- ComputeDatabase | rocprof-compute - +-----------------+----------------+ + +------------------------------------+ + | Database (abstract) | + | +-- SqliteDatabase (abstract) | + | +-- QueryManager | table/query engine + | | +-- ProfileDatabase | rocpd / rocprof + | | | +-- RocpdDatabase | legacy schema + | | | +-- RocprofDatabase| modern + multinode + | | +-- GoogleTraceProcessor| chrome / perfetto + | +-- ComputeDatabase | rocprof-compute + +-----------------+------------------+ | uses binding callbacks v +----------------------------------+ @@ -464,7 +491,7 @@ Adds SQLite plumbing on top of `Database`. Key concepts: - **`Sqlite3Exec` / `ExecuteSQLQuery` overloads:** the canonical way to run SQL. The internal `Sqlite3Exec` mimics `sqlite3_exec` using `sqlite3_prepare_v2` so callbacks receive a real `sqlite3_stmt*` - and can use the typed `Sqlite3Column*` helpers. There are eight + and can use the typed `Sqlite3Column*` helpers. There are **eleven** `ExecuteSQLQuery` overloads covering: result-less queries, single-row scalars (string / uint64 / uint32), multi-row queries with handle context, multi-row queries with cache table name, and @@ -490,25 +517,46 @@ Adds SQLite plumbing on top of `Database`. Key concepts: `DropSQLTable`, `DropSQLIndex`, `GetRocpdIndexes`, `DetectTable`. Use these instead of inline DDL. -### 4.3 `ProfileDatabase` (`rocprofvis_db_profile.h`) +### 4.2b `QueryManager` (`rocprofvis_db_query_manager.h`) -Common base for `RocpdDatabase` and `RocprofDatabase`. Key -responsibilities: +**The layer between `SqliteDatabase` and everything that answers table +queries.** It is easy to miss, because `ProfileDatabase` is what most +call sites name - but `QueryManager` is where the table machinery +actually lives, and a change to querying usually belongs here rather +than one level down. Both `ProfileDatabase` and `GoogleTraceProcessor` +derive from it. - Owns the global string table (`StringTable m_string_table`). +- Owns the `TableProcessor` array: **three**, not four, one per + `rocprofvis_db_compound_table_type` - + `kRPVTableDataTypeEvent`, `kRPVTableDataTypeSample`, + `kRPVTableDataTypeSearch`, terminated by the `kRPVTableDataTypesNum` + sentinel. There is no `analysis` processor; earlier revisions of this + guide listed one. +- Implements `BuildTableQuery`, `ReadTraceSlice`, `ExecuteQuery`, and + `ExportTableCSV`. Compute queries are explicitly stubbed out here + (`ROCPROFVIS_ASSERT_ALWAYS_MSG_RETURN("Systems database does not + build compute query")`). +- Declares `rocprofvis_db_query_type_t`, + `rocprofvis_db_sqlite_query_type_t`, `rocprofvis_dm_track_search_id_t`, + and `rocprofvis_event_data_category_map_t`. +- Carries `SINGLE_THREAD_RECORDS_COUNT_LIMIT` / + `NO_THREAD_RECORDS_COUNT_LIMIT` alongside the `SqliteDatabase` copies. + +### 4.3 `ProfileDatabase` (`rocprofvis_db_profile.h`) + +Common base for `RocpdDatabase` and `RocprofDatabase`, deriving from +`QueryManager`. Key responsibilities: + - Holds level-calculation cache: `m_event_levels[op]` (`unordered_map>`) and `m_event_levels_id_to_index[op]`. -- Holds the four `TableProcessor`s, one per - `rocprofvis_db_compound_table_type` (event, sample, search, - analysis). -- Exposes `Detect(filename, multinode_files)` to identify between - rocpd, rocprof, and rocprof-multinode SQLite formats. -- Implements the shared `BuildTrackQuery` / `BuildSliceQuery` / - `BuildTableQuery`, `ReadTraceSlice`, `ExecuteQuery`, - `ExportTableCSV` flows. Compute queries are explicitly stubbed out - here (`ROCPROFVIS_ASSERT_ALWAYS_MSG_RETURN("Systems database does - not build compute query")`). +- Exposes `Detect(filename, multinode_files)`, which identifies + **only** rocpd, rocprof, rocprof-multinode, and compute SQLite (or + `kAutodetect` on no match). Chrome/Perfetto detection is a different + function - see section 4.6b. +- Implements `BuildTrackQuery` and `BuildSliceQueryMap`. The rest of + the query surface is on `QueryManager` above. - Houses every standard query callback used during metadata load: `CallBackAddTrack`, `CallBackLoadTrack`, `CallbackCacheTable`, `CallbackAddFlowTrace`, `CallbackAddStackTrace`, @@ -553,10 +601,11 @@ The modern SQLite schema; supports **multinode**. Important traits: is captured into `m_memalloc_activity` (per-PID `vector`), then materialized into a `roc_optiq_memory_activity` SQL table during - `LoadMemoryActivityData`. The memory-allocation level enums and - type enums are part of the public model header + `LoadMemoryActivityData`. The memory-allocation level and type enums (`kRPVMemActivityAlloc`/`Free`/`Realloc`/`Reclaim`, - `kRPVMemLevelReal`/`Virtual`/`Scratch`). + `kRPVMemLevelReal`/`Virtual`/`Scratch`) are **internal** - they live + in `src/model/src/database/rocprofvis_db_rocprof.h`, not in any + `inc/` header, so they are not part of the public ABI. - String interning is keyed on `(string_id, guid_id, string_type)` (`rocprofvis_db_string_id_t`) because rocprof unifies strings and kernel symbol names into one array. @@ -604,6 +653,32 @@ databases: - `ComputeWorkloadTopKernelsMeanAndMedian(table)` post-processes top kernels to stamp mean / median into the table. +### 4.6b `GoogleTraceProcessor` (`rocprofvis_db_trace_processor.h`) + +The Chrome / Perfetto adapter, and the fourth database flavour. It +derives from `QueryManager` (a sibling of `ProfileDatabase`, not a +child) and **the whole translation unit is behind +`#ifdef ROCPROFVIS_PERFETTO_ENABLED`**, which is set only when Perfetto +is available rather than by a user-facing option. `thirdparty/perfetto` +is the dependency. + +- Serves `kChromeTrace`, `kPerfettoTrace`, and `kGoogleSqlite`. +- **Detection does not go through `ProfileDatabase::Detect`.** + `GoogleTraceProcessor::Detect` is tried *first* in + `rocprofvis_c_interface.cpp`, and only on no match does the profile + detector run. If you are tracing why a file picked one adapter over + another, start here. +- `TraceConverter::Convert(source_path, output_path, progress_callback)` + converts the source trace into a SQLite cache file before anything is + queried, which is what lets the rest of the stack stay SQLite-only. +- `s_perfetto_categorized_data` is the third categorized-data map, + beside `s_rocpd_categorized_data` and `s_rocprof_categorized_data`. +- **Trimming is unsupported**: `SaveTrimmedData` returns + `kRocProfVisDmResultNotSupported`. + +`rocprofvis_db_trace_processor_dll.cpp` is a Perfetto wrapper DLL and +is deliberately excluded from the `datamodel` target. + ### 4.7 Other database/ files - `rocprofvis_db_future.h` - the `Future` (`DataModel` namespace), @@ -984,7 +1059,7 @@ typedef struct { constructor populates `m_binding_info` with the trace-side static methods. 2. Caller opens a database: `rocprofvis_db_open_database(path, type)`. -3. Caller binds: `rocprofvis_dm_bind_trace_to_database(trace, db)`. +3. Caller binds: `rocprofvis_dm_bind_trace_to_database(trace, db, config_path)`. This calls `Trace::BindDatabase` which writes the database-side pointers into `m_binding_info` (`FuncFindCachedTableValue`, `FuncGetInfoTable*`) and hands the populated struct back to the @@ -1012,9 +1087,15 @@ agnostic to which Database flavor is bound. ## 7. Trace File Formats & Adapters -ROCm Optiq consumes four trace shapes, all SQLite-backed. -`rocprofvis_db_identify_type(filename)` -> `ProfileDatabase::Detect` -sniffs: +All trace shapes are SQLite-backed by the time they are queried; +Chrome/Perfetto inputs are converted first (section 4.6b). + +**Detection is two detectors, in order.** +`rocprofvis_db_identify_type(filename)` tries +`GoogleTraceProcessor::Detect` first, then falls back to +`ProfileDatabase::Detect` - which recognizes only +`kRocprofMultinodeSqlite`, `kRocprofSqlite`, `kRocpdSqlite`, +`kComputeSqlite`, or `kAutodetect`. The formats: - **`kRocpdSqlite`** - legacy rocpd schema. One file. Decoded by `RocpdDatabase`. String table is per-GPU duplicated and is @@ -1214,11 +1295,12 @@ of metric IDs. ### 8.4 `BuildTableQuery` flow The View / controller calls `rocprofvis_db_build_table_query(...)` -(public ABI). The model dispatches via `Database::BuildTableQuery` -to either `ProfileDatabase::BuildTableQuery` (the four system use -cases) or `ComputeDatabase::BuildTableQuery` (asserts because compute -does not produce per-track tables - those go through -`BuildComputeQuery` instead). +(public ABI). The model dispatches via `Database::BuildTableQuery` to +either **`QueryManager::BuildTableQuery`** (the three system use cases - +the override is on `QueryManager`, not `ProfileDatabase`) or +`ComputeDatabase::BuildTableQuery` (asserts because compute does not +produce per-track tables - those go through `BuildComputeQuery` +instead). Args passed all the way through: @@ -1226,18 +1308,23 @@ Args passed all the way through: - `num_tracks`, `tracks` - subset of track IDs to include. - `where`, `filter`, `group`, `group_cols` - SQL fragment overrides. - `sort_column`, `sort_order` - per-page sorting. -- `num_string_table_filters`, `string_table_filters` - free-text - search; the database resolves these via `BuildTableStringIdFilter` - which finds matching string IDs and rewrites them into a - `WHERE IN (...)`. -- `include_substring` - how those filters are matched against the - string table. `true` (the default) matches any string containing a - filter, `false` only strings equal to it; both are case insensitive. - Exact matching is only satisfiable with a single distinct filter, - since a string cannot equal two different values at once. - `max_count`, `offset` - paging. - `count_only` - return a `SELECT COUNT(*) ...` shape. +**Free-text search is a different entry point.** +`rocprofvis_db_build_event_search_query` is where +`num_string_table_filters` / `string_table_filters` live, resolved via +`BuildTableStringIdFilter` into a `WHERE IN (...)` over matching string +IDs. Its three flags: + +- `include_substring` - `true` matches any string containing a filter, + `false` only strings equal to it; both case insensitive. +- `include_category` - also match the event category, not just the name. +- `partial_matching` - `true` means match **any** filter, `false` means + match **all** of them. This is why exact, all-of matching is only + satisfiable with a single distinct filter: a string cannot equal two + different values at once. + The output is a `char* out_query` the caller is responsible for freeing. @@ -1262,24 +1349,24 @@ public: void AddRow(); void PlaceValue(col, double|uint64_t value); - Numeric GetMergeTableValue(uint8_t op, row, col, ProfileDatabase*) const; + Numeric GetMergeTableValue(uint8_t op, row, col, QueryManager*) const; uint8_t GetOperationValue(row) const; void RemoveDuplicates(); void CreateSortOrderArray(); - void SortByColumn(db, column_name, ascending); + void SortByColumn(QueryManager*, column_name, ascending); bool SetupAggregation(agg_spec, num_threads); void FinalizeAggregation(); void ClearAggregation(); - void AggregateRow(db, row_index, map_index); - void SortAggregationByColumn(db, column, ascending); + void AggregateRow(QueryManager*, row_index, map_index); + void SortAggregationByColumn(QueryManager*, column, ascending); void Merge(vector>&); void ManageColumns(vector>&); void RemoveRowsForSetOfTracks(selected, unselected, remove_all); - static const char* ConvertSqlStringReference(db, col, idx, node, &numeric_string); + static const char* ConvertSqlStringReference(QueryManager*, col, idx, node, &numeric_string); static uint8_t ColumnTypeSize(ColumnType); void ResetTrackIdetifiers(); }; @@ -1328,10 +1415,33 @@ to round-trip through SQLite. ### 9.3 `TableProcessor` (`rocprofvis_db_table_processor.h`) The mediator between SQL execution and the in-memory `PackedTable`s. -Per `ProfileDatabase` there are four `TableProcessor`s, one for each -`rocprofvis_db_compound_table_type` (event / sample / search / -analysis), so concurrent queries against different "kinds" of tables -do not stomp on each other's caches. +Per **`QueryManager`** (not `ProfileDatabase`) there are **three** +`TableProcessor`s, one for each `rocprofvis_db_compound_table_type` - +event, sample, search - so concurrent queries against different "kinds" +of tables do not stomp on each other's caches. The fourth enumerator, +`kRPVTableDataTypesNum`, is the array-size sentinel; there is no +`analysis` processor. + +**The embedded command vocabulary** parsed out of the `-- CMD:` tag is +`OFFSET`, `LIMIT`, `FILTER`, `GROUP`, `SORT`, and `COUNT`. + +`FILTER` does not remove rows. It fills `m_filter_lookup`, an +`unordered_set` of surviving row indices, and the row data +stays put. The work fans out across +`(RowCount() + 10000) / 10000` workers, capped at +`hardware_concurrency() - 1`, with a final worker picking up the +leftover rows when the count does not divide evenly - **that leftover +worker's end index must be the merged-table row count, not the leftover +count**, or the tail rows are silently dropped (fixed in 425a2292). +The filter string is memoized in `m_last_filter_str` so an unchanged +filter re-runs nothing, and adding or removing tracks goes through +`set_difference` plus `RemoveRowsForSetOfTracks` rather than a rebuild. + +**Note the `GROUP` path does not carry FILTER's guards.** FILTER checks +`thread_count == 0` and `leftover_rows_count > 0`; the GROUP fan-out +about sixty lines below evaluates `i < thread_count - 1`, which +underflows to a huge unsigned value when `thread_count` is 0. That is +reachable on a single-core machine. Highlights: @@ -1340,15 +1450,17 @@ Highlights: `-- CMD: ` directives and the explicit multi-track / multi-guid format used by combined views. - **`ExecuteCompoundQuery(future, queries, tracks, commands, - handle, type, query_updated)`** drives a multi-stage pipeline: - run each per-node query in parallel into a `PackedTable`, merge - via `m_merged_table.Merge(...)`, apply filter / sort / group via - the embedded commands, then emit into the public `Table` (or to - CSV for export). -- **`RestartableTimer m_timer`** clears the cached - `m_merged_table` and `m_tracks` after a configurable idle window - (default 1s) so the same table query coming back later can avoid - re-fetching, but a long pause does not pin memory. + handle, query_updated)`** - six parameters, no `type` - drives a + multi-stage pipeline: run each per-node query in parallel into a + `PackedTable`, merge via `m_merged_table.Merge(...)`, apply filter / + sort / group via the embedded commands, then emit into the public + `Table` (or to CSV for export). +- **`RestartableTimer m_timer`** clears the cached `m_merged_table` and + `m_tracks` after an idle window so the same table query coming back + later can avoid re-fetching, but a long pause does not pin memory. + **The window is 60 seconds.** `m_delay{1000}` in the header looks like + a one-second default, but `restart()` always overwrites it and the + only call site passes `std::chrono::seconds(60)`. ### 9.4 `DatabaseCache`, `TableCache`, `StringTable` File: `rocprofvis_db_cache.h`. @@ -1445,6 +1557,33 @@ Subclasses: with four tables (track info, kernel-dispatch level, region level, histogram) and a similar dependency mask. +`roc_optiq_table_version_t` in full. Most tables share one value +through aliases, which is deliberate: bumping +`kRocOptiqTableVersionForLevelCalculation` invalidates every +level-calculation table at once. + +| Constant | Value | +|---|---| +| `kRocOptiqTableVersionMemoryActivity` | `0x0002` | +| `kRocOptiqTableVersionMemoryAllocate` | `0x0001` | +| `kRocOptiqTableVersionForLevelCalculation` | `0x0002` | +| `kRocOptiqTableVersionKernelDispatchLevel` | alias of the above | +| `kRocOptiqTableVersionRegionLevel` | alias | +| `kRocOptiqTableVersionRegionSampleLevel` | alias | +| `kRocOptiqTableVersionMemoryAllocLevel` | alias | +| `kRocOptiqTableVersionMemoryCopyLevel` | alias | +| `kRocOptiqTableVersionHistogram` | `0x0001` - **dead, see below** | +| `kRocOptiqTableVersionTrackInfo` | `0x0003` | + +**`kRocOptiqTableVersionHistogram` is never used.** Both +`RocprofMetadataVersionControl` and `RocpdMetadataVersionControl` +register `roc_optiq_histogram` with +`kRocOptiqTableVersionMemoryCopyLevel` instead, so the histogram's +effective version is `0x0002` and it is silently invalidated whenever +the level-calculation version moves. That looks like a copy-paste slip +rather than intent; if you touch this, either wire the constant up or +delete it, and do not assume the named constant reflects what ships. + When you add a new derived table: 1. Append a value to the subclass's `roc_optiq_tables` enum. @@ -1494,6 +1633,9 @@ Avoid conditional compilation and validate changes with `rocprofvis_c_interface.h` for direct C/C++ consumers. `src/model/python/rocprofvis_cffi_test.py` covers the Python side. +That test still calls `rocprofvis_dm_bind_trace_to_database` with two +arguments; the public ABI takes three (`config_path` last, pass +`None` / `nullptr`). If you change a public ABI signature: - Update `rocprofvis_interface.h` (single C-overload only). @@ -1710,10 +1852,18 @@ exploratory testing during development. `rocprofvis_db_string_id_t`. - `rocprofvis_db_sqlite.h` -> `SqliteDatabase`, `MAX_CONNECTIONS`, `RpvSqliteExecuteQueryCallback`, `rocprofvis_db_sqlite_db_node_t`, - `rocprofvis_db_sqlite_callback_parameters`, the seven + `rocprofvis_db_sqlite_callback_parameters`, the eleven `ExecuteSQLQuery` overloads, `Sqlite3Column*` helpers. -- `rocprofvis_db_profile.h` -> `ProfileDatabase`, the four - `TableProcessor`s, `m_event_levels`, the long callback list +- `rocprofvis_db_query_manager.h` -> `QueryManager`, the three + `TableProcessor`s, `m_string_table`, `BuildTableQuery`, + `rocprofvis_db_compound_table_type`, `rocprofvis_db_query_type_t`, + `rocprofvis_db_sqlite_query_type_t`, + `rocprofvis_dm_track_search_id_t`, + `rocprofvis_event_data_category_map_t`. +- `rocprofvis_db_trace_processor.{h,cpp}` -> `GoogleTraceProcessor`, + `TraceConverter` (`ROCPROFVIS_PERFETTO_ENABLED`). +- `rocprofvis_db_profile.h` -> `ProfileDatabase`, + `m_event_levels`, the long callback list (`CallBackAddTrack`, `CallbackAddFlowTrace`, `CallbackAddStackTrace`, `CallbackAddEssentialInfo`, `CallbackAddArgumentsInfo`, `CalculateEventLevels`, diff --git a/.agents/SCRIPTING.md b/.agents/SCRIPTING.md index 6e307558c..9a1e63b99 100644 --- a/.agents/SCRIPTING.md +++ b/.agents/SCRIPTING.md @@ -6,13 +6,23 @@ alongside this file. When `CODING.md` disagrees with this file, `CODING.md` wins. When this file disagrees with source, the source wins; update this file in the same change. -This is a planned feature. Enable with -`ROCPROFVIS_ENABLE_SCRIPTING=ON`. Phase 0 (runtime skeleton), the -Phase 1 **read path** (query-table alloc, `optiq.trace` / `selection` / +**This feature is partly shipped, not merely planned** - the phases +below are a live roadmap, so check a phase's status before trusting its +tense. Enable with `ROCPROFVIS_ENABLE_SCRIPTING=ON` (default OFF, "in +development"). + +In tree and working today: Phase 0 (runtime skeleton), the Phase 1 +**read path** (query-table alloc, `optiq.trace` / `selection` / `table().fetch()` / `Track.events()`, Catch2 against a sample trace), -and Phase 1b (DataProvider execute + floating script editor) are in -tree. The `run_analysis_script` half of Phase 3 is also in tree; the -vendored CPython half is not. +Phase 1b (DataProvider execute + the **docked Script tab**, not the +floating editor this document originally planned), and the +`run_analysis_script` half of Phase 3, which is how the Ask Optiq +assistant offers a script. Not in tree: the vendored CPython half of +Phase 3, and Phases 2 and 4 in full. + +Sections describing shipped phases document code you can read; sections +describing Phase 2 and Phase 4 are design intent and may not match any +source file. Section 8 has the authoritative per-phase status. --- @@ -105,19 +115,46 @@ Table tab. ## 3. Interpreter library (`src/python/`) -C ABI sketch (owned by `roc-optiq-python`): +The C ABI, in full, from `src/python/inc/rocprofvis_python_runtime.h`. +Note the return type is `rocprofvis_python_result_t`, **not** +`rocprofvis_result_t` - the runtime shares no types with the controller: ```c -rocprofvis_result_t rocprofvis_python_init(char const* runtime_root); -rocprofvis_result_t rocprofvis_python_exec( - char const* source, - rocprofvis_python_result_t (*prepare_globals)(void* py_dict, void* user), - void (*teardown_globals)(void* py_dict, void* user), - void* user); +typedef enum rocprofvis_python_result_t +{ + kRocProfVisPythonSuccess = 0, + kRocProfVisPythonError = 1, + kRocProfVisPythonInvalidArgument = 2, + kRocProfVisPythonCancelled = 3, + kRocProfVisPythonNotInitialized = 4, +} rocprofvis_python_result_t; + +typedef rocprofvis_python_result_t (*rocprofvis_python_prepare_globals_t)( + void* py_dict, void* user); +typedef void (*rocprofvis_python_teardown_globals_t)(void* py_dict, void* user); +typedef void (*rocprofvis_python_done_t)(void* user, + rocprofvis_python_result_t result, + char const* error_message); + +rocprofvis_python_result_t rocprofvis_python_init(char const* runtime_root); +rocprofvis_python_result_t rocprofvis_python_exec( + char const* source, + rocprofvis_python_prepare_globals_t prepare_globals, + rocprofvis_python_teardown_globals_t teardown_globals, void* user, + rocprofvis_python_done_t done, unsigned long long timeout_ms); void rocprofvis_python_interrupt(void); // raise into interpreter thread void rocprofvis_python_shutdown(void); +unsigned long long rocprofvis_python_interpreter_thread_id(void); // tests only ``` +`rocprofvis_python_exec` returns as soon as the work is queued; `done` +is what reports the outcome, on the interpreter thread with the GIL +released, and its `error_message` is valid only for the duration of the +callback. `timeout_ms` of 0 takes the built-in `SCRIPT_TIMEOUT_MS` +(30000). **A timeout is reported as an error, not a cancellation**, +because a script that outstays its budget is a script to fix, while only +an explicit `rocprofvis_python_interrupt` is a cancellation. + `prepare_globals` is provided by the controller: it creates the `optiq` module and stuffs `optiq.trace` / `optiq.selection` into the exec dict. The runtime never includes `rocprofvis_controller.h`. @@ -313,9 +350,18 @@ script's rows. ## 5. Bindings (`src/controller/src/python/`) -One translation unit may include `Python.h`. Every `rocprofvis_handle_t*` -is a capsule (`owns=0` borrowed, `owns=1` for alloc'd future/array/table). -Bindings call only the C ABI. +One translation unit may include `Python.h`. Bindings call only the C +ABI. + +**There is no capsule-per-handle scheme, and no `owns` flag.** Earlier +drafts of this section described one; it was never built. There are +exactly two capsules, both created with a null destructor: +`rocprofvis.script_session` (`SESSION_CAPSULE_NAME`) and +`rocprofvis.script_result` (`RESULT_CAPSULE_NAME`), stashed as +`optiq._session` and `optiq.result._result`. Controller handles are +held as plain raw pointers inside `TrackObject` / `TraceObject` / +`TableObject`, and what keeps them from outliving their run is not +ownership tracking but `check_generation` - see §3's *Run generation*. Before adding a wrapper that stores a controller or a session, read §3's *Run generation* - every entry point that reads either needs @@ -325,23 +371,39 @@ pointers is not obvious. User-facing surface (injected as `optiq`, not imported from disk): ```python -optiq.trace # current rocprofvis_controller_t* -optiq.selection # tracks + time range from context args +optiq.trace # optiq.Trace, or None +optiq.selection # optiq.Selection, or None: .tracks, .start, .end optiq.result.text(str) -optiq.on_progress(cb) # optional; default is none +print(...) # injected; appends a line to the result t = optiq.table() # table_alloc; NOT the UI singleton -t.fetch(tracks=..., start=..., end=..., where=..., group=...) + # refuses a compute trace +t.fetch(tracks=, start=, end=, where=, filter=, group=, group_columns=, + sort_column=, sort_order=, start_index=, count=, type=) for row in t.rows(): ... -for e in optiq.selection.tracks[0].events(): +for e in optiq.selection.tracks[0].events(start=None, end=None): gap = e.end - e.start ``` -Property getters wrap `get_uint64` / `get_double` / `get_string` / -`get_object` for a small set of names (`track.id`, `event.start`, -…). Raw property ids can remain an escape hatch. +All twelve `fetch` keywords are listed above; `count` defaults to +`DEFAULT_TABLE_FETCH_COUNT` (10000). Six module constants come with it: +`TRACK_TYPE_SAMPLES`, `TRACK_TYPE_EVENTS`, `TABLE_TYPE_EVENTS`, +`TABLE_TYPE_SAMPLES`, `SORT_ASCENDING`, `SORT_DESCENDING`. `optiq` is +also registered in `sys.modules`, so `import optiq` works even though +nothing is on `sys.path`. + +**`optiq.on_progress` does not exist.** It appeared in earlier drafts +and was never implemented; §5.1 below describes the polling that stands +in for it. + +`Track` exposes `id`, `type`, `name`, `sub_name`, `min_time`, +`max_time`, `num_entries` plus `events()`; `Trace` exposes `tracks`. +Those getters wrap `get_uint64` / `get_double` / `get_string` / +`get_object`. There is **no** raw-property escape hatch - an earlier +draft said one "can remain", but scripts cannot reach arbitrary property +ids. `Event` is a **copy**, not a live handle: `id`, `start`, `end`, `level`, `name`, `category`, `value`. `copy_event` fills it from the event @@ -454,21 +516,28 @@ mutation still belongs to `OptiqActions`. Four things it relies on: chain as every other UI action, so nothing in `agenticprofiling/` keeps a pointer to a view that a closing tab could take away. - **The wait is on a person, so it gets its own deadline.** - `AssistantToolStartResult::timeout_seconds` overrides the 45s a fetch - runs under; the script tool asks for 300. The panel also routes a + `AssistantToolStartResult::timeout_seconds` overrides the default + fetch deadline (`ASSISTANT_FETCH_TIMEOUT_SECONDS`, 350s); the script + tool asks for 300 via + `ASSISTANT_SCRIPT_APPROVAL_TIMEOUT_SECONDS`. The panel also routes a timed-out `kScript` fetch back through `FinishAssistantScriptFetch` rather than reporting a generic timeout, because only that side knows whether the user never answered or the run was abandoned, and it has an outstanding offer to clear. -- **`ScriptApproval` is the whole state machine.** `kPending` -> - `kRunning` -> `kFinished` on approval, `kRejected` on refusal, and - `kFailedToStart` when an approved run could not begin - which exists - so a script that never started still answers the assistant instead of - waiting out the full five minutes. `AssistantScriptFetchPending` is - true for `kPending` and `kRunning` only. -- **An offer is pinned to its trace.** `Run` refuses when the tab in - front is not the trace the script was written against, the same - mistake `m_turn_project_id` guards elsewhere. +- **`ScriptApproval` is the whole state machine.** Idle is `kNone` + (manual runs stay there). `kPending` -> `kRunning` -> `kFinished` on + approval, `kRejected` on refusal, and `kFailedToStart` when an + approved run could not begin - which exists so a script that never + started still answers the assistant instead of waiting out the full + five minutes. `AssistantScriptFetchPending` is true for `kPending` + and `kRunning` only. +- **An offer is pinned to its trace by construction, not by a check.** + There is one `ScriptEditor` per trace and each holds its own + `m_data_provider`, so a run can only ever go to the trace the script + was written against. `Run()` itself tests only that the provider is + `ProviderState::kReady` - there is no "wrong tab in front" refusal, so + do not go looking for one. The cross-trace guard that *does* exist is + the assistant's `m_turn_project_id`, which is a different layer. - **Events are filtered by source id.** Every editor hears every `ScriptExecuteCompleteEvent`, and there is one editor per trace, so each answers only events carrying the trace it started a run on. @@ -634,8 +703,10 @@ table (Catch2 + sample trace). Even-spacing is the acceptance script. ### Phase 1b — Minimal UI -**Done when:** a floating editor can run a script and show text (including the -even-spacing example). +**Complete.** Shipped as the docked **Script tab** of the details panel +rather than the floating editor planned here; see §6 for why. A script +can be run from it and its text result shown, including the +even-spacing example. - `DataProvider` request type + poll + progress callback. - Floating `ScriptEditor` (`InputTextMultiline` + text result, Load/Save, @@ -663,7 +734,9 @@ and a release build does not require a system Python. tree, along with the exec deadline and tracebacks that make an unattended script safe to run and possible to fix. - ~~Editor shows the source before or as it runs.~~ In tree via - `ShowGeneratedScript`. + `ScriptEditor::ProposeScript`, reached through + `OptiqActions::ProposeScript`. (There is no `ShowGeneratedScript`; + earlier drafts named one.) - Vendor embeddable CPython into the package; CI builds against it. - Tighten restriction (optional RestrictedPython, scratch-dir `open`). - ~~Decide about raw `where` / `group`.~~ **Decided: they stay raw, and @@ -708,11 +781,18 @@ and a release build does not require a system Python. --- -## 10. Suggested first implementation slice +## 10. What to pick up next + +Phases 0, 1, and 1b are in tree, and so is the `run_analysis_script` +half of Phase 3. The two open pieces are **Phase 2** (result tables in +the view - `optiq.result` is text-only today) and the **vendored +CPython** half of Phase 3, which is what would let the app ship without +depending on a build-machine Python. -Phases 0, 1, and 1b are in tree. Next is Phase 2 (result tables in -the view). Do not start Ask Optiq or vendored CPython until that -presentation path is stable. +This section used to read "Do not start Ask Optiq or vendored CPython +until that presentation path is stable." Ask Optiq shipped first, so +that ordering no longer describes the project; the sequencing advice is +kept only as history. --- @@ -786,9 +866,6 @@ reasoning moved to the section it belongs to. ### Docs -- `PYTHON.md` tells users to open Script from the **compute** toolbar - (compute traces have no Script tab) and says `print` is unavailable - (it is injected into globals). - `optiq.on_progress` from §5 is still unimplemented, and implementing it naively would break cancel-safety — see the note in §5. diff --git a/.agents/UI.md b/.agents/UI.md index 6fab32589..9558e9a55 100644 --- a/.agents/UI.md +++ b/.agents/UI.md @@ -34,9 +34,11 @@ It also pairs with sibling agent guides under `.agents/`: change touches `src/model/`. The brief model summary in section 5 of this file is the high-level pass; `DATABASE.md` is the source of truth. -- [`.agents/SCRIPTING.md`](./SCRIPTING.md) - planned in-app Python - analysis (editor sends source strings; controller owns execution). - Read this when adding script UI or Ask Optiq script tools. +- [`.agents/SCRIPTING.md`](./SCRIPTING.md) - in-app Python analysis + (interpreter lib, controller ABI, phases). Partly shipped behind + `ROCPROFVIS_ENABLE_SCRIPTING`; the doc is a live roadmap, so check + a phase's status before trusting its tense. Read this when adding + script UI or Ask Optiq script tools. When humans and `CODING.md` disagree with this file, `CODING.md` wins. @@ -105,21 +107,48 @@ Output executable: - Linux: `build//roc-optiq` - macOS: `build//roc-optiq` -CMake options worth knowing: +CMake options worth knowing. **Every `ROCPROFVIS_*` option defaults to +OFF**, so a plain clone builds none of the gated features: - `ROCPROFVIS_ENABLE_INTERNAL_BANNER` - draws a watermark on internal builds. -- `ROCPROFVIS_DEVELOPER_MODE` - enables extra menus, the Debug Window, and - `ComputeTester`. Guarded with `#ifdef ROCPROFVIS_DEVELOPER_MODE` in code. -- `ROCPROFVIS_ENABLE_PROFILER` - enables the in-app profiler launcher - (default off). -- `ROCPROFVIS_ENABLE_REMOTE` - enables SSH connection, browse, transfer, - and remote-trace UI (default off). Remote profiling needs both remote - and profiler support. -- `ROCPROFVIS_ENABLE_TRACE_COMPARE` - enables the in-development trace - comparison UI (default off). -- `ROCPROFVIS_MULTI_WINDOW` - enables the in-development multi-window - support (default off). -- `USE_NATIVE_FILE_DIALOG` - off disables `nativefiledialog-extended`. +- `ROCPROFVIS_DEVELOPER_MODE` - enables extra menus, the Debug Window, + `ComputeTester`, **and the `File > Compare` entry point**. Guarded + with `#ifdef ROCPROFVIS_DEVELOPER_MODE` in code. +- `ROCPROFVIS_ENABLE_PROFILER` - the in-app profiler launcher. +- `ROCPROFVIS_ENABLE_REMOTE` - SSH connection, browse, transfer, and + remote-trace UI. Remote profiling needs both remote and profiler + support. +- `ROCPROFVIS_ENABLE_AGENTIC_PROFILING` - the Ask Optiq assistant. +- `ROCPROFVIS_ENABLE_SCRIPTING` - embedded Python analysis, including + the details panel's Script tab. +- `ROCPROFVIS_ENABLE_UI_TESTS` - the Dear ImGui UI tests. +- `ROCPROFVIS_MULTI_WINDOW` - in-development multi-window support. + **This one does not reach `src/view/` at all**: all nine of its + `#ifdef` sites are in `src/app/src/main.cpp`, most of them further + narrowed to `__linux__`. +- `USE_NATIVE_FILE_DIALOG` - **defaults ON**, unlike the list above. + Turning it off disables `nativefiledialog-extended`. + +Two macros the code tests are *not* options and cannot be set directly: + +- `ROCPROFVIS_HAVE_NATIVE_FILE_DIALOG` - derived from + `USE_NATIVE_FILE_DIALOG`. This is the name to grep for; all twelve + sites are in `rocprofvis_appwindow.{h,cpp}`. +- `ROCPROFVIS_PERFETTO_ENABLED` - set only when Perfetto is available. + It changes which trace formats the welcome page and open dialog + advertise. +- `ROCPROFVIS_HAVE_LIBSECRET` - target-scoped, selects the Linux + credential backend in `SecretStore`. + +**`ROCPROFVIS_ENABLE_TRACE_COMPARE` currently gates nothing.** The +option exists and the `-D` is injected, but there is not one +`#ifdef ROCPROFVIS_ENABLE_TRACE_COMPARE` anywhere in `src/`. Compare is +gated by `ROCPROFVIS_DEVELOPER_MODE` instead, and only at the menu item +- `CompareFilesDialog` itself is always compiled. Do not reach for this +flag expecting it to do something. + +**There is no PC-sampling build flag.** PC sampling is gated at runtime +on the trace's schema version, not at compile time. The CLI flag `--file-dialog={auto|imgui|native}` overrides dialog selection at runtime (see `src/app/src/rocprofvis_cli_parser.h`). @@ -143,16 +172,21 @@ at runtime (see `src/app/src/rocprofvis_cli_parser.h`). | | +-- src/datamodel # In-memory dm_* types (events, tracks, samples) | | +-- src/test*/ # Unit tests (Catch2) | +-- controller/ # Coordinates model + view, owns async futures/handles -| | +-- inc/ # rocprofvis_controller.h + rocprofvis_profiler.h +| | +-- inc/ # rocprofvis_controller.h, _script.h, rocprofvis_profiler.h | | +-- src/ # Generic controller plumbing | | +-- src/system/ # System-profile-specific (events, samples, tracks...) | | +-- src/compute/ # Compute-profile-specific (kernels, roofline...) +| | +-- src/python/ # optiq Python bindings (optional) | | +-- src/remote/ # SSH bridge/client/known-host handling | | +-- src/profiler/ # Local/remote profiler process execution -| | +-- tests/ # System & compute Catch2 tests +| | +-- tests/ # Catch2 tests (note: NOT under src/) +| +-- python/ # roc-optiq-python: embedded CPython runtime (optional) +| | +-- inc/ # rocprofvis_python.h, rocprofvis_python_runtime.h +| | +-- src/ | \-- view/ # << This is the UI. Read sections 6-14. >> | +-- inc/ # rocprofvis_view_module.h (entry: init/render/destroy) | +-- src/ # All UI classes +| | +-- agenticprofiling/ # Ask Optiq assistant (optional) | | +-- compute/ # Compute-only views | | +-- icons/ # Icon font glyph constants & ranges | | +-- model/ # UI-side data models (cached projections) @@ -161,6 +195,7 @@ at runtime (see `src/app/src/rocprofvis_cli_parser.h`). | | +-- remote/ # SSH profiles, sessions, dialogs (optional) | | +-- welcome/ # WelcomePage empty state | | +-- widgets/ # Reusable widget library (use these first!) +| \-- test/ # rocprofvis_view_test_access.h (*TestPeer structs) +-- thirdparty/ # Vendored libs (imgui, glfw, sqlite3, spdlog, ...) +-- resources/ # Icons, fonts, AMD logo, embedded assets +-- sample/ # Sample traces (.db, .rpd, .rpv) @@ -179,7 +214,8 @@ at runtime (see `src/app/src/rocprofvis_cli_parser.h`). \-- .agents/ # AI/agent architecture guides +-- UI.md # This file: View-layer guide +-- CONTROLLER.md # Controller-layer deep dive - \-- DATABASE.md # Model/database-layer deep dive + +-- DATABASE.md # Model/database-layer deep dive + \-- SCRIPTING.md # Python analysis: runtime, ABI, phases ``` ## 4. Architectural Pillars @@ -340,14 +376,18 @@ Internal source layout under `src/controller/src/`: control, graphs, memory management, samples (and sample LOD), segments, summary (+metrics), tables, timeline, topology, traces, tracks, call stack. -- `compute/` - per-domain modules covering kernel, metrics container, - plots (+compute, +series), roofline, table compute (+pivot), trace - compute, workload. +- `compute/` - seven modules: kernel, metrics container, pc_sampling, + roofline, table compute pivot, trace compute, workload. There is no + controller-side plot layer; see `.agents/CONTROLLER.md` section 6.8. +- `python/` - the `optiq` Python bindings for in-app analysis scripts + (`ROCPROFVIS_ENABLE_SCRIPTING`). See `.agents/SCRIPTING.md`. - `remote/` - SSH bridge/client/known-host implementation behind the controller C API; the View never links libssh2 directly. - `profiler/` - profiler command construction, process execution, and optional SSH executor. -- `tests/` - Catch2 system + compute tests. + +Catch2 tests are in `src/controller/tests/`, one level up - **not** +under `src/controller/src/`. ### `src/view/` @@ -366,9 +406,22 @@ void rocprofvis_view_set_texture_backend(...); std::string rocprofvis_get_application_config_path(); bool rocprofvis_view_is_remote_display_session(); std::string rocprofvis_get_application_log_path(); +bool rocprofvis_view_get_drag_repair_enabled(); +void rocprofvis_view_set_drag_repair_enabled(bool enabled); bool rocprofvis_view_wants_continuous_render(); ``` +The header also defines the three types `app/` needs to call them: +`rocprofvis_view_render_options_t` +(`kRocProfVisViewRenderOption_None`, `_RequestExit`), +`rocprofvis_view_notification_t` (`_Exit_App`, `_Toggle_Fullscreen`), +and `rocprofvis_view_file_dialog_preference_t` (`_Auto`, `_Native`, +`_ImGui`). + +The drag-repair pair is the Linux/Wayland pointer-routing workaround +behind `UserSettings::linux_drag_repair` and the `--drag-repair` CLI +flag; it is a no-op elsewhere. + These are the only symbols `app/` should call from `view/`. Internally they all forward to `RocProfVis::View::AppWindow`. @@ -630,6 +683,15 @@ splitter dragging. with `SetEventSourceName(...)`. Toggle close/change events via `EnableSendCloseEvent` / `EnableSendChangeEvent`. Used in `AppWindow` for the project tabs and in `ComputeView` for sub-tabs. +- `AddTab`, `RemoveTab(id|index)`, `SetActiveTab(id|index)`, + `SetTabLabel`, `GetActiveTab`, `GetTabs`, `SetAllowToolTips`. +- **`FindTabByLabel(label)` is deliberately fuzzy**, and its rules + matter because this is how a tab gets selected by name rather than by + index: an exact match always wins; a substring match is accepted only + if the query is at least `MIN_SUBSTRING_LABEL_MATCH` (3) characters + *and* matches exactly one tab. It returns `nullptr` both when nothing + matched and when several did, so a caller cannot tell those apart - + treat either as "ask the user". ### 7.6 `rocprofvis_gui_helpers.{h,cpp}` - low-level UI helpers @@ -676,8 +738,9 @@ Use these instead of inlining their logic anywhere new. - `enum Alignment { Left, Center, Right }` and `ElidedText(text, available_width, tooltip_width, alignment, align_to_frame)` - ellipsizes text and shows the full text in a tooltip on hover. -- `ElideWithEllipsis(text, available_width)` - returns an elided - string for callers that draw text themselves. +- `ElideWithEllipsis(text, max_width, max_chars)` - returns an elided + string for callers that draw text themselves. Note the third + parameter: it caps characters as well as pixels. - `CenterNextTextItem` / `CenterNextItem` - centers the next ImGui draw within the current row. - `XButton(id, tool_tip, settings)` - the small "x" close button @@ -687,6 +750,26 @@ Use these instead of inlining their logic anywhere new. - `TableRowHeight()` - canonical row height; align tables to this. - `DrawInternalBuildBanner(text)` - watermark, gated by `ROCPROFVIS_ENABLE_INTERNAL_BANNER`. +- `RenderLoadingIndicatorDots` / `MeasureLoadingIndicatorDots`, `PI`, + `PositionCell`, `CaptureCellRightClick`, `AddCopyRowCellMenuItems`. + +**The panel-card vocabulary is the app's design language, and this +header is its single source of truth.** Prefer it over hand-rolled +`BeginChild` / `PushStyleColor` sequences: + +- `BeginPanelCard` / `EndPanelCard` with + `enum class PanelCardTone { kFrame, kPanel, kMain }` and + `PANEL_CARD_ROUNDING` (10.0f). +- `PanelFieldLabel`, `PanelIcon`. +- `AccentButton` (the primary action) and `ColoredButton` (everything + else). +- `RenderRemoteDownloadPopup`. + +The profiler launcher has a parallel set in +`profiler/rocprofvis_launch_shared_tabs.h` (`BeginLaunchCard`, `Chip`, +`StatusPill`, ...) - see section 13.2. Compare panes have a third, +`BeginCompareCard`. Use whichever belongs to the surface you are in +rather than mixing them. ### 7.7 `rocprofvis_image_helpers.{h,cpp}` - GPU textures @@ -768,8 +851,9 @@ subclassing this; don't roll your own.** ### 7.13 `rocprofvis_debug_window.{h,cpp}` (developer mode only) - `class DebugWindow` - singleton. Aggregates spdlog output into a - scrollable panel. `AddDebugMessage`, `AddPersitentDebugMessage` - (sic), max-message limit, transient/persistent split panes. + scrollable panel. The public entry point is `AddDebugMessage`, and it + is the only one: earlier revisions of this guide also listed an + `AddPersitentDebugMessage`, which does not exist. ### 7.14 `rocprofvis_log_viewer.{h,cpp}` - production log viewer @@ -1022,12 +1106,32 @@ Public surface: - `GetTracks()` exposes the shared track vector to the sidebar and project settings. -Subscribed events: `kNewTrackData`, `kHandleUserGraphNavigationEvent` -(carries `ScrollToTrackEvent`), `kSetViewRange`, `kFontSizeChanged`, -`kGoToTimelineSpot` (carries `NavigationEvent`), -`kTimelineTimeRangeChanged`. Always keep these tokens released in the +Subscribed events - seven, not six: `kNewTrackData`, +`kHandleUserGraphNavigationEvent` (carries `ScrollToTrackEvent`), +`kSetViewRange`, `kFontSizeChanged`, `kGoToTimelineSpot` (carries +`NavigationEvent`), `kTimelineTimeRangeChanged`, and +`kTrackVisibilityChanged`. Always keep these tokens released in the destructor. +**Track sorting lives here.** `enum class TrackSortMode { kTopology, +kDefault, kCustom }` with `SortTracksBy`, `BuildTopologyOrder`, +`HasCustomOrder`, `ApplyTrackOrder`, `RebuildTrackVectorFromMetadata`, +`LoadSortSettings`, and `RenderTrackSortMenu`. The chosen order is +persisted through `TimelineViewProjectSettings::HasSortSettings` / +`SortMode` / `CustomOrder`, and pushed down to the provider with +`DataProvider::SetTrackIndex`. + +**The measurement duration label is draggable**, which is what +`MeasurementRulerDragTarget { kNone, kStart, kEnd }`, +`MeasurementLabelRect`, `HandleMeasurementLabelInput`, and the +session-only `m_measure_label_offset_y` are for. Right-click copy is +`MeasurementCopyTarget { kNone, kStart, kEnd, kDuration }`, and +`ZoomToMeasurement` frames the span. + +**The track-count strip** above the tracks is `TrackTypeCounts` plus +`CalculateTrackCounts`, `BuildTrackCountLabels`, `RenderTrackInfo`, and +`RenderTrackStats`. + Note: there is no `kScrollToTrack` enum value. The `ScrollToTrackEvent` payload class is dispatched on `kHandleUserGraphNavigationEvent` and disambiguated by `dynamic_pointer_cast`. @@ -1117,15 +1221,21 @@ with: ### `AnalysisView` (`rocprofvis_analysis_view.{h,cpp}`) -The bottom-right tabbed panel. Hosts, in source order: +The bottom-right tabbed panel. Hosts, in source order (the labels are +what the user sees, and what `SelectTab` matches against): - `m_tab_container` (`TabContainer`) with sub-tabs: - - `MultiTrackTable` (event table) - cross-track event listing. - - `MultiTrackTable` (sample table) - cross-track sample listing. - - `EventsView` - per-event detail tab (basic info, ext data, flow, + - `"Event Table"` - `MultiTrackTable`, cross-track event listing. + - `"Sample Table"` - `MultiTrackTable`, cross-track sample listing. + - `"Event Details"` - `EventsView` (basic info, ext data, flow, callstack, args). - - `TrackDetails` - selected-track summary tab. - - `TopEventsView` - category-specific top event/sample tables. - - `AnnotationView` - sticky-note list. + - `"Track Details"` - `TrackDetails`, selected-track summary. + - `"Top Events"` - `TopEventsView`, category-specific tables. + - `"Annotations"` - `AnnotationView`, sticky-note list. + - `"Script"` - `ScriptEditor`, last and only under + `ROCPROFVIS_ENABLE_SCRIPTING`. See section 7.15. +- `ListTabs()`, `SelectTab(name)`, and `ActiveTab()` are the accessors + the assistant drives this panel through; `GetScriptEditor()` is + scripting-gated. - Listens to track / range / event selection events to keep tabs in sync. - **Compare mode (two compare sources).** Every applicable tab splits @@ -1498,11 +1608,13 @@ performing the scroll. ### Compute data plumbing -- `ComputeDataProvider` (`rocprofvis_compute_data_provider.{h,cpp}`) - is a separate provider used by the **older** dedicated compute UI - flows. New code should use the unified `DataProvider` (which has +- **There is one provider.** `ComputeView` owns a plain `DataProvider` + like every other `RootView`, and reaches compute data through `ComputeModel()`, `FetchMetrics`, `FetchMetricPivotTable`, and - `SetFetchMetricsCallback`). + `SetFetchMetricsCallback`. Earlier revisions of this guide described + a separate `ComputeDataProvider` in a + `compute/rocprofvis_compute_data_provider.{h,cpp}` for "older" + compute flows; **neither the class nor the file exists.** - `ComputeDataModel` (`model/compute/rocprofvis_compute_data_model.{h,cpp}`) holds `WorkloadInfo`, `KernelInfo`, `MetricValue` per `(store_id, kernel_id|workload_id)`. @@ -1647,19 +1759,25 @@ EventManager::GetInstance()->AddEvent( ### `enum class RocEvents` - the canonical event IDs -The full list is in `rocprofvis_events.h`. Examples used widely: -`kNewTrackData`, `kNewTableData`, `kTabClosed`, `kTabSelected`, -`kTimelineTrackSelectionChanged`, `kTimelineTimeRangeChanged`, -`kTimelineEventSelectionChanged`, `kTimelineEventHighlightChanged`, -`kHandleUserGraphNavigationEvent`, `kTrackMetadataChanged`, -`kFontSizeChanged`, `kSetViewRange`, -`kGoToTimelineSpot`, `kTimeFormatChanged`, -`kRequestProgressUpdate`, `kProfilerStatusChanged`, -`kRemoteStatusChanged`. Compute-only: -`kComputeWorkloadSelectionChanged`, +All 25, in declaration order - `kInvalidEvent` is the only explicitly +valued one, and new values go at the end: + +`kInvalidEvent = -1`, `kNewTrackData`, `kNewTableData`, `kTabClosed`, +`kTabSelected`, `kTimelineTrackSelectionChanged`, +`kTimelineTimeRangeChanged`, `kTimelineEventSelectionChanged`, +`kTimelineEventHighlightChanged`, `kHandleUserGraphNavigationEvent`, +`kTrackMetadataChanged`, `kTrackVisibilityChanged`, `kFontSizeChanged`, +`kSetViewRange`, `kGoToTimelineSpot`, `kTimeFormatChanged`, +`kRequestProgressUpdate`, `kScriptExecuteComplete`, +`kProfilerStatusChanged`, `kRemoteStatusChanged`, +`kRevealTrackInTopology`, `kComputeWorkloadSelectionChanged`, `kComputeKernelSelectionChanged`, `kComputeMetricsFetched`, `kComputeShowMetricInKernelDetails`. +The last four are compute-only. `kScriptExecuteComplete` carries +`ScriptExecuteCompleteEvent` and exists only in spirit without +`ROCPROFVIS_ENABLE_SCRIPTING`. + When adding a new event: 1. Add a new value at the end of `RocEvents` (don't reorder). 2. If it carries data, add a `RocEvent` subclass and a new @@ -1701,8 +1819,16 @@ through this** - never hardcode `IM_COL32(...)` in feature code. `SettingsManager::CompactSidebar()` hides the topology sidebar's per-row icons in favor of the right-click menus (see `SideBar` in section 9). -- `UserSettings::log_viewer` stores level mask, entry limit, search and - presentation preferences for `LogViewer`. +- `UserSettings::log_viewer` (`LogViewerSettings`) holds exactly + `level_mask`, `auto_scroll`, `use_regex`, `relative_time`, and + `visible`. **The entry limit is not in it** - it is the sibling field + `UserSettings::log_viewer_max_entries`, bounded by + `LOG_VIEWER_MAX_ENTRIES_DEFAULT` (512), `_MIN` (64), `_MAX` (100000), + `_STEP` (64), and `_STEP_FAST` (512). The default mask is + `LOG_VIEWER_DEFAULT_LEVEL_MASK` (`0x3F`). +- `UserSettings::linux_drag_repair` - the Linux/Wayland pointer-routing + repair described in section 5, set from `--drag-repair` and ignored + on other platforms. - `GetInternalSettings()` -> recent files (`MAX_RECENT_FILES = 5`). Use `AddRecentFile / RemoveRecentFile / ClearRecentFiles`. - `GetAppWindowSettings()` -> show/hide flags (`show_toolbar`, @@ -1909,6 +2035,14 @@ Layered, transport at the bottom and the panel at the top: OpenAI chat-completions endpoint over cpp-httplib, plus the reply parser (including a "harmony" inline tool-call fallback). Knows nothing about traces. +- `rocprofvis_ai_prompts.{h,cpp}` - `AssistantSystemPrompt()`, the + standing instructions sent before every round. A function rather + than a constant because it appends the scripting paragraph itself + (see below). It is its own file so the prompt can be read and + revised as prose, without scrolling past the turn machinery. Rebuilt + per call rather than cached: it is assembled once per HTTP round on + a worker thread, and the copy is dwarfed by the request it goes + into. - `rocprofvis_ai_tool_schema.{h,cpp}` - `BuildAssistantToolsJson`, the description of the tool set the model receives. Builds JSON out of string literals and touches no view state, which is what makes it @@ -1923,14 +2057,21 @@ Layered, transport at the bottom and the panel at the top: string literals, and escaped `LIKE` wildcards paired with an explicit `ESCAPE` clause. - `rocprofvis_ai_tools.{h,cpp}` - the public executor surface plus - `StartAssistantTool`, which parses the arguments, refuses everything - but `offer_next_steps` when no trace is ready, then searches the UI - handler table and the data handler table in that order. Also defines - the handful of helpers both body files need. Reads go through - `DataProvider` and the view-side models only - never SQLite, never - `src/model/`. + `StartAssistantTool`, the dispatcher. It parses the arguments, then + applies three gates that every tool except `offer_next_steps` passes + through - no provider is refused outright, a compute trace is refused + with an explanation, and a provider that is not yet + `ProviderState::kReady` **parks** on `kTraceLoading` rather than + failing - and only then searches the UI, data, and script handler + tables in that order (UI first: they are the cheap ones, and none + shares a name with a query). An unmatched name comes back with + `AssistantToolNameList()` attached. Also defines the + handful of helpers the body files share (`DoneResult`, `JsonU64`, + `TrimAssistantText`, `CheckArrayLength`, `SelectedOrFullTimeRange`). + Reads go through `DataProvider` and the view-side models only - never + SQLite, never `src/model/`. - `rocprofvis_ai_tools_internal.h` - private wiring between the three - executor files: the shared helpers and the two handler-table + executor files: the shared helpers and the three handler-table accessors. Nothing outside `agenticprofiling/` includes it. - `rocprofvis_ai_ui_tools.cpp` - the tools that change Optiq rather than read it: `goto`, `show_panel`, `switch_tab`, `flow_arrows`, @@ -1948,9 +2089,10 @@ Layered, transport at the bottom and the panel at the top: formatter they use, and `FinishAssistantFetch`. Most of these cannot answer in one call: they queue a fetch and hand the panel a set of `DataProvider` request ids to poll, then format the rows once they - land. Those request ids are shared with the normal UI, which is why - each body checks `IsRequestPending` before issuing its own query and - reports whether it actually started the fetch. + land. Most of those ids are the assistant's own (see the client-id + note below), but some are still shared with the normal UI, which is + why every body checks `IsRequestPending` before issuing its own query + and reports whether it actually started the fetch. - `rocprofvis_ai_script_tools.cpp` - `run_analysis_script`, which is neither of the above: the model writes Python, the interpreter computes the answer, and what comes back is a conclusion rather than @@ -1960,10 +2102,12 @@ Layered, transport at the bottom and the panel at the top: fills the Script tab of the details panel and selects it, and the user presses Run or Reject, so the model cannot execute code unattended. That wait is on a person, so the tool sets its own `timeout_seconds` - instead of the 45s a fetch gets, and the panel asks + (`ASSISTANT_SCRIPT_APPROVAL_TIMEOUT_SECONDS`, 300s) rather than + taking the default fetch deadline, and the panel asks `AssistantScriptFetchPending` rather than polling a request id, - because an approval has no request behind it. **Compiled only when `ROCPROFVIS_ENABLE_SCRIPTING` is also - on**; with it off the handler table is empty, the schema never + because an approval has no request behind it. **Compiled only when + `ROCPROFVIS_ENABLE_SCRIPTING` is also on**; with it off the handler + table is empty, the schema never registers the tool, and the prompt never names it, so the model is not offered something that would always fail. It owns `AssistantFetchKind::kScript` and the `FinishAssistantScriptFetch` @@ -1973,22 +2117,203 @@ Layered, transport at the bottom and the panel at the top: click, a drag, a menu item) including the event traffic the rest of the app listens for. **Add a capability here, as one method, rather than wiring widgets from inside a tool.** - -**Adding a tool is three edits, and none of them is the dispatcher:** a -schema entry in `rocprofvis_ai_tool_schema.cpp`, a body in whichever of -`rocprofvis_ai_ui_tools.cpp`, `rocprofvis_ai_data_tools.cpp`, or -`rocprofvis_ai_script_tools.cpp` matches what it touches, and an entry -in that same file's own handler table. A body without a schema entry is -unreachable; a schema entry without a body comes back to the model as an -unknown tool. The label list at the top of the schema file has to grow -with it, since that is what `AssistantToolNameList` reports and what the -panel shows as a status line. +- `rocprofvis_ai_trace_view_actions.cpp` - **`TraceView` members whose + bodies live here instead of in `rocprofvis_trace_view.cpp`**, because + the assistant is their only caller and an agentic-off build should + not compile them at all. This is where `SetMinimapVisibility`, + `SetFlowArrowsVisible`, `SetFlowRenderChained`, `ListAnalysisTabs`, + `SelectAnalysisTab`, `ActiveAnalysisTab`, `ProposeScript`, + `ScriptProposalState`, `ClearScriptProposal`, `ResetView`, + `SetAnnotationsVisible`, `ListBookmarks`, `SaveBookmark`, + `GotoBookmark`, `RemoveBookmark`, `MeasureRange`, `ClearMeasurement`, + `ZoomToRange`, and `AddNote` are defined - so grepping + `rocprofvis_trace_view.cpp` for them finds nothing. The file states + its own eviction rule: a capability belongs here only while the + assistant is its sole caller, and moves back to + `rocprofvis_trace_view.cpp` the moment a toolbar or menu needs it. - `rocprofvis_ai_assistant.{h,cpp}` - `AssistantPanel`, a lazy - singleton like `LogViewer`. Owns the transcript, the docked column, - and the turn loop. The composer shows **Explain this view** on an - empty chat, then replaces it with up to three stacked next-step - buttons from `offer_next_steps`. Clicking a step sends that text as - the next user message. + singleton like `LogViewer`. Owns the transcript, the conversation + sent upstream, and the turn loop. **The turn machinery only** - it + emits no ImGui. +- `rocprofvis_ai_assistant_render.cpp` - the panel's drawing half: the + dock and its splitter, the transcript cards, the composer, the + activity strip, and the toolbar button. Everything here reads panel + state and emits ImGui; the two halves meet only through + `AssistantPanel` members, so a change to how a turn advances and a + change to how it looks are rarely the same edit. The composer shows + **Explain this view** on an empty chat, then replaces it with up to + three stacked next-step buttons from `offer_next_steps`. Clicking a + step sends that text as the next user message. +- `rocprofvis_ai_settings.cpp` - `SettingsPanel` and `SettingsManager` + members for the assistant settings page and the credential-store + access behind its API-key field. Same reasoning as the `TraceView` + actions file: they are members because the page edits the same + `UserSettings` copy and OK/Cancel state as every other page, but the + bodies live beside the assistant so the shared files carry none of + it. **The `Serialize`/`Deserialize` pair is the deliberate + exception** and stays in `rocprofvis_settings_manager.cpp` with every + other one, unguarded, so an assistant-off build round-trips a saved + configuration instead of erasing it. + +**The tool set, as the model sees it.** Twenty tools at schema indices +0-19, plus `run_analysis_script` at index 20 when scripting is built +in. The three handler tables are `k_data_tool_handlers` +(`rocprofvis_ai_data_tools.cpp`), `k_ui_tool_handlers` +(`rocprofvis_ai_ui_tools.cpp`), and `k_script_tool_handlers` +(`rocprofvis_ai_script_tools.cpp`), searched in the order UI, data, +script. + +| Tool | Kind | Notes | +| --- | --- | --- | +| `trace_overview` | data | Free - no query. The prompt requires it first. | +| `get_summary` | data | Free. Shares `SUMMARY_REQUEST_ID` with the UI. | +| `list_tracks` | data | | +| `top_events` | data | Aggregate; carries no `__uuid`. | +| `kernel_instances` | data | Returns individual events. | +| `kernel_metrics` | data | Aggregate; carries no `__uuid`. | +| `track_events` | data | Shares `ToolTrackRows` with `track_samples`. | +| `track_samples` | data | Same body, split by `tool_name`. | +| `event_details` | data | Point lookup; needs a `__uuid`. Issues three fetches. | +| `track_statistics` | data | Aggregate; derived shared request id. | +| `search_events` | data | Whole-trace name search. Returns events. | +| `goto` | ui | Zoom, select, highlight. | +| `show_panel` | ui | | +| `switch_tab` | ui | | +| `flow_arrows` | ui | | +| `annotate` | ui | Outlives the conversation - saved with the project. | +| `bookmark` | ui | | +| `measure` | ui | | +| `reset_view` | ui | | +| `offer_next_steps` | ui | Fills the follow-up buttons. | +| `run_analysis_script` | script | Only with `ROCPROFVIS_ENABLE_SCRIPTING`. | + +**`track_events` and `track_samples` are one body.** That is the only +reason `AssistantToolHandler` is passed the name it was called by; no +other handler reads it. + +**Adding a tool is four edits, and none of them is the dispatcher:** + +1. A schema entry in `rocprofvis_ai_tool_schema.cpp`, via `AddTool` with + the next contiguous index. +2. A body in whichever of `rocprofvis_ai_ui_tools.cpp`, + `rocprofvis_ai_data_tools.cpp`, or `rocprofvis_ai_script_tools.cpp` + matches what it touches. +3. An entry in that same file's own handler table, plus a row in + `ASSISTANT_TOOL_LABELS` at the top of the schema file. +4. **The `Tools:` line in `ASSISTANT_SYSTEM_PROMPT`**, which names the + whole set in one line. This is the one that gets forgotten, and a + tool missing from it is one the model has no reason to call. + +A body without a schema entry is unreachable; a schema entry without a +body comes back to the model as an unknown tool. `ASSISTANT_TOOL_LABELS` +feeds two things: `AssistantToolNameList`, which is what the model is +told when it invents a name, and `AssistantToolStatusLabel`, which is +the line under the transcript while the tool runs. Its order is *not* +the schema order, so do not use one to index the other. Each body file +owns its table and hands it over through `GetAssistant*ToolHandlers()`, +so the handlers keep internal linkage and no file has to know the others +exist. + +**The turn machinery.** `AssistantPanel::Phase` is three states - +`kIdle`, `kHttpWait`, `kToolWait` - and a turn walks them in a loop: +`StartHttpRequest` posts and moves to `kHttpWait`; `HandleHttpResult` +either queues tool calls (`BeginToolQueue` -> `RunNextTool`, `kToolWait`) +or, when the model stops calling tools, spends one more round through +`BeginFinalAnswer`. A tool that cannot answer immediately parks in +`FetchWait` and `PollToolFetch` retries it each frame until the request +ids clear, the deadline passes, or the trace changes underneath it. + +**Before the first question there is a briefing and a warm-up.** +`BuildAssistantBriefing` puts topology and headline totals in front of +the model so it is not asking blind, and `NeedsBriefing` / +`m_briefed_project_id` re-send it only when the trace it describes is +not the trace being asked about - a follow-up about the same trace does +not repeat it. `TryStartSummaryWarmup` can park the whole question +behind a summary preload; that wait is flagged `warmup` because it has +no tool call to answer, so when it lands `BeginQueuedTurn` resumes the +question instead of replying to a tool. The prompt still insists on at +least one real tool call, because the briefing carries no kernel names, +per-kernel times, or event rows - answering from it alone is guessing. + +**A turn is bounded four ways, and they measure different things.** +`ASSISTANT_MAX_TOOL_ROUNDS` (20) caps how long an investigation may run. +`ASSISTANT_MAX_TOOL_CALLS_PER_ROUND` (16, in the client) caps one +reply's worth of calls. Then the history is bounded twice over: +`ASSISTANT_MAX_CONVERSATION_MESSAGES` (60) and +`ASSISTANT_MAX_CONVERSATION_CHARS` (120000). **Both are needed.** Every +round re-sends the whole conversation, so cost grows with the square of +its length; but a message count alone cannot bound a single +investigation, because that is one user message followed by round after +round of tool replies and so never reaches a second user message to cut +at. `TrimConversation` compacts old tool replies to +`ASSISTANT_COMPACTED_TOOL_REPLY`, which says plainly that the rows can +be fetched again so a model that still needs them asks rather than +invents - and it never touches the newest +`ASSISTANT_RECENT_TOOL_REPLIES` (8), since those are what the answer is +about to be written from. + +**Timeouts, and why they differ:** + +| Constant | Value | Applies to | +| --- | --- | --- | +| `ASSISTANT_CONNECT_TIMEOUT_SECONDS` | 15 | TCP connect | +| `ASSISTANT_HTTP_TIMEOUT_SECONDS` | 120 | One read/write | +| `ASSISTANT_FETCH_TIMEOUT_SECONDS` | 350 | Default parked fetch | +| `ASSISTANT_SLOW_FETCH_NOTICE_SECONDS` | 5 | When the status starts counting up | +| `ASSISTANT_SCRIPT_APPROVAL_TIMEOUT_SECONDS` | 300 | Waiting on the user to approve a script | +| `ASSISTANT_TRACE_LOADING_TIMEOUT_SECONDS` | 600 | Waiting for a trace to finish opening | + +The fetch deadline is generous on purpose: cutting a live query off and +telling the model it timed out is worse than making the user wait, +because the model then answers around the gap instead of reporting it. +Clear cancels at any point. + +**A tool called while the trace is still opening waits for it.** +`AssistantFetchKind::kTraceLoading` parks the turn with no query issued +at all, and the tool runs for real once the trace is ready. A large file +on a slow disk is waited out rather than reported as a failure the model +would answer around - and if the wait does run out, the reply says the +trace is still opening rather than naming the tool, which never got as +far as running. The full set of kinds is `kNone`, `kSummary`, +`kTopEvents`, `kKernelInstances`, `kDataTable`, `kEventDetails`, +`kTrackStatistics`, `kScript`, `kTraceLoading`; the kind is what decides +how `FinishAssistantFetch` formats the rows. + +**The transcript can hold a chart, not just text.** `Speaker::kChart` +is a line rendered from live model data rather than a stored snapshot: +`AppendChart(track_id)` records only the id, and `RenderActivityChart` +redraws it from `GetAssistantActivityBins` / +`GetAssistantActivityRows` each frame. A tool asks for one by setting +`chart` and `chart_track_id` on its result. The rows of a set share one +scale, so their brightness is comparable to each other. + +**What `OptiqActions` can actually do**, since a tool should reach for +an existing method before adding one: panels (`ShowPanel` over the +`OptiqPanel` enum - `kMinimap`, `kHistogram`, `kTopology`, `kDetails`, +`kSummary`, `kLogViewer`, `kToolbar`, `kAnnotations`), tabs (`ListTabs`, +`ActiveTab`, `SelectTab`, and the details panel's inner +`ListAnalysisTabs` / `ActiveAnalysisTab` / `SelectAnalysisTab`), +timeline options (`SetFlowArrowsVisible`, `SetFlowRenderChained`, +`ZoomToRange`), navigation (`SelectRange`, `ShowRange`, +`ScrollToTrack`), events (`ClickEvent`, `HighlightEvent`, +`ClearHighlights`, `NavigateToEvent`), what gets left behind (`AddNote`, +`SaveBookmark`, `GotoBookmark`, `RemoveBookmark`, `ListBookmarks`, +`MeasureRange`, `ClearMeasurement`, `ResetView`), and - under +`ROCPROFVIS_ENABLE_SCRIPTING` - `ProposeScript`, `ScriptProposalState`, +`ClearScriptProposal`. Every method returns false rather than doing half +the work when the part of the app it needs is missing. + +**`PanelFromName` matches loosely, and that is load-bearing.** "navbar" +and "tree" both resolve to topology; case, spacing, punctuation, and +trailing filler like "view" or "panel" are ignored, so the prompt can +tell the model to pass the user's own words straight through. Every +alias is unique across panels, so a name never resolves two ways. + +**`ClickEvent` is the literal equivalent of a click, so do not help +it.** It drops the previous selection and selects the event, and *that* +is what makes `TraceView` load the details, flow arrows, and call stack. +Pairing it with a manual `DataProvider::FetchEvent` collides on the +shared request id and the duplicate is dropped. Integration points: @@ -1999,7 +2324,13 @@ Integration points: - `TraceView` and `AnalysisView` expose the plain accessors `OptiqActions` needs (`ZoomToRange`, `SelectAnalysisTab`, `ListBookmarks`, and friends). Reuse those rather than reaching into - their members. + their members. They are declared in `rocprofvis_trace_view.h` but + **defined in `agenticprofiling/rocprofvis_ai_trace_view_actions.cpp`**, + so look there, not in `rocprofvis_trace_view.cpp`. +- `DataProvider` carries the assistant's own client id and request ids, + and `rocprofvis_data_provider_script.cpp` carries the script fetch. + `TableType::kAssistant*` in `model/rocprofvis_tables_model.h` are its + private result slots. Rules that are easy to get wrong here: @@ -2019,22 +2350,70 @@ Rules that are easy to get wrong here: - **`ASSISTANT_SCRIPT_PROMPT` is appended, not merged.** The base prompt names its tools in one line, so it must never name a tool the build might not have. The scripting paragraph is a second constant - concatenated in `StartHttpRequest` under the same `#ifdef` that - registers the tool, and it says only *when* to reach for a script - - what a script may call is in the tool's schema description, which is - the one place the model reads about an API. Say it once, in the place - that ships with the tool. + concatenated by `AssistantSystemPrompt()` itself, in + `rocprofvis_ai_prompts.cpp`, under the same `#ifdef` that registers + the tool - deliberately there and not at the call site, so that + everything the model is told lives in one file and the panel just + asks for a prompt. It says only *when* to reach for a script: what a + script may call is in the tool's schema description, which is the one + place the model reads about an API. Say it once, in the place that + ships with the tool. - **The diagnostic knowledge lives in `ASSISTANT_SYSTEM_PROMPT`, for now.** Its `WHAT TO LOOK FOR` list is the catalogue of things worth - checking (idle GPU, launch-bound, transfer cost, register spilling, - launch geometry, imbalance, and so on), each named alongside the tool - and columns that evidence it. Two consequences. Every entry must be - answerable with the tools and the column whitelist as they stand, or - the model will invent an argument that does not exist. And the list is - re-sent on every round of every turn, so it earns its tokens only - while it stays a one-line-per-check list - the moment thresholds need - arithmetic, move them into a C++ tool that returns findings, which is - both cheaper and testable. + checking, in worst-offenders-first order and currently eleven + numbered entries: GPU idle, launch-bound, one slow kernel, transfer + cost, no overlap, blocking synchronization, register spilling, launch + geometry, instability, imbalance, and allocation in the hot path. + Each is named alongside the tool and the columns that evidence it. + Two consequences. Every entry must be answerable with the tools and + the column whitelist as they stand, or the model will invent an + argument that does not exist. And the list is re-sent on every round + of every turn, so it earns its tokens only while it stays a + one-line-per-check list - the moment thresholds need arithmetic, move + them into a C++ tool that returns findings, which is both cheaper and + testable. +- **The prompt's other sections, so an edit lands in the right one.** + In order: the opening role line, `VOICE`, `TWO PASSES, AND THE FIRST + ONE IS CHEAP`, `KNOW WHICH TOOLS RETURN AN EVENT`, `IT IS FINE IF + NOTHING IS WRONG`, an "always call at least one tool" rule, `THREE + LEVELS, EACH NARROWING THE NEXT`, `WHAT TO LOOK FOR`, `LIMITS`, + `AGREEING AND DISAGREEING`, a `Tools:` block that names the tool set + and carries the column whitelist, and `FINISHING`. The scripting + addendum appends `RUNNING A SCRIPT, WHICH IS LEVEL 4`. +- **`THREE LEVELS` is the cost model, and the tool descriptions cannot + replace it.** Level 1 is free (`trace_overview`, `get_summary` - + already built, no query); level 2 is a query (`top_events`, + `kernel_instances`, `track_events`, `track_samples`, `search_events`), + costing whatever window and track count it is given; level 3 is a + point lookup (`event_details`), cheap but reachable only with a + `__uuid` from level 2. The scripting paragraph adds level 4. The point + is the funnel: each level's output narrows the next, and skipping one + is how the model ends up reading a whole trace to answer a question + about one kernel. Numbering them is also what lets the scripting + addendum place itself without editing the base prompt. +- **`KNOW WHICH TOOLS RETURN AN EVENT` exists because aggregates carry + no `__uuid`.** `top_events`, `get_summary`, `kernel_metrics`, and + `track_statistics` group by name and hand back counts and totals; + only `kernel_instances`, `search_events`, and `track_events` return + individual events, and only they can supply the `__uuid` and + `__trackId` that `goto` needs to actually click something. An + aggregate row is a lead, not something clickable - so naming a kernel + and being able to show it are two different tool calls. Related: the + prompt requires every `__uuid` to come back **as a quoted string**, + digit for digit, because they run to 19 digits and a bare JSON number + loses the low bits and names a different event. The tools reject one + rather than answer about the wrong thing. +- **The column whitelist is written out twice, and both must agree.** + `ASSISTANT_QUERY_COLUMNS` in `rocprofvis_ai_tool_query.cpp` is what + `IsAllowedQueryColumn` enforces, and the `Tools:` block of the prompt + spells the same list out so the model does not have to guess; + `AssistantAllowedQueryColumnList()` builds it from the array to report + back when a call names something else. Adding a queryable column means + editing both, or the model is either told about a column that will be + rejected or never told about one that would work. **Treat it as an + interface change**, not a tweak: the array is the boundary between + model input and SQL. `ASSISTANT_MAX_FILTERS` (8) caps how many + conditions one call may carry. - **`AGREEING AND DISAGREEING` is the anti-sycophancy rule.** A question with a claim inside it is a claim to check, not a premise to build on, and the model holds its position when pushed unless a *number* moves - @@ -2075,11 +2454,19 @@ Rules that are easy to get wrong here: itself, a play-by-play of each tool call) plus the required closing `Checked:` line, which gives tool attribution in one line instead of narration throughout. -- **The settings page is URL, model, and API key.** There is no - shipped endpoint URL. The default model is `ASSISTANT_DEFAULT_MODEL` - (`gpt-5.6-luna`). Nothing about the endpoint shape is persisted: - `AssistantProvider` holds only name, URL, and model, and the client - works the rest out from the URL. +- **The settings page is a list of endpoints, one of them active.** + `AssistantSettings` holds `std::vector providers` + plus an `active` index; `SettingsManager::GetActiveAssistantProvider` + resolves it and `SettingsPanel::RenderAssistantSettings` edits the + active one. Each `AssistantProvider` is name, URL, and model, and + nothing else - there is no shipped endpoint URL, the default model is + `ASSISTANT_DEFAULT_MODEL` (`gpt-5.6-luna`), and the default name is + `ASSISTANT_DEFAULT_PROVIDER_NAME` (`Default`). Nothing about the + endpoint *shape* is persisted: the client works that out from the + URL. A half-written settings file still runs, because + `ApplyAssistantEndpointDefaults` fills an empty name or model and the + panel re-seeds an empty list with + `MakeDefaultAssistantProvider()`. - **Two endpoint shapes, decided by `EndpointFlavour`.** Stock OpenAI gets `/chat/completions` appended, names the model in the body, sends `Authorization: Bearer`, and uses `max_completion_tokens`. Azure-style @@ -2093,6 +2480,23 @@ Rules that are easy to get wrong here: `/openai/deployments//chat/completions` with `apiVersion`. Everything that differs between the two keys off the flavour in one place, rather than being sniffed at each use. +- **Three rules exist solely to keep the key off the wire and off other + hosts. Do not relax them for convenience.** The client sets + `set_follow_location(false)`: cpp-httplib does not strip headers when + a redirect crosses hosts, and the Azure subscription key is not one it + would drop, so following one could hand the key to whatever host the + redirect names. A chat endpoint has no reason to redirect, so a 3xx + comes back as `ASSISTANT_REDIRECT_ERROR` instead. `OriginMayCarryToken` + refuses to send a key over plain `http` unless the origin is loopback + (`ASSISTANT_INSECURE_URL_ERROR`) - note it refuses the *key*, not the + request, so an unauthenticated local endpoint still works. And + `StripDeploymentlessCompletions` repairs an Azure URL that already has + `/chat/completions` glued on without a deployment in between, because + Azure answers that with a 404 and a user who pasted the failing URL + back into settings would otherwise be stuck with it. +- **A malformed reply cannot name unbounded tools.** The harmony + fallback parser stops at `ASSISTANT_MAX_HARMONY_CALLS` (4), and the + normal path at `ASSISTANT_MAX_TOOL_CALLS_PER_ROUND` (16). - **`temperature` and `reasoning_effort` are never sent.** Reasoning models reject a non-default temperature, and each model's own default effort is what we want. Keeping the body free of model-specific @@ -2101,11 +2505,32 @@ Rules that are easy to get wrong here: - **Tools only ever run from `Update()`.** They toggle panel visibility and rebuild layout, so running them from `Render()` would mutate widgets halfway through the frame that draws them. -- **A tool only formats rows from a fetch it started.** Several table - request IDs and result slots are shared with the normal UI. If a tool - finds one busy, it waits for that owner and retries its own query - against the original timeout deadline. It returns a timeout rather - than treating the other query's rows as its own. +- **The assistant is its own `DataProvider` client, and that is what + keeps it out of the tabs' way.** `DataProvider::ASSISTANT_CLIENT_ID` + buys its table reads their own request ids, their own controller + tables, and their own model slots (`TableType::kAssistantEventTable`, + `kAssistantSampleTable`, `kAssistantSearchTable`, + `kAssistantSummaryKernelTable`, `kAssistantTopEventsTable`, reached + through `DataProvider::ASSISTANT_*_REQUEST_ID`). A background reader + on the shared ids would be refused whenever a tab happened to be + loading and, worse, would overwrite the rows that tab was showing + once it was not. **Anything else that reads tables without being a + tab wants the same treatment and a client id of its own** - that is + the rule the constant exists to state. +- **Not every tool got a private slot, so the contention protocol + stays.** `get_summary` still rides `DataProvider::SUMMARY_REQUEST_ID` + and `TableType::kSummaryKernelTable`, `event_details` shares the + extended-data, flow, and call-stack ids, and `track_statistics` + derives a shared id through + `RequestIdBuilder::MakeTrackDataRequestId`. So every body still + checks `IsRequestPending` before issuing its own query and reports + `started_fetch`, and the panel keeps that flag: rows that land for a + fetch the tool did not start answer somebody else's query, so the + tool has to run again rather than format them. A tool that finds an + id busy waits for that owner and retries against the *original* + timeout deadline (`m_fetch_retries` is what keeps the clock from + restarting), and returns a timeout rather than treating the other + query's rows as its own. - **HTTP runs on a worker via `std::async`, and must stay cancellable.** `AssistantChatCall::Cancel()` closes the socket; `CancelPendingRequest()` is what lets the panel be destroyed without blocking on a @@ -2115,8 +2540,21 @@ Rules that are easy to get wrong here: The flip side is that the trace in front can change mid-turn, which is what `m_turn_project_id` detects. - **The API key lives in `SecretStore`, never in settings JSON and - never in a log line.** `AssistantProvider` is the saved record in - `UserSettings::assistant`, and holds only name, URL, and model. + never in a log line.** `UserSettings::assistant` stores only the + provider records. **The key is scoped per endpoint**, under + `ASSISTANT_TOKEN_SECRET_KEY + "/" + provider.name` - which is why + `AssistantProvider::name` is a credential-store key rather than a + display label, and why renaming an endpoint orphans its token + (`ApplyAssistantTokenEdits` clears the orphan). Scoping is a security + property, not filing: a single shared key would let any endpoint + without one of its own inherit it, which is how a key for one + provider ends up posted to another's host. + `MigrateLegacyAssistantToken()` moves a key written before endpoints + were named onto the first endpoint, once at load, then erases the + unnamed entry - deliberately a one-shot migration rather than a live + fallback, for exactly that reason. The four + `{Has,Get,Set,Clear}AssistantToken` methods are the only users of + `SecretStore` outside remote, and take the provider name. ## 13. Remote / SSH and Profiler Launch UI @@ -2150,10 +2588,22 @@ downloaded/generated trace is passed to `AppWindow::OpenFile()`: - `RenderSshAuthModal(session)` - renders keyboard-interactive and host-key requests. The owning dialog must call it every frame while the session exists; it is not rendered globally by `AppWindow`. -- `SshSettingsDialog` - connection-profile CRUD modal. -- `SshTestDialog` - remote trace opener/browser owned lazily by - `AppWindow`; its entry point requires both - `ROCPROFVIS_ENABLE_REMOTE` and `ROCPROFVIS_DEVELOPER_MODE`. +- `SshSettingsDialog` (`rocprofvis_ssh_settings_dialog.{h,cpp}`) - + connection-profile CRUD modal. +- `SshTestDialog` (`rocprofvis_ssh_test_dialog.{h,cpp}`) - remote trace + opener/browser owned lazily by `AppWindow`; its entry point requires + both `ROCPROFVIS_ENABLE_REMOTE` and `ROCPROFVIS_DEVELOPER_MODE`. +- `RemoteFileBrowser` (`rocprofvis_remote_file_browser.{h,cpp}`) - the + shared remote file picker: `Open(seed_path, mode, ...)`, `Render()`, + `IsOpen()`, and a `PickMode` enum. Reuse it rather than building + another browse UI on top of `SshSession`'s `Browse` operation. +- `SecretStore` (`rocprofvis_secret_store.{h,cpp}`) - four statics, + `IsAvailable` / `Set` / `Get` / `Erase`, over the OS credential + vault. **It lives under `remote/` but is not gated on remote alone**: + it compiles when `ROCPROFVIS_ENABLE_REMOTE` *or* + `ROCPROFVIS_ENABLE_AGENTIC_PROFILING` is on, because the assistant's + API keys use it too. On Linux the backend is selected by + `ROCPROFVIS_HAVE_LIBSECRET`. - `PromptRequest`, `HostKeyRequest`, `ExecutionOutput`, `FileStat`, and `RemoteDir` in `rocprofvis_ssh_fetch.*` are mutex-protected snapshots; consume updates without holding locks across ImGui calls. @@ -2263,11 +2713,16 @@ express. **Shared form helpers (`rocprofvis_launch_shared_tabs.h`)** - reuse these instead of re-authoring launcher UI: `RenderTargetSection`, -`RenderToolLocationSection`, `RenderRawEnvVarsTab`, `BuildCommandPreviewString`, +`RenderToolLocationSection`, `BuildCommandPreviewString`, `RenderCommandPreview`, `RenderOutputConsole` (+ `ConsoleStatusLevel {kIdle, kRunning, kSuccess, kError}`), `RenderSavedProfileBar`. The -connection-mode selector and SSH UI live in the dialog -(`RenderRemoteSection`), not here. +same header also exports the launcher's card and chip vocabulary: +`BeginLaunchCard` / `EndLaunchCard`, `LaunchCardHeader`, +`LaunchSubHeader`, `ToggleSwitch`, `Chip`, `RenderConfigChips`, +`StatusPill`, and `enum class PillAction`. The connection-mode selector +and SSH UI live in the dialog (`RenderRemoteSection`), not here. +(There is no `RenderRawEnvVarsTab`, despite earlier revisions of this +guide listing one.) **`ProfilerLauncherDialog`** owns `m_backends`, the `LaunchPresetManager`, the `ProfilerLaunchOrchestrator`, `m_config`, and @@ -2435,8 +2890,10 @@ through `AppMonitor` instead of blocking: `rocprofvis_profiler_cancel`. To add a backend: implement `IProfilerBackend`, register it in -`ProfilerLauncherDialog`, map its `rocprofvis_profiler_type_t` in -`ResolveProfilerType`, and implement `ParseTraceOutputPath`. +`ProfilerLauncherDialog`, and implement `ParseTraceOutputPath`. Earlier +revisions named a third step, mapping `rocprofvis_profiler_type_t` in a +`ResolveProfilerType`; **neither that function nor that type exists +under `src/view/`**, so the recipe was unfollowable as written. ## 14. Data Flow: Requests, Remote Operations, and Profiler Runs @@ -2954,14 +3411,19 @@ All under `agenticprofiling/`, compiled only with - `OptiqActions`, `OptiqPanel` -> `agenticprofiling/rocprofvis_ai_actions.h` -> The only code that mutates the UI on the assistant's behalf. -- `AssistantToolContext`, `AssistantFetchState`, - `AssistantToolStartResult`, `StartAssistantTool`, - `FinishAssistantFetch`, `BuildAssistantBriefing` -> +- `AssistantToolContext`, `AssistantFetchState`, `AssistantFetchKind`, + `AssistantToolStartResult`, `AssistantActivityRow`, + `StartAssistantTool`, `FinishAssistantFetch`, + `AssistantScriptFetchPending`, `BuildAssistantBriefing`, + `GetAssistantActivityBins`, `GetAssistantActivityRows` -> `agenticprofiling/rocprofvis_ai_tools.h`. The dispatcher lives in `rocprofvis_ai_tools.cpp`; the bodies are split by what they touch into `rocprofvis_ai_ui_tools.cpp`, `rocprofvis_ai_data_tools.cpp`, and `rocprofvis_ai_script_tools.cpp`, each owning its own handler table. +- `AssistantSystemPrompt` -> `agenticprofiling/rocprofvis_ai_prompts.h` + -> The standing instructions, appended with the scripting paragraph + when that option is on. Product behaviour; edit as prose. - `AssistantToolEntry`, `AssistantToolTable`, `GetAssistantUiToolHandlers`, `GetAssistantDataToolHandlers`, `GetAssistantScriptToolHandlers`, `FinishAssistantScriptFetch` -> @@ -2971,19 +3433,39 @@ All under `agenticprofiling/`, compiled only with -> Runs model-written Python through `DataProvider::ExecuteScript`. Needs `ROCPROFVIS_ENABLE_SCRIPTING` as well; the table is empty otherwise. -- `BuildAssistantToolsJson`, `AssistantToolStatusLabel` -> +- `BuildAssistantToolsJson`, `AssistantToolStatusLabel`, + `AssistantToolNameList` -> `agenticprofiling/rocprofvis_ai_tool_schema.h` -> Thread-safe, reads - no view state. + no view state. `ASSISTANT_TOOL_LABELS` in the `.cpp` is the name and + status-label table. - `BuildAssistantWhereClause`, `AssistantGroupByFromArgs`, - `ResolveAssistantSortColumn` -> + `ResolveAssistantSortColumn`, `ResolveAssistantSortColumnNamed`, + `AssistantSortOrderFromArgs`, `AssistantAllowedQueryColumnList` -> `agenticprofiling/rocprofvis_ai_tool_query.h` -> Model arguments to - SQL fragments, via whitelists. + SQL fragments, via whitelists. `ASSISTANT_QUERY_COLUMNS` in the + `.cpp` is the column whitelist. - `AssistantChatCall`, `AssistantChatRequest`, `AssistantChatResult`, `AssistantMessage`, `AssistantToolCall` -> - `agenticprofiling/rocprofvis_ai_client.h`. -- `AssistantSettings`, `AssistantProvider` -> - `rocprofvis_settings_manager.h` -> Saved URL and model; the token - itself lives in `SecretStore`. + `agenticprofiling/rocprofvis_ai_client.h`. `EndpointFlavour` is + internal to the `.cpp`. +- `AssistantSettings`, `AssistantProvider`, + `ASSISTANT_DEFAULT_MODEL`, `ASSISTANT_DEFAULT_PROVIDER_NAME`, + `ASSISTANT_TOKEN_SECRET_KEY` -> `rocprofvis_settings_manager.h` -> + A list of saved endpoints (name, URL, model) plus an active index; + each endpoint's token lives in `SecretStore` under its own key. The + page and the `{Has,Get,Set,Clear}AssistantToken` bodies are in + `agenticprofiling/rocprofvis_ai_settings.cpp`, but + serialization stays in `rocprofvis_settings_manager.cpp`, unguarded. + +Two files in the folder define no type of their own and are easy to +miss when grepping: + +- `agenticprofiling/rocprofvis_ai_assistant_render.cpp` -> every + `AssistantPanel` method that emits ImGui. +- `agenticprofiling/rocprofvis_ai_trace_view_actions.cpp` -> the + `TraceView` members that exist only for the assistant, declared in + `rocprofvis_trace_view.h` but **not** defined in + `rocprofvis_trace_view.cpp`. ### Events / pubsub @@ -3052,11 +3534,15 @@ All under `agenticprofiling/`, compiled only with `compute/rocprofvis_compute_tester.h`. - `ComputeIsaView`, `BaseCodeWidget`, `SourceCodeWidget`, `IsaCodeWidget` -> `compute/rocprofvis_compute_isa_view.h`. -- `ComputeDataProvider`, `ComputeTableModel`, `ComputeTableCellModel`, - `ComputePlotModel`, `ComputePlotAxisModel`, `ComputePlotSeriesModel`, - `ComputeMetricModel` -> `compute/rocprofvis_compute_data_provider.h`. - `ComputeDataModel`, `ComputeKernelSelectionTable` -> - `model/compute/rocprofvis_compute_data_model.h`. + `model/compute/rocprofvis_compute_data_model.h`. This is the whole + compute model layer. Earlier revisions of this index listed + `ComputeDataProvider`, `ComputeTableModel`, `ComputeTableCellModel`, + `ComputePlotModel`, `ComputePlotAxisModel`, + `ComputePlotSeriesModel`, and `ComputeMetricModel` in a + `compute/rocprofvis_compute_data_provider.h`; **none of those, nor + that file, exist.** Compute views read through the shared + `DataProvider` like everything else. - `AvailableMetrics`, `PcSampleState`, `InstructionSourceLine`, `InstructionLine`, `KernelSymbol`, `CodeObjectStore`, `SourceLine`, `SourceFile`, `PcSamplingData`, `KernelInfo`, `WorkloadInfo`, diff --git a/AGENTS.md b/AGENTS.md index 91b43fe13..10a874640 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -4,15 +4,18 @@ The root file is the repo entry point. Layer-specific AI/agent guides live in `.agents/`: > - [`.agents/UI.md`](./.agents/UI.md) - View-layer architecture, -> widgets, timelines, profiler launch, and remote UI +> widgets, timelines, profiler launch, remote UI, and the Ask Optiq +> agentic-profiling assistant > - [`.agents/CONTROLLER.md`](./.agents/CONTROLLER.md) - deep dive on > `src/controller/` (C ABI, async fetch, memory manager, segment > timeline) > - [`.agents/DATABASE.md`](./.agents/DATABASE.md) - deep dive on > `src/model/` (SQLite adapters, query pipeline, packed table, > data model, topology, metadata versioning) -> - [`.agents/SCRIPTING.md`](./.agents/SCRIPTING.md) - planned in-app -> Python analysis (interpreter lib, controller ABI, phases) +> - [`.agents/SCRIPTING.md`](./.agents/SCRIPTING.md) - in-app Python +> analysis (interpreter lib, controller ABI, phases). Partly shipped +> behind `ROCPROFVIS_ENABLE_SCRIPTING`; the doc is a live roadmap, so +> check a phase's status before trusting its tense. **If you are an AI coding assistant** (Cursor, Codex, Claude Code, Copilot agent, etc.), read `.agents/UI.md` in full before making @@ -21,8 +24,12 @@ If your change touches `src/controller/`, also read `.agents/CONTROLLER.md`. If it touches `src/model/` (the database / data-model layer), also read `.agents/DATABASE.md`. If it touches in-app Python scripting (`src/python/`, `rocprofvis_script_*`, or -script UI), also read `.agents/SCRIPTING.md`. Together these -guides are the single source of truth for: +script UI), also read `.agents/SCRIPTING.md`. If it touches the Ask +Optiq assistant (`src/view/src/agenticprofiling/`, `rocprofvis_ai_*`), +read the **Ask Optiq assistant** section of `.agents/UI.md` in full: +the tool set, the prompt, and the turn machinery are product behaviour, +and the prompt text is not incidental. Together these guides are the +single source of truth for: - Project identity, build, and repo layout - Module boundaries (`app` / `core` / `model` / `controller` / `view`) @@ -31,6 +38,9 @@ guides are the single source of truth for: - UI models and cross-cutting services (events, settings, monitoring, logging, hotkeys, notifications) - Compare, measurement, profiler-launch, and remote/SSH workflows +- The Ask Optiq assistant: tool set, prompt, and turn machinery +- Which features are gated behind a `ROCPROFVIS_ENABLE_*` CMake option + (all of them default OFF) - Data flow (click -> request -> event -> pixels) - Coding conventions, comment style, and reuse catalog - Common pitfalls and a quick-reference index of every UI class diff --git a/PYTHON.md b/PYTHON.md index 55799a87d..de6520ec9 100644 --- a/PYTHON.md +++ b/PYTHON.md @@ -12,7 +12,9 @@ filesystem APIs. Enable scripting at build time with ## How to run a script 1. Open a system trace (`.rpd` or equivalent). -2. Open the **Script** editor from the timeline or compute toolbar. +2. Select the **Script** tab in the details panel at the bottom of the + trace view. Scripting is a system-trace feature: compute traces have + no details panel, so no Script tab. 3. Write (or keep) a Python source string and click **Run**. The editor sends the source plus the current selection (tracks and time @@ -39,7 +41,9 @@ it. `import optiq` is also allowed. | `optiq.table()` | `Table` | Allocates a **private** query table. Does not touch the UI Event Table or Sample Table. | | `optiq.result.text(s)` | function | Append a line of text to the script result. Call more than once; lines are joined with newlines. | -`print` is not available. Use `optiq.result.text`. +`print(...)` is available too, and appends to the same result as +`optiq.result.text`. It accepts `sep`; `end` is ignored, because each +call is one line. ### Constants diff --git a/src/model/README.md b/src/model/README.md index 31c0ad512..aa054b265 100644 --- a/src/model/README.md +++ b/src/model/README.md @@ -1,3 +1,14 @@ +> **Parts of this file are out of date.** Several signatures below have +> drifted from `src/model/inc/` - `bind_trace_to_database` now takes a +> third config-path argument, `rocprofvis_db_type_t` has eight values +> rather than the three listed, the event id is 52/8/4 bits rather than +> 60/4, and more than a dozen public functions are missing entirely. +> The PC-sampling section at the end is current. +> +> For the authoritative description of this layer, read +> [`.agents/DATABASE.md`](../../.agents/DATABASE.md), and treat +> `src/model/inc/` as the final word on any signature. + The Data Model package include two components:
1. Data Model - layered data storage, with public interface to access data properties and possibility of freeing redundant objects.
2. Database - database query manager and processor, with public interface to add data model objects. Supports asynchronous database access.
@@ -44,14 +55,14 @@ Database interface --- **Opens database of provided path and type**
- kAutodetect = 0,
- kRocpdSqlite = 1,
- kRocprofSqlite = 2
+ `rocprofvis_db_type_t` has eight values; see + `src/model/inc/rocprofvis_interface_types.h` for the current list, + which also covers multinode rocprof, rocprof-compute, and the + Chrome / Perfetto / Google SQLite shapes.
`param` filename path of the database file
`param` type type enumeration, kAutodetect for automatic detection
`return` handler to database object
- `note` Currently only old rocpd schema fully supported. Working on rocprof schema
``` rocprofvis_dm_database_t rocprofvis_db_open_database( @@ -232,13 +243,15 @@ rocprofvis_dm_result_t rocprofvis_dm_delete_trace( `param` trace trace object handle created with rocprofvis_dm_create_trace()
`param` database database object handle created with rocprofvis_db_open_database()
+ `param` config_path optional application config directory; pass nullptr
`return` status of operation
``` rocprofvis_dm_result_t rocprofvis_dm_bind_trace_to_database( rocprofvis_dm_trace_t, - rocprofvis_dm_database_t); + rocprofvis_dm_database_t, + rocprofvis_dm_charptr_t); ``` ---