Version 1.5.6#41
Conversation
WalkthroughThis release (v1.5.6) introduces performance optimizations and stability improvements. Key changes include debounced emission of UI signals to reduce overhead, increased prefetch worker capacity and radius, cooperative shutdown with Ctrl-C handling, Qt-thread-safe callback ordering in prefetchers, and debug instrumentation for cache/timing analysis. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant SigHandler as Signal Handler
participant QtTimer as Qt Timer
participant AppController
participant MainExecutor as Main/Prefetch Executor
participant ThumbnailExecutor as Thumbnail Executor
User->>SigHandler: Ctrl-C (SIGINT)
SigHandler->>QtTimer: Request App Shutdown
QtTimer->>AppController: Call shutdown()
AppController->>AppController: shutdown_qt()
Note over AppController: Stop timers, detach engine
AppController->>MainExecutor: executor.shutdown(cancel_futures=True, wait=False)
MainExecutor->>MainExecutor: Cancel pending tasks
AppController->>ThumbnailExecutor: executor.shutdown(cancel_futures=True, wait=False)
ThumbnailExecutor->>ThumbnailExecutor: Cancel pending tasks
AppController->>AppController: shutdown_nonqt()
Note over AppController: Cleanup background resources
AppController->>User: Application exits
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@ChangeLog.md`:
- Around line 7-12: Update the two subsection headings from "#### Performance"
and "#### Stability" to use H3 (###) instead of H4 so they follow the parent H2
and match other sections (e.g., "### Changed", "### Fixed"); edit the heading
lines containing "Performance" and "Stability" to start with ###.
🧹 Nitpick comments (7)
faststack/ui/provider.py (1)
41-45: Moveimport timeto module level; preferlogoverprint().
import timeinsiderequestImageruns on every QML image request. While Python caches imports, the convention in this file is module-level imports. More importantly, the[DBGCACHE]output usesprint()while the rest of the module (andprefetch.py's similar instrumentation) uses theloglogger — this bypasses log-level filtering and any configured log handlers.Suggested changes
At the top of the file (e.g., after line 5):
import timeThen in the method body:
- import time _debug = getattr(self.app_controller, 'debug_cache', False) if _debug: _t_start = time.perf_counter() - print(f"[DBGCACHE] {_t_start*1000:.3f} requestImage: START id={id}") + log.info("[DBGCACHE] %.3f requestImage: START id=%s", _t_start*1000, id)Apply the same
log.infochange on lines 86 and 136.faststack/thumbnail_view/prefetcher.py (2)
93-102: Qt emitter setup assumes construction on the Qt/main thread.The
_ReadyEmitterQObject will have thread affinity to whatever thread constructsThumbnailPrefetcher. TheQueuedConnectionon line 100 only delivers to the receiver's thread, so this works correctly as long as the prefetcher is constructed on the main thread. If that invariant ever changes, callbacks would not land on the Qt thread as intended.Worth a brief comment or assertion if you want to harden this.
311-324: Consider logging cancelled-future exceptions at debug level.The
try/except Exception: passon lines 321-324 silently swallows errors duringcancel(). While this is a shutdown path, alog.debugwould aid diagnosis if cancellation misbehaves.Optional improvement
for f in futures: try: f.cancel() - except Exception: - pass + except Exception as exc: + log.debug("Error cancelling future during cancel_all: %s", exc)faststack/imaging/prefetch.py (1)
221-224: Use module-levelimport timeandloginstead ofprint().
import timeappears insideupdate_prefetch(line 222),submit_task(line 338), and_decode_and_cache(line 404). While cached by Python, scattering imports across methods is unconventional — move to the top of the file. Additionally,print()on lines 224, 322, and 341 bypasses the configured logging infrastructure; preferlog.info()/log.debug()for consistency with the other timing instrumentation already in this file.Suggested fix (module top-level import)
import logging import os import io +import time import hashlibThen remove the inline
import timestatements from lines 222, 338, and 404.Also applies to: 318-322, 338-341
faststack/app.py (3)
722-724: Consider usinglog.debug()instead ofprint()for debug instrumentation.All
[DBGCACHE]instrumentation usesprint(), bypassing the logging infrastructure. This means these messages can't be filtered by log level, won't appear in log files, and won't include standard log metadata (timestamps, module name). Sincedebug_cacheis already a dedicated flag,log.debug()with the[DBGCACHE]prefix would integrate better with the existing logging setup while preserving the same gating behavior.
3497-3506: Log exceptions during shutdown cleanup instead of silently swallowing them.The
try/except/passblocks here mask potential issues during shutdown. Even during cleanup, a brieflog.debughelps diagnose problems when shutdown behaves unexpectedly.♻️ Proposed fix
try: self._metadata_debounce_timer.stop() - except Exception: - pass + except Exception as e: + log.debug("Failed to stop debounce timer: %s", e) - # Stop QFileSystemWatcher if it's Qt-based try: self.watcher.stop() - except Exception: - pass + except Exception as e: + log.debug("Failed to stop watcher: %s", e)
5582-5636: Robust shutdown-with-timeout pattern; minor cleanup opportunity.The
threading.Timerbackstop +faulthandler.dump_traceback_latercombo is a solid approach for preventing hung shutdowns while providing diagnostic output. Two small items:
import threadingat line 5583 is redundant — it's already imported at the top of the file (line 27).- The
try/except/passat lines 5616-5619 should log the exception for consistency with the shutdown debugging intent.♻️ Proposed cleanup
- import threading import faulthandlertry: timer.stop() - except Exception: - pass + except Exception as e: + log.debug("Failed to stop SIGINT timer: %s", e)
Summary by CodeRabbit
New Features
Bug Fixes