-
Notifications
You must be signed in to change notification settings - Fork 175
GH-5343 Make LMDBSail Size() 36000x Faster 🚀🚀🚀🚀 #5345
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -115,7 +115,7 @@ public CloseableIteration<? extends Namespace> getNamespaces() throws SailExcept | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (added == null && removed == null) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return namespaces; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
final Iterator<Map.Entry<String, String>> addedIter = added; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
final Iterator<Entry<String, String>> addedIter = added; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
final Set<String> removedSet = removed; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return new AbstractCloseableIteration<>() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -383,4 +383,15 @@ private boolean isDeprecated(Triple triple, List<Statement> deprecatedStatements | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return true; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
@Override | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
public long size(final Resource subj, final IRI pred, final Value obj, final Resource... contexts) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// Fast path: no approved or deprecated | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (!changes.hasApproved() && !changes.hasDeprecated()) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return derivedFrom.size(subj, pred, obj, contexts); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// Fallback path: iterate over all matching statements | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return getStatements(subj, pred, obj, contexts).stream().count(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @odysa Does this call materialize/resolve all values in LmdbStore? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No. It calls rdf4j/core/sail/lmdb/src/main/java/org/eclipse/rdf4j/sail/lmdb/LmdbSailStore.java Lines 927 to 941 in d78001b
And eventually calls getNextElement rdf4j/core/sail/lmdb/src/main/java/org/eclipse/rdf4j/sail/lmdb/LmdbStatementIterator.java Lines 53 to 78 in d78001b
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As in:
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,6 +17,7 @@ | |
import java.util.concurrent.atomic.AtomicBoolean; | ||
import java.util.stream.Stream; | ||
|
||
import org.eclipse.rdf4j.common.annotation.Experimental; | ||
import org.eclipse.rdf4j.common.iteration.CloseableIteration; | ||
import org.eclipse.rdf4j.common.order.StatementOrder; | ||
import org.eclipse.rdf4j.common.transaction.IsolationLevel; | ||
|
@@ -1033,4 +1034,21 @@ private boolean hasStatement(SailDataset dataset, Resource subj, IRI pred, Value | |
} | ||
} | ||
|
||
/** | ||
* Returns the number of statements in the snapshot, optionally including inferred statements, for the given | ||
* contexts. This method reads the size directly from the dataset within the current isolation level. | ||
* | ||
* @param includeInferred whether to include inferred statements in the count | ||
* @param contexts the RDF contexts (named graphs) to restrict the count to; if none are provided, counts all | ||
* contexts | ||
* @return the number of statements in the dataset | ||
* @throws SailException if an error occurs while accessing the Sail store | ||
*/ | ||
@Experimental | ||
protected long getSizeFromSnapshot(final boolean includeInferred, final Resource... contexts) throws SailException { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I kinda wish we could just use the existing sizeInternal(...) method, but I get that it's better to be safe and introduce a separate method for now and then later on we can consider changing up sizeInternal(...). |
||
try (SailSource branch = branch(IncludeInferred.fromBoolean(includeInferred))) { | ||
return branch.dataset(getIsolationLevel()).size(null, null, null, contexts); | ||
} | ||
} | ||
|
||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe that this also needs to be closed.