diff --git a/src/main/java/net/openhft/chronicle/hash/impl/CompactOffHeapLinearHashTable.java b/src/main/java/net/openhft/chronicle/hash/impl/CompactOffHeapLinearHashTable.java index 267304918..8eb3a633e 100644 --- a/src/main/java/net/openhft/chronicle/hash/impl/CompactOffHeapLinearHashTable.java +++ b/src/main/java/net/openhft/chronicle/hash/impl/CompactOffHeapLinearHashTable.java @@ -133,8 +133,17 @@ public abstract class CompactOffHeapLinearHashTable { public abstract long stepBack(long pos); + /** + * Used when we assume we are holding at least update lock, and during recovery (because we + * don't care about concurrency during recovery) + */ public abstract long readEntry(long addr, long pos); + /** + * Used when we are under read lock and aware of possible concurrent insertions + */ + public abstract long readEntryVolatile(long addr, long pos); + public abstract void writeEntryVolatile(long addr, long pos, long key, long value); public abstract void writeEntry(long addr, long pos, long newEntry); @@ -148,6 +157,7 @@ public abstract class CompactOffHeapLinearHashTable { long posToShift = posToRemove; while (true) { posToShift = step(posToShift); + // volatile read not needed because removed is performed under exclusive lock long entryToShift = readEntry(addr, posToShift); if (empty(entryToShift)) break; diff --git a/src/main/java/net/openhft/chronicle/hash/impl/IntCompactOffHeapLinearHashTable.java b/src/main/java/net/openhft/chronicle/hash/impl/IntCompactOffHeapLinearHashTable.java index 16188a369..2c83cafa0 100644 --- a/src/main/java/net/openhft/chronicle/hash/impl/IntCompactOffHeapLinearHashTable.java +++ b/src/main/java/net/openhft/chronicle/hash/impl/IntCompactOffHeapLinearHashTable.java @@ -46,6 +46,11 @@ public final class IntCompactOffHeapLinearHashTable extends CompactOffHeapLinear return OS.memory().readInt(addr + pos); } + @Override + public long readEntryVolatile(long addr, long pos) { + return OS.memory().readVolatileInt(addr + pos); + } + @Override public void writeEntryVolatile(long addr, long pos, long key, long value) { OS.memory().writeVolatileInt(null, addr + pos, (int) entry(key, value)); diff --git a/src/main/java/net/openhft/chronicle/hash/impl/LongCompactOffHeapLinearHashTable.java b/src/main/java/net/openhft/chronicle/hash/impl/LongCompactOffHeapLinearHashTable.java index d580f49d5..f56b3ccf5 100644 --- a/src/main/java/net/openhft/chronicle/hash/impl/LongCompactOffHeapLinearHashTable.java +++ b/src/main/java/net/openhft/chronicle/hash/impl/LongCompactOffHeapLinearHashTable.java @@ -46,6 +46,11 @@ public final class LongCompactOffHeapLinearHashTable extends CompactOffHeapLinea return OS.memory().readLong(addr + pos); } + @Override + public long readEntryVolatile(long addr, long pos) { + return OS.memory().readVolatileLong(addr + pos); + } + @Override public void writeEntryVolatile(long addr, long pos, long key, long value) { OS.memory().writeVolatileLong(null, addr + pos, entry(key, value)); diff --git a/src/main/java/net/openhft/chronicle/hash/impl/stage/entry/HashLookupSearch.java b/src/main/java/net/openhft/chronicle/hash/impl/stage/entry/HashLookupSearch.java index 54081a0ee..8953f3f3e 100644 --- a/src/main/java/net/openhft/chronicle/hash/impl/stage/entry/HashLookupSearch.java +++ b/src/main/java/net/openhft/chronicle/hash/impl/stage/entry/HashLookupSearch.java @@ -54,7 +54,9 @@ public abstract class HashLookupSearch { public long nextPos() { long pos = hlp.hashLookupPos; while (true) { - long entry = hl().readEntry(addr(), pos); + // read volatile to make a happens-before edge between entry insertion from concurrent + // thread under update lock and this thread (reading the entry) + long entry = hl().readEntryVolatile(addr(), pos); if (hl().empty(entry)) { hlp.setHashLookupPos(pos); return -1L; @@ -96,6 +98,8 @@ public abstract class HashLookupSearch { } public boolean checkSlotContainsExpectedKeyAndValue(long value) { + // volatile read not needed here because this method is for verifying within-thread + // invariants long entry = hl().readEntry(addr(), hlp.hashLookupPos); return hl().key(entry) == searchKey && hl().value(entry) == value; } diff --git a/src/main/java/net/openhft/chronicle/hash/impl/stage/iter/HashSegmentIteration.java b/src/main/java/net/openhft/chronicle/hash/impl/stage/iter/HashSegmentIteration.java index f5b73ab79..edfbc231c 100644 --- a/src/main/java/net/openhft/chronicle/hash/impl/stage/iter/HashSegmentIteration.java +++ b/src/main/java/net/openhft/chronicle/hash/impl/stage/iter/HashSegmentIteration.java @@ -105,6 +105,7 @@ public abstract class HashSegmentIteration> boolean interrupted = false; long startPos = 0L; CompactOffHeapLinearHashTable hashLookup = hh.h().hashLookup; + // volatile read not needed because iteration is performed at least under update lock while (!hashLookup.empty(hashLookup.readEntry(currentTierBaseAddr, startPos))) { startPos = hashLookup.step(startPos); } @@ -119,6 +120,7 @@ public abstract class HashSegmentIteration> currentHashLookupPos = hashLookup.step(hlp.hashLookupPos); steps++; hlp.setHashLookupPos(currentHashLookupPos); + // volatile read not needed because iteration is performed at least under update lock long entry = hashLookup.readEntry(currentTierBaseAddr, currentHashLookupPos); initHashLookupEntry(entry); if (!hashLookup.empty(entry)) { diff --git a/src/main/java/net/openhft/chronicle/map/impl/stage/entry/MapEntryStages.java b/src/main/java/net/openhft/chronicle/map/impl/stage/entry/MapEntryStages.java index 6b862dee2..bd44de4a6 100644 --- a/src/main/java/net/openhft/chronicle/map/impl/stage/entry/MapEntryStages.java +++ b/src/main/java/net/openhft/chronicle/map/impl/stage/entry/MapEntryStages.java @@ -205,9 +205,10 @@ public abstract class MapEntryStages extends HashEntryStages freeExtraAllocatedChunks(); CompactOffHeapLinearHashTable hl = hh.h().hashLookup; - long oldEntry = hl.readEntry(oldHashLookupAddr, oldHashLookupPos); + long hashLookupKey = hl.key(hl.readEntry(oldHashLookupAddr, oldHashLookupPos)); hl.checkValueForPut(pos); - hl.writeEntryVolatile(s.tierBaseAddr, hlp.hashLookupPos, hl.key(oldEntry), pos); + // volatile not needed, because we are in write lock context + hl.writeEntry(s.tierBaseAddr, hlp.hashLookupPos, hl.entry(hashLookupKey, pos)); if (tierHasChanged) hl.remove(oldHashLookupAddr, oldHashLookupPos); }