8344833: CTW: Make failing on zero classes optional

[lepestock]

For CTW, zero classes in provided jar is now a failure.
This creates noisy and blocking false positives in fuzzy/mass scale runs, where we use jar archives from random sources, unchecked or randomly generated, etc.

This PR makes this behaviour controllable. Default reaction is a failure, like before.

---

Progress

• Change must be properly reviewed (1 review required, with at least 1 Reviewer)
• Change must not contain extraneous whitespace
• Commit message must refer to an issue

Issue

• JDK-8344833: CTW: Make failing on zero classes optional (Enhancement - P4)

Reviewers

• Tobias Hartmann (@TobiHartmann - Reviewer)
• Vladimir Kozlov (@vnkozlov - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/22320/head:pull/22320
$ git checkout pull/22320

Update a local copy of the PR:
$ git checkout pull/22320
$ git pull https://git.openjdk.org/jdk.git pull/22320/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 22320

View PR using the GUI difftool:
$ git pr show -t 22320

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/22320.diff

Using Webrev

Link to Webrev Comment

[bridgekeeper]

👋 Welcome back enikitin! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[openjdk]

@lepestock This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8344833: CTW: Make failing on zero classes optional

Reviewed-by: thartmann, kvn

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 446 new commits pushed to the master branch:

• f6021a9: 8344668: Unnecessary array allocations and copying in TextLine
• 874d68a: 8343747: C2: TestReplicateAtConv.java crashes with -XX:MaxVectorSize=8
• d9a2213: 8345299: C2: some nodes can still have incorrect control after do_range_check()
• 2286fae: 8345159: RISCV: Fix -Wzero-as-null-pointer-constant warning in emit_static_call_stub
• 8403285: 8268145: [macos] Rendering artifacts is seen when text inside the JTable with TableCellEditor having JTextfield
• aa38284: 8345435: Eliminate tier1_compiler_not_xcomp group
• 9284602: 8345628: [BACKOUT] JDK-8287122 Use gcc12 -ftrivial-auto-var-init=pattern in debug builds
• 41c8971: 8287122: Use gcc12 -ftrivial-auto-var-init=pattern in debug builds
• 5da0eee: 8285692: Enable _FORTIFY_SOURCE=2 when building with Clang
• daa2ba5: 8339622: Regression in make open-hotspot-xcode-project
• ... and 436 more: https://git.openjdk.org/jdk/compare/a91d4c022f1ccaec68d7e3ce21be402ffc568eca...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

As you do not have Committer status in this project an existing Committer must agree to sponsor your change. Possible candidates are the reviewers of this PR (@TobiHartmann, @vnkozlov) but any other Committer may sponsor as well.

➡️ To flag this PR as ready for integration with the above commit message, type /integrate in a new comment. (Afterwards, your sponsor types /sponsor in a new comment to perform the integration).

[openjdk]

@lepestock The following label will be automatically applied to this pull request:

• hotspot-compiler

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

[mlbridge]

Webrevs

• 02: Full - Incremental (d1e57aa6)
• 01: Full - Incremental (a091e15d)
• 00: Full (286c47e0)

[vnkozlov]

What is default value of allow_zero_class_count and where it is set?

Why you even need this to be controlled and not default behavior? What is benefit of having error vs warning for empty jar?

Should you check totalClassCount too to catch empty jar? As I see classCount could be 0 if specified classStart and classStop as the same which could happened regardless number of classes in jar file.

[lepestock]

What is default value of allow_zero_class_count and where it is set?

It's False, as per Boolean.getBoolean(...) specification

Why you even need this to be controlled and not default behavior? What is benefit of having error vs warning for empty jar?

For mass-running of CTW against unchecked/random jars from various jar repositories, like Maven Central. Another solution would be filtering out such jars in advance, but that's a more difficult (read the jar file, check the class' count, etc.) solution.

Should you check totalClassCount too to catch empty jar? As I see classCount could be 0 if specified classStart and classStop as the same which could happened regardless number of classes in jar file.

I've missed it. You're right, relying on totalClassCount seems a better idea. Fixed.

[vnkozlov]

Why you even need this to be controlled and not default behavior? What is benefit of having error vs warning for empty jar?

For mass-running of CTW against unchecked/random jars from various jar repositories, like Maven Central. Another solution would be filtering out such jars in advance, but that's a more difficult (read the jar file, check the class' count, etc.) solution.

So for our normal testing it will be still error? Okay.

[vnkozlov]

Good.

[lepestock]

/integrate

[openjdk]

@lepestock
Your change (at version d1e57aa) is now ready to be sponsored by a Committer.

[vnkozlov]

I leave to @TobiHartmann to finish his review and sponsor if approved.

[TobiHartmann]

Looks good.

[TobiHartmann]

/sponsor

[openjdk]

Going to push as commit 0e2a285.
Since your change was applied there have been 446 commits pushed to the master branch:

• f6021a9: 8344668: Unnecessary array allocations and copying in TextLine
• 874d68a: 8343747: C2: TestReplicateAtConv.java crashes with -XX:MaxVectorSize=8
• d9a2213: 8345299: C2: some nodes can still have incorrect control after do_range_check()
• 2286fae: 8345159: RISCV: Fix -Wzero-as-null-pointer-constant warning in emit_static_call_stub
• 8403285: 8268145: [macos] Rendering artifacts is seen when text inside the JTable with TableCellEditor having JTextfield
• aa38284: 8345435: Eliminate tier1_compiler_not_xcomp group
• 9284602: 8345628: [BACKOUT] JDK-8287122 Use gcc12 -ftrivial-auto-var-init=pattern in debug builds
• 41c8971: 8287122: Use gcc12 -ftrivial-auto-var-init=pattern in debug builds
• 5da0eee: 8285692: Enable _FORTIFY_SOURCE=2 when building with Clang
• daa2ba5: 8339622: Regression in make open-hotspot-xcode-project
• ... and 436 more: https://git.openjdk.org/jdk/compare/a91d4c022f1ccaec68d7e3ce21be402ffc568eca...master

Your commit was automatically rebased without conflicts.

[openjdk]

@TobiHartmann @lepestock Pushed as commit 0e2a285.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.