Skip to content

7903671: jcstress: Update buffer tests for JDK-8318966#160

Closed
judovana wants to merge 1 commit into
openjdk:masterfrom
judovana:jdk23buffers
Closed

7903671: jcstress: Update buffer tests for JDK-8318966#160
judovana wants to merge 1 commit into
openjdk:masterfrom
judovana:jdk23buffers

Conversation

@judovana

@judovana judovana commented Feb 26, 2025

Copy link
Copy Markdown

This is another possible fix for https://bugs.openjdk.org/browse/CODETOOLS-7903671 - : jcstress: Update buffer tests for JDK-8318966 (Bug - P4)
This one is attempting Experimental reduction of (0, 8) -> (0, 1) in all affected calls. Brief experiments seesm the change works. But whether it is still testing what it should.. I try to decrypt it, but up to now now proof.

I have not yet tested all affected tests. That is in progress.


Progress

  • Change must not contain extraneous whitespace
  • Change must be properly reviewed (1 review required, with at least 1 Committer)

Issue

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 160

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

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jcstress/pull/160.diff

Using Webrev

Link to Webrev Comment

@bridgekeeper

bridgekeeper Bot commented Feb 26, 2025

Copy link
Copy Markdown

👋 Welcome back jvanek! 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

openjdk Bot commented Feb 26, 2025

Copy link
Copy Markdown

❗ This change is not yet ready to be integrated.
See the Progress checklist in the description for automated requirements.

@openjdk openjdk Bot added the rfr Pull request is ready for review label Feb 26, 2025
@judovana

judovana commented Feb 26, 2025

Copy link
Copy Markdown
Author

Ah... not fulfilling its promises....
With jdk21 it was passing ok. But on 23 it got occasionally

    Check test failed
    java.lang.IllegalStateException: Atomic access not supported for heap buffer

But about half passed. I guess Ihad picked up wrong subset for initial testing

@judovana judovana closed this Feb 26, 2025
@judovana

Copy link
Copy Markdown
Author

Reopening for a while... maye somebody around will know why.

@judovana judovana reopened this Feb 26, 2025
@mlbridge

mlbridge Bot commented Feb 26, 2025

Copy link
Copy Markdown

Webrevs

@judovana

judovana commented Mar 3, 2025

Copy link
Copy Markdown
Author

Ok. I made a much more deeper look, and consulted with Aph and Thomas, and this fix indeed looks wrong from all points of view.

To reduce it to (1,1),except obvious failures, seesm to no longer check the issue. Also it seems that for jdk 23 and up, the correct replacement are direct buffers - which - feel free to correct me - are well covered in jcstress,

Thus the correct fix seesm to be #159 or to remove the.

Closing this in favour of #159

@judovana judovana closed this Mar 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

rfr Pull request is ready for review

Development

Successfully merging this pull request may close these issues.

1 participant