From 1001bc01c9ea06785c2d5261ca9fe7ac0eac0625 Mon Sep 17 00:00:00 2001 From: Tomas Kolda Date: Fri, 15 Jan 2021 17:03:11 -0800 Subject: [PATCH] Read Options to support direct slice (#7132) Summary: This request is adding support for using DirectSlice in ReadOptions lower/upper bounds. To be more efficient I have added setLength to DirectSlice so I can just update the length to be used by slice from direct buffer. It is also needed, because when one creates iterator it keep pointer to original slice so setting new slice in options does not help (it needs to reuse existing one). Using this approach one can modify the slice any time during operations with iterator. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7132 Reviewed By: zhichao-cao Differential Revision: D25840092 Pulled By: jay-zhuang fbshipit-source-id: 760167baf61568c9a35138145c4bf9b06824cb71 --- java/rocksjni/slice.cc | 11 ++++++++ .../main/java/org/rocksdb/DirectSlice.java | 5 ++++ .../main/java/org/rocksdb/ReadOptions.java | 26 +++++++++---------- .../java/org/rocksdb/ReadOptionsTest.java | 4 +++ 4 files changed, 32 insertions(+), 14 deletions(-) diff --git a/java/rocksjni/slice.cc b/java/rocksjni/slice.cc index d9e58992b..00ee8e7e2 100644 --- a/java/rocksjni/slice.cc +++ b/java/rocksjni/slice.cc @@ -228,6 +228,17 @@ void Java_org_rocksdb_Slice_removePrefix0(JNIEnv* /*env*/, jobject /*jobj*/, slice->remove_prefix(length); } +/* + * Class: org_rocksdb_DirectSlice + * Method: setLength0 + * Signature: (JI)V + */ +void Java_org_rocksdb_DirectSlice_setLength0(JNIEnv* /*env*/, jobject /*jobj*/, + jlong handle, jint length) { + auto* slice = reinterpret_cast(handle); + slice->size_ = length; +} + /* * Class: org_rocksdb_Slice * Method: disposeInternalBuf diff --git a/java/src/main/java/org/rocksdb/DirectSlice.java b/java/src/main/java/org/rocksdb/DirectSlice.java index b0d35c3cc..02fa3511f 100644 --- a/java/src/main/java/org/rocksdb/DirectSlice.java +++ b/java/src/main/java/org/rocksdb/DirectSlice.java @@ -110,6 +110,10 @@ public class DirectSlice extends AbstractSlice { this.internalBufferOffset += n; } + public void setLength(final int n) { + setLength0(getNativeHandle(), n); + } + @Override protected void disposeInternal() { final long nativeHandle = getNativeHandle(); @@ -127,6 +131,7 @@ public class DirectSlice extends AbstractSlice { private native void clear0(long handle, boolean internalBuffer, long internalBufferOffset); private native void removePrefix0(long handle, int length); + private native void setLength0(long handle, int length); private native void disposeInternalBuf(final long handle, long internalBufferOffset); } diff --git a/java/src/main/java/org/rocksdb/ReadOptions.java b/java/src/main/java/org/rocksdb/ReadOptions.java index 1f1510568..2d3d2f3b8 100644 --- a/java/src/main/java/org/rocksdb/ReadOptions.java +++ b/java/src/main/java/org/rocksdb/ReadOptions.java @@ -440,13 +440,12 @@ public class ReadOptions extends RocksObject { * @param iterateLowerBound Slice representing the upper bound * @return the reference to the current ReadOptions. */ - public ReadOptions setIterateLowerBound(final Slice iterateLowerBound) { + public ReadOptions setIterateLowerBound(final AbstractSlice iterateLowerBound) { assert(isOwningHandle()); - if (iterateLowerBound != null) { - // Hold onto a reference so it doesn't get garbage collected out from under us. - iterateLowerBoundSlice_ = iterateLowerBound; - setIterateLowerBound(nativeHandle_, iterateLowerBoundSlice_.getNativeHandle()); - } + setIterateLowerBound( + nativeHandle_, iterateLowerBound == null ? 0 : iterateLowerBound.getNativeHandle()); + // Hold onto a reference so it doesn't get garbage collected out from under us. + iterateLowerBoundSlice_ = iterateLowerBound; return this; } @@ -485,13 +484,12 @@ public class ReadOptions extends RocksObject { * @param iterateUpperBound Slice representing the upper bound * @return the reference to the current ReadOptions. */ - public ReadOptions setIterateUpperBound(final Slice iterateUpperBound) { + public ReadOptions setIterateUpperBound(final AbstractSlice iterateUpperBound) { assert(isOwningHandle()); - if (iterateUpperBound != null) { - // Hold onto a reference so it doesn't get garbage collected out from under us. - iterateUpperBoundSlice_ = iterateUpperBound; - setIterateUpperBound(nativeHandle_, iterateUpperBoundSlice_.getNativeHandle()); - } + setIterateUpperBound( + nativeHandle_, iterateUpperBound == null ? 0 : iterateUpperBound.getNativeHandle()); + // Hold onto a reference so it doesn't get garbage collected out from under us. + iterateUpperBoundSlice_ = iterateUpperBound; return this; } @@ -570,8 +568,8 @@ public class ReadOptions extends RocksObject { // freely leave scope without us losing the Java Slice object, which during // close() would also reap its associated rocksdb::Slice native object since // it's possibly (likely) to be an owning handle. - private Slice iterateLowerBoundSlice_; - private Slice iterateUpperBoundSlice_; + private AbstractSlice iterateLowerBoundSlice_; + private AbstractSlice iterateUpperBoundSlice_; private native static long newReadOptions(); private native static long newReadOptions(final boolean verifyChecksums, diff --git a/java/src/test/java/org/rocksdb/ReadOptionsTest.java b/java/src/test/java/org/rocksdb/ReadOptionsTest.java index 675023ef3..689c48cb0 100644 --- a/java/src/test/java/org/rocksdb/ReadOptionsTest.java +++ b/java/src/test/java/org/rocksdb/ReadOptionsTest.java @@ -159,6 +159,8 @@ public class ReadOptionsTest { Slice upperBound = buildRandomSlice(); opt.setIterateUpperBound(upperBound); assertThat(Arrays.equals(upperBound.data(), opt.iterateUpperBound().data())).isTrue(); + opt.setIterateUpperBound(null); + assertThat(opt.iterateUpperBound()).isNull(); } } @@ -175,6 +177,8 @@ public class ReadOptionsTest { Slice lowerBound = buildRandomSlice(); opt.setIterateLowerBound(lowerBound); assertThat(Arrays.equals(lowerBound.data(), opt.iterateLowerBound().data())).isTrue(); + opt.setIterateLowerBound(null); + assertThat(opt.iterateLowerBound()).isNull(); } }