Skip to content

Improve Geronimo Parallelism#202

Merged
jmfinelli merged 1 commit intojbosstm:mainfrom
jmfinelli:improve_geronimo_parallelism
May 5, 2026
Merged

Improve Geronimo Parallelism#202
jmfinelli merged 1 commit intojbosstm:mainfrom
jmfinelli:improve_geronimo_parallelism

Conversation

@jmfinelli
Copy link
Copy Markdown
Contributor

@jmfinelli jmfinelli commented Apr 23, 2026

Trying to address #203

@jmfinelli jmfinelli marked this pull request as draft April 23, 2026 14:21
Copy link
Copy Markdown
Member

@mmusgrov mmusgrov left a comment

Choose a reason for hiding this comment

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

The PR is still running but the changes look good (I agree that we should strive to get the best results possible for other TMs so that we get a true comparison).

I don't know what exactly the checksumEnabled flag does (the geronimo javadoc for it doesn't say) but the ApacheTomEE config seems to set it to true by default. I wonder what the impact of not setting it is.

10,// "threadsWaitingForceThreshold"});
xidFactory,
new File(buildDir)
"org.objectweb.howl.log.BlockLogBuffer",
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.

Thanks @jmfinelli for your PR, as I am not familiar with Geronimo configuration it might be useful to keep the information from the comments we had?

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.

org.apache.geronimo.transaction.log.HOWLLog already reports those variable names, that's why I removed them

@jmfinelli
Copy link
Copy Markdown
Contributor Author

The PR is still running but the changes look good (I agree that we should strive to get the best results possible for other TMs so that we get a true comparison).

I don't know what exactly the checksumEnabled flag does (the geronimo javadoc for it doesn't say) but the ApacheTomEE config seems to set it to true by default. I wonder what the impact of not setting it is.

I should have opened a Jira to explain why I raised this PR. Sorry about that. Apparently, we're experiencing a strange failure in the PERFORMANCE job related to Geronimo: its transaction logs gets corrupted and the job gets stuck. I'm trying to fix this issue by modifying the HOWL parameters. So far, no luck. I suppose that the limited resources of the runner are the problem...but we cannot do much about that.

@jmfinelli
Copy link
Copy Markdown
Contributor Author

I don't know what exactly the checksumEnabled flag does (the geronimo javadoc for it doesn't say) but the ApacheTomEE config seems to set it to true by default. I wonder what the impact of not setting it is.

checksumEnabled makes Geronimo compute a checksum while using HOWL. By disabling it, I'm trying to reduce the pressure on the runner's CPUs. Anyway, modifying those parameters didn't help as the job failed again.

@jmfinelli jmfinelli force-pushed the improve_geronimo_parallelism branch from efbabd1 to 6dddfcc Compare April 29, 2026 10:56
@jmfinelli jmfinelli force-pushed the improve_geronimo_parallelism branch from c7d0720 to 8d39465 Compare April 30, 2026 16:35
@jmfinelli jmfinelli marked this pull request as ready for review May 1, 2026 09:33
@jmfinelli jmfinelli requested review from marcosgopen and mmusgrov May 1, 2026 09:33
@jmfinelli
Copy link
Copy Markdown
Contributor Author

I'm going to re-run all GH actions in this PR but I would like to highlight that a full passed was already achieved

@jmfinelli jmfinelli force-pushed the improve_geronimo_parallelism branch from 151feca to 89e716f Compare May 1, 2026 10:46
@jmfinelli jmfinelli marked this pull request as draft May 1, 2026 12:18
@jmfinelli jmfinelli marked this pull request as ready for review May 1, 2026 12:19
Copy link
Copy Markdown
Member

@marcosgopen marcosgopen left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @jmfinelli !

name: Testing PERFORMANCE with JDK ${{ inputs.jdk_version }}
# ---------------------------------------------------------------------------
# Build Narayana once and determine which benchmarks to run
# ---------------------------------------------------------------------------
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.

great improvement, thanks!

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.

Thanks Marco! I will squash all commits together.

As a note, the performance benchmark with 1600 threads can still fail because of Geronimo but the probability is definitely smaller than before. cc @mmusgrov @tomjenkinson

@jmfinelli jmfinelli force-pushed the improve_geronimo_parallelism branch from c353691 to 77399ec Compare May 5, 2026 08:49
@jmfinelli jmfinelli merged commit 301b3e1 into jbosstm:main May 5, 2026
jmfinelli added a commit that referenced this pull request May 5, 2026
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.

3 participants