From 5558f4eec6a1f24e9881f5b8c84e14db42c1b867 Mon Sep 17 00:00:00 2001 From: leventov Date: Sat, 3 Jan 2015 03:18:52 +0700 Subject: [PATCH] Fixed several bugs in contexts API implementation and in VanillaChronicleMap.putValue() --- .../chronicle/map/Int24Int24MultiMap.java | 13 +- .../openhft/chronicle/map/IntIntMultiMap.java | 13 +- .../net/openhft/chronicle/map/MultiMap.java | 2 +- .../chronicle/map/ReplicatedChronicleMap.java | 18 +- .../chronicle/map/ShortShortMultiMap.java | 13 +- .../chronicle/map/VanillaChronicleMap.java | 188 ++++++++++++------ 6 files changed, 153 insertions(+), 94 deletions(-) diff --git a/src/main/java/net/openhft/chronicle/map/Int24Int24MultiMap.java b/src/main/java/net/openhft/chronicle/map/Int24Int24MultiMap.java index b1a35ff06..214912be2 100644 --- a/src/main/java/net/openhft/chronicle/map/Int24Int24MultiMap.java +++ b/src/main/java/net/openhft/chronicle/map/Int24Int24MultiMap.java @@ -238,15 +238,18 @@ class Int24Int24MultiMap implements MultiMap { } @Override - public void replacePrevPos(SearchState searchState, long newValue) { + public void replacePrevPos(SearchState searchState, long newValue, + boolean oldValueInPositions) { checkValueForPut(newValue); long prevPos = searchState.searchPos; if (!searchState.putAfterFailedSearch) prevPos = stepBack(prevPos); - long oldEntry = bytes.readInt48(prevPos); - long oldValue = value(oldEntry); - checkValueForRemove(oldValue); - positions.clear(oldValue); + if (oldValueInPositions) { + long oldEntry = bytes.readInt48(prevPos); + long oldValue = value(oldEntry); + checkValueForRemove(oldValue); + positions.clear(oldValue); + } positions.set(newValue); bytes.writeInt48(prevPos, entry(searchState.searchHash, newValue)); } diff --git a/src/main/java/net/openhft/chronicle/map/IntIntMultiMap.java b/src/main/java/net/openhft/chronicle/map/IntIntMultiMap.java index 44761766c..5b0ee3b05 100644 --- a/src/main/java/net/openhft/chronicle/map/IntIntMultiMap.java +++ b/src/main/java/net/openhft/chronicle/map/IntIntMultiMap.java @@ -239,15 +239,18 @@ class IntIntMultiMap implements MultiMap { } @Override - public void replacePrevPos(SearchState searchState, long newValue) { + public void replacePrevPos(SearchState searchState, long newValue, + boolean oldValueInPositions) { checkValueForPut(newValue); long prevPos = searchState.searchPos; if (!searchState.putAfterFailedSearch) prevPos = stepBack(prevPos); - long oldEntry = bytes.readLong(prevPos); - long oldValue = value(oldEntry); - checkValueForRemove(oldValue); - positions.clear(oldValue); + if (oldValueInPositions) { + long oldEntry = bytes.readLong(prevPos); + long oldValue = value(oldEntry); + checkValueForRemove(oldValue); + positions.clear(oldValue); + } positions.set(newValue); bytes.writeLong(prevPos, entry(searchState.searchHash, newValue)); } diff --git a/src/main/java/net/openhft/chronicle/map/MultiMap.java b/src/main/java/net/openhft/chronicle/map/MultiMap.java index 6f335e549..fd09274b1 100644 --- a/src/main/java/net/openhft/chronicle/map/MultiMap.java +++ b/src/main/java/net/openhft/chronicle/map/MultiMap.java @@ -65,7 +65,7 @@ interface MultiMap { void removePrevPos(SearchState searchState); - void replacePrevPos(SearchState searchState, long newValue); + void replacePrevPos(SearchState searchState, long newValue, boolean oldValueInPositions); void putAfterFailedSearch(SearchState searchState, long value); diff --git a/src/main/java/net/openhft/chronicle/map/ReplicatedChronicleMap.java b/src/main/java/net/openhft/chronicle/map/ReplicatedChronicleMap.java index 25cfaccaa..b68533ca9 100644 --- a/src/main/java/net/openhft/chronicle/map/ReplicatedChronicleMap.java +++ b/src/main/java/net/openhft/chronicle/map/ReplicatedChronicleMap.java @@ -749,7 +749,8 @@ final class ReplicatedChronicleMap, checkReallyUsingValue(usingValue, mustReuseValue, v); - lock.copies = copies; - lock.metaKeyInterop = metaKeyInterop; - lock.keyInterop = keyInterop; - lock.keySize = keySize; - lock.segmentHash = segmentHash; + lock.initKey(copies, metaKeyInterop, keyInterop, keySize, segmentHash, key); if (!nativeValueClass && lockType == LockType.WRITE_LOCK && v == null) v = usingValue; - lock.value(v); - lock.key(key); return (T) lock; } @@ -1373,24 +1367,21 @@ class VanillaChronicleMap, , V, VI, MVI extends MetaBytesInterop> ReadLocked readLocked(VanillaChronicleMap.Segment segment) { - readLocked.segment = segment; - readLocked.map = segment.map(); + readLocked.initSegment(segment); return readLocked; } , V, VI, MVI extends MetaBytesInterop> WriteLocked nativeWriteLocked(VanillaChronicleMap.Segment segment) { - nativeWriteLocked.segment = segment; - nativeWriteLocked.map = segment.map(); + nativeWriteLocked.initSegment(segment); return nativeWriteLocked; } , V, VI, MVI extends MetaBytesInterop> WriteLocked heapWriteLocked(VanillaChronicleMap.Segment segment) { - heapWriteLocked.segment = segment; - heapWriteLocked.map = segment.map(); + heapWriteLocked.initSegment(segment); return heapWriteLocked; } } @@ -1720,12 +1711,14 @@ class VanillaChronicleMap, RV result = createEntryOnAcquire(copies, segmentState, metaKeyInterop, keyInterop, key, keySize, toKey, readValue, usingValue, toValue, entry); + entryCreated(lock); + return result; + } + final void entryCreated(MutableLockedEntry lock) { // notify the context that the entry was created if (lock instanceof WriteLocked) ((WriteLocked) lock).created(true); - - return result; } final , RV> @@ -1911,7 +1904,7 @@ class VanillaChronicleMap, } // putValue may relocate entry and change offset - putValue(pos, offset, entry, valueSizePos, entryEndAddr, segmentState, + putValue(pos, offset, entry, valueSizePos, entryEndAddr, entryIsDeleted, segmentState, metaValueInterop, valueInterop, value, valueSize, searchedHashLookup, sizeOfEverythingBeforeValue); @@ -2288,7 +2281,8 @@ class VanillaChronicleMap, MVBI metaValueInterop = getNewValueInterops.getMetaValueInterop( copies, valueInterop, newValue); long newValueSize = metaValueInterop.size(valueInterop, newValue); - putValue(pos, offset, entry, valueSizePos, entryEndAddr, segmentState, + boolean entryIsDeleted = false; // couldn't replace deleted entry + putValue(pos, offset, entry, valueSizePos, entryEndAddr, entryIsDeleted, segmentState, metaValueInterop, valueInterop, newValue, newValueSize, searchedHashLookup, sizeOfEverythingBeforeValue); @@ -2321,6 +2315,7 @@ class VanillaChronicleMap, */ final long putValue( long pos, long offset, MultiStoreBytes entry, long valueSizePos, long entryEndAddr, + boolean entryIsDeleted, SegmentState segmentState, MetaBytesWriter metaElemWriter, EW elemWriter, E newElem, long newElemSize, @@ -2351,7 +2346,8 @@ class VanillaChronicleMap, long newPos = alloc(allocatedChunks); // putValue() is called from put() and replace() // after successful search by key - searchedHashLookup.replacePrevPos(segmentState.searchState, newPos); + searchedHashLookup.replacePrevPos(segmentState.searchState, newPos, + !entryIsDeleted); long newOffset = offsetFromPos(newPos); reuse(entry, newOffset); // Moving metadata, key size and key. @@ -2416,10 +2412,6 @@ class VanillaChronicleMap, return new WriteThroughEntry(key, value); } - boolean isDeleted(long pos) { - return false; - } - /** * Check there is no garbage in freeList. */ @@ -2669,13 +2661,13 @@ class VanillaChronicleMap, V, VI, MVI extends MetaBytesInterop> implements Context { final SegmentState segmentState; - VanillaChronicleMap map; - VanillaChronicleMap.Segment segment; - ThreadLocalCopies copies; - MKI metaKeyInterop; - KI keyInterop; - long keySize; - long segmentHash; + private VanillaChronicleMap map; + private VanillaChronicleMap.Segment segment; + private ThreadLocalCopies copies; + private MKI metaKeyInterop; + private KI keyInterop; + private long keySize; + private long segmentHash; private K key; private V value; @@ -2684,22 +2676,72 @@ class VanillaChronicleMap, this.segmentState = segmentState; } - public final K key() { - return key; + final void initSegment(VanillaChronicleMap.Segment segment) { + this.map = segment.map(); + this.segment = segment; + init(); } - final void key(K key) { + final void initKey(ThreadLocalCopies copies, MKI metaKeyInterop, KI keyInterop, + long keySize, long segmentHash, K key) { + this.copies = copies; + this.metaKeyInterop = metaKeyInterop; + this.keyInterop = keyInterop; + this.keySize = keySize; + this.segmentHash = segmentHash; this.key = key; + init(); + } + + void init() { + // nothing + } + + final VanillaChronicleMap map() { + return map; + } + + final VanillaChronicleMap.Segment segment() { + return segment; + } + + final ThreadLocalCopies copies() { + return copies; + } + + final MKI metaKeyInterop() { + return metaKeyInterop; + } + + final KI keyInterop() { + return keyInterop; + } + + final long keySize() { + return keySize; + } + + final long segmentHash() { + return segmentHash; + } + + public final K key() { + return key; } public final V value() { return value; } - void value(V value) { + final void value(V value) { this.value = value; } + @Override + public void close() { + segmentState.close(); + segment().readUnlock(); + } } private static class ReadLocked, @@ -2707,17 +2749,10 @@ class VanillaChronicleMap, extends MutableLockedEntry implements ReadContext { - ReadLocked(SegmentState segmentState) { super(segmentState); } - @Override - public void close() { - segmentState.close(); - segment.readUnlock(); - } - /** * @return if the value is not null */ @@ -2731,12 +2766,20 @@ class VanillaChronicleMap, extends MutableLockedEntry implements WriteContext { - private boolean created = false; + private boolean created; + boolean removed; WriteLocked(SegmentState segmentState) { super(segmentState); } + @Override + void init() { + super.init(); + created = false; + removed = false; + } + /** * @return if the entry was created */ @@ -2757,32 +2800,36 @@ class VanillaChronicleMap, super(segmentState); } - @Override public void close() { - // TODO optimize -- keep replication bytes offset in SegmentState to jump directly - long pos = segmentState.pos; - long offset = segment.offsetFromPos(pos); - MultiStoreBytes entry = segment.reuse(segmentState.tmpBytes, offset); - long keySize = map.keySizeMarshaller.readSize(entry); - entry.skip(keySize); - segment.manageReplicationBytes(entry, true, false); - map.onPut(segment, pos); - segmentState.close(); - segment.writeUnlock(); + if (!removed) { + // TODO optimize -- keep replication bytes offset in SegmentState to jump directly + long pos = segmentState.pos; + VanillaChronicleMap.Segment segment = segment(); + VanillaChronicleMap map = map(); + long offset = segment.offsetFromPos(pos); + MultiStoreBytes entry = segment.reuse(segmentState.tmpBytes, offset); + long keySize = map.keySizeMarshaller.readSize(entry); + entry.skip(keySize); + segment.manageReplicationBytes(entry, true, false); + map.onPut(segment, pos); + } + super.close(); } @Override public void dontPutOnClose() { throw new IllegalStateException( - "This method is not supported for native value classes"); + "dontPutOnClose() method is not supported for native value classes"); } @Override public void removeEntry() { - segment.removeWithoutLock(copies, segmentState, - metaKeyInterop, keyInterop, key(), keySize, map.keyIdentity(), - map, null, map.valueIdentity(), segmentHash, map, true); + removed = true; + VanillaChronicleMap map = map(); + segment().removeWithoutLock(copies(), segmentState, + metaKeyInterop(), keyInterop(), key(), keySize(), map.keyIdentity(), + map, null, map.valueIdentity(), segmentHash(), map, true); } } @@ -2790,18 +2837,28 @@ class VanillaChronicleMap, V, VI, MVI extends MetaBytesInterop> extends WriteLocked { - private boolean putOnClose = true; + private boolean putOnClose; HeapWriteLocked(SegmentState segmentState) { super(segmentState); } + @Override + void init() { + super.init(); + putOnClose = true; + } + @Override public void close() { + VanillaChronicleMap.Segment segment = segment(); if (putOnClose) { + assert !removed; // TODO optimize -- keep keySize, valueSizePos, entryEndAddr, etc. inside // segmentState long pos = segmentState.pos; + VanillaChronicleMap map = map(); + ThreadLocalCopies copies = copies(); long offset = segment.offsetFromPos(pos); MultiStoreBytes entry = segment.reuse(segmentState.tmpBytes, offset); long keySize = map.keySizeMarshaller.readSize(entry); @@ -2817,28 +2874,31 @@ class VanillaChronicleMap, MVI metaValueInterop = map.metaValueInteropProvider .get(copies, map.originalMetaValueInterop, valueInterop, value); long newValueSize = metaValueInterop.size(valueInterop, value); - segment.putValue(pos, offset, entry, valueSizePos, entryEndAddr, segmentState, + segment.putValue(pos, offset, entry, valueSizePos, entryEndAddr, removed, + segmentState, metaValueInterop, valueInterop, value, newValueSize, segment.hashLookup(), sizeOfEverythingBeforeValue); map.onPut(segment, segmentState.pos); } - putOnClose = true; - segmentState.close(); - segment.writeUnlock(); + super.close(); } @Override public void dontPutOnClose() { + if (removed) + throw new IllegalStateException("Shouldn't call this method after removeEntry()"); putOnClose = false; } @Override public void removeEntry() { putOnClose = false; - segment.removeWithoutLock(copies, segmentState, - metaKeyInterop, keyInterop, key(), keySize, map.keyIdentity(), - map, null, map.valueIdentity(), segmentHash, map, true); + removed = true; + VanillaChronicleMap map = map(); + segment().removeWithoutLock(copies(), segmentState, + metaKeyInterop(), keyInterop(), key(), keySize(), map.keyIdentity(), + map, null, map.valueIdentity(), segmentHash(), map, true); } }