Skip to content

Perf: cache UseStatementSniff getUseStatements across fix iterations#66

Merged
dereuromark merged 1 commit into
masterfrom
perf-cache-usestatement-and-fqcn
May 13, 2026
Merged

Perf: cache UseStatementSniff getUseStatements across fix iterations#66
dereuromark merged 1 commit into
masterfrom
perf-cache-usestatement-and-fqcn

Conversation

@dereuromark
Copy link
Copy Markdown
Contributor

@dereuromark dereuromark commented May 13, 2026

Summary

Adds a per-file cache to UseStatementSniff::getUseStatements(), the same shape as #64 and #65.

UseStatementSniff has its own getUseStatements() implementation that duplicates UseStatementsTrait::getUseStatements() (the one cached in #64). The existing instance-level cache via $existingStatements covers repeated calls within a single phpcs pass; the new static cache adds coverage across phpcbf fix iterations, where populateTokenListeners() creates fresh sniff instances per pass and resets the instance cache.

Cache invalidation

Same fingerprint-based scheme introduced in #64. Token count alone is not strong enough — an alias rename keeps it constant. Cached entries record a content fingerprint of each use statement range and re-verify them against the live tokens before being trusted.

Defensive belt-and-braces: refuse to serve a cached empty result. Files with no use statements don't benefit from caching anyway, and skipping the cache for empty results means we won't serve stale state in the corner case where a fix adds a first use statement while another simultaneous fix happens to keep the overall token count unchanged.

Why not the trait

UseStatementSniff::getUseStatements() is almost a duplicate of UseStatementsTrait::getUseStatements() (the one cached in #64), but with a slightly different result shape (no 'statement' field). Refactoring this sniff onto the trait is doable but out of scope for a perf-only PR — same cache shape, kept local.

Benchmarks

Measured on the same 1095-file CakePHP 5 app from #62 / #63 / #64 / #65. Per-sniff CPU times from phpcs --parallel=1 --report=performance:

Sniff Before this PR After this PR
PhpCollective.Namespaces.UseStatement ~3.36s ~2.08s (-38%)

Note

An earlier draft of this PR also added a cache to FullyQualifiedClassNameInDocBlockSniff::parseUseStatements() / ::getNamespace(). Those changes were dropped after #65 landed with a different (monotonic-stack-pointer) caching scheme covering the same hot path.

Verification

  • Existing PHPUnit suite passes (100 tests / 122 assertions).
  • composer cs-check and composer stan on this repo: clean.
  • Output diff on the activity-pro test corpus: identical violations before vs after.
  • codex review --uncommitted on the original (broader) draft flagged P2 cache-invalidation concerns; the empty-result skip in this final version was added in response.

UseStatementSniff has its own getUseStatements() implementation that
duplicates UseStatementsTrait::getUseStatements() (cached in #64). It
already has an instance-level cache via existingStatements, which
covers repeated calls within a single phpcs pass; the new static
cache adds coverage across phpcbf fix iterations, where
populateTokenListeners() creates fresh sniff instances per pass and
resets the instance cache.

Cache invalidation follows the same fingerprint-based scheme as #64:
token count alone is not strong enough, since an alias rename keeps
it constant. Cached entries record a content fingerprint of each
use statement range and re-verify them against the live tokens
before being trusted. The cache also refuses to serve an empty
result so it cannot return stale state for a file where a fix added
a first use statement while another simultaneous fix happened to
keep the file's overall token count unchanged.

Measured on the same CakePHP 5 app from #62 / #63 / #64 / #65
(parallel=1, --report=performance):

  PhpCollective.Namespaces.UseStatement   3.36s -> 2.08s

Existing test suite (100 tests / 122 assertions) passes unchanged.

The FQCN cache changes from the earlier draft of this PR were
dropped in favour of the cache that landed via #65.
@dereuromark dereuromark force-pushed the perf-cache-usestatement-and-fqcn branch from 2178955 to 9f2a6d7 Compare May 13, 2026 21:57
@dereuromark dereuromark changed the title Perf: cache UseStatementSniff getUseStatements + FullyQualifiedClassNameInDocBlock parseUseStatements/getNamespace Perf: cache UseStatementSniff getUseStatements across fix iterations May 13, 2026
@dereuromark dereuromark merged commit c9b7e70 into master May 13, 2026
4 checks passed
@dereuromark dereuromark deleted the perf-cache-usestatement-and-fqcn branch May 13, 2026 22:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant