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

Lock-free BlockingIdentifierGenerator LoHi #1610

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import java.util.Objects;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.CompletionStage;
import java.util.concurrent.atomic.AtomicIntegerFieldUpdater;

import static org.hibernate.reactive.util.impl.CompletionStages.completedFuture;

Expand Down Expand Up @@ -56,22 +57,52 @@ public abstract class BlockingIdentifierGenerator implements ReactiveIdentifierG
//to reason about what the current state is and what the CombinerExecutor is
//supposed to work on.
private static class GeneratorState {
private int loValue;
private long hiValue;

private static final class LoHi {

private static final AtomicIntegerFieldUpdater<LoHi> LO_UPDATER = AtomicIntegerFieldUpdater.newUpdater(LoHi.class, "lo");
private final long hi;
private volatile long lo;
Copy link
Member

Choose a reason for hiding this comment

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

should this be an int ? Especially as you're using a AtomicIntegerFieldUpdater ?

Copy link
Author

Choose a reason for hiding this comment

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

It's for 2 reasons: it won't change based on JOL and will make life easier due to the "blind" getAndIncrement (which scale much buffer then a compare and set loop).


LoHi(long hi) {
this.hi = hi;
this.lo = 1;
}

public long next(int blockSize) {
final long nextLo = LO_UPDATER.getAndIncrement(this);
if (nextLo < blockSize) {
return hi + nextLo;
}
return -1;
}
}

private volatile LoHi loHi;

public long hi(long hi) {
loHi = new LoHi(hi);
Copy link
Author

Choose a reason for hiding this comment

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

@Sanne
This could be made much more strict and based on the previous value while failing if it happens concurrently too (by using a compareAndSet)

return hi;
}

public long next(int blockSize) {
final LoHi loHi = this.loHi;
if (loHi == null) {
return -1;
}
return loHi.next(blockSize);
}
}

//Critical section: needs to be accessed exclusively via the CombinerExecutor
//when there's contention; direct invocation is allowed in the fast path.
private synchronized long next() {
return state.loValue > 0 && state.loValue < getBlockSize()
? state.hiValue + state.loValue++
: -1; //flag value indicating that we need to hit db
private long next() {
return state.next(getBlockSize());
}

//Critical section: needs to be accessed exclusively via the CombinerExecutor
private synchronized long next(long hi) {
state.hiValue = hi;
state.loValue = 1;
private long next(long hi) {
state.hi(hi);
return hi;
}

Expand Down