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

Adding UnpaddedAtomic queues #388

Closed
wants to merge 3 commits into from

Conversation

franz1981
Copy link
Collaborator

Fixes #387

@nitsanw I didn't tried yet to reuse some unpadded gen code, but the most of the work should be done, if the naming is fine for you.

@franz1981
Copy link
Collaborator Author

@jponge ;)

final int offsetInOld = modifiedCalcCircularRefElementOffset(pIndex, oldMask);
final int offsetInNew = modifiedCalcCircularRefElementOffset(pIndex, newMask);
// element in new array
soRefElement(newBuffer, offsetInNew, e == null ? s.get() : e);

Check warning

Code scanning / CodeQL

Dereferenced variable may be null Warning

Variable
s
may be null at this access because of
this
null argument.
Variable
s
may be null at this access as suggested by
this
null guard.
Variable
s
may be null at this access as suggested by
this
null guard.
@franz1981
Copy link
Collaborator Author

I see now that I have created the queues under unpadded.atomic instead of atomic.unpadded, likely because I've decided for names like UnpaddedAtomicArrayQueue, because I love to keep AtomicQueue together, but if you prefer differently, I can do it; i really have no strong preferences on this

@franz1981
Copy link
Collaborator Author

@nitsanw If you prefer atomic.unpadded I created an alternative branch (and can squash the commits if is better) -> https://github.com/franz1981/JCTools/tree/atomic_unpadded

@nitsanw
Copy link
Contributor

nitsanw commented Feb 7, 2024

There's already an unpadded code generator, I think it would be better to enhance it rather than add code to all the different atomic queue generators.
Also, please stick with atomic.unpadded.
Thanks!

@franz1981
Copy link
Collaborator Author

franz1981 commented Feb 7, 2024

There's already an unpadded code generator,

In term of code changes, and complexity, I found much easier to modify the atomic generators, given that the most of the work is there, while unpadding (from unsafe versions) seems a relatively easier job (mostly removing comments and unpadded fields).
I will open a PR based on https://github.com/franz1981/JCTools/tree/atomic_unpadded

@nitsanw
Copy link
Contributor

nitsanw commented Feb 7, 2024

I think it makes sense to put the unpadding into one place, I gave it a few minutes and it seems doable, but if you think it's not I don't mind that much.

@franz1981
Copy link
Collaborator Author

Ok @nitsanw i will give it a shot and see where I go, I would have avoided to copy parts/refactor atomic generator into the existing unpadded one

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unpadded variants could avoid using Unsafe
2 participants