Skip to content

Commit 74b817c

Browse files
committed
More JNI write batch performance fixes
Second instance of - When passing a direct bytebuffer, slice() it first before passing over JNI Use critical section instead of env->GetByteArrayElements to avoid a copy of a potentially very large array, only part of which we may need. It has drawbacks, but for demonstrating performance and working round the byte[] version’s performance bug, it is invaluable.
1 parent 1e7c9e5 commit 74b817c

File tree

3 files changed

+29
-8
lines changed

3 files changed

+29
-8
lines changed

java/jmh/WriteBatch.md

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -481,7 +481,7 @@ WriteBatchBenchmarks.putWriteBatchNative 10000000 16
481481
WriteBatchBenchmarks.putWriteBatchNativeBB 10000000 16 20 4096 131072 false thrpt 5 775264.705 ± 21106.839 ops/s
482482
WriteBatchBenchmarks.putWriteBatchNativeBB 10000000 16 20 4096 1048576 false thrpt 5 128399.860 ± 9716.046 ops/s
483483

484-
Performance bugfix for native direct buffers:
484+
Performance bugfix for native direct buffers, and re-run the test above (names have changed):
485485
```
486486
java -jar target/rocksdbjni-jmh-1.0-SNAPSHOT-benchmarks.jar WriteBatchBenchmarks.putWriteBatch -p keySize="16" -p valueSize="4096" -p keyCount="10000000" -p numOpsPerBatch="20" -p writeBatchAllocation="131072","1048576" -p writeToDB="false"
487487
```
@@ -496,6 +496,24 @@ WriteBatchBenchmarks.putWriteBatchNativeD 10000000 16
496496
WriteBatchBenchmarks.putWriteBatchNativeI 10000000 16 20 4096 131072 false thrpt 5 1656172.994 ± 65982.655 ops/s
497497
WriteBatchBenchmarks.putWriteBatchNativeI 10000000 16 20 4096 1048576 false thrpt 5 608036.465 ± 148801.987 ops/s
498498

499+
It turns out that the *indirect* version (`putWriteBatchNativeI`) uses `JNI.GetByteArrayElements()` and that does a copy in the tests I am running; I have previously seen it not do
500+
a copy. If we use a critical section instead (and there are difficulties in general with this, see the JNI documentation etc) we get back the performance that has gone
501+
missing.
502+
503+
```
504+
java -jar target/rocksdbjni-jmh-1.0-SNAPSHOT-benchmarks.jar WriteBatchBenchmarks.putWriteBatch -p keySize="16" -p valueSize="4096" -p keyCount="10000000" -p numOpsPerBatch="20" -p writeBatchAllocation="131072","1048576" -p writeToDB="false"
505+
```
506+
507+
Benchmark (keyCount) (keySize) (numOpsPerBatch) (valueSize) (writeBatchAllocation) (writeToDB) Mode Cnt Score Error Units
508+
WriteBatchBenchmarks.putWriteBatchD 10000000 16 20 4096 131072 false thrpt 5 1749305.054 ± 31228.287 ops/s
509+
WriteBatchBenchmarks.putWriteBatchD 10000000 16 20 4096 1048576 false thrpt 5 1739204.577 ± 34841.845 ops/s
510+
WriteBatchBenchmarks.putWriteBatchI 10000000 16 20 4096 131072 false thrpt 5 1330461.139 ± 30573.622 ops/s
511+
WriteBatchBenchmarks.putWriteBatchI 10000000 16 20 4096 1048576 false thrpt 5 1344626.672 ± 66659.690 ops/s
512+
WriteBatchBenchmarks.putWriteBatchNativeD 10000000 16 20 4096 131072 false thrpt 5 3654634.857 ± 792083.344 ops/s
513+
WriteBatchBenchmarks.putWriteBatchNativeD 10000000 16 20 4096 1048576 false thrpt 5 2978031.325 ± 562536.419 ops/s
514+
WriteBatchBenchmarks.putWriteBatchNativeI 10000000 16 20 4096 131072 false thrpt 5 3053646.626 ± 521910.691 ops/s
515+
WriteBatchBenchmarks.putWriteBatchNativeI 10000000 16 20 4096 1048576 false thrpt 5 3930996.978 ± 2093063.739 ops/s
516+
499517

500518

501519

java/rocksjni/write_batch_java_native.cc

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -63,12 +63,13 @@ jlong Java_org_rocksdb_WriteBatchJavaNative_flushWriteBatchJavaNativeArray(
6363
wb = reinterpret_cast<ROCKSDB_NAMESPACE::WriteBatchJavaNative*>(jwb_handle);
6464
}
6565

66-
jbyte* buf = env->GetByteArrayElements(jbuf, nullptr);
66+
jboolean is_copy;
67+
jbyte* buf = reinterpret_cast<jbyte *>(env->GetPrimitiveArrayCritical(jbuf, &is_copy));
6768
if (env->ExceptionCheck()) {
6869
// exception thrown: OutOfMemoryError
6970
return -1L;
7071
}
71-
72+
7273
try {
7374
wb->Append(
7475
ROCKSDB_NAMESPACE::Slice(reinterpret_cast<char*>(buf), jbuf_len));
@@ -77,7 +78,7 @@ jlong Java_org_rocksdb_WriteBatchJavaNative_flushWriteBatchJavaNativeArray(
7778
return e.Code();
7879
}
7980

80-
env->ReleaseByteArrayElements(jbuf, buf, JNI_ABORT);
81+
env->ReleasePrimitiveArrayCritical(jbuf, buf, JNI_ABORT);
8182

8283
return GET_CPLUSPLUS_POINTER(wb);
8384
}
@@ -140,12 +141,13 @@ jlong Java_org_rocksdb_WriteBatchJavaNative_writeWriteBatchJavaNativeArray(
140141
wb = reinterpret_cast<ROCKSDB_NAMESPACE::WriteBatchJavaNative*>(jwb_handle);
141142
}
142143

143-
jbyte* buf = env->GetByteArrayElements(jbuf, nullptr);
144+
jboolean is_copy;
145+
jbyte* buf = reinterpret_cast<jbyte *>(env->GetPrimitiveArrayCritical(jbuf, &is_copy));
144146
if (env->ExceptionCheck()) {
145147
// exception thrown: OutOfMemoryError
146148
return -1L;
147149
}
148-
150+
149151
try {
150152
wb->Append(
151153
ROCKSDB_NAMESPACE::Slice(reinterpret_cast<char*>(buf), jbuf_len));
@@ -154,7 +156,7 @@ jlong Java_org_rocksdb_WriteBatchJavaNative_writeWriteBatchJavaNativeArray(
154156
return e.Code();
155157
}
156158

157-
env->ReleaseByteArrayElements(jbuf, buf, JNI_ABORT);
159+
env->ReleasePrimitiveArrayCritical(jbuf, buf, JNI_ABORT);
158160

159161
ROCKSDB_NAMESPACE::Status s = db->Write(*write_options, wb);
160162

java/src/main/java/org/rocksdb/WriteBatchJavaNative.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -221,10 +221,11 @@ void write(final RocksDB db, final WriteOptions writeOptions) throws RocksDBExce
221221

222222
buffer.flip();
223223
if (buffer.isDirect()) {
224+
ByteBuffer slice = buffer.slice();
224225
setNativeHandle(
225226
writeWriteBatchJavaNativeDirect(db.getNativeHandle(), writeOptions.getNativeHandle(),
226227
// assert position == 0 (we just flipped)
227-
getNativeHandle(), buffer.capacity(), buffer, buffer.position(), buffer.limit()));
228+
getNativeHandle(), buffer.capacity(), slice, 0, slice.limit()));
228229
} else {
229230
setNativeHandle(
230231
writeWriteBatchJavaNativeArray(db.getNativeHandle(), writeOptions.getNativeHandle(),

0 commit comments

Comments
 (0)