Adding Atomic Unpadded queues #389
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| 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
franz1981
force-pushed
the
atomic_unpadded
branch
from
e1d445e
to
4b295bb
Compare
franz1981
force-pushed
the
atomic_unpadded
branch
from
4b295bb
to
b10375b
Compare
franz1981
force-pushed
the
atomic_unpadded
branch
from
b10375b
to
3a063e3
Compare
|
IDK if e1d445e vs this one matches some of your requests: I didn't enhanced the unpadded generator, because the most of the parts I needed to reuse were from the atomic generators, while unpadding just requires to remove unpadded fields & comments. I tried to extract the parts from the atomic generator, to enhance the unpadded one, without much luck, due to lack of time (admit), but if you have any hint I'm open to do things differently, bud |
| generator.organiseImports(cu); | ||
| generator.cleanupComments(cu); | ||
|
|
||
| String outputFileName = generator.translateQueueName(file.getName().replace(".java", "")) + ".java"; |
There was a problem hiding this comment.
I could make JCToolsGenerator to use a single method String generate(String queueName) which return the translated queue name instead of having 3 different methods, executing a generic generation process
There was a problem hiding this comment.
I'm saving to add more abstractions @nitsanw because I don't know yet if this PR is decent enough; IDK if makes sense to create abstractions of any kind for some task like this, if not for the sake of re-using code, given that generating code isn't a proper domain which require being strongly defined (in order to get used elsewhere - in other layers), but I see it more as a set of transformation steps/scripts which we want to write once and modify after 4-5 years (unless bugs).
| import static org.jctools.queues.atomic.unpadded.AtomicQueueUtil.calcCircularRefElementOffset; | ||
| import static org.jctools.queues.atomic.unpadded.AtomicQueueUtil.lvRefElement; | ||
|
|
||
| abstract class AtomicReferenceArrayQueue<E> extends AbstractQueue<E> implements IndexedQueue, QueueProgressIndicators, MessagePassingQueue<E>, SupportsIterator |
There was a problem hiding this comment.
We can avoid the copy by making the original public and annotating as InternalAPI
|
|
||
| import java.util.concurrent.atomic.AtomicReference; | ||
|
|
||
| public final class LinkedQueueAtomicNode<E> extends AtomicReference<LinkedQueueAtomicNode<E>> |
There was a problem hiding this comment.
Can reuse original which is unpadded
|
Nice one, ok @nitsanw I will reuse as much as we can 👍 thanks for taking a look! |
|
@nitsanw PTAL |
| @@ -173,6 +173,57 @@ | |||
| </arguments> | |||
| </configuration> | |||
| </execution> | |||
| <execution> | |||
There was a problem hiding this comment.
What's happening with the indentation here?
This is replacing #388
Right now it modifies the atomic generators, which are doing already the heavy-lifting of replacing methods/primitive utils and classes to allow
Atomics to be used, but adding the 2 required behaviour:It looked like a natural thing to me, given that it would add unpadding to the existing atomic generation mechanism.
As promised on #388 (comment), I'm exploring how to modify the existing
JavaParsingUnpaddedQueueGeneratorto createAtomicvariants, but TBH it seems to add more changes (probably taking them - by decoration or copy - from the existing Atomic generators)