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

Make use of relaxed*() methods in PoolThreadCache.MemoryRegionCache when possible #13734

Draft
wants to merge 2 commits into
base: 4.1
Choose a base branch
from

Conversation

laosijikaichele
Copy link
Contributor

@laosijikaichele laosijikaichele commented Dec 15, 2023

Motivation:

This is inspired by #11960.
We should be able to make use of MessagePassingQueue.relaxedPoll() methods in PoolThreadCache.MemoryRegionCache to reduce the overhead.

Modification:

Use MessagePassingQueue.relaxedPoll(...).

Result:

Less overhead.

@@ -368,7 +369,7 @@ protected abstract void initBuf(PoolChunk<T> chunk, ByteBuffer nioBuffer, long h
@SuppressWarnings("unchecked")
public final boolean add(PoolChunk<T> chunk, ByteBuffer nioBuffer, long handle, int normCapacity) {
Entry<T> entry = newEntry(chunk, nioBuffer, handle, normCapacity);
boolean queued = queue.offer(entry);
boolean queued = queue.relaxedOffer(entry);
Copy link
Contributor

Choose a reason for hiding this comment

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

relaxed offer on mpsc doesn't make much difference https://github.com/JCTools/JCTools/blob/master/jctools-core/src/main/java/org/jctools/queues/MpscArrayQueue.java#L456

while relaxedPoll would save it to spin awaiting the elements (and breaking linearizability if the queue);

I don't suggest to go there unless there's some evidence of pathologic cases which can by fixing by this change...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

relaxed offer on mpsc doesn't make much difference

True for relaxedOffer.

About the relaxedPoll part, I may try some benchmark if possible.

Copy link
Member

Choose a reason for hiding this comment

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

@laosijikaichele any update ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@normanmaurer Sorry for the delay, been busy with other things recently, I may still need some time to back on this PR...

Copy link
Member

Choose a reason for hiding this comment

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

please ping me once you are ready

…che.MemoryRegionCache to reduce the overhead
@normanmaurer normanmaurer marked this pull request as draft April 20, 2024 19:15
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.

None yet

3 participants