Skip to content

Commit

Permalink
GH-5059 remove calls to E(...) when the return value is already being…
Browse files Browse the repository at this point in the history 
… checked (#5066)
  • Loading branch information
@hmottestad
hmottestad authored Jul 9, 2024
2 parents ae61579 + e5faa2c commit 5ed5bad
Show file tree
Hide file tree
Showing 8 changed files with 114 additions and 80 deletions.
14 changes: 7 additions & 7 deletions core/sail/lmdb/src/main/java/org/eclipse/rdf4j/sail/lmdb/LmdbContextIdIterator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,12 +104,12 @@ public long[] next() {
Varint.writeUnsigned(minKeyBuf, record[0]);
minKeyBuf.flip();
keyData.mv_data(minKeyBuf);
lastResult = E(mdb_cursor_get(cursor, keyData, valueData, MDB_SET));
if (lastResult != 0) {
lastResult = mdb_cursor_get(cursor, keyData, valueData, MDB_SET);
if (lastResult != MDB_SUCCESS) {
// use MDB_SET_RANGE if key was deleted
lastResult = E(mdb_cursor_get(cursor, keyData, valueData, MDB_SET_RANGE));
lastResult = mdb_cursor_get(cursor, keyData, valueData, MDB_SET_RANGE);
}
if (lastResult != 0) {
if (lastResult != MDB_SUCCESS) {
closeInternal(false);
return null;
}
Expand All @@ -119,16 +119,16 @@ public long[] next() {
}

if (fetchNext) {
lastResult = E(mdb_cursor_get(cursor, keyData, valueData, MDB_NEXT));
lastResult = mdb_cursor_get(cursor, keyData, valueData, MDB_NEXT);
fetchNext = false;
} else {
if (minKeyBuf != null) {
// set cursor to min key
keyData.mv_data(minKeyBuf);
lastResult = E(mdb_cursor_get(cursor, keyData, valueData, MDB_SET_RANGE));
lastResult = mdb_cursor_get(cursor, keyData, valueData, MDB_SET_RANGE);
} else {
// set cursor to first item
lastResult = E(mdb_cursor_get(cursor, keyData, valueData, MDB_NEXT));
lastResult = mdb_cursor_get(cursor, keyData, valueData, MDB_NEXT);
}
}

Expand Down
18 changes: 8 additions & 10 deletions core/sail/lmdb/src/main/java/org/eclipse/rdf4j/sail/lmdb/LmdbRecordIterator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -138,12 +138,12 @@ public long[] next() {
index.toKey(minKeyBuf, quad[0], quad[1], quad[2], quad[3]);
minKeyBuf.flip();
keyData.mv_data(minKeyBuf);
lastResult = E(mdb_cursor_get(cursor, keyData, valueData, MDB_SET));
if (lastResult != 0) {
lastResult = mdb_cursor_get(cursor, keyData, valueData, MDB_SET);
if (lastResult != MDB_SUCCESS) {
// use MDB_SET_RANGE if key was deleted
lastResult = E(mdb_cursor_get(cursor, keyData, valueData, MDB_SET_RANGE));
lastResult = mdb_cursor_get(cursor, keyData, valueData, MDB_SET_RANGE);
}
if (lastResult != 0) {
if (lastResult != MDB_SUCCESS) {
closeInternal(false);
return null;
}
Expand All @@ -153,16 +153,16 @@ public long[] next() {
}

if (fetchNext) {
lastResult = E(mdb_cursor_get(cursor, keyData, valueData, MDB_NEXT));
lastResult = mdb_cursor_get(cursor, keyData, valueData, MDB_NEXT);
fetchNext = false;
} else {
if (minKeyBuf != null) {
// set cursor to min key
keyData.mv_data(minKeyBuf);
lastResult = E(mdb_cursor_get(cursor, keyData, valueData, MDB_SET_RANGE));
lastResult = mdb_cursor_get(cursor, keyData, valueData, MDB_SET_RANGE);
} else {
// set cursor to first item
lastResult = E(mdb_cursor_get(cursor, keyData, valueData, MDB_NEXT));
lastResult = mdb_cursor_get(cursor, keyData, valueData, MDB_NEXT);
}
}

Expand All @@ -172,7 +172,7 @@ public long[] next() {
lastResult = MDB_NOTFOUND;
} else if (groupMatcher != null && !groupMatcher.matches(keyData.mv_data())) {
// value doesn't match search key/mask, fetch next value
lastResult = E(mdb_cursor_get(cursor, keyData, valueData, MDB_NEXT));
lastResult = mdb_cursor_get(cursor, keyData, valueData, MDB_NEXT);
} else {
// Matching value found
index.keyToQuad(keyData.mv_data(), quad);
Expand All @@ -183,8 +183,6 @@ public long[] next() {
}
closeInternal(false);
return null;
} catch (IOException e) {
throw new SailException(e);
} finally {
txnLock.unlockRead(stamp);
}
Expand Down
19 changes: 14 additions & 5 deletions core/sail/lmdb/src/main/java/org/eclipse/rdf4j/sail/lmdb/LmdbSailStore.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -572,12 +572,14 @@ public void approve(Resource subj, IRI pred, Value obj, Resource ctx) throws Sai

@Override
public void approveAll(Set<Statement> approved, Set<Resource> approvedContexts) {
Statement last = null;

sinkStoreAccessLock.lock();
try {
startTransaction(true);

for (Statement statement : approved) {
last = statement;
Resource subj = statement.getSubject();
IRI pred = statement.getPredicate();
Value obj = statement.getObject();
Expand All @@ -604,13 +606,20 @@ public void approveAll(Set<Statement> approved, Set<Resource> approvedContexts)
}

}
} catch (IOException e) {
} catch (IOException | RuntimeException e) {
rollback();
if (multiThreadingActive) {
logger.error("Encountered an unexpected problem while trying to add a statement.", e);
} else {
logger.error(
"Encountered an unexpected problem while trying to add a statement. Last statement that was attempted to be added: [ {} ]",
last, e);
}

if (e instanceof RuntimeException) {
throw (RuntimeException) e;
}
throw new SailException(e);
} catch (RuntimeException e) {
rollback();
logger.error("Encountered an unexpected problem while trying to add a statement", e);
throw e;
} finally {
sinkStoreAccessLock.unlock();
}
Expand Down
11 changes: 9 additions & 2 deletions core/sail/lmdb/src/main/java/org/eclipse/rdf4j/sail/lmdb/LmdbUtil.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

import static org.lwjgl.system.MemoryStack.stackPush;
import static org.lwjgl.system.MemoryUtil.NULL;
import static org.lwjgl.util.lmdb.LMDB.MDB_DBS_FULL;
import static org.lwjgl.util.lmdb.LMDB.MDB_KEYEXIST;
import static org.lwjgl.util.lmdb.LMDB.MDB_NOTFOUND;
import static org.lwjgl.util.lmdb.LMDB.MDB_RDONLY;
Expand All @@ -39,12 +40,16 @@
import org.lwjgl.system.Pointer;
import org.lwjgl.util.lmdb.MDBCmpFuncI;
import org.lwjgl.util.lmdb.MDBVal;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

/**
* Utility class for working with LMDB.
*/
final class LmdbUtil {

private static final Logger logger = LoggerFactory.getLogger(LmdbUtil.class);

/**
* Minimum free space in an LMDB db before automatically resizing the map.
*/
Expand All @@ -61,7 +66,9 @@ private LmdbUtil() {

static int E(int rc) throws IOException {
if (rc != MDB_SUCCESS && rc != MDB_NOTFOUND && rc != MDB_KEYEXIST) {
throw new IOException(mdb_strerror(rc));
IOException ioException = new IOException(mdb_strerror(rc));
logger.info("Possible LMDB error: {}", mdb_strerror(rc), ioException);
throw ioException;
}
return rc;
}
Expand Down Expand Up @@ -105,7 +112,7 @@ static <T> T transaction(long env, Transaction<T> transaction) throws IOExceptio
int err;
try {
ret = transaction.exec(stack, txn);
err = E(mdb_txn_commit(txn));
err = mdb_txn_commit(txn);
} catch (Throwable t) {
mdb_txn_abort(txn);
throw t;
Expand Down
30 changes: 28 additions & 2 deletions core/sail/lmdb/src/main/java/org/eclipse/rdf4j/sail/lmdb/PersistentSet.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@

import static org.eclipse.rdf4j.sail.lmdb.LmdbUtil.E;
import static org.lwjgl.system.MemoryUtil.NULL;
import static org.lwjgl.util.lmdb.LMDB.MDB_MAP_FULL;
import static org.lwjgl.util.lmdb.LMDB.MDB_NEXT;
import static org.lwjgl.util.lmdb.LMDB.MDB_NOOVERWRITE;
import static org.lwjgl.util.lmdb.LMDB.MDB_SET;
Expand All @@ -24,6 +25,7 @@
import static org.lwjgl.util.lmdb.LMDB.mdb_del;
import static org.lwjgl.util.lmdb.LMDB.mdb_drop;
import static org.lwjgl.util.lmdb.LMDB.mdb_put;
import static org.lwjgl.util.lmdb.LMDB.mdb_strerror;
import static org.lwjgl.util.lmdb.LMDB.mdb_txn_abort;
import static org.lwjgl.util.lmdb.LMDB.mdb_txn_begin;

Expand All @@ -43,12 +45,16 @@
import org.lwjgl.PointerBuffer;
import org.lwjgl.system.MemoryStack;
import org.lwjgl.util.lmdb.MDBVal;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

/**
* A LMDB-based persistent set.
*/
class PersistentSet<T extends Serializable> extends AbstractSet<T> {

private static final Logger logger = LoggerFactory.getLogger(PersistentSet.class);

private PersistentSetFactory<T> factory;
private final int dbi;
private int size;
Expand Down Expand Up @@ -126,15 +132,35 @@ private synchronized boolean update(Object element, boolean add) throws IOExcept
keyVal.mv_data(keyBuf);

if (add) {
if (E(mdb_put(factory.writeTxn, dbi, keyVal, dataVal, MDB_NOOVERWRITE)) == MDB_SUCCESS) {
int rc = mdb_put(factory.writeTxn, dbi, keyVal, dataVal, MDB_NOOVERWRITE);
if (rc == MDB_SUCCESS) {
size++;
return true;
} else if (rc == MDB_MAP_FULL) {
factory.ensureResize();
if (mdb_put(factory.writeTxn, dbi, keyVal, dataVal, MDB_NOOVERWRITE) == MDB_SUCCESS) {
size++;
return true;
}
return false;
} else {
logger.debug("Failed to add element due to error {}: {}", mdb_strerror(rc), element);
}
} else {
// delete element
if (mdb_del(factory.writeTxn, dbi, keyVal, dataVal) == MDB_SUCCESS) {
int rc = mdb_del(factory.writeTxn, dbi, keyVal, dataVal);
if (rc == MDB_SUCCESS) {
size--;
return true;
} else if (rc == MDB_MAP_FULL) {
factory.ensureResize();
if (mdb_del(factory.writeTxn, dbi, keyVal, dataVal) == MDB_SUCCESS) {
size--;
return true;
}
return false;
} else {
logger.debug("Failed to remove element due to error {}: {}", mdb_strerror(rc), element);
}
}
return false;
Expand Down
40 changes: 20 additions & 20 deletions core/sail/lmdb/src/main/java/org/eclipse/rdf4j/sail/lmdb/TripleStore.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -561,7 +561,7 @@ protected void filterUsedIds(Collection<Long> ids) throws IOException {
keyBuf.clear();
Varint.writeUnsigned(keyBuf, id);
keyData.mv_data(keyBuf.flip());
if (E(mdb_get(txn, contextsDbi, keyData, valueData)) == MDB_SUCCESS) {
if (mdb_get(txn, contextsDbi, keyData, valueData) == MDB_SUCCESS) {
it.remove();
}
}
Expand All @@ -587,15 +587,15 @@ protected void filterUsedIds(Collection<Long> ids) throws IOException {

if (fullScan) {
long[] quad = new long[4];
int rc = E(mdb_cursor_get(cursor, keyData, valueData, MDB_FIRST));
int rc = mdb_cursor_get(cursor, keyData, valueData, MDB_FIRST);
while (rc == MDB_SUCCESS && !ids.isEmpty()) {
index.keyToQuad(keyData.mv_data(), quad);
ids.remove(quad[0]);
ids.remove(quad[1]);
ids.remove(quad[2]);
ids.remove(quad[3]);

rc = E(mdb_cursor_get(cursor, keyData, valueData, MDB_NEXT));
rc = mdb_cursor_get(cursor, keyData, valueData, MDB_NEXT);
}
} else {
for (Iterator<Long> it = ids.iterator(); it.hasNext();) {
Expand Down Expand Up @@ -625,15 +625,15 @@ protected void filterUsedIds(Collection<Long> ids) throws IOException {

// set cursor to min key
keyData.mv_data(keyBuf);
int rc = E(mdb_cursor_get(cursor, keyData, valueData, MDB_SET_RANGE));
int rc = mdb_cursor_get(cursor, keyData, valueData, MDB_SET_RANGE);
boolean exists = false;
while (!exists && rc == 0) {
while (!exists && rc == MDB_SUCCESS) {
if (mdb_cmp(txn, dbi, keyData, maxKey) > 0) {
// id was not found
break;
} else if (!matcher.matches(keyData.mv_data())) {
// value doesn't match search key/mask, fetch next value
rc = E(mdb_cursor_get(cursor, keyData, valueData, MDB_NEXT));
rc = mdb_cursor_get(cursor, keyData, valueData, MDB_NEXT);
} else {
exists = true;
}
Expand Down Expand Up @@ -708,24 +708,24 @@ protected double cardinality(long subj, long pred, long obj, long context) throw

// set cursor to min key
keyData.mv_data(keyBuf);
int rc = E(mdb_cursor_get(cursor, keyData, valueData, MDB_SET_RANGE));
if (rc != 0 || mdb_cmp(txn, dbi, keyData, maxKey) >= 0) {
int rc = mdb_cursor_get(cursor, keyData, valueData, MDB_SET_RANGE);
if (rc != MDB_SUCCESS || mdb_cmp(txn, dbi, keyData, maxKey) >= 0) {
break;
} else {
Varint.readListUnsigned(keyData.mv_data(), s.minValues);
}

// set cursor to max key
keyData.mv_data(maxKeyBuf);
rc = E(mdb_cursor_get(cursor, keyData, valueData, MDB_SET_RANGE));
if (rc != 0) {
rc = mdb_cursor_get(cursor, keyData, valueData, MDB_SET_RANGE);
if (rc != MDB_SUCCESS) {
// directly go to last value
rc = E(mdb_cursor_get(cursor, keyData, valueData, MDB_LAST));
rc = mdb_cursor_get(cursor, keyData, valueData, MDB_LAST);
} else {
// go to previous value of selected key
rc = E(mdb_cursor_get(cursor, keyData, valueData, MDB_PREV));
rc = mdb_cursor_get(cursor, keyData, valueData, MDB_PREV);
}
if (rc == 0) {
if (rc == MDB_SUCCESS) {
Varint.readListUnsigned(keyData.mv_data(), s.maxValues);
// this is required to correctly estimate the range size at a later point
s.startValues[s.MAX_BUCKETS] = s.maxValues;
Expand All @@ -747,7 +747,7 @@ protected double cardinality(long subj, long pred, long obj, long context) throw
keyData.mv_data(keyBuf);

int currentSamplesCount = 0;
rc = E(mdb_cursor_get(cursor, keyData, valueData, MDB_SET_RANGE));
rc = mdb_cursor_get(cursor, keyData, valueData, MDB_SET_RANGE);
while (rc == MDB_SUCCESS && currentSamplesCount < s.MAX_SAMPLES_PER_BUCKET) {
if (mdb_cmp(txn, dbi, keyData, maxKey) >= 0) {
endOfRange = true;
Expand Down Expand Up @@ -776,8 +776,8 @@ protected double cardinality(long subj, long pred, long obj, long context) throw
}
}
}
rc = E(mdb_cursor_get(cursor, keyData, valueData, MDB_NEXT));
if (rc != 0) {
rc = mdb_cursor_get(cursor, keyData, valueData, MDB_NEXT);
if (rc != MDB_SUCCESS) {
// no more elements are available
endOfRange = true;
}
Expand Down Expand Up @@ -873,14 +873,14 @@ public boolean storeTriple(long subj, long pred, long obj, long context, boolean
return recordCache.storeRecord(quad, explicit);
}

int rc = E(mdb_put(writeTxn, mainIndex.getDB(explicit), keyVal, dataVal, MDB_NOOVERWRITE));
int rc = mdb_put(writeTxn, mainIndex.getDB(explicit), keyVal, dataVal, MDB_NOOVERWRITE);
if (rc != MDB_SUCCESS && rc != MDB_KEYEXIST) {
throw new IOException(mdb_strerror(rc));
}
stAdded = rc == MDB_SUCCESS;
boolean foundImplicit = false;
if (explicit && stAdded) {
foundImplicit = E(mdb_del(writeTxn, mainIndex.getDB(false), keyVal, dataVal)) == MDB_SUCCESS;
foundImplicit = mdb_del(writeTxn, mainIndex.getDB(false), keyVal, dataVal) == MDB_SUCCESS;
}

if (stAdded) {
Expand Down Expand Up @@ -920,7 +920,7 @@ private void incrementContext(MemoryStack stack, long context) throws IOExceptio
idVal.mv_data(bb);
MDBVal dataVal = MDBVal.calloc(stack);
long newCount = 1;
if (E(mdb_get(writeTxn, contextsDbi, idVal, dataVal)) == MDB_SUCCESS) {
if (mdb_get(writeTxn, contextsDbi, idVal, dataVal) == MDB_SUCCESS) {
// update count
newCount = Varint.readUnsigned(dataVal.mv_data()) + 1;
}
Expand All @@ -944,7 +944,7 @@ private boolean decrementContext(MemoryStack stack, long context) throws IOExcep
bb.flip();
idVal.mv_data(bb);
MDBVal dataVal = MDBVal.calloc(stack);
if (E(mdb_get(writeTxn, contextsDbi, idVal, dataVal)) == MDB_SUCCESS) {
if (mdb_get(writeTxn, contextsDbi, idVal, dataVal) == MDB_SUCCESS) {
// update count
long newCount = Varint.readUnsigned(dataVal.mv_data()) - 1;
if (newCount <= 0) {
Expand Down
Loading

0 comments on commit 5ed5bad

Please sign in to comment.