perf(test): replace per-instance _database_initialized flag with process-global cache #735

New issue

Closed

opened 2026-03-12 13:19:48 +00:00 by CoreRasurae · 3 comments

CoreRasurae commented

2026-03-12 13:19:48 +00:00

Member

Metadata

Commit Message: perf(test): replace per-instance _database_initialized flag with process-global cache
Branch: perf/m3-global-db-initialized-cache

Summary

Even after the PR #727 fix (early return for non-empty test DBs), the monkey-patched _fast_init_or_upgrade still executes on every new UnitOfWork instance: it parses the URL, extracts the file path, checks prefixes against _SCENARIO_DB_PREFIXES, and calls stat(). A process-global or thread-local set of "already-initialized" DB paths would skip all of this work for repeated calls to the same path.

Background and Context

Identified during the performance review of PR #727 (Performance Finding P5 by @hurui200320, rated Medium severity). The _database_initialized flag at unit_of_work.py:55 is per-instance, not global, so new UnitOfWork objects targeting the same DB file re-trigger the full check. With ~7 calls per scenario across ~10,700 scenarios, this amounts to ~65,000 unnecessary function-body executions per full test run.

Current Behavior

# src/cleveragents/infrastructure/database/unit_of_work.py:55
self._database_initialized = False  # Per-instance flag

Each new UnitOfWork instance calls _ensure_database_initialized() → init_or_upgrade(), which (after PR #727) hits the monkey-patched _fast_init_or_upgrade. Even though the patched function now returns early for non-empty DBs, it still executes URL parsing, path extraction, prefix matching, and a stat() syscall on every call.

Expected Behavior

A process-global set of already-initialized DB paths should short-circuit repeated calls:

_INITIALIZED_DBS: set[str] = set()

def _fast_init_or_upgrade(self, **kwargs):
    db_url = getattr(self, "database_url", "")
    if db_url in _INITIALIZED_DBS:
        return
    # ... existing logic ...
    _INITIALIZED_DBS.add(db_url)

This eliminates all per-call overhead (URL parsing, path extraction, prefix matching, stat syscall) for subsequent calls to the same DB path.

Acceptance Criteria

A process-global (or thread-local) cache of initialized DB paths is implemented
Repeated _fast_init_or_upgrade calls for the same DB skip all logic
The cache is cleared in before_scenario / after_scenario to prevent cross-scenario leaks
All tests pass under both sequential and parallel execution
Measurable reduction in per-scenario overhead (profiling optional but recommended)

Supporting Information

Identified in: PR #727 Performance Review, Finding P5
Review URL: #727
Reviewer: @hurui200320
Estimated savings: ~65,000 unnecessary function executions eliminated per full test run

Subtasks

Add _INITIALIZED_DBS: set[str] = set() to features/environment.py
Add early return in _fast_init_or_upgrade when URL is in the set
Add URL to set after successful initialization/template copy
Clear the set in before_scenario to prevent cross-scenario state
Run nox -s unit_tests with --processes 1 and --processes 16
Verify no cross-scenario database state leaks

Definition of Done

This issue is complete when:

All subtasks above are completed and checked off.
A Git commit is created where the first line of the commit message matches the Commit Message in Metadata exactly, followed by a blank line, then additional lines providing relevant details about the implementation.
The commit is pushed to the remote on the branch matching the Branch in Metadata exactly.
The commit is submitted as a pull request to master, reviewed, and merged before this issue is marked done.

## Metadata - **Commit Message**: `perf(test): replace per-instance _database_initialized flag with process-global cache` - **Branch**: `perf/m3-global-db-initialized-cache` ## Summary Even after the PR #727 fix (early return for non-empty test DBs), the monkey-patched `_fast_init_or_upgrade` still executes on every new `UnitOfWork` instance: it parses the URL, extracts the file path, checks prefixes against `_SCENARIO_DB_PREFIXES`, and calls `stat()`. A process-global or thread-local set of "already-initialized" DB paths would skip all of this work for repeated calls to the same path. ## Background and Context Identified during the performance review of PR #727 (Performance Finding P5 by @hurui200320, rated Medium severity). The `_database_initialized` flag at `unit_of_work.py:55` is per-instance, not global, so new `UnitOfWork` objects targeting the same DB file re-trigger the full check. With ~7 calls per scenario across ~10,700 scenarios, this amounts to ~65,000 unnecessary function-body executions per full test run. ## Current Behavior ```python # src/cleveragents/infrastructure/database/unit_of_work.py:55 self._database_initialized = False # Per-instance flag ``` Each new `UnitOfWork` instance calls `_ensure_database_initialized()` → `init_or_upgrade()`, which (after PR #727) hits the monkey-patched `_fast_init_or_upgrade`. Even though the patched function now returns early for non-empty DBs, it still executes URL parsing, path extraction, prefix matching, and a `stat()` syscall on every call. ## Expected Behavior A process-global set of already-initialized DB paths should short-circuit repeated calls: ```python _INITIALIZED_DBS: set[str] = set() def _fast_init_or_upgrade(self, **kwargs): db_url = getattr(self, "database_url", "") if db_url in _INITIALIZED_DBS: return # ... existing logic ... _INITIALIZED_DBS.add(db_url) ``` This eliminates all per-call overhead (URL parsing, path extraction, prefix matching, stat syscall) for subsequent calls to the same DB path. ## Acceptance Criteria - [x] A process-global (or thread-local) cache of initialized DB paths is implemented - [x] Repeated `_fast_init_or_upgrade` calls for the same DB skip all logic - [x] The cache is cleared in `before_scenario` / `after_scenario` to prevent cross-scenario leaks - [x] All tests pass under both sequential and parallel execution - [x] Measurable reduction in per-scenario overhead (profiling optional but recommended) ## Supporting Information - Identified in: PR #727 Performance Review, Finding P5 - Review URL: https://git.cleverthis.com/cleveragents/cleveragents-core/pulls/727 - Reviewer: @hurui200320 - Estimated savings: ~65,000 unnecessary function executions eliminated per full test run ## Subtasks - [x] Add `_INITIALIZED_DBS: set[str] = set()` to `features/environment.py` - [x] Add early return in `_fast_init_or_upgrade` when URL is in the set - [x] Add URL to set after successful initialization/template copy - [x] Clear the set in `before_scenario` to prevent cross-scenario state - [x] Run `nox -s unit_tests` with `--processes 1` and `--processes 16` - [x] Verify no cross-scenario database state leaks ## Definition of Done This issue is complete when: - All subtasks above are completed and checked off. - A Git commit is created where the **first line** of the commit message matches the Commit Message in Metadata exactly, followed by a blank line, then additional lines providing relevant details about the implementation. - The commit is pushed to the remote on the branch matching the **Branch** in Metadata exactly. - The commit is submitted as a **pull request** to `master`, reviewed, and **merged** before this issue is marked done.

CoreRasurae added the

Rows
Columns

perf(test): replace per-instance _database_initialized flag with process-global cache #735

Metadata

Summary

Background and Context

Current Behavior

Expected Behavior

Acceptance Criteria

Supporting Information

Subtasks

Definition of Done

Implementation Notes

Design Decisions

Key Discovery: Behave exec_file Namespace Isolation

Files Changed

Test Results

Key Discovery: Behave `exec_file` Namespace Isolation