Skip to content

Resolve patterns and entry text once per analysis - #36

Open
pavog wants to merge 1 commit into
masterfrom
pattern-analyser-cache-entry-text
Open

pavog wants to merge 1 commit into
masterfrom
pattern-analyser-cache-entry-text

Conversation

@pavog

@pavog pavog commented Nov 13, 2026

Copy link
Copy Markdown
Member

PatternAnalyser::analyse() called getPatterns() again for every entry and insight class, and passed the entry object to preg_match_all(), so Entry::__toString() joined all lines of the entry once per pattern.

Patterns are now resolved once per analysis and every entry is converted to a string once.
Analysing all 229 test logs of codex-minecraft goes from about 500 ms to about 195 ms with unchanged results.

To do this without passing an entry and its text side by side, the analysis is split by responsibility:

  • analyse() resolves the patterns and adds the insights of every entry to the analysis
  • analyseEntry() handles one entry against all patterns and holds the entry text as a local variable
  • findMatches() only knows a pattern and a text
  • createInsight() builds the insight for one match

Breaking changes

  • analyseEntry() has a new signature: analyseEntry(EntryInterface $entry, array $patternsByClass): array.
    It is called once per entry instead of once per pattern and always returns an array instead of null.
    Subclasses that override it have to be adapted; codex-minecraft, codex-minecraft-clientmod, codex-minecraft-fabricclient and codex-hytale don't override it and pass their tests unchanged.
  • An insight class that is added twice is only used once, because the patterns are keyed by class name.
    Before, it matched twice and doubled the counter of its insights.

Decisions

  • The patterns are only reused within one analyse() call and not cached statically, so changes to the possible insight classes or their patterns still take effect.
  • The entry text is not cached in Entry itself, because Line::setText() is public and doesn't know its entry, so the cache could not be invalidated.
  • A pattern for which preg_match_all() returns false is still skipped silently, as before.
    Reporting these errors is left for a separate PR.

Changes

  • add resolvePatterns(), findMatches() and createInsight() to PatternAnalyser
  • change analyseEntry() to analyse one entry against all patterns
  • use a strict comparison in findPossibleInsightClass()
  • add tests that patterns are resolved once and every entry is converted to a string once
  • add tests for the pattern key, an insight class added twice, a removed insight class and a failing pattern

@pavog pavog added this to the v5.0.0 milestone Nov 13, 2026
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.

2 participants