- Notifications
You must be signed in to change notification settings - Fork124
Iterator performance#270
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:master
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
… key using DUPSORT
stroomdev66 commentedNov 4, 2025
This is just a draft for discussion. If the approach is accepted then the current iterator would be replaced to maintain API compatability. |
codecovbot commentedNov 4, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@## master #270 +/- ##============================================- Coverage 89.06% 82.64% -6.43%- Complexity 413 449 +36============================================ Files 32 34 +2 Lines 1482 2040 +558 Branches 125 227 +102 ============================================+ Hits 1320 1686 +366- Misses 92 261 +169- Partials 70 93 +23 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
| if (keyLen ==Integer.BYTES) { | ||
| returnbb(Integer.parseInt(key.trim())); | ||
| }else { | ||
| returnbb(Long.parseLong(key.trim())); |
Check notice
Code scanning / CodeQL
Missing catch of NumberFormatException Note test
Show autofix suggestionHide autofix suggestion
Copilot Autofix
AI 2 days ago
To fix the problem, surround the parse operations in theparseKey method (lines 216, 218, 222, and 224) with try-catch blocks forNumberFormatException. If a parsing error occurs, the method should handle it appropriately; in this case, returningnull would gracefully indicate an unparseable key without propagating the exception. No additional imports are required, asNumberFormatException is injava.lang. The required change is limited to the body ofparseKey withinsrc/test/java/org/lmdbjava/LmdbStreamRangeTest.java. No changes needed to imports, method signatures, or globals.
| @@ -211,18 +211,23 @@ | ||
| privateByteBufferparseKey(finalStringkey,finalintkeyLen,finalByteOrderbyteOrder) { | ||
| if (key !=null) { | ||
| if (ByteOrder.LITTLE_ENDIAN.equals(byteOrder)) { | ||
| if (keyLen ==Integer.BYTES) { | ||
| returnbbLeInt(Integer.parseInt(key.trim())); | ||
| try { | ||
| if (ByteOrder.LITTLE_ENDIAN.equals(byteOrder)) { | ||
| if (keyLen ==Integer.BYTES) { | ||
| returnbbLeInt(Integer.parseInt(key.trim())); | ||
| }else { | ||
| returnbbLeLong(Long.parseLong(key.trim())); | ||
| } | ||
| }else { | ||
| returnbbLeLong(Long.parseLong(key.trim())); | ||
| if (keyLen ==Integer.BYTES) { | ||
| returnbb(Integer.parseInt(key.trim())); | ||
| }else { | ||
| returnbb(Long.parseLong(key.trim())); | ||
| } | ||
| } | ||
| }else { | ||
| if (keyLen ==Integer.BYTES) { | ||
| returnbb(Integer.parseInt(key.trim())); | ||
| }else { | ||
| returnbb(Long.parseLong(key.trim())); | ||
| } | ||
| }catch (NumberFormatExceptione) { | ||
| // Handle invalid key format by returning null | ||
| returnnull; | ||
| } | ||
| } | ||
| returnnull; |
| } | ||
| }else { | ||
| if (keyLen ==Integer.BYTES) { | ||
| returnbb(Integer.parseInt(key.trim())); |
Check notice
Code scanning / CodeQL
Missing catch of NumberFormatException Note test
Show autofix suggestionHide autofix suggestion
Copilot Autofix
AI 2 days ago
To fix the problem, we should wrap each call toInteger.parseInt andLong.parseLong withinparseKey in a try-catch block that catchesNumberFormatException. When a parsing error occurs, we should handle it in a way that fits the function's use case (for tests, often throwing an unchecked exception with a clear message is suitable, or, alternatively, returningnull or bubbling up as a specific checked/unchecked exception). Since this is a test utility, the best approach is probably to throw anIllegalArgumentException with an informative message so test failures are explicit and debuggable. Only the code withinparseKey (lines 212–229) needs to be changed, and no new imports are necessary sinceNumberFormatException andIllegalArgumentException are injava.lang.
| @@ -211,18 +211,23 @@ | ||
| privateByteBufferparseKey(finalStringkey,finalintkeyLen,finalByteOrderbyteOrder) { | ||
| if (key !=null) { | ||
| if (ByteOrder.LITTLE_ENDIAN.equals(byteOrder)) { | ||
| if (keyLen ==Integer.BYTES) { | ||
| returnbbLeInt(Integer.parseInt(key.trim())); | ||
| try { | ||
| if (ByteOrder.LITTLE_ENDIAN.equals(byteOrder)) { | ||
| if (keyLen ==Integer.BYTES) { | ||
| returnbbLeInt(Integer.parseInt(key.trim())); | ||
| }else { | ||
| returnbbLeLong(Long.parseLong(key.trim())); | ||
| } | ||
| }else { | ||
| returnbbLeLong(Long.parseLong(key.trim())); | ||
| if (keyLen ==Integer.BYTES) { | ||
| returnbb(Integer.parseInt(key.trim())); | ||
| }else { | ||
| returnbb(Long.parseLong(key.trim())); | ||
| } | ||
| } | ||
| }else { | ||
| if (keyLen ==Integer.BYTES) { | ||
| returnbb(Integer.parseInt(key.trim())); | ||
| }else { | ||
| returnbb(Long.parseLong(key.trim())); | ||
| } | ||
| }catch (NumberFormatExceptione) { | ||
| thrownewIllegalArgumentException("Could not parse key string '" +key +"' as a " + | ||
| (keyLen ==Integer.BYTES ?"integer" :"long") +".",e); | ||
| } | ||
| } | ||
| returnnull; |
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
| () -> { | ||
| longcount =0; | ||
| try (finalTxn<ByteBuffer>txn =env.txnRead()) { | ||
| for (finalKeyVal<ByteBuffer>ignored :dbi.iterate(txn)) { |
Check notice
Code scanning / CodeQL
Unread local variable Note test
Show autofix suggestionHide autofix suggestion
Copilot Autofix
AI 2 days ago
To fix the problem, we should eliminate the declaration of the unused variable in the for-each loop at line 164. The current loop:
for (finalKeyVal<ByteBuffer>ignored :dbi.iterate(txn)) {count++;}
can be rewritten as a standard for-each loop that simply increments the counter for each element, without naming the unused variable, but in Java this isn't technically possible without at least binding the variable. Therefore, as an alternative, we could use a normalfor loop over the result set or, if the variable truly serves no purpose, simply use theforEach method with a lambda that increments the count. This change minimizes the presence of an unread variable and makes the intent clear.
Best fix: Replace the for-each loop with a call to.forEach(e -> count++), which is idiomatic in Java when the loop variable is unused.
Where to change: Only the for-each loop at line 164, withintestIterationMethods insrc/test/java/org/lmdbjava/TestLmdbStreamBenchmark.java.
What is needed: No additional imports or definitions are needed. Only a code change to the loop construct.
| @@ -161,9 +161,7 @@ | ||
| () -> { | ||
| longcount =0; | ||
| try (finalTxn<ByteBuffer>txn =env.txnRead()) { | ||
| for (finalKeyVal<ByteBuffer>ignored :dbi.iterate(txn)) { | ||
| count++; | ||
| } | ||
| dbi.iterate(txn).forEach(e ->count++); | ||
| } | ||
| assertThat(count).isEqualTo(totalRows); | ||
| }); |
Uh oh!
There was an error while loading.Please reload this page.
… key using DUPSORT
… key using DUPSORT
… key using DUPSORT
Uh oh!
There was an error while loading.Please reload this page.
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.
Closesgh-269