Skip to content
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.

Sign up for GitHub

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

GH-3989 improve DynamicModel and LinkedHashModel #3990

Draft
wants to merge 2 commits into
base: develop
Choose a base branch
from
Draft

GH-3989 improve DynamicModel and LinkedHashModel #3990

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a26254a
GH-3989 allow a ModelFactory to create a model from a collection
hmottestad Jun 15, 2022
e572a9c
GH-3989 cache the hashCode for ModelStatement and switch from Context…
hmottestad Jun 15, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions core/model-api/src/main/java/org/eclipse/rdf4j/model/ModelFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
*******************************************************************************/
package org.eclipse.rdf4j.model;

import java.util.Collection;

/**
* Factory to create empty {@link Model} implementations.
*
Expand All @@ -19,4 +21,10 @@ public interface ModelFactory {
*/
Model createEmptyModel();

default Model createFrom(Collection<? extends Statement> collection) {
Model newModel = createEmptyModel();
newModel.addAll(collection);
return newModel;
}

}
92 changes: 80 additions & 12 deletions core/model/src/main/java/org/eclipse/rdf4j/model/impl/DynamicModel.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

import java.util.AbstractSet;
import java.util.Collection;
import java.util.Collections;
import java.util.ConcurrentModificationException;
import java.util.Iterator;
import java.util.LinkedHashMap;
import java.util.LinkedHashSet;
Expand All @@ -35,7 +35,7 @@
* retrieving and removing data. The model will upgrade to a full model (provided by the modelFactory) if more complex
* operations are called, for instance removing data according to a pattern (eg. all statements with rdf:type as
* predicate).
*
* <p>
* DynamicModel is thread safe to the extent that the underlying LinkedHashMap or Model is. The upgrade path is
* protected by the actual upgrade method being synchronized. The LinkedHashMap storage is not removed once upgraded, so
* concurrent reads that have started reading from the LinkedHashMap can continue to read even during an upgrade. We do
Expand All @@ -49,10 +49,10 @@ public class DynamicModel extends AbstractSet<Statement> implements Model {

private static final Resource[] NULL_CTX = new Resource[] { null };

private Map<Statement, Statement> statements = new LinkedHashMap<>();
private volatile Map<Statement, Statement> statements = new LinkedHashMap<>();
final Set<Namespace> namespaces = new LinkedHashSet<>();

volatile private Model model = null;
private volatile Model model = null;

private final ModelFactory modelFactory;

Expand Down Expand Up @@ -115,11 +115,15 @@ public boolean add(Resource subj, IRI pred, Value obj, Resource... contexts) {
}

if (model == null) {
Map<Statement, Statement> statements = this.statements;
if (statements == null) {
throw new ConcurrentModificationException();
}

boolean added = false;
for (Resource context : contexts) {
Statement statement = SimpleValueFactory.getInstance().createStatement(subj, pred, obj, context);
added = added
| statements.put(statement, statement) == null;
added = added | statements.put(statement, statement) == null;
}
return added;
} else {
Expand All @@ -140,6 +144,11 @@ public boolean remove(Resource subj, IRI pred, Value obj, Resource... contexts)
}

if (model == null) {
Map<Statement, Statement> statements = this.statements;
if (statements == null) {
throw new ConcurrentModificationException();
}

boolean removed = false;
for (Resource context : contexts) {
removed = removed
Expand Down Expand Up @@ -185,6 +194,11 @@ public Set<Resource> contexts() {
@Override
public int size() {
if (model == null) {
Map<Statement, Statement> statements = this.statements;
if (statements == null) {
throw new ConcurrentModificationException();
}

return statements.size();
}
return model.size();
Expand All @@ -193,6 +207,11 @@ public int size() {
@Override
public boolean isEmpty() {
if (model == null) {
Map<Statement, Statement> statements = this.statements;
if (statements == null) {
throw new ConcurrentModificationException();
}

return statements.isEmpty();
}
return model.isEmpty();
Expand All @@ -201,6 +220,11 @@ public boolean isEmpty() {
@Override
public boolean contains(Object o) {
if (model == null) {
Map<Statement, Statement> statements = this.statements;
if (statements == null) {
throw new ConcurrentModificationException();
}

return statements.containsKey(o);
}
return model.contains(o);
Expand All @@ -209,6 +233,10 @@ public boolean contains(Object o) {
@Override
public Iterator<Statement> iterator() {
if (model == null) {
Map<Statement, Statement> statements = this.statements;
if (statements == null) {
throw new ConcurrentModificationException();
}
return statements.values().iterator();
}

Expand All @@ -218,6 +246,10 @@ public Iterator<Statement> iterator() {
@Override
public Object[] toArray() {
if (model == null) {
Map<Statement, Statement> statements = this.statements;
if (statements == null) {
throw new ConcurrentModificationException();
}
return statements.values().toArray();
}
return model.toArray();
Expand All @@ -226,6 +258,10 @@ public Object[] toArray() {
@Override
public <T> T[] toArray(T[] a) {
if (model == null) {
Map<Statement, Statement> statements = this.statements;
if (statements == null) {
throw new ConcurrentModificationException();
}
return statements.values().toArray(a);
}
return model.toArray(a);
Expand All @@ -235,6 +271,10 @@ public <T> T[] toArray(T[] a) {
public boolean add(Statement statement) {
Objects.requireNonNull(statement);
if (model == null) {
Map<Statement, Statement> statements = this.statements;
if (statements == null) {
throw new ConcurrentModificationException();
}
return statements.put(statement, statement) == null;
}
return model.add(statement);
Expand All @@ -244,6 +284,10 @@ public boolean add(Statement statement) {
public boolean remove(Object o) {
Objects.requireNonNull(o);
if (model == null) {
Map<Statement, Statement> statements = this.statements;
if (statements == null) {
throw new ConcurrentModificationException();
}
return statements.remove(o) != null;
}
return model.remove(o);
Expand All @@ -253,6 +297,10 @@ public boolean remove(Object o) {
public boolean containsAll(Collection<?> c) {
Objects.requireNonNull(c);
if (model == null) {
Map<Statement, Statement> statements = this.statements;
if (statements == null) {
throw new ConcurrentModificationException();
}
return statements.keySet().containsAll(c);
}
return model.containsAll(c);
Expand All @@ -264,6 +312,10 @@ public boolean addAll(Collection<? extends Statement> c) {
if (model == null) {
return c.stream()
.map(s -> {
Map<Statement, Statement> statements = this.statements;
if (statements == null) {
throw new ConcurrentModificationException();
}
Objects.requireNonNull(s);
return statements.put(s, s) == null;
})
Expand All @@ -276,6 +328,10 @@ public boolean addAll(Collection<? extends Statement> c) {
@Override
public boolean retainAll(Collection<?> c) {
if (model == null) {
Map<Statement, Statement> statements = this.statements;
if (statements == null) {
throw new ConcurrentModificationException();
}
return statements.keySet().retainAll(c);
}
return model.retainAll(c);
Expand All @@ -284,6 +340,10 @@ public boolean retainAll(Collection<?> c) {
@Override
public boolean removeAll(Collection<?> c) {
if (model == null) {
Map<Statement, Statement> statements = this.statements;
if (statements == null) {
throw new ConcurrentModificationException();
}
return c
.stream()
.map(statements::remove)
Expand All @@ -297,6 +357,10 @@ public boolean removeAll(Collection<?> c) {
@Override
public void clear() {
if (model == null) {
Map<Statement, Statement> statements = this.statements;
if (statements == null) {
throw new ConcurrentModificationException();
}
statements.clear();
} else {
model.clear();
Expand All @@ -307,6 +371,11 @@ public void clear() {
public Iterable<Statement> getStatements(Resource subject, IRI predicate, Value object, Resource... contexts) {
if (model == null && subject != null && predicate != null && object != null && contexts != null
&& contexts.length == 1) {
Map<Statement, Statement> statements = this.statements;
if (statements == null) {
throw new ConcurrentModificationException();
}

Statement statement = SimpleValueFactory.getInstance()
.createStatement(subject, predicate, object, contexts[0]);
Statement foundStatement = statements.get(statement);
Expand All @@ -331,12 +400,11 @@ private void upgrade() {

synchronized private void synchronizedUpgrade() {
if (model == null) {
// make statements unmodifiable first, to increase chance of an early failure if the user is doing
// concurrent write with reads
statements = Collections.unmodifiableMap(statements);
Model tempModel = modelFactory.createEmptyModel();
tempModel.addAll(statements.values());
model = tempModel;
// Increase the chance of detecting if the user is doing concurrent write with reads by setting the
// statements field to null while we upgrade.
Map<Statement, Statement> statements = this.statements;
this.statements = null;
model = modelFactory.createFrom(statements.values());
}
}

Expand Down
1 change: 0 additions & 1 deletion core/model/src/main/java/org/eclipse/rdf4j/model/impl/DynamicModelFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@

/**
* Creates {@link DynamicModel}.
*
*/
public class DynamicModelFactory implements ModelFactory {

Expand Down
Loading