From 0d14d7fc7816e1e79d2bf1a75e42b3c7b7db19e8 Mon Sep 17 00:00:00 2001 From: leventov Date: Wed, 31 Dec 2014 06:46:22 +0700 Subject: [PATCH] HCOLL-284 better assert-independent bounds checking --- .../internal/CopyingMetaBytesInterop.java | 40 ++--------- .../internal/DataValueMetaBytesInterop.java | 3 +- .../internal/DirectBytesBuffer.java | 66 +++++++++++++++---- .../chronicle/map/SerializationBuilder.java | 2 +- 4 files changed, 63 insertions(+), 48 deletions(-) diff --git a/src/main/java/net/openhft/chronicle/hash/serialization/internal/CopyingMetaBytesInterop.java b/src/main/java/net/openhft/chronicle/hash/serialization/internal/CopyingMetaBytesInterop.java index 292e03038..995eea719 100644 --- a/src/main/java/net/openhft/chronicle/hash/serialization/internal/CopyingMetaBytesInterop.java +++ b/src/main/java/net/openhft/chronicle/hash/serialization/internal/CopyingMetaBytesInterop.java @@ -20,7 +20,6 @@ package net.openhft.chronicle.hash.serialization.internal; import net.openhft.chronicle.hash.serialization.BytesWriter; import net.openhft.lang.MemoryUnit; -import net.openhft.lang.io.Bytes; import net.openhft.lang.io.serialization.BytesMarshaller; import net.openhft.lang.threadlocal.ThreadLocalCopies; @@ -49,28 +48,6 @@ public abstract class CopyingMetaBytesInterop extends BasicCopyingMetaByte super(buffer); } - void init(W writer, E e, boolean mutable, long maxSize) { - if (mutable || writer != this.writer || e != cur) { - this.writer = writer; - cur = e; - while (true) { - try { - Bytes buffer = this.buffer.obtain(maxSize); - innerWrite(writer, buffer, e); - buffer.flip(); - size = buffer.remaining(); - hash = 0L; - return; - } catch (Exception ex) { - checkMaxSizeStillReasonable(maxSize, ex); - maxSize *= 2L; - } - } - } - } - - abstract void innerWrite(W writer, Bytes bytes, E e); - DirectBytesBuffer buffer() { return buffer; } @@ -88,7 +65,7 @@ public abstract class CopyingMetaBytesInterop extends BasicCopyingMetaByte public static > MetaProvider> providerForBytesMarshaller(boolean mutable, long maxSize) { - return new BytesMarshallerCopyingMetaBytesInteropProvider(mutable, maxSize); + return new BytesMarshallerCopyingMetaBytesInteropProvider<>(mutable, maxSize); } private static class BytesMarshallerCopyingMetaBytesInteropProvider extends BasicCopyingMetaByte } public static > - MetaProvider> providerForBytesWriter(boolean mutable, - long maxSize) { - return new BytesWriterCopyingMetaBytesInteropProvider(mutable, maxSize); + MetaProvider> providerForBytesWriter(boolean mutable) { + return new BytesWriterCopyingMetaBytesInteropProvider<>(mutable); } private static class BytesWriterCopyingMetaBytesInteropProvider> extends BasicCopyingMetaBytesInteropProvider> { private static final long serialVersionUID = 0L; private final boolean mutable; - private final long maxSize; - public BytesWriterCopyingMetaBytesInteropProvider(boolean mutable, long maxSize) { + public BytesWriterCopyingMetaBytesInteropProvider(boolean mutable) { this.mutable = mutable; - this.maxSize = maxSize; } @Override - public CopyingMetaBytesInterop get( - ThreadLocalCopies copies, + public CopyingMetaBytesInterop get(ThreadLocalCopies copies, CopyingMetaBytesInterop originalMetaWriter, W writer, E e) { DirectBytesBuffer.ForBytesWriter forBytesWriter = provider.get(copies, originalMetaWriter.buffer()).forBytesWriter; - forBytesWriter.buffer.obtain(writer.size(e)); - forBytesWriter.init(writer, e, mutable, maxSize); + forBytesWriter.init(writer, e, mutable); return forBytesWriter; } } diff --git a/src/main/java/net/openhft/chronicle/hash/serialization/internal/DataValueMetaBytesInterop.java b/src/main/java/net/openhft/chronicle/hash/serialization/internal/DataValueMetaBytesInterop.java index 3c4425869..87eeb84b5 100644 --- a/src/main/java/net/openhft/chronicle/hash/serialization/internal/DataValueMetaBytesInterop.java +++ b/src/main/java/net/openhft/chronicle/hash/serialization/internal/DataValueMetaBytesInterop.java @@ -34,9 +34,10 @@ public abstract class DataValueMetaBytesInterop } void init(BytesWriter writer, E e, long size) { - Bytes buffer = this.buffer.obtain(size); + Bytes buffer = this.buffer.obtain(size, false); writer.write(buffer, e); buffer.flip(); + assert buffer.remaining() == size; this.size = size; hash = 0L; } diff --git a/src/main/java/net/openhft/chronicle/hash/serialization/internal/DirectBytesBuffer.java b/src/main/java/net/openhft/chronicle/hash/serialization/internal/DirectBytesBuffer.java index 5a78d6dfa..c01ceb59d 100644 --- a/src/main/java/net/openhft/chronicle/hash/serialization/internal/DirectBytesBuffer.java +++ b/src/main/java/net/openhft/chronicle/hash/serialization/internal/DirectBytesBuffer.java @@ -17,6 +17,7 @@ package net.openhft.chronicle.hash.serialization.internal; import net.openhft.chronicle.hash.serialization.BytesWriter; +import net.openhft.lang.io.BoundsCheckingNativeBytes; import net.openhft.lang.io.Bytes; import net.openhft.lang.io.DirectBytes; import net.openhft.lang.io.DirectStore; @@ -33,6 +34,7 @@ class DirectBytesBuffer private static final long serialVersionUID = 0L; private final Serializable identity; transient DirectBytes buffer; + transient BoundsCheckingNativeBytes checkingBuffer; transient ForBytesMarshaller forBytesMarshaller; transient ForBytesWriter forBytesWriter; @@ -54,19 +56,33 @@ class DirectBytesBuffer forDataValueWriter = new ForDataValueWriter(); } - Bytes obtain(long maxSize) { + Bytes obtain(long maxSize, boolean boundsChecking) { DirectBytes buf; if ((buf = buffer) != null) { if (maxSize <= buf.capacity()) { - return buf.clear(); + if (!boundsChecking) + return buf.clear(); + Bytes checkingBuf; + if ((checkingBuf = checkingBuffer) != null) { + return checkingBuf.clear(); + } else { + return (checkingBuffer = new BoundsCheckingNativeBytes(buf)).clear(); + } } else { DirectStore store = (DirectStore) buf.store(); - store.resize(maxSize, false); - return buffer = store.bytes(); + store.resize(maxSize, true); + buf = buffer = store.bytes(); + if (!boundsChecking) + return buf; + return (checkingBuffer = new BoundsCheckingNativeBytes(buf)); } } else { - buffer = new DirectStore(JDKObjectSerializer.INSTANCE, maxSize, false).bytes(); - return buffer; + buf = buffer = new DirectStore(JDKObjectSerializer.INSTANCE, + // don't allocate 0 bytes + Math.max(1, maxSize), true).bytes(); + if (!boundsChecking) + return buf; + return (checkingBuffer = new BoundsCheckingNativeBytes(buf)); } } @@ -88,9 +104,26 @@ class DirectBytesBuffer super(DirectBytesBuffer.this); } - @Override - void innerWrite(M writer, Bytes bytes, E e) { - writer.write(bytes, e); + void init(M writer, E e, boolean mutable, long maxSize) { + if (mutable || writer != this.writer || e != cur) { + this.writer = writer; + cur = e; + while (true) { + try { + Bytes buffer = this.buffer.obtain(maxSize, true); + writer.write(buffer, e); + buffer.flip(); + long size = this.size = buffer.remaining(); + this.buffer.buffer.position(0L); + this.buffer.buffer.limit(size); + hash = 0L; + return; + } catch (Exception ex) { + checkMaxSizeStillReasonable(maxSize, ex); + maxSize *= 2L; + } + } + } } } @@ -102,9 +135,18 @@ class DirectBytesBuffer super(DirectBytesBuffer.this); } - @Override - void innerWrite(W writer, Bytes bytes, E e) { - writer.write(bytes, e); + void init(W writer, E e, boolean mutable) { + if (mutable || writer != this.writer || e != cur) { + this.writer = writer; + cur = e; + long size = writer.size(e); + Bytes buffer = this.buffer.obtain(size, false); + writer.write(buffer, e); + buffer.flip(); + this.size = size; + assert size == buffer.remaining(); + hash = 0L; + } } } diff --git a/src/main/java/net/openhft/chronicle/map/SerializationBuilder.java b/src/main/java/net/openhft/chronicle/map/SerializationBuilder.java index ac1f597b5..98846efb3 100644 --- a/src/main/java/net/openhft/chronicle/map/SerializationBuilder.java +++ b/src/main/java/net/openhft/chronicle/map/SerializationBuilder.java @@ -240,7 +240,7 @@ final class SerializationBuilder implements Cloneable, Serializable { } else if (copyingInterop == CopyingInterop.FROM_WRITER) { this.maxSize = maxSize; metaInteropProvider(CopyingMetaBytesInterop - .>providerForBytesWriter(instancesAreMutable, maxSize)); + .>providerForBytesWriter(instancesAreMutable)); } return this; }