Skip to content

Cache duration Pattern in GenerationalHeapParser CMS remark path - #579

Merged
Martijn Verburg (karianna) merged 3 commits into
microsoft:mainfrom
Hashim1999164:fix/cache-duration-group-pattern-567
Aug 24, 2026
Merged

Martijn Verburg (karianna) merged 3 commits into
microsoft:mainfrom
Hashim1999164:fix/cache-duration-group-pattern-567

Conversation

@Hashim1999164

@Hashim1999164 Hashim Khan (Hashim1999164) commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Summary

splitRemarkReferenceWithWeakReferenceSplitBug recompiled Pattern.compile(".* " + PAUSE_TIME) on every call. That matches other parser patterns that already use static final compiled patterns.

This change introduces DURATION_GROUP_PATTERN and reuses it for matching.

Fixes #567

Test plan

  1. ./mvnw -pl parser -am -DskipTests compile
  2. Optional: run parser tests that cover CMS remark / weak reference split bug lines

Compile DURATION_GROUP_PATTERN once as a static final field instead of
on every CMS remark weak-reference split parse call.
Fixes microsoft#567.
Replace greedy .* with a line bounded prefix so matcher.find still
selects the last pause time. Guard null parse results that SonarCloud
flags as reliability bugs.
Comment thread parser/src/main/java/com/microsoft/gctoolkit/parser/GenerationalHeapParser.java Outdated
@sonarqubecloud

Copy link
Copy Markdown

@skrcode

Copy link
Copy Markdown

I ran JAIPilot Cloud against this exact PR head. It found one additional internal allocation cleanup in the changed ParNew promotion-failure path: collect sizes in a growable primitive int array instead of boxing each value into an ArrayList and then unboxing into the final array.

Tests pin zero, one, and 28 captured blocks and their exact order. The same focused and 124 parser tests passed before and after; the seven-module reactor passed. A fixed 28-block collection fixture eliminated 28 boxed Integers plus ArrayList growth, with a five-trial isolated median of 733.82 ms to 537.30 ms over 5,000,000 iterations.

PR directly onto this source branch: Hashim1999164#1
Cloud-generated draft and full evidence: skrcode#2

Regex parsing dominates real log processing and the populated field currently has no downstream reader, so this is an optional internal allocation cleanup rather than a broad throughput claim. Microsoft CLA handling may require folding the change into the contributor commit.

@karianna

Copy link
Copy Markdown
Member

John (@johnoliver) Can you approve and merge

@karianna
Martijn Verburg (karianna) merged commit 61b968f into microsoft:main Aug 24, 2026
10 checks passed
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.

Parser bug: Pattern.compile() called per-invocation in GenerationalHeapParser.splitRemarkReferenceWithWeakReferenceSplitBug

3 participants