From e4a171ff23b2ea9dfb42e80580589451fa3aafdf Mon Sep 17 00:00:00 2001 From: leventov Date: Tue, 30 Dec 2014 10:40:32 +0700 Subject: [PATCH] HCOLL-244 fixed potential problems with chunks over-allocatation in Vanilla/Replicated ChronicleMaps --- .../chronicle/map/ReplicatedChronicleMap.java | 24 ++++++--- .../chronicle/map/VanillaChronicleMap.java | 54 +++++++++++++++---- 2 files changed, 60 insertions(+), 18 deletions(-) diff --git a/src/main/java/net/openhft/chronicle/map/ReplicatedChronicleMap.java b/src/main/java/net/openhft/chronicle/map/ReplicatedChronicleMap.java index c5faed2ba..da945b2b2 100644 --- a/src/main/java/net/openhft/chronicle/map/ReplicatedChronicleMap.java +++ b/src/main/java/net/openhft/chronicle/map/ReplicatedChronicleMap.java @@ -724,7 +724,9 @@ final class ReplicatedChronicleMap, final long entrySize(long keySize, long valueSize) { long sizeOfEverythingBeforeValue = sizeOfEverythingBeforeValue(keySize, valueSize); + return innerEntrySize(sizeOfEverythingBeforeValue, valueSize); + } + + private long innerEntrySize(long sizeOfEverythingBeforeValue, long valueSize) { if (constantlySizedEntry) { return alignment.alignAddr(sizeOfEverythingBeforeValue + valueSize); } else if (couldNotDetermineAlignmentBeforeAllocation) { @@ -1894,7 +1897,9 @@ class VanillaChronicleMap, long valueSize = metaValueInterop.size(valueInterop, value); long valueSizePos = entry.position(); - long prevValueSize = readValueSize(entry); + long prevValueSize = valueSizeMarshaller.readSize(entry); + long sizeOfEverythingBeforeValue = entry.position(); + alignment.alignPositionAddr(entry); long valueAddr = entry.positionAddr(); long entryEndAddr = valueAddr + prevValueSize; @@ -1910,7 +1915,8 @@ class VanillaChronicleMap, // putValue may relocate entry and change offset putValue(pos, offset, entry, valueSizePos, entryEndAddr, segmentState, - metaValueInterop, valueInterop, value, valueSize, searchedHashLookup); + metaValueInterop, valueInterop, value, valueSize, searchedHashLookup, + sizeOfEverythingBeforeValue); // put callbacks onPutMaybeRemote(segmentState.pos, remote); @@ -1944,7 +1950,8 @@ class VanillaChronicleMap, MultiStoreBytes entry, boolean writeDefaultInitialReplicationValues) { long valueSize = metaElemWriter.size(elemWriter, elem); long entrySize = entrySize(keySize, valueSize); - long pos = alloc(inChunks(entrySize)); + int allocatedChunks = inChunks(entrySize); + long pos = alloc(allocatedChunks); segmentState.pos = pos; long offset = offsetFromPos(pos); clearMetaData(offset); @@ -1960,12 +1967,22 @@ class VanillaChronicleMap, alignment.alignPositionAddr(entry); metaElemWriter.write(elemWriter, entry, elem); + freeExtraAllocatedChunks(pos, allocatedChunks, entry); + hashLookup.putAfterFailedSearch(segmentState.searchState, pos); incrementSize(); return valueSize; } + final void freeExtraAllocatedChunks(long pos, int allocatedChunks, Bytes entry) { + int actuallyUsedChunks; + if (!constantlySizedEntry && couldNotDetermineAlignmentBeforeAllocation && // fast path + (actuallyUsedChunks = inChunks(entry.position())) < allocatedChunks) { + free(pos + actuallyUsedChunks, allocatedChunks - actuallyUsedChunks); + } + } + void manageReplicationBytes(Bytes entry, boolean writeDefaultInitialReplicationValues, boolean remove) { // do nothing @@ -2249,7 +2266,9 @@ class VanillaChronicleMap, ReadValue readValue, InstanceOrBytesToInstance toValue, long pos, long offset, MultiStoreBytes entry, MultiMap searchedHashLookup) { long valueSizePos = entry.position(); - long valueSize = readValueSize(entry); + long valueSize = valueSizeMarshaller.readSize(entry); + long sizeOfEverythingBeforeValue = entry.position(); + alignment.alignPositionAddr(entry); long entryEndAddr = entry.positionAddr() + valueSize; RV prevValue; @@ -2273,7 +2292,8 @@ class VanillaChronicleMap, copies, valueInterop, newValue); long newValueSize = metaValueInterop.size(valueInterop, newValue); putValue(pos, offset, entry, valueSizePos, entryEndAddr, segmentState, - metaValueInterop, valueInterop, newValue, newValueSize, searchedHashLookup); + metaValueInterop, valueInterop, newValue, newValueSize, searchedHashLookup, + sizeOfEverythingBeforeValue); // put callbacks onPut(this, segmentState.pos); @@ -2307,7 +2327,7 @@ class VanillaChronicleMap, SegmentState segmentState, MetaBytesWriter metaElemWriter, EW elemWriter, E newElem, long newElemSize, - MultiMap searchedHashLookup) { + MultiMap searchedHashLookup, long sizeOfEverythingBeforeValue) { long entryStartAddr = entry.address(); long valueSizeAddr = entryStartAddr + valueSizePos; long newValueAddr = alignment.alignAddr( @@ -2329,7 +2349,9 @@ class VanillaChronicleMap, // RELOCATION free(pos, oldSizeInChunks); onRelocation(this, pos); - pos = alloc(newSizeInChunks); + int allocatedChunks = + inChunks(innerEntrySize(sizeOfEverythingBeforeValue, newElemSize)); + pos = alloc(allocatedChunks); // putValue() is called from put() and replace() // after successful search by key searchedHashLookup.replacePrevPos(segmentState.searchState, pos); @@ -2341,6 +2363,15 @@ class VanillaChronicleMap, NativeBytes.UNSAFE.copyMemory(entryStartAddr, newEntryStartAddr, valueSizeAddr - entryStartAddr); entry = reuse(entry, offset); + entry.position(valueSizePos); + valueSizeMarshaller.writeSize(entry, newElemSize); + alignment.alignPositionAddr(entry); + metaElemWriter.write(elemWriter, entry, newElem); + + freeExtraAllocatedChunks(pos, allocatedChunks, entry); + + segmentState.pos = pos; + return offset; // END OF RELOCATION } else if (newSizeInChunks < oldSizeInChunks) { // Freeing extra chunks @@ -2780,7 +2811,9 @@ class VanillaChronicleMap, entry.skip(keySize); segment.manageReplicationBytes(entry, true, false); long valueSizePos = entry.position(); - long valueSize = map.readValueSize(entry); + long valueSize = map.valueSizeMarshaller.readSize(entry); + long sizeOfEverythingBeforeValue = entry.position(); + map.alignment.alignPositionAddr(entry); long entryEndAddr = entry.positionAddr() + valueSize; VI valueInterop = map.valueInteropProvider.get(copies, map.originalValueInterop); V value = value(); @@ -2788,7 +2821,8 @@ class VanillaChronicleMap, .get(copies, map.originalMetaValueInterop, valueInterop, value); long newValueSize = metaValueInterop.size(valueInterop, value); segment.putValue(pos, offset, entry, valueSizePos, entryEndAddr, segmentState, - metaValueInterop, valueInterop, value, newValueSize, segment.hashLookup()); + metaValueInterop, valueInterop, value, newValueSize, segment.hashLookup(), + sizeOfEverythingBeforeValue); map.onPut(segment, segmentState.pos); } putOnClose = true;