Skip to content

feat(#1284): benchmark unlint-non-existing-defect with +unlint metas - #1285

Open
VasilevNStas wants to merge 1 commit into
objectionary:masterfrom
VasilevNStas:fix/issue-1284-unlint-benchmark
Open

feat(#1284): benchmark unlint-non-existing-defect with +unlint metas#1285
VasilevNStas wants to merge 1 commit into
objectionary:masterfrom
VasilevNStas:fix/issue-1284-unlint-benchmark

Conversation

@VasilevNStas

Copy link
Copy Markdown
Contributor

fix #1284

What

Adds a JMH benchmark that measures LtUnlintNonExistingDefect on an XMIR that actually contains +unlint metas.

Why

The existing scansXmir benchmark is driven by real-world bytecode-derived XMIR (com.sun.jna.*, org.apache.hadoop...FSNamesystem), none of which contains a single +unlint meta. So it only measures the "no +unlint at all" fast path of LtUnlintNonExistingDefect.

#1233 changed the other path too: when +unlint metas are present, the lint now filters this.lints down to the referenced names before computing existing(). Until now there was no fixture exercising that path, so a regression there would go unnoticed by the benchmark.

How

New benchmark method scansXmirWithUnlints in SourceBench, which parses the existing fixture unlint-ascii-only.eo (it has +unlint ascii-only and a non-ASCII comment that produces a real defect) and runs the full lint set on it.

Measured result (JMH, average time):

SourceBench.scansXmirWithUnlints  N/A  avgt  3  9.097 ± 31.283  ms/op

This gives a timing signal for the +unlint code path, so future changes to LtUnlintNonExistingDefect (faster, same or slower) become visible.

Both mvn test (606 tests) and mvn clean install -Pqulice pass.

@VasilevNStas

Copy link
Copy Markdown
Contributor Author

@volodya-lombrozo plz review this

@volodya-lombrozo volodya-lombrozo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@VasilevNStas Currently we create S ... XXL benchmarks using jeo-maven-plugin which works pretty well, but it generates XMIR that is a bit of artifical. Maybe we can reconsider our strategy to generating these files? Maybe it's better to generate them by parsing real EO programs? At least, we can substitute S, M examples. What do u think?

@VasilevNStas VasilevNStas left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@volodya-lombrozo

Good suggestion about using real EO programs instead of bytecode-derived XMIR for the S/M benchmarks. That would make the benchmark data closer to what the lints actually process in production. I see two ways to do it and would like your call on the scope:

  1. A focused follow-up that only adds this scansXmirWithUnlints benchmark for the +unlint path (what this PR does), and
  2. A bigger refactor that replaces jeo-maven-plugin fixtures for S/M (or all sizes) with parsed EO programs.

I kept this PR to option 1 because switching the existing S..XXL fixtures touches SourceSize, BenchmarkState and the README table, and it is a separate concern from #1284 (which is specifically about the +unlint path having no coverage at all).

If you prefer, I can prepare option 2 as another PR after this one lands. Meanwhile, note this PR also surfaced a pre-existing infra issue: the proguard profile (active on JDK < 22) fails on the rultor merge (JDK 21) and locally on JDK 21 with Obfuscation failed, unrelated to the diff — I described it in #1238 with the MAVEN_OPTS/-P!proguard suggestion.

Happy to proceed either way.

@VasilevNStas

Copy link
Copy Markdown
Contributor Author

@volodya-lombrozo
plz re-review

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.

unlint-non-existing-defect benchmark lacks coverage for documents with +unlint metas, potentially missing regressions

2 participants