Skip to content

perf(image-redactor): index analyzer bboxes by position in get_pii_bboxes - #2251

Open
SiddhaBasu wants to merge 1 commit into
data-privacy-stack:mainfrom
SiddhaBasu:perf/image-analyzer-bbox-matching
Open

SiddhaBasu wants to merge 1 commit into
data-privacy-stack:mainfrom
SiddhaBasu:perf/image-analyzer-bbox-matching

Conversation

@SiddhaBasu

@SiddhaBasu SiddhaBasu commented Sep 12, 2026

Copy link
Copy Markdown

Change Description

ImageAnalyzerEngine.get_pii_bboxes rescanned the entire analyzer_bboxes list for every ocr_bbox to find a positional match (left/top/width/height equality), an O(len(ocr_bboxes) × len(analyzer_bboxes)) scan. This builds a dict keyed by (left, top, width, height) from analyzer_bboxes once up front, so each ocr_bbox is resolved with a single O(1) dict lookup instead — O(n) + O(m) total.

No behavior change. The original loop used break on the first matching analyzer_bbox, so when multiple analyzer bboxes shared a position, the first one in list order always won. A naive dict[key] = value build would flip that to last-wins, so this uses dict.setdefault(key, analyzer_bbox) to keep identical first-match semantics. Verified with:

  • A new regression test asserting the first of two same-position analyzer bboxes is the one kept.
  • A new equivalence test comparing this implementation against a preserved copy of the original O(n·m) logic on 500 bboxes — output is identical.

Scope note: get_pii_bboxes is called from ImagePiiVerifyEngine.verify() and DicomImagePiiVerifyEngine — the verification/visualization tooling that draws PII/non-PII bounding boxes on an image for inspection. The core ImageRedactorEngine.redact() path does not call this method, so this improves the verify/QA tooling, not the redaction pipeline itself.

Issue reference

N/A — found while auditing presidio-image-redactor for nested-loop hotspots; no existing issue tracked this.

Testing

  • pytest tests/test_image_analyzer_engine.py: 5 existing get_pii_bboxes tests pass, plus 3 new tests added.
  • pytest tests/test_dicom_image_pii_verify_engine.py (a direct caller of this method): unchanged pass/fail counts before and after this change.
  • Full presidio-image-redactor suite: identical failure/error counts before and after this change (confirmed via git stash); only difference is the added tests passing.
  • ruff check / ruff format --check on the changed source file: clean.
  • Could not produce a numeric coverage percentage locally — coverage.py instrumentation crashes on an unrelated torch/thinc import ordering issue in my local environment, reproducible on unmodified main as well. Every branch touched by this diff (dict build, match found, no match, duplicate-position tie-break) is exercised by the test suite.

Checklist

  • I have reviewed the contribution guidelines
  • I agree to follow this project's Code of Conduct
  • I confirm that I have the right to submit this contribution and that it does not knowingly contain proprietary or confidential code
  • My code includes unit tests
  • All unit tests and lint checks pass locally
  • My PR contains documentation updates / additions if required (none required — no public API/behavior change)

…oxes

get_pii_bboxes rescanned the full analyzer_bboxes list for every
ocr_bbox to find a positional match, an O(ocr_bboxes x analyzer_bboxes)
scan. Build a dict keyed by (left, top, width, height) once instead,
turning the lookup into O(n + m).

No behavior change: dict.setdefault preserves the original loop's
first-match-wins semantics when multiple analyzer bboxes share a
position, verified by a new duplicate-position test and an
equivalence test against the original O(n*m) logic on 500 bboxes.

This affects ImagePiiVerifyEngine.verify() and
DicomImagePiiVerifyEngine, the verification/visualization tooling
that annotates images with detected-PII bounding boxes; the core
redact() path does not call this method.
@SiddhaBasu
SiddhaBasu force-pushed the perf/image-analyzer-bbox-matching branch from ec75e6b to 7527f94 Compare September 12, 2026 18:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant