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); ``` ---