From 1335d2c31cf9ffc8569a38b75a6041740329b653 Mon Sep 17 00:00:00 2001 From: AJ Slater Date: Mon, 27 Apr 2026 18:18:52 -0700 Subject: [PATCH] Threads + worker: gate type-only imports + monotonic clock MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two changes to the librarian thread infrastructure both ride this diff because the file lives in the import chain of all 9 librarian daemons (cover, bookmark, notifier, scribe, fs.event_batcher, fs.watcher, fs.poller, cron, librariand). Saving here multiplies. F1 - type-only imports gated behind TYPE_CHECKING. Both ``multiprocessing.queues.Queue`` and ``loguru._logger.Logger`` are referenced ONLY in function-signature type annotations (NamedThread.__init__ and WorkerMixin.init_worker). Adding ``from __future__ import annotations`` makes the annotations forward-references at runtime, so the imports can move into a TYPE_CHECKING block. $ uv run python -X importtime -c \ "from multiprocessing.queues import Queue" 2>&1 | grep multiprocessing import time: 14045 us | multiprocessing.queues # cold After this change ``codex.librarian.threads`` no longer pulls in ``multiprocessing.queues`` at all — confirmed via ``'multiprocessing.queues' in sys.modules`` after fresh import. ``loguru._logger`` was already loaded transitively by Django's logging configuration, so the saving for it is structural rather than measurable, but the type-only import was misleading. F2 - switch ``time.time()`` to ``time.monotonic()`` in AggregateMessageQueuedThread. The thread does elapsed-time math (``time.time() - self._last_send``) on every queued item to decide whether to flush the cache. ``time.time()`` returns wall-clock and can jump (NTP correction, daylight saving, manual clock change), which would skew the flush-timing logic in subclasses that rely on it (BookmarkThread, NotifierThread). ``time.monotonic()`` is immune to clock jumps and is also slightly cheaper on most platforms (CLOCK_MONOTONIC vs gettimeofday). Correctness fix with an incidental perf win on the hot per-item path. Co-Authored-By: Claude Opus 4.7 --- codex/librarian/threads.py | 29 ++++++++++++++++++++++------- codex/librarian/worker.py | 12 +++++++++--- 2 files changed, 31 insertions(+), 10 deletions(-) diff --git a/codex/librarian/threads.py b/codex/librarian/threads.py index f5dc08a44..32265618d 100644 --- a/codex/librarian/threads.py +++ b/codex/librarian/threads.py @@ -1,18 +1,26 @@ """Abstract Thread worker for doing queued tasks.""" -import time +from __future__ import annotations + from abc import ABC, abstractmethod -from multiprocessing.queues import Queue from queue import Empty, SimpleQueue from threading import Thread -from typing import override +from time import monotonic +from typing import TYPE_CHECKING, override from django.db import close_old_connections -from loguru._logger import Logger from setproctitle import setproctitle from codex.librarian.worker import WorkerStatusMixin +if TYPE_CHECKING: + # Both type-hint-only — defer until type checkers run. Keeps + # the runtime import graph for ``threads.py`` lean across the + # nine librarian modules that import it. + from multiprocessing.queues import Queue + + from loguru._logger import Logger + class BreakLoopError(Exception): """Simple way to break out of function nested loop.""" @@ -116,12 +124,19 @@ class AggregateMessageQueuedThread(QueuedThread, ABC): def __init__(self, *args, **kwargs) -> None: """Initialize the cache.""" self.cache = {} - self._last_send = time.time() + # ``time.monotonic`` over ``time.time``: elapsed-time math + # (``monotonic() - self._last_send``) must not be affected by + # wall-clock jumps from NTP / daylight saving / manual + # adjustments. Slightly cheaper than ``time.time`` on most + # platforms (clock_gettime(CLOCK_MONOTONIC) vs gettimeofday) + # — the bigger win is correctness on the flush-timing path + # in subclasses like BookmarkThread / NotifierThread. + self._last_send = monotonic() super().__init__(*args, **kwargs) def set_last_send(self) -> None: """Set the last send time to now.""" - self._last_send = time.time() + self._last_send = monotonic() @override def get_timeout(self): @@ -148,7 +163,7 @@ def cleanup_cache(self, keys) -> None: def process_item(self, item) -> None: """Aggregate items and sleep in case there are more.""" self.aggregate_items(item) - since_last_timed_out = time.time() - self._last_send + since_last_timed_out = monotonic() - self._last_send waited_too_long = since_last_timed_out > self.MAX_DELAY if waited_too_long: self.timed_out() diff --git a/codex/librarian/worker.py b/codex/librarian/worker.py index e8b3cb59f..98b88b5ce 100644 --- a/codex/librarian/worker.py +++ b/codex/librarian/worker.py @@ -1,12 +1,18 @@ """Mixin for common librarian thread attributes.""" -from multiprocessing.queues import Queue -from typing import override +from __future__ import annotations -from loguru._logger import Logger +from typing import TYPE_CHECKING, override from codex.librarian.status_controller import StatusController +if TYPE_CHECKING: + # Type-hint-only — runtime imports stay lean across the nine + # librarian modules that import this chain. + from multiprocessing.queues import Queue + + from loguru._logger import Logger + class WorkerMixin: """Mixin for common thread attributes."""