Iterator performance #270
Iterator performance #270
Conversation
There are now essentially three ways of configuring comparators when creating a Dbi. **null comparator** LMDB will use its own comparator & CursorIterable will call down to mdb_cmp for comparisons between the current cursor key and the range start/stop key. **provided comparator** LMDB will use its own comparator & CursorIterable will use the provided comparator for comparisons between the current cursor key and the range start/stop key. **provided comparator with nativeCb==true** LMDB will call back to java for all comparator duties. CursorIterable will use the same provided comparator for comparisons between the current cursor key and the range start/stop key. The methods `getSignedComparator()` and `getUnsignedComparator()` have been made public so users of this library can access them.
Refactor DbiBuilder and Dbi ctor to use DbiFlagSet.
Replace Env#copy(File, CopyFlags...) with copy(File, CopyFlagSet). As there is only one flag this should not be a breaking change. Deprecate Env#txn(Txn, TxnFlags...) as there is now Env#txn(Txn) Env#txn(Txn, TxnFlags) Env#txn(Txn, TxnFlagSet)
Also improve javadoc and refactor some tests to use DbiBuilder. Some tests are failing.
… key using DUPSORT
|
This is just a draft for discussion. If the approach is accepted then the current iterator would be replaced to maintain API compatability. |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #270 +/- ##
============================================
- Coverage 89.06% 81.31% -7.76%
- Complexity 413 598 +185
============================================
Files 32 57 +25
Lines 1482 2531 +1049
Branches 125 279 +154
============================================
+ Hits 1320 2058 +738
- Misses 92 336 +244
- Partials 70 137 +67 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…ava into iterator-performance
…ava into iterator-performance # Conflicts: # src/test/java/org/lmdbjava/CursorIterableIntegerDupTest.java # src/test/java/org/lmdbjava/CursorIterableRangeTest.java
| CursorIterable( | ||
| final Txn<T> txn, final Dbi<T> dbi, final KeyRange<T> range, final Comparator<T> comparator) { | ||
| this.cursor = dbi.openCursor(txn); | ||
| final Txn<T> txn, |
Check notice
Code scanning / CodeQL
Useless parameter Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 1 month ago
The best way to fix a useless parameter is to remove it from both the parameter list of the method and from any places where that method is called, ensuring that its absence does not break functionality. In this case, the constructor for CursorIterable in src/main/java/org/lmdbjava/CursorIterable.java takes a Txn<T> txn parameter which is never used. We should remove final Txn<T> txn from the constructor's parameter list. The constructor is not using txn, and does not store it, so no other changes within the constructor are needed.
The file changes required:
- Remove
final Txn<T> txnfrom the parameter list at line 48. - Update the parameter list to start with
final Dbi<T> dbi. - If the constructor is called elsewhere and we can see those calls, we should update those calls to remove the first parameter (unless the surrounding context requires it). However, the context provided does not show any use/calls, so we only update the definition here.
No new methods or imports are necessary.
| @@ -45,7 +45,6 @@ | ||
| private State state = REQUIRES_INITIAL_OP; | ||
|
|
||
| CursorIterable( | ||
| final Txn<T> txn, | ||
| final Dbi<T> dbi, | ||
| final Cursor<T> cursor, | ||
| final KeyRange<T> range, |
| final Txn<T> txn, final Dbi<T> dbi, final KeyRange<T> range, final Comparator<T> comparator) { | ||
| this.cursor = dbi.openCursor(txn); | ||
| final Txn<T> txn, | ||
| final Dbi<T> dbi, |
Check notice
Code scanning / CodeQL
Useless parameter Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 1 month ago
To fix the problem, the unnecessary dbi parameter should be removed from the constructor in CursorIterable. This involves:
- Removing
final Dbi<T> dbifrom the parameter list of theCursorIterableconstructor. - Updating all usages/calls of the constructor in the provided file (if any) to remove the argument for
dbi. - The removal must ensure that existing code functionality is retained and that the constructor still works as intended with the remaining parameters.
Since only this constructor is supplied and no usages appear within this file, only the region containing the constructor (lines 47-57) needs to be edited.
| @@ -46,7 +46,6 @@ | ||
|
|
||
| CursorIterable( | ||
| final Txn<T> txn, | ||
| final Dbi<T> dbi, | ||
| final Cursor<T> cursor, | ||
| final KeyRange<T> range, | ||
| final RangeComparator rangeComparator) { |
| } | ||
|
|
||
| private static <T> Comparator<KeyVal<T>> createEntryComparator( | ||
| final RangeComparator rangeComparator) { |
Check notice
Code scanning / CodeQL
Useless parameter Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 1 month ago
To fix the problem, the useless parameter rangeComparator should be removed from the method signature of createEntryComparator. Specifically, this means changing both the method declaration and any usages/calls of this method within the file to remove the argument. In the file src/main/java/org/lmdbjava/LmdbStream.java, locate the method starting on line 164, change its signature to remove the parameter, and ensure no rangeComparator argument is passed in calls to createEntryComparator. If there are calls in the code we've seen (e.g., in createSpliterator or constructors), update those call sites. Only change visible code.
| @@ -161,8 +161,7 @@ | ||
| } | ||
| } | ||
|
|
||
| private static <T> Comparator<KeyVal<T>> createEntryComparator( | ||
| final RangeComparator rangeComparator) { | ||
| private static <T> Comparator<KeyVal<T>> createEntryComparator() { | ||
| return null; | ||
| // return (o1, o2) -> comparator.compare(o1.key(), o2.key()); | ||
| } |
| } | ||
|
|
||
| private static <T> Comparator<KeyVal<T>> createReversedEntryComparator( | ||
| final RangeComparator rangeComparator) { |
Check notice
Code scanning / CodeQL
Useless parameter Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 1 month ago
To resolve this, remove the unused rangeComparator parameter from the createReversedEntryComparator method in LmdbStream.java. Also, update every location where this method is called to no longer pass a rangeComparator argument. Within the snippet you provided, direct usages are not shown, so, per instruction, only the method declaration should be altered, and the parameter should be removed from both the signature and invocation if shown. No imports or method definitions are required for this minor change.
| @@ -167,8 +167,7 @@ | ||
| // return (o1, o2) -> comparator.compare(o1.key(), o2.key()); | ||
| } | ||
|
|
||
| private static <T> Comparator<KeyVal<T>> createReversedEntryComparator( | ||
| final RangeComparator rangeComparator) { | ||
| private static <T> Comparator<KeyVal<T>> createReversedEntryComparator() { | ||
| return null; | ||
| // return (o1, o2) -> comparator.compare(o1.key(), o2.key()); | ||
| } |
| import java.util.List; | ||
| import java.util.stream.Collectors; | ||
|
|
||
| class CopyFlagSetTest extends AbstractFlagSetTest<CopyFlags, CopyFlagSet> { |
Check notice
Code scanning / CodeQL
Unused classes and interfaces Note test
Copilot Autofix
AI about 1 month ago
Copilot could not generate an autofix suggestion
Copilot could not generate an autofix suggestion for this alert. Try pushing a new commit or if the problem persists contact support.
| import java.util.List; | ||
| import java.util.stream.Collectors; | ||
|
|
||
| class DbiFlagSetTest extends AbstractFlagSetTest<DbiFlags, DbiFlagSet> { |
Check notice
Code scanning / CodeQL
Unused classes and interfaces Note test
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 1 month ago
The best way to fix this problem is to remove the unused class DbiFlagSetTest entirely from the source file src/test/java/org/lmdbjava/DbiFlagSetTest.java. This means deleting its definition. If the whole file only contains this class and it is not referenced elsewhere or required by any testing framework, removing the entire file is even better. If this file contains only the code shown, delete all lines in the file.
| @@ -1,59 +1 @@ | ||
| /* | ||
| * Copyright © 2016-2025 The LmdbJava Open Source Project | ||
| * | ||
| * Licensed under the Apache License, Version 2.0 (the "License"); | ||
| * you may not use this file except in compliance with the License. | ||
| * You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
| package org.lmdbjava; | ||
|
|
||
| import java.util.Arrays; | ||
| import java.util.Collection; | ||
| import java.util.List; | ||
| import java.util.stream.Collectors; | ||
|
|
||
| class DbiFlagSetTest extends AbstractFlagSetTest<DbiFlags, DbiFlagSet> { | ||
|
|
||
| @Override | ||
| List<DbiFlags> getAllFlags() { | ||
| return Arrays.stream(DbiFlags.values()).collect(Collectors.toList()); | ||
| } | ||
|
|
||
| @Override | ||
| DbiFlagSet getEmptyFlagSet() { | ||
| return DbiFlagSet.empty(); | ||
| } | ||
|
|
||
| @Override | ||
| AbstractFlagSet.Builder<DbiFlags, DbiFlagSet> getBuilder() { | ||
| return DbiFlagSet.builder(); | ||
| } | ||
|
|
||
| @Override | ||
| Class<DbiFlags> getFlagType() { | ||
| return DbiFlags.class; | ||
| } | ||
|
|
||
| @Override | ||
| DbiFlagSet getFlagSet(Collection<DbiFlags> flags) { | ||
| return DbiFlagSet.of(flags); | ||
| } | ||
|
|
||
| @Override | ||
| DbiFlagSet getFlagSet(DbiFlags[] flags) { | ||
| return DbiFlagSet.of(flags); | ||
| } | ||
|
|
||
| @Override | ||
| DbiFlagSet getFlagSet(DbiFlags flag) { | ||
| return DbiFlagSet.of(flag); | ||
| } | ||
| } |
| import java.util.List; | ||
| import java.util.stream.Collectors; | ||
|
|
||
| class EnvFlagSetTest extends AbstractFlagSetTest<EnvFlags, EnvFlagSet> { |
Check notice
Code scanning / CodeQL
Unused classes and interfaces Note test
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 1 month ago
To fix the unused class issue, the best approach is to remove the entire EnvFlagSetTest class from the file. This involves deleting all lines pertaining to the class definition: line 23 through 59 (inclusive) in src/test/java/org/lmdbjava/EnvFlagSetTest.java. This resolves the issue by eliminating the maintenance burden of an unused class and reducing clutter in the codebase.
| @@ -20,40 +20,4 @@ | ||
| import java.util.List; | ||
| import java.util.stream.Collectors; | ||
|
|
||
| class EnvFlagSetTest extends AbstractFlagSetTest<EnvFlags, EnvFlagSet> { | ||
|
|
||
| @Override | ||
| List<EnvFlags> getAllFlags() { | ||
| return Arrays.stream(EnvFlags.values()).collect(Collectors.toList()); | ||
| } | ||
|
|
||
| @Override | ||
| EnvFlagSet getEmptyFlagSet() { | ||
| return EnvFlagSet.empty(); | ||
| } | ||
|
|
||
| @Override | ||
| AbstractFlagSet.Builder<EnvFlags, EnvFlagSet> getBuilder() { | ||
| return EnvFlagSet.builder(); | ||
| } | ||
|
|
||
| @Override | ||
| EnvFlagSet getFlagSet(Collection<EnvFlags> flags) { | ||
| return EnvFlagSet.of(flags); | ||
| } | ||
|
|
||
| @Override | ||
| EnvFlagSet getFlagSet(EnvFlags[] flags) { | ||
| return EnvFlagSet.of(flags); | ||
| } | ||
|
|
||
| @Override | ||
| EnvFlagSet getFlagSet(EnvFlags flag) { | ||
| return EnvFlagSet.of(flag); | ||
| } | ||
|
|
||
| @Override | ||
| Class<EnvFlags> getFlagType() { | ||
| return EnvFlags.class; | ||
| } | ||
| } |
| .setEnvFlags(MDB_NOSUBDIR) | ||
| .open(file)) { | ||
| final Dbi<ByteBuffer> dbi = | ||
| env.openDbi(DB_1, comparator, nativeCb, flags.toArray(new DbiFlags[0])); |
Check notice
Code scanning / CodeQL
Deprecated method or constructor invocation Note test
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 1 month ago
To fix the Deprecated method or constructor invocation, we should replace the call to the deprecated env.openDbi(DB_1, comparator, nativeCb, flags.toArray(new DbiFlags[0])). The LmdbJava Env class provides several overloaded versions of openDbi. The deprecated version typically combines comparator and nativeCb arguments, but newer versions require passing either a comparator OR using the nativeCb flag, not both. According to standard migration in LmdbJava, the newer method is likely openDbi(String name, Comparator<ByteBuffer> comparator, DbiFlags... flags) if a comparator is provided, or openDbi(String name, DbiFlags... flags) otherwise. The boolean nativeCb argument has been removed in newer APIs.
Therefore, the recommended fix is:
- If
comparatoris not null, useenv.openDbi(DB_1, comparator, ...)with flags. - If
comparatoris null, useenv.openDbi(DB_1, ...)with flags only.
Since the function signature always provides a comparator (whose value is variable), we need to check its value. The replacement involves changing line 187 in testCSV() to:
final Dbi<ByteBuffer> dbi = comparator != null
? env.openDbi(DB_1, comparator, flags.toArray(new DbiFlags[0]))
: env.openDbi(DB_1, flags.toArray(new DbiFlags[0]));This edit ensures backward compatibility and uses the modern recommended overload.
No additional imports or dependencies are required as we are using existing core APIs.
| @@ -184,7 +184,9 @@ | ||
| .setEnvFlags(MDB_NOSUBDIR) | ||
| .open(file)) { | ||
| final Dbi<ByteBuffer> dbi = | ||
| env.openDbi(DB_1, comparator, nativeCb, flags.toArray(new DbiFlags[0])); | ||
| comparator != null | ||
| ? env.openDbi(DB_1, comparator, flags.toArray(new DbiFlags[0])) | ||
| : env.openDbi(DB_1, flags.toArray(new DbiFlags[0])); | ||
| dbPopulator.accept(env, dbi); | ||
| try (final Writer writer = new StringWriter()) { | ||
| final KeyRangeType keyRangeType = KeyRangeType.valueOf(keyType.trim()); |
| .setEnvFlags(MDB_NOSUBDIR) | ||
| .open(file)) { | ||
| final Dbi<ByteBuffer> dbi = | ||
| env.openDbi(DB_1, comparator, nativeCb, flags.toArray(new DbiFlags[0])); |
Check notice
Code scanning / CodeQL
Deprecated method or constructor invocation Note test
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 1 month ago
To fix this error, replace the deprecated env.openDbi(DB_1, comparator, nativeCb, flags.toArray(new DbiFlags[0])) call with the recommended, non-deprecated API for opening a database in LmdbJava. The current recommended method is typically env.openDbi(DB_1, DbiFlags...) without extra arguments, or using a builder/configurable pattern to specify comparators and native callbacks if available.
However, your code needs to specify a Comparator<ByteBuffer>, a native callback flag, and pass flags as an array. In recent LmdbJava versions, the recommended approach is to use the Dbi.Builder, chain the desired configuration, and call build() or open().
Therefore, the fix in testCSV is to refactor line 179 as follows:
- Use
env.openDbi()orenv.dbiBuilder(); configure the comparator and flags through the builder API. - If a native callback is required, use the builder's method for setting it, or note that the comparator will act as a native comparator if appropriate.
- Pass the flags as a varargs (directly).
- Remove the deprecated multi-argument method.
Required changes:
- Replace line 179 with code that uses the builder (
env.dbiBuilder(DB_1)) to set the comparator and flags. - Call
build()oropen()to obtain the configured Dbi. - No additional imports are needed if builder API is already available; otherwise, if unavailable, use the recommended open method that accepts the arguments.
| @@ -176,7 +176,11 @@ | ||
| .setEnvFlags(MDB_NOSUBDIR) | ||
| .open(file)) { | ||
| final Dbi<ByteBuffer> dbi = | ||
| env.openDbi(DB_1, comparator, nativeCb, flags.toArray(new DbiFlags[0])); | ||
| env.dbiBuilder(DB_1) | ||
| .withComparator(comparator) | ||
| .withNativeComparator(nativeCb) | ||
| .withFlags(flags.toArray(new DbiFlags[0])) | ||
| .build(); | ||
| dbPopulator.accept(env, dbi); | ||
| try (final Writer writer = new StringWriter()) { | ||
| final KeyRangeType keyRangeType = KeyRangeType.valueOf(keyType.trim()); |
| import java.util.List; | ||
| import java.util.stream.Collectors; | ||
|
|
||
| class TxnFlagSetTest extends AbstractFlagSetTest<TxnFlags, TxnFlagSet> { |
Check notice
Code scanning / CodeQL
Unused classes and interfaces Note test
Copilot Autofix
AI about 1 month ago
Copilot could not generate an autofix suggestion
Copilot could not generate an autofix suggestion for this alert. Try pushing a new commit or if the problem persists contact support.
This is just a draft for now to show iterator performance improvements when using specific classes for cursor direction etc and removing the state machine approach.
Closes gh-269