JBTM-3734 Replace synchronized with JUCL

[imperatorx]

https://issues.redhat.com/browse/JBTM-3734
Replaced synchronized blocks with reentrantlocks as advised in JEP 425 to support virtual threads.

CORE

Edit:
The synchronized parts were replaced that caused thread pinnings in my own limited tests. These events occured at transaction start and end. To make locking consistent, all synchronized(this) and non-static synchronized methods were replaced in all subclasses of StateManager. Additionally, I changed the _syncLock to be a ReentrantLock in TwoPhaseCoordinator.

This should be enough for basic virtual thread work, although many other subclasses of StateManager contain different synchronized(someCustomLock) synchronizations, which will need to be investigated if someone hits thread pinnings on hot paths again.

[jmfinelli]

Thank you very much for your PR @imperatorx!

We're in the middle of releasing Narayana 6.0 and, unfortunately, we're not able to use our CI to test your PR. Sorry about that!

As soon as we release the major version of Narayana, we'll test and review your PR.

[mmusgrov]

Is there a JBTM for this one?

[imperatorx]

Is there a JBTM for this one?

https://issues.redhat.com/browse/JBTM-3734

[mmusgrov]

@imperatorx How did you track down these potentially long lived pinned threads? The JEP says the diagnostics tool (to help decide whether or not to change instances of synchronized to a lock) is to run with -Djdk.tracePinnedThreads=full; did you use that or something else?

Some of the code you have changed may do I/O which can be "unfriendly" but other code is short lived manipulation of collections so I was wondering why those needed to change.

Our code base makes entensive use of synchronized blocks, do you think we need to analyse these many other usages.

[imperatorx]

Yes, I've used the tracePinnedThreads utility. The short lived non-IO blocks are only changed to use the same locks as the IO operations use, so I don't break the synchronization consistency (if two places synchronized on the same lock, they still do, but now on a JUCL lock instead of a monitor).

• BasicAction had synchronized methods and synchronized(this) blocks, so I created a new lock object. (Made it protected, so child classes can use the same lock if they used synchronized(this) or had synchronized methods)
• TwoPhaseCoordinator had an Object to synchronize on, so I changed it to a ReentrantLock

These two classes gave me problems on the hot paths, there might be more. I think for recovery stuff and background work this problem is not really relevant since those task are run in custom threads, e.g. reaper, recovery etc.

[mmusgrov]

Yes, I've used the tracePinnedThreads utility. The short lived non-IO blocks are only changed to use the same locks as the IO operations use, so I don't break the synchronization consistency (if two places synchronized on the same lock, they still do, but now on a JUCL lock instead of a monitor).

A yes of course, I should have realised. Thanks for the explanation.

[mmusgrov]

I applied the Hold label since we are in the middle of a release.

[jbosstm-bot]

CORE profile tests failed (https://ci-jenkins-csb-narayana.apps.ocp-c1.prod.psi.redhat.com/job/btny-pulls-narayana/PROFILE=CORE,jdk=openJDK11,label=jnlp-agent/33/): Narayana rebase on main failed. Please rebase it manually

[jbosstm-bot]

AS_TESTS profile tests failed (https://ci-jenkins-csb-narayana.apps.ocp-c1.prod.psi.redhat.com/job/btny-pulls-narayana/PROFILE=AS_TESTS,jdk=openJDK11,label=jnlp-agent/33/): Narayana rebase on main failed. Please rebase it manually

[jbosstm-bot]

XTS profile tests failed (https://ci-jenkins-csb-narayana.apps.ocp-c1.prod.psi.redhat.com/job/btny-pulls-narayana/PROFILE=XTS,jdk=openJDK17,label=jnlp-agent/33/): Narayana rebase on main failed. Please rebase it manually

[jbosstm-bot]

QA_JTS_OPENJDKORB profile tests failed (https://ci-jenkins-csb-narayana.apps.ocp-c1.prod.psi.redhat.com/job/btny-pulls-narayana/PROFILE=QA_JTS_OPENJDKORB,jdk=openJDK17,label=jnlp-agent/33/): Narayana rebase on main failed. Please rebase it manually

[jbosstm-bot]

JACOCO profile tests failed (https://ci-jenkins-csb-narayana.apps.ocp-c1.prod.psi.redhat.com/job/btny-pulls-narayana/PROFILE=JACOCO,jdk=openJDK17,label=jnlp-agent/33/): Narayana rebase on main failed. Please rebase it manually

[jbosstm-bot]

JACOCO profile tests failed (https://ci-jenkins-csb-narayana.apps.ocp-c1.prod.psi.redhat.com/job/btny-pulls-narayana/PROFILE=JACOCO,jdk=openJDK11,label=jnlp-agent/33/): Narayana rebase on main failed. Please rebase it manually

[jbosstm-bot]

RTS profile tests failed (https://ci-jenkins-csb-narayana.apps.ocp-c1.prod.psi.redhat.com/job/btny-pulls-narayana/PROFILE=RTS,jdk=openJDK17,label=jnlp-agent/33/): Narayana rebase on main failed. Please rebase it manually

[jbosstm-bot]

QA_JTS_OPENJDKORB profile tests failed (https://ci-jenkins-csb-narayana.apps.ocp-c1.prod.psi.redhat.com/job/btny-pulls-narayana/PROFILE=QA_JTS_OPENJDKORB,jdk=openJDK11,label=jnlp-agent/33/): Narayana rebase on main failed. Please rebase it manually

[jbosstm-bot]

TOMCAT profile tests failed (https://ci-jenkins-csb-narayana.apps.ocp-c1.prod.psi.redhat.com/job/btny-pulls-narayana/PROFILE=TOMCAT,jdk=openJDK17,label=jnlp-agent/33/): Narayana rebase on main failed. Please rebase it manually

[jbosstm-bot]

LRA profile tests failed (https://ci-jenkins-csb-narayana.apps.ocp-c1.prod.psi.redhat.com/job/btny-pulls-narayana/PROFILE=LRA,jdk=openJDK17,label=jnlp-agent/33/): Narayana rebase on main failed. Please rebase it manually

[jbosstm-bot]

AS_TESTS profile tests failed (https://ci-jenkins-csb-narayana.apps.ocp-c1.prod.psi.redhat.com/job/btny-pulls-narayana/PROFILE=AS_TESTS,jdk=openJDK17,label=jnlp-agent/33/): Narayana rebase on main failed. Please rebase it manually

[jbosstm-bot]

CORE profile tests failed (https://ci-jenkins-csb-narayana.apps.ocp-c1.prod.psi.redhat.com/job/btny-pulls-narayana/PROFILE=CORE,jdk=openJDK17,label=jnlp-agent/33/): Narayana rebase on main failed. Please rebase it manually

[jbosstm-bot]

QA_JTA profile tests failed (https://ci-jenkins-csb-narayana.apps.ocp-c1.prod.psi.redhat.com/job/btny-pulls-narayana/PROFILE=QA_JTA,jdk=openJDK11,label=jnlp-agent/33/): Narayana rebase on main failed. Please rebase it manually

[jbosstm-bot]

XTS profile tests failed (https://ci-jenkins-csb-narayana.apps.ocp-c1.prod.psi.redhat.com/job/btny-pulls-narayana/PROFILE=XTS,jdk=openJDK11,label=jnlp-agent/33/): Narayana rebase on main failed. Please rebase it manually

[jbosstm-bot]

QA_JTA profile tests failed (https://ci-jenkins-csb-narayana.apps.ocp-c1.prod.psi.redhat.com/job/btny-pulls-narayana/PROFILE=QA_JTA,jdk=openJDK17,label=jnlp-agent/33/): Narayana rebase on main failed. Please rebase it manually

[jbosstm-bot]

LRA profile tests failed (https://ci-jenkins-csb-narayana.apps.ocp-c1.prod.psi.redhat.com/job/btny-pulls-narayana/PROFILE=LRA,jdk=openJDK11,label=jnlp-agent/33/): Narayana rebase on main failed. Please rebase it manually

[jbosstm-bot]

TOMCAT profile tests failed (https://ci-jenkins-csb-narayana.apps.ocp-c1.prod.psi.redhat.com/job/btny-pulls-narayana/PROFILE=TOMCAT,jdk=openJDK11,label=jnlp-agent/33/): Narayana rebase on main failed. Please rebase it manually

[jbosstm-bot]

PERFORMANCE profile tests failed (https://ci-jenkins-csb-narayana.apps.ocp-c1.prod.psi.redhat.com/job/btny-pulls-narayana/PROFILE=PERFORMANCE,jdk=openJDK17,label=jnlp-agent/33/): Narayana rebase on main failed. Please rebase it manually

[jbosstm-bot]

PERFORMANCE profile tests failed (https://ci-jenkins-csb-narayana.apps.ocp-c1.prod.psi.redhat.com/job/btny-pulls-narayana/PROFILE=PERFORMANCE,jdk=openJDK11,label=jnlp-agent/33/): Narayana rebase on main failed. Please rebase it manually

[jbosstm-bot]

Started testing this pull request with JACOCO profile: https://ci-jenkins-csb-narayana.apps.ocp-c1.prod.psi.redhat.com/job/btny-pulls-narayana/PROFILE=JACOCO,jdk=openJDK11,label=jnlp-agent/190/

[jbosstm-bot]

CORE profile tests passed - Job complete https://ci-jenkins-csb-narayana.apps.ocp-c1.prod.psi.redhat.com/job/btny-pulls-narayana/PROFILE=CORE,jdk=openJDK17,label=jnlp-agent/190/

[jbosstm-bot]

Benchmark output (please refer to the article https://developer.jboss.org/wiki/PerformanceGatesForAcceptingPerformanceFixesInNarayana for information on our testing procedures.

If you just want to run a single benchmark then please refer to the README.md file in our benchmark repository at https://github.com/jbosstm/performance/tree/main/narayana

For information on the hardware config used for this PR please consult the CI job artefact hwinfo.txt or the job output

[jbosstm-bot]

PERFORMANCE profile tests failed (https://ci-jenkins-csb-narayana.apps.ocp-c1.prod.psi.redhat.com/job/btny-pulls-narayana/PROFILE=PERFORMANCE,jdk=openJDK17,label=jnlp-agent/190/): there were regressions in one or more of the benchmarks (see previous PR comment for details

[jbosstm-bot]

Started testing this pull request with CORE profile: https://ci-jenkins-csb-narayana.apps.ocp-c1.prod.psi.redhat.com/job/btny-pulls-narayana/PROFILE=CORE,jdk=openJDK11,label=jnlp-agent/190/

[jbosstm-bot]

Started testing this pull request with JACOCO profile: https://ci-jenkins-csb-narayana.apps.ocp-c1.prod.psi.redhat.com/job/btny-pulls-narayana/PROFILE=JACOCO,jdk=openJDK17,label=jnlp-agent/190/

[jbosstm-bot]

Benchmark output (please refer to the article https://developer.jboss.org/wiki/PerformanceGatesForAcceptingPerformanceFixesInNarayana for information on our testing procedures.

If you just want to run a single benchmark then please refer to the README.md file in our benchmark repository at https://github.com/jbosstm/performance/tree/main/narayana

For information on the hardware config used for this PR please consult the CI job artefact hwinfo.txt or the job output

[jbosstm-bot]

PERFORMANCE profile tests failed (https://ci-jenkins-csb-narayana.apps.ocp-c1.prod.psi.redhat.com/job/btny-pulls-narayana/PROFILE=PERFORMANCE,jdk=openJDK11,label=jnlp-agent/190/): there were regressions in one or more of the benchmarks (see previous PR comment for details

[jbosstm-bot]

Started testing this pull request with QA_JTA profile: https://ci-jenkins-csb-narayana.apps.ocp-c1.prod.psi.redhat.com/job/btny-pulls-narayana/PROFILE=QA_JTA,jdk=openJDK17,label=jnlp-agent/190/

[jbosstm-bot]

CORE profile tests passed - Job complete https://ci-jenkins-csb-narayana.apps.ocp-c1.prod.psi.redhat.com/job/btny-pulls-narayana/PROFILE=CORE,jdk=openJDK11,label=jnlp-agent/190/

[jbosstm-bot]

TOMCAT profile tests failed (https://ci-jenkins-csb-narayana.apps.ocp-c1.prod.psi.redhat.com/job/btny-pulls-narayana/PROFILE=TOMCAT,jdk=openJDK17,label=jnlp-agent/190/): Narayana rebase on main failed. Please rebase it manually

[jbosstm-bot]

AS_TESTS profile tests failed (https://ci-jenkins-csb-narayana.apps.ocp-c1.prod.psi.redhat.com/job/btny-pulls-narayana/PROFILE=AS_TESTS,jdk=openJDK17,label=jnlp-agent/190/): Narayana rebase on main failed. Please rebase it manually

[jbosstm-bot]

XTS profile tests passed - Job complete https://ci-jenkins-csb-narayana.apps.ocp-c1.prod.psi.redhat.com/job/btny-pulls-narayana/PROFILE=XTS,jdk=openJDK11,label=jnlp-agent/190/

[jbosstm-bot]

XTS profile tests passed - Job complete https://ci-jenkins-csb-narayana.apps.ocp-c1.prod.psi.redhat.com/job/btny-pulls-narayana/PROFILE=XTS,jdk=openJDK17,label=jnlp-agent/190/

[jbosstm-bot]

QA_JTA profile tests passed - Job complete https://ci-jenkins-csb-narayana.apps.ocp-c1.prod.psi.redhat.com/job/btny-pulls-narayana/PROFILE=QA_JTA,jdk=openJDK11,label=jnlp-agent/190/

[jbosstm-bot]

QA_JTA profile tests passed - Job complete https://ci-jenkins-csb-narayana.apps.ocp-c1.prod.psi.redhat.com/job/btny-pulls-narayana/PROFILE=QA_JTA,jdk=openJDK17,label=jnlp-agent/190/

[jbosstm-bot]

AS_TESTS profile tests failed (https://ci-jenkins-csb-narayana.apps.ocp-c1.prod.psi.redhat.com/job/btny-pulls-narayana/PROFILE=AS_TESTS,jdk=openJDK11,label=jnlp-agent/190/): AS tests failed

[jbosstm-bot]

QA_JTS_OPENJDKORB profile tests passed - Job complete https://ci-jenkins-csb-narayana.apps.ocp-c1.prod.psi.redhat.com/job/btny-pulls-narayana/PROFILE=QA_JTS_OPENJDKORB,jdk=openJDK17,label=jnlp-agent/190/

[jbosstm-bot]

QA_JTS_OPENJDKORB profile tests passed - Job complete https://ci-jenkins-csb-narayana.apps.ocp-c1.prod.psi.redhat.com/job/btny-pulls-narayana/PROFILE=QA_JTS_OPENJDKORB,jdk=openJDK11,label=jnlp-agent/190/

[jbosstm-bot]

JACOCO profile tests passed - Job complete https://ci-jenkins-csb-narayana.apps.ocp-c1.prod.psi.redhat.com/job/btny-pulls-narayana/PROFILE=JACOCO,jdk=openJDK17,label=jnlp-agent/190/

[jbosstm-bot]

JACOCO profile tests passed - Job complete https://ci-jenkins-csb-narayana.apps.ocp-c1.prod.psi.redhat.com/job/btny-pulls-narayana/PROFILE=JACOCO,jdk=openJDK11,label=jnlp-agent/190/

[mmusgrov]

Thanks @imperatorx for this contribution and I approve of it and thanks for the explanation of why you changed only a subset of synchronized primitives (#2077 (comment)). Sorry it took us so long to attempt verification. Since the PR changes public APIs we are going to release the change in a major version (7.0.0.Final).

[mmusgrov]

@imperatorx Please can you rebase the PR (it rebases quite easily - just the header in BasicAction).
I fixed the conflicts using the github interface but the PR is still reporting conflicts so it needs a force push.

[jbosstm-bot]

CORE profile tests failed (https://ci-jenkins-csb-narayana.apps.ocp-c1.prod.psi.redhat.com/job/btny-pulls-narayana/PROFILE=CORE,jdk=openJDK11,label=jnlp-agent/192/): Narayana rebase on main failed. Please rebase it manually

[jbosstm-bot]

CORE profile tests failed (https://ci-jenkins-csb-narayana.apps.ocp-c1.prod.psi.redhat.com/job/btny-pulls-narayana/PROFILE=CORE,jdk=openJDK17,label=jnlp-agent/192/): Narayana rebase on main failed. Please rebase it manually

[mmusgrov]

I merged the PR even though the tests failed with a rebase request - the PR does rebase cleanly and the CI run does pass after the merge.