Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid volatile writes from Atomics default values #22373

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

dreis2211
Copy link
Contributor

Hi,

this PR avoids volatile writes by removing the default parameters from Atomic... classes.

Cheers,
Christoph

Checklist:

  • Labels (Team:, Type:, Source:, Module:) and Milestone set
  • Label Add to Release Notes or Not Release Notes content set
  • Request reviewers if possible
  • Send backports/forwardports if fix needs to be applied to past/future releases
  • New public APIs have @Nonnull/@Nullable annotations
  • New public APIs have @since tags in Javadoc

@dreis2211 dreis2211 requested a review from a team as a code owner October 2, 2022 14:02
@hz-devops-test hz-devops-test added the Source: Community PR or issue was opened by a community user label Oct 2, 2022
@devOpsHazelcast
Copy link
Collaborator

Can one of the admins verify this patch?

11 similar comments
@devOpsHazelcast
Copy link
Collaborator

Can one of the admins verify this patch?

@devOpsHazelcast
Copy link
Collaborator

Can one of the admins verify this patch?

@devOpsHazelcast
Copy link
Collaborator

Can one of the admins verify this patch?

@devOpsHazelcast
Copy link
Collaborator

Can one of the admins verify this patch?

@devOpsHazelcast
Copy link
Collaborator

Can one of the admins verify this patch?

@devOpsHazelcast
Copy link
Collaborator

Can one of the admins verify this patch?

@devOpsHazelcast
Copy link
Collaborator

Can one of the admins verify this patch?

@devOpsHazelcast
Copy link
Collaborator

Can one of the admins verify this patch?

@devOpsHazelcast
Copy link
Collaborator

Can one of the admins verify this patch?

@devOpsHazelcast
Copy link
Collaborator

Can one of the admins verify this patch?

@devOpsHazelcast
Copy link
Collaborator

Can one of the admins verify this patch?

@JackPGreen
Copy link
Contributor

run-lab-run

@JackPGreen
Copy link
Contributor

Thanks for your contribution - I think this is a positive change.

I'm concerned about the number and scope of changes that might cause conflicts with other open branches.

Is it possible to focus these changes on any critical paths of the code to get the best improvement on a smaller scale?

@dreis2211
Copy link
Contributor Author

dreis2211 commented Oct 18, 2023

Most of these are inside tests. I can't tell what the "critical paths" are for you and what might conflict with other branches. You have a better overview of that.

@devOpsHazelcast
Copy link
Collaborator

PR closed by Hazelcast automation as no activity (>6 months). Please reopen with comments, if necessary. Thank you for using Hazelcast and your valuable contributions

@dreis2211
Copy link
Contributor Author

dreis2211 commented Apr 19, 2024

This is still relevant. Please reopen this

@JackPGreen
Copy link
Contributor

JackPGreen commented Apr 19, 2024

When benchmarking, on average they initialise ~10% faster when using implicit (over explicit) defaults.

@JackPGreen JackPGreen reopened this Apr 19, 2024
@hz-devops-test
Copy link

The job Hazelcast-pr-builder of your PR failed. (Hazelcast internal details: build log, artifacts).
Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log file
---------ERRORS-----------
--------------------------
[ERROR] Tests run: 13, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 3.545 s <<< FAILURE! -- in com.hazelcast.internal.tpcengine.nio.NioAsyncServerSocketTest
--------------------------
[ERROR] com.hazelcast.internal.tpcengine.nio.NioAsyncServerSocketTest.test_createCloseLoop_withSameReactor -- Time elapsed: 0.343 s <<< ERROR!
--------------------------
[ERROR] Errors: 
--------------------------
[ERROR]   NioAsyncServerSocketTest>AsyncServerSocketTest.test_createCloseLoop_withSameReactor:255 ? UncheckedIO Failed to bind to /127.0.0.1:5001
--------------------------
[ERROR] Tests run: 292, Failures: 0, Errors: 1, Skipped: 0
--------------------------
[ERROR] 
--------------------------

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Automation: PR auto closed Source: Community PR or issue was opened by a community user
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants