feat: Enhance PerformanceProfiler to handle shutdown state and prevent crashes during destruction
This commit is contained in:
@@ -14,7 +14,7 @@ This plan decomposes the design additions (Sections 11–15 of `E6-z3ed-cli-desi
|
|||||||
| Verification Pipeline | Build layered testing + CI coverage. | Phase 6+ | Integrates with harness + CLI suites. |
|
| Verification Pipeline | Build layered testing + CI coverage. | Phase 6+ | Integrates with harness + CLI suites. |
|
||||||
| Telemetry & Learning | Capture signals to improve prompts + heuristics. | Phase 8 | Optional/opt-in features. |
|
| Telemetry & Learning | Capture signals to improve prompts + heuristics. | Phase 8 | Optional/opt-in features. |
|
||||||
|
|
||||||
### Progress snapshot — 2025-10-01 final update (Phase 6 + AW-03 Complete)
|
### Progress snapshot — 2025-10-01 final update (Phase 6 + AW-03 Complete + Graphics Fix)
|
||||||
|
|
||||||
- ✅ CLI global flag passthrough now preserves subcommand options, letting `agent describe` and palette routines accept both space-separated and `--flag=value` styles alongside the updated help text.
|
- ✅ CLI global flag passthrough now preserves subcommand options, letting `agent describe` and palette routines accept both space-separated and `--flag=value` styles alongside the updated help text.
|
||||||
- ✅ `agent describe --format yaml` writes catalog data end-to-end; JSON format also working correctly.
|
- ✅ `agent describe --format yaml` writes catalog data end-to-end; JSON format also working correctly.
|
||||||
@@ -32,6 +32,7 @@ This plan decomposes the design additions (Sections 11–15 of `E6-z3ed-cli-desi
|
|||||||
- ✅ **Regenerated API documentation** (`docs/api/z3ed-resources.yaml`) with all new agent commands.
|
- ✅ **Regenerated API documentation** (`docs/api/z3ed-resources.yaml`) with all new agent commands.
|
||||||
- ✅ **Implemented ProposalDrawer ImGui component** with proposal list, detail view, and accept/reject/delete actions (AW-03).
|
- ✅ **Implemented ProposalDrawer ImGui component** with proposal list, detail view, and accept/reject/delete actions (AW-03).
|
||||||
- ✅ **Fixed linker errors** by adding CLI service sources to app/emu/lib build targets in CMake configuration.
|
- ✅ **Fixed linker errors** by adding CLI service sources to app/emu/lib build targets in CMake configuration.
|
||||||
|
- ✅ **Fixed RAII shutdown crash** in `PerformanceProfiler` - added shutdown flag and validity checks to prevent segfault during static destruction (see `docs/gfx-raii-shutdown-fix.md`).
|
||||||
|
|
||||||
## 2. Task Backlog
|
## 2. Task Backlog
|
||||||
|
|
||||||
@@ -67,9 +68,12 @@ _Status Legend: Prototype · In Progress · Planned · Blocked · Done_
|
|||||||
5. ✅ **COMPLETED**: Added `agent list` command to enumerate all proposals with status filtering.
|
5. ✅ **COMPLETED**: Added `agent list` command to enumerate all proposals with status filtering.
|
||||||
6. ✅ **COMPLETED**: Added `--proposal-id` flag to `agent diff` for viewing specific proposals.
|
6. ✅ **COMPLETED**: Added `--proposal-id` flag to `agent diff` for viewing specific proposals.
|
||||||
7. ✅ **COMPLETED**: Updated resource catalog with agent list and diff actions including arguments and return schemas.
|
7. ✅ **COMPLETED**: Updated resource catalog with agent list and diff actions including arguments and return schemas.
|
||||||
8. **PLANNED**: Add ImGui drawer for proposals with accept/reject controls (AW-03).
|
8. ✅ **COMPLETED**: Implemented ProposalDrawer ImGui component with proposal list, detail view, and accept/reject/delete actions (AW-03).
|
||||||
9. **PLANNED**: Spike IPC options for `ImGuiTestHarness` (socket vs. HTTP vs. shared memory) and document findings (IT-01).
|
9. ✅ **COMPLETED**: Fixed RAII shutdown crash in `PerformanceProfiler` for clean application exit.
|
||||||
10. **PLANNED**: Integrate schema export with TUI command palette + help overlays (RC-04).
|
10. **IN PROGRESS**: Test ProposalDrawer in running application with live proposals.
|
||||||
|
11. **PLANNED**: Complete ROM merging in AcceptProposal method (AW-03 TODO).
|
||||||
|
12. **PLANNED**: Spike IPC options for `ImGuiTestHarness` (socket vs. HTTP vs. shared memory) and document findings (IT-01).
|
||||||
|
13. **PLANNED**: Integrate schema export with TUI command palette + help overlays (RC-04).
|
||||||
|
|
||||||
## 4. Open Questions
|
## 4. Open Questions
|
||||||
|
|
||||||
|
|||||||
@@ -16,28 +16,36 @@ PerformanceProfiler& PerformanceProfiler::Get() {
|
|||||||
return instance;
|
return instance;
|
||||||
}
|
}
|
||||||
|
|
||||||
PerformanceProfiler::PerformanceProfiler() : enabled_(true) {
|
PerformanceProfiler::PerformanceProfiler() : enabled_(true), is_shutting_down_(false) {
|
||||||
// Initialize with memory pool for efficient data storage
|
// Initialize with memory pool for efficient data storage
|
||||||
// Reserve space for common operations to avoid reallocations
|
// Reserve space for common operations to avoid reallocations
|
||||||
active_timers_.reserve(50);
|
active_timers_.reserve(50);
|
||||||
operation_times_.reserve(100);
|
operation_times_.reserve(100);
|
||||||
operation_totals_.reserve(100);
|
operation_totals_.reserve(100);
|
||||||
operation_counts_.reserve(100);
|
operation_counts_.reserve(100);
|
||||||
|
|
||||||
|
// Register destructor to set shutdown flag
|
||||||
|
std::atexit([]() {
|
||||||
|
Get().is_shutting_down_ = true;
|
||||||
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
void PerformanceProfiler::StartTimer(const std::string& operation_name) {
|
void PerformanceProfiler::StartTimer(const std::string& operation_name) {
|
||||||
if (!enabled_) return;
|
if (!enabled_ || is_shutting_down_) return;
|
||||||
|
|
||||||
active_timers_[operation_name] = std::chrono::high_resolution_clock::now();
|
active_timers_[operation_name] = std::chrono::high_resolution_clock::now();
|
||||||
}
|
}
|
||||||
|
|
||||||
void PerformanceProfiler::EndTimer(const std::string& operation_name) {
|
void PerformanceProfiler::EndTimer(const std::string& operation_name) {
|
||||||
if (!enabled_) return;
|
if (!enabled_ || is_shutting_down_) return;
|
||||||
|
|
||||||
auto timer_iter = active_timers_.find(operation_name);
|
auto timer_iter = active_timers_.find(operation_name);
|
||||||
if (timer_iter == active_timers_.end()) {
|
if (timer_iter == active_timers_.end()) {
|
||||||
SDL_Log("Warning: EndTimer called for operation '%s' that was not started",
|
// During shutdown, silently ignore missing timers to avoid log spam
|
||||||
operation_name.c_str());
|
if (!is_shutting_down_) {
|
||||||
|
SDL_Log("Warning: EndTimer called for operation '%s' that was not started",
|
||||||
|
operation_name.c_str());
|
||||||
|
}
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -264,13 +272,15 @@ double PerformanceProfiler::CalculateMedian(std::vector<double> values) {
|
|||||||
// ScopedTimer implementation
|
// ScopedTimer implementation
|
||||||
ScopedTimer::ScopedTimer(const std::string& operation_name)
|
ScopedTimer::ScopedTimer(const std::string& operation_name)
|
||||||
: operation_name_(operation_name) {
|
: operation_name_(operation_name) {
|
||||||
if (PerformanceProfiler::IsEnabled()) {
|
if (PerformanceProfiler::IsEnabled() && PerformanceProfiler::IsValid()) {
|
||||||
PerformanceProfiler::Get().StartTimer(operation_name_);
|
PerformanceProfiler::Get().StartTimer(operation_name_);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
ScopedTimer::~ScopedTimer() {
|
ScopedTimer::~ScopedTimer() {
|
||||||
if (PerformanceProfiler::IsEnabled()) {
|
// Check if profiler is still valid (not shutting down) to prevent
|
||||||
|
// crashes during static destruction order issues
|
||||||
|
if (PerformanceProfiler::IsEnabled() && PerformanceProfiler::IsValid()) {
|
||||||
PerformanceProfiler::Get().EndTimer(operation_name_);
|
PerformanceProfiler::Get().EndTimer(operation_name_);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -62,6 +62,14 @@ class PerformanceProfiler {
|
|||||||
return Get().enabled_;
|
return Get().enabled_;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* @brief Check if the profiler is in a valid state (not shutting down)
|
||||||
|
* This prevents crashes during static destruction order issues
|
||||||
|
*/
|
||||||
|
static bool IsValid() {
|
||||||
|
return !Get().is_shutting_down_;
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* @brief Start timing an operation
|
* @brief Start timing an operation
|
||||||
* @param operation_name Name of the operation to time
|
* @param operation_name Name of the operation to time
|
||||||
@@ -161,6 +169,7 @@ class PerformanceProfiler {
|
|||||||
std::unordered_map<std::string, int> operation_counts_; // Count per operation
|
std::unordered_map<std::string, int> operation_counts_; // Count per operation
|
||||||
|
|
||||||
bool enabled_ = true; // Performance monitoring enabled by default
|
bool enabled_ = true; // Performance monitoring enabled by default
|
||||||
|
bool is_shutting_down_ = false; // Flag to prevent operations during destruction
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* @brief Calculate median value from a sorted vector
|
* @brief Calculate median value from a sorted vector
|
||||||
|
|||||||
@@ -36,21 +36,22 @@ void CanvasPerformanceIntegration::StartMonitoring() {
|
|||||||
}
|
}
|
||||||
|
|
||||||
void CanvasPerformanceIntegration::StopMonitoring() {
|
void CanvasPerformanceIntegration::StopMonitoring() {
|
||||||
if (frame_timer_active_) {
|
// Release timers in reverse order of creation to ensure clean shutdown
|
||||||
frame_timer_.reset();
|
if (modal_timer_active_) {
|
||||||
frame_timer_active_ = false;
|
modal_timer_.reset();
|
||||||
}
|
modal_timer_active_ = false;
|
||||||
if (draw_timer_active_) {
|
|
||||||
draw_timer_.reset();
|
|
||||||
draw_timer_active_ = false;
|
|
||||||
}
|
}
|
||||||
if (interaction_timer_active_) {
|
if (interaction_timer_active_) {
|
||||||
interaction_timer_.reset();
|
interaction_timer_.reset();
|
||||||
interaction_timer_active_ = false;
|
interaction_timer_active_ = false;
|
||||||
}
|
}
|
||||||
if (modal_timer_active_) {
|
if (draw_timer_active_) {
|
||||||
modal_timer_.reset();
|
draw_timer_.reset();
|
||||||
modal_timer_active_ = false;
|
draw_timer_active_ = false;
|
||||||
|
}
|
||||||
|
if (frame_timer_active_) {
|
||||||
|
frame_timer_.reset();
|
||||||
|
frame_timer_active_ = false;
|
||||||
}
|
}
|
||||||
|
|
||||||
util::logf("Stopped performance monitoring for canvas: %s", canvas_id_.c_str());
|
util::logf("Stopped performance monitoring for canvas: %s", canvas_id_.c_str());
|
||||||
|
|||||||
Reference in New Issue
Block a user