diff --git a/src/main/java/net/openhft/chronicle/hash/impl/stage/data/bytes/EntryKeyBytesData.java b/src/main/java/net/openhft/chronicle/hash/impl/stage/data/bytes/EntryKeyBytesData.java index 83298b98d..d26323b57 100644 --- a/src/main/java/net/openhft/chronicle/hash/impl/stage/data/bytes/EntryKeyBytesData.java +++ b/src/main/java/net/openhft/chronicle/hash/impl/stage/data/bytes/EntryKeyBytesData.java @@ -17,20 +17,25 @@ package net.openhft.chronicle.hash.impl.stage.data.bytes; +import net.openhft.chronicle.algo.hashing.LongHashFunction; import net.openhft.chronicle.bytes.Bytes; import net.openhft.chronicle.bytes.RandomDataInput; import net.openhft.chronicle.hash.AbstractData; +import net.openhft.chronicle.hash.impl.VanillaChronicleHashHolder; import net.openhft.chronicle.hash.impl.stage.entry.HashEntryStages; import net.openhft.chronicle.hash.impl.stage.entry.SegmentStages; import net.openhft.chronicle.hash.impl.stage.hash.CheckOnEachPublicOperation; import net.openhft.chronicle.hash.impl.stage.hash.KeyBytesInterop; +import net.openhft.chronicle.hash.serialization.impl.IntegerDataAccess; +import net.openhft.chronicle.hash.serialization.impl.WrongXxHash; import net.openhft.sg.Stage; import net.openhft.sg.StageRef; import net.openhft.sg.Staged; @Staged public class EntryKeyBytesData extends AbstractData { - + + @StageRef VanillaChronicleHashHolder hh; @StageRef KeyBytesInterop ki; @StageRef SegmentStages s; @StageRef HashEntryStages entry; @@ -62,6 +67,17 @@ public class EntryKeyBytesData extends AbstractData { return entry.keySize; } + @Override + public long hash(LongHashFunction f) { + checkOnEachPublicOperation.checkOnEachPublicOperation(); + if (f == LongHashFunction.xx_r39() && entry.keySize == 4 && + hh.h().keyDataAccess instanceof IntegerDataAccess) { + return WrongXxHash.hashInt(s.segmentBS.readInt(entry.keyOffset)); + } else { + return super.hash(f); + } + } + @Override public K get() { checkOnEachPublicOperation.checkOnEachPublicOperation(); diff --git a/src/main/java/net/openhft/chronicle/hash/impl/stage/iter/IterationKeyHashCode.java b/src/main/java/net/openhft/chronicle/hash/impl/stage/iter/IterationKeyHashCode.java index cb57c42b9..4a9f2f200 100644 --- a/src/main/java/net/openhft/chronicle/hash/impl/stage/iter/IterationKeyHashCode.java +++ b/src/main/java/net/openhft/chronicle/hash/impl/stage/iter/IterationKeyHashCode.java @@ -18,15 +18,20 @@ package net.openhft.chronicle.hash.impl.stage.iter; import net.openhft.chronicle.algo.hashing.LongHashFunction; +import net.openhft.chronicle.core.OS; +import net.openhft.chronicle.hash.impl.VanillaChronicleHashHolder; import net.openhft.chronicle.hash.impl.stage.entry.HashEntryStages; import net.openhft.chronicle.hash.impl.stage.entry.KeyHashCode; import net.openhft.chronicle.hash.impl.stage.entry.SegmentStages; +import net.openhft.chronicle.hash.serialization.impl.IntegerDataAccess; +import net.openhft.chronicle.hash.serialization.impl.WrongXxHash; import net.openhft.sg.StageRef; import net.openhft.sg.Staged; @Staged public class IterationKeyHashCode implements KeyHashCode { + @StageRef VanillaChronicleHashHolder hh; @StageRef SegmentStages s; @StageRef HashEntryStages e; @@ -35,7 +40,11 @@ public class IterationKeyHashCode implements KeyHashCode { void initKeyHash() { long addr = s.tierBaseAddr + e.keyOffset; long len = e.keySize; - keyHash = LongHashFunction.xx_r39().hashMemory(addr, len); + if (len == 4 && hh.h().keyDataAccess instanceof IntegerDataAccess) { + keyHash = WrongXxHash.hashInt(OS.memory().readInt(addr)); + } else { + keyHash = LongHashFunction.xx_r39().hashMemory(addr, len); + } } @Override diff --git a/src/main/java/net/openhft/chronicle/hash/impl/stage/iter/TierRecovery.java b/src/main/java/net/openhft/chronicle/hash/impl/stage/iter/TierRecovery.java index cd87edc40..ce8ea31ea 100644 --- a/src/main/java/net/openhft/chronicle/hash/impl/stage/iter/TierRecovery.java +++ b/src/main/java/net/openhft/chronicle/hash/impl/stage/iter/TierRecovery.java @@ -181,11 +181,13 @@ public class TierRecovery { long entry = hashLookup.readEntry(currentTierBaseAddr, hlPos); if (!hashLookup.empty(entry)) { e.readExistingEntry(hashLookup.value(entry)); - Data key = (Data) e.key(); + Data key = e.key(); try (ExternalMapQueryContext c = m.queryContext(key)) { MapEntry entry2 = c.entry(); Data key2 = ((MapEntry) c).key(); - if (key2.bytes().address(key2.offset()) != key.bytes().address(key.offset())) { + long keyAddress = key.bytes().address(key.offset()); + long key2Address = key2.bytes().address(key2.offset()); + if (key2Address != keyAddress) { lh.LOG.error("entries with duplicate key {} in segment {}: " + "with values {} and {}, removing the latter", key, c.segmentIndex(), diff --git a/src/main/java/net/openhft/chronicle/hash/serialization/impl/IntegerDataAccess.java b/src/main/java/net/openhft/chronicle/hash/serialization/impl/IntegerDataAccess.java index 05c079ab8..63bc4b4a9 100644 --- a/src/main/java/net/openhft/chronicle/hash/serialization/impl/IntegerDataAccess.java +++ b/src/main/java/net/openhft/chronicle/hash/serialization/impl/IntegerDataAccess.java @@ -29,6 +29,14 @@ import net.openhft.chronicle.wire.WireOut; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; +/** + * @deprecated use {@link IntegerDataAccess_3_13} instead. This class is not removed only because + * users who created a Chronicle Map with older version of the library with this class as the key or + * value data access should be able to read and access the map with the present version of the + * Chronicle Map library. + */ +@SuppressWarnings("DeprecatedIsStillUsed") +@Deprecated public final class IntegerDataAccess extends AbstractData implements DataAccess, Data { @@ -78,7 +86,11 @@ public final class IntegerDataAccess extends AbstractData @Override public long hash(LongHashFunction f) { - return f.hashInt(instance); + if (f == LongHashFunction.xx_r39()) { + return WrongXxHash.hashInt(instance); + } else { + return f.hashInt(instance); + } } @Override @@ -115,6 +127,7 @@ public final class IntegerDataAccess extends AbstractData } @Override + @SuppressWarnings("deprecation") public DataAccess copy() { return new IntegerDataAccess(); } diff --git a/src/main/java/net/openhft/chronicle/hash/serialization/impl/IntegerDataAccess_3_13.java b/src/main/java/net/openhft/chronicle/hash/serialization/impl/IntegerDataAccess_3_13.java new file mode 100644 index 000000000..bacede19e --- /dev/null +++ b/src/main/java/net/openhft/chronicle/hash/serialization/impl/IntegerDataAccess_3_13.java @@ -0,0 +1,120 @@ +/* + * Copyright (C) 2016 Roman Leventov + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Lesser General Public License as published by + * the Free Software Foundation, either version 3 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program. If not, see . + */ + +package net.openhft.chronicle.hash.serialization.impl; + +import net.openhft.chronicle.algo.hashing.LongHashFunction; +import net.openhft.chronicle.bytes.BytesStore; +import net.openhft.chronicle.bytes.RandomDataInput; +import net.openhft.chronicle.bytes.RandomDataOutput; +import net.openhft.chronicle.hash.AbstractData; +import net.openhft.chronicle.hash.Data; +import net.openhft.chronicle.hash.serialization.DataAccess; +import net.openhft.chronicle.wire.WireIn; +import net.openhft.chronicle.wire.WireOut; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +public final class IntegerDataAccess_3_13 extends AbstractData + implements DataAccess, Data { + + // Cache fields + private transient boolean bsInit; + private transient BytesStore bs; + + /** State field */ + private transient Integer instance; + + public IntegerDataAccess_3_13() { + initTransients(); + } + + private void initTransients() { + bs = BytesStore.wrap(new byte[4]); + } + + @Override + public RandomDataInput bytes() { + if (!bsInit) { + bs.writeInt(0, instance); + bsInit = true; + } + return bs; + } + + @Override + public long offset() { + return 0; + } + + @Override + public long size() { + return 4; + } + + @Override + public Integer get() { + return instance; + } + + @Override + public Integer getUsing(@Nullable Integer using) { + return instance; + } + + @Override + public long hash(LongHashFunction f) { + return f.hashInt(instance); + } + + @Override + public boolean equivalent(RandomDataInput source, long sourceOffset) { + return source.readInt(sourceOffset) == instance; + } + + @Override + public void writeTo(RandomDataOutput target, long targetOffset) { + target.writeInt(targetOffset, instance); + } + + @Override + public Data getData(@NotNull Integer instance) { + this.instance = instance; + bsInit = false; + return this; + } + + @Override + public void uninit() { + instance = null; + } + + @Override + public void readMarshallable(@NotNull WireIn wireIn) { + // no config fields to read + initTransients(); + } + + @Override + public void writeMarshallable(@NotNull WireOut wireOut) { + // no config fields to write + } + + @Override + public DataAccess copy() { + return new IntegerDataAccess_3_13(); + } +} diff --git a/src/main/java/net/openhft/chronicle/hash/serialization/impl/IntegerMarshaller.java b/src/main/java/net/openhft/chronicle/hash/serialization/impl/IntegerMarshaller.java index 1b701f601..282720d97 100644 --- a/src/main/java/net/openhft/chronicle/hash/serialization/impl/IntegerMarshaller.java +++ b/src/main/java/net/openhft/chronicle/hash/serialization/impl/IntegerMarshaller.java @@ -24,7 +24,7 @@ import org.jetbrains.annotations.Nullable; /** * As a writer for top-level Chronicle Map's key or value type (Integer), {@code IntegerMarshaller} - * is deprecated in favor of {@link IntegerDataAccess}. As reader and element writer for {@link + * is deprecated in favor of {@link IntegerDataAccess_3_13}. As reader and element writer for {@link * ListMarshaller} and similar composite marshallers, {@code IntegerMarshaller} is not deprecated. */ public final class IntegerMarshaller diff --git a/src/main/java/net/openhft/chronicle/hash/serialization/impl/SerializationBuilder.java b/src/main/java/net/openhft/chronicle/hash/serialization/impl/SerializationBuilder.java index 46b7e1d3b..09a5a53c9 100644 --- a/src/main/java/net/openhft/chronicle/hash/serialization/impl/SerializationBuilder.java +++ b/src/main/java/net/openhft/chronicle/hash/serialization/impl/SerializationBuilder.java @@ -122,7 +122,7 @@ public final class SerializationBuilder implements Cloneable { sizeMarshaller(constant(8)); } else if (tClass == Integer.class) { reader((SizedReader) IntegerMarshaller.INSTANCE); - dataAccess((DataAccess) new IntegerDataAccess()); + dataAccess((DataAccess) new IntegerDataAccess_3_13()); sizeMarshaller(constant(4)); } else if (tClass == byte[].class) { reader((SizedReader) ByteArraySizedReader.INSTANCE); diff --git a/src/main/java/net/openhft/chronicle/hash/serialization/impl/WrongXxHash.java b/src/main/java/net/openhft/chronicle/hash/serialization/impl/WrongXxHash.java new file mode 100644 index 000000000..879844cb0 --- /dev/null +++ b/src/main/java/net/openhft/chronicle/hash/serialization/impl/WrongXxHash.java @@ -0,0 +1,51 @@ +/* + * Copyright (C) 2016 Roman Leventov + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Lesser General Public License as published by + * the Free Software Foundation, either version 3 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program. If not, see . + */ + +package net.openhft.chronicle.hash.serialization.impl; + +/** + * @deprecated this class exists for compatibility with old versions of Chronicle Map, which used + * wrong version of xxHash. + */ +@Deprecated +public final class WrongXxHash { + + // xxHash constants + private static final long P1 = -7046029288634856825L; + private static final long P2 = -4417276706812531889L; + private static final long P3 = 1609587929392839161L; + private static final long P5 = 2870177450012600261L; + + public static long hashInt(int input) { + long hash = P5 + 4; + // Preserve old, wrong behaviour. In the correct version, input is widened to long as + // unsigned here. + hash ^= input * P1; + hash = Long.rotateLeft(hash, 23) * P2 + P3; + return finalize(hash); + } + + private static long finalize(long hash) { + hash ^= hash >>> 33; + hash *= P2; + hash ^= hash >>> 29; + hash *= P3; + hash ^= hash >>> 32; + return hash; + } + + private WrongXxHash() {} +} diff --git a/src/test/java/net/openhft/chronicle/map/ChronicleMap3_12IntegerKeyCompatibilityTest.java b/src/test/java/net/openhft/chronicle/map/ChronicleMap3_12IntegerKeyCompatibilityTest.java new file mode 100644 index 000000000..8952790af --- /dev/null +++ b/src/test/java/net/openhft/chronicle/map/ChronicleMap3_12IntegerKeyCompatibilityTest.java @@ -0,0 +1,64 @@ +/* + * Copyright (C) 2016 Roman Leventov + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Lesser General Public License as published by + * the Free Software Foundation, either version 3 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program. If not, see . + */ + +package net.openhft.chronicle.map; + +import com.google.common.io.Files; +import net.openhft.chronicle.set.Builder; +import org.junit.Test; + +import java.io.File; +import java.net.URL; + +import static org.junit.Assert.assertEquals; + +public class ChronicleMap3_12IntegerKeyCompatibilityTest { + + @Test + public void testWithChecksums() throws Exception { + ClassLoader cl = Thread.currentThread().getContextClassLoader(); + URL fileUrl = cl.getResource("chronicle-map-3-12-with-checksums.dat"); + File file = new File(fileUrl.toURI()); + File persistenceFile = Builder.getPersistenceFile(); + Files.copy(file, persistenceFile); + try (ChronicleMap map = ChronicleMap.of(Integer.class, String.class) + .averageValue("1") + .entries(1) + .recoverPersistedTo(persistenceFile, false)) { + assertEquals(2, map.size()); + assertEquals("1", map.get(1)); + assertEquals("-1", map.get(-1)); + } + } + + @Test + public void testNoChecksums() throws Exception { + ClassLoader cl = Thread.currentThread().getContextClassLoader(); + URL fileUrl = cl.getResource("chronicle-map-3-12-no-checksums.dat"); + File file = new File(fileUrl.toURI()); + File persistenceFile = Builder.getPersistenceFile(); + Files.copy(file, persistenceFile); + try (ChronicleMap map = ChronicleMap.of(Integer.class, String.class) + .averageValue("1") + .entries(1) + .checksumEntries(false) + .recoverPersistedTo(persistenceFile, false)) { + assertEquals(2, map.size()); + assertEquals("1", map.get(1)); + assertEquals("-1", map.get(-1)); + } + } +} diff --git a/src/test/resources/chronicle-map-3-12-no-checksums.dat b/src/test/resources/chronicle-map-3-12-no-checksums.dat new file mode 100644 index 000000000..9476f9445 Binary files /dev/null and b/src/test/resources/chronicle-map-3-12-no-checksums.dat differ diff --git a/src/test/resources/chronicle-map-3-12-with-checksums.dat b/src/test/resources/chronicle-map-3-12-with-checksums.dat new file mode 100644 index 000000000..2c2323d3d Binary files /dev/null and b/src/test/resources/chronicle-map-3-12-with-checksums.dat differ