diff --git a/core/rio/turtle/src/main/java/org/eclipse/rdf4j/rio/turtle/TurtleParser.java b/core/rio/turtle/src/main/java/org/eclipse/rdf4j/rio/turtle/TurtleParser.java index 174c8ab76c6..40695cf58e1 100644 --- a/core/rio/turtle/src/main/java/org/eclipse/rdf4j/rio/turtle/TurtleParser.java +++ b/core/rio/turtle/src/main/java/org/eclipse/rdf4j/rio/turtle/TurtleParser.java @@ -278,12 +278,11 @@ protected void parsePrefixID() throws IOException, RDFParseException, RDFHandler skipWSC(); // Read the namespace URI - IRI namespace = parseURI(); + String namespaceStr = parseURI().toString(); - // Store and report this namespace mapping String prefixStr = prefixID.toString(); - String namespaceStr = namespace.toString(); + // Store and report this namespace mapping setNamespace(prefixStr, namespaceStr); if (rdfHandler != null) { diff --git a/core/sail/memory/src/main/java/org/eclipse/rdf4j/sail/memory/MemorySailStore.java b/core/sail/memory/src/main/java/org/eclipse/rdf4j/sail/memory/MemorySailStore.java index a065726cc80..b7629025d9d 100644 --- a/core/sail/memory/src/main/java/org/eclipse/rdf4j/sail/memory/MemorySailStore.java +++ b/core/sail/memory/src/main/java/org/eclipse/rdf4j/sail/memory/MemorySailStore.java @@ -40,6 +40,8 @@ import org.eclipse.rdf4j.sail.base.SailSink; import org.eclipse.rdf4j.sail.base.SailSource; import org.eclipse.rdf4j.sail.base.SailStore; +import org.eclipse.rdf4j.sail.memory.model.CloseableIterator; +import org.eclipse.rdf4j.sail.memory.model.MemBNode; import org.eclipse.rdf4j.sail.memory.model.MemIRI; import org.eclipse.rdf4j.sail.memory.model.MemResource; import org.eclipse.rdf4j.sail.memory.model.MemStatement; @@ -782,20 +784,25 @@ public CloseableIteration getContextIDs() thr Lock stLock = openStatementsReadLock(); try { - synchronized (valueFactory) { - int snapshot = getCurrentSnapshot(); - for (MemResource memResource : valueFactory.getMemURIs()) { + int snapshot = getCurrentSnapshot(); + try (CloseableIterator memIRIsIterator = valueFactory.getMemIRIsIterator()) { + while (memIRIsIterator.hasNext()) { + MemResource memResource = memIRIsIterator.next(); if (isContextResource(memResource, snapshot)) { contextIDs.add(memResource); } } + } - for (MemResource memResource : valueFactory.getMemBNodes()) { + try (CloseableIterator memBNodesIterator = valueFactory.getMemBNodesIterator()) { + while (memBNodesIterator.hasNext()) { + MemResource memResource = memBNodesIterator.next(); if (isContextResource(memResource, snapshot)) { contextIDs.add(memResource); } } } + } finally { stLock.release(); } diff --git a/core/sail/memory/src/main/java/org/eclipse/rdf4j/sail/memory/model/CloseableIterator.java b/core/sail/memory/src/main/java/org/eclipse/rdf4j/sail/memory/model/CloseableIterator.java new file mode 100644 index 00000000000..c7af55549ee --- /dev/null +++ b/core/sail/memory/src/main/java/org/eclipse/rdf4j/sail/memory/model/CloseableIterator.java @@ -0,0 +1,17 @@ +/******************************************************************************* + * Copyright (c) 2022 Eclipse RDF4J contributors. + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Eclipse Distribution License v1.0 + * which accompanies this distribution, and is available at + * http://www.eclipse.org/org/documents/edl-v10.php. + ******************************************************************************/ + +package org.eclipse.rdf4j.sail.memory.model; + +import java.util.Iterator; + +public interface CloseableIterator extends Iterator, AutoCloseable { + + @Override + void close(); +} diff --git a/core/sail/memory/src/main/java/org/eclipse/rdf4j/sail/memory/model/MemValueFactory.java b/core/sail/memory/src/main/java/org/eclipse/rdf4j/sail/memory/model/MemValueFactory.java index d9ffa6532f6..388dc9c1851 100644 --- a/core/sail/memory/src/main/java/org/eclipse/rdf4j/sail/memory/model/MemValueFactory.java +++ b/core/sail/memory/src/main/java/org/eclipse/rdf4j/sail/memory/model/MemValueFactory.java @@ -22,6 +22,7 @@ import org.eclipse.rdf4j.model.datatypes.XMLDatatypeUtil; import org.eclipse.rdf4j.model.util.Literals; import org.eclipse.rdf4j.model.util.URIUtil; +import org.eclipse.rdf4j.model.util.Values; import org.eclipse.rdf4j.model.vocabulary.XSD; /** @@ -41,7 +42,7 @@ public class MemValueFactory extends AbstractValueFactory { * Registry containing the set of MemURI objects as used by a MemoryStore. This registry enables the reuse of * objects, minimizing the number of objects in main memory. */ - private final WeakObjectRegistry uriRegistry = new WeakObjectRegistry<>(); + private final WeakObjectRegistry iriRegistry = new WeakObjectRegistry<>(); /** * Registry containing the set of MemTriple objects as used by a MemoryStore. This registry enables the reuse of @@ -72,7 +73,7 @@ public class MemValueFactory extends AbstractValueFactory { *---------*/ public void clear() { - uriRegistry.clear(); + iriRegistry.clear(); bnodeRegistry.clear(); literalRegistry.clear(); namespaceRegistry.clear(); @@ -118,18 +119,18 @@ public MemResource getMemResource(Resource resource) { /** * See getMemValue() for description. */ - public synchronized MemIRI getMemURI(IRI uri) { + public MemIRI getMemURI(IRI uri) { if (isOwnMemValue(uri)) { return (MemIRI) uri; } else { - return uriRegistry.get(uri); + return iriRegistry.get(uri); } } /** * See getMemValue() for description. */ - public synchronized MemBNode getMemBNode(BNode bnode) { + public MemBNode getMemBNode(BNode bnode) { if (isOwnMemValue(bnode)) { return (MemBNode) bnode; } else { @@ -140,7 +141,7 @@ public synchronized MemBNode getMemBNode(BNode bnode) { /** * See getMemValue() for description. */ - public synchronized MemLiteral getMemLiteral(Literal literal) { + public MemLiteral getMemLiteral(Literal literal) { if (isOwnMemValue(literal)) { return (MemLiteral) literal; } else { @@ -159,23 +160,25 @@ private boolean isOwnMemValue(Value value) { /** * Gets all URIs that are managed by this value factory. *

- * Warning: This method is not synchronized. To iterate over the returned set in a thread-safe way, this - * method should only be called while synchronizing on this object. + * Warning: This method is not synchronized. * * @return An unmodifiable Set of MemURI objects. + * @deprecated Use getMemIRIsIterator() instead. */ + @Deprecated(forRemoval = true, since = "4.0.0") public Set getMemURIs() { - return Collections.unmodifiableSet(uriRegistry); + return Collections.unmodifiableSet(iriRegistry); } /** * Gets all bnodes that are managed by this value factory. *

- * Warning: This method is not synchronized. To iterate over the returned set in a thread-safe way, this - * method should only be called while synchronizing on this object. + * Warning: This method is not synchronized. * * @return An unmodifiable Set of MemBNode objects. + * @deprecated Use getMemBNodesIterator() instead. */ + @Deprecated(forRemoval = true, since = "4.0.0") public Set getMemBNodes() { return Collections.unmodifiableSet(bnodeRegistry); } @@ -183,15 +186,43 @@ public Set getMemBNodes() { /** * Gets all literals that are managed by this value factory. *

- * Warning: This method is not synchronized. To iterate over the returned set in a thread-safe way, this - * method should only be called while synchronizing on this object. + * Warning: This method is not synchronized. * * @return An unmodifiable Set of MemURI objects. + * @deprecated Use getMemLiteralsIterator() instead. */ + @Deprecated(forRemoval = true, since = "4.0.0") public Set getMemLiterals() { return Collections.unmodifiableSet(literalRegistry); } + /** + * Gets all URIs that are managed by this value factory. + * + * @return An autocloseable iterator. + */ + public CloseableIterator getMemIRIsIterator() { + return iriRegistry.closeableIterator(); + } + + /** + * Gets all bnodes that are managed by this value factory. + * + * @return An autocloseable iterator. + */ + public CloseableIterator getMemBNodesIterator() { + return bnodeRegistry.closeableIterator(); + } + + /** + * Gets all literals that are managed by this value factory. + * + * @return An autocloseable iterator. + */ + public CloseableIterator getMemLiteralsIterator() { + return literalRegistry.closeableIterator(); + } + /** * Gets or creates a MemValue for the supplied Value. If the factory already contains a MemValue object that is * equivalent to the supplied value then this equivalent value will be returned. Otherwise a new MemValue will be @@ -228,13 +259,10 @@ public MemResource getOrCreateMemResource(Resource resource) { /** * See {@link #getOrCreateMemValue(Value)} for description. */ - public synchronized MemIRI getOrCreateMemURI(IRI uri) { - MemIRI memURI = getMemURI(uri); - - if (memURI == null) { - // Namespace strings are relatively large objects and are shared - // between uris + public MemIRI getOrCreateMemURI(IRI uri) { + return iriRegistry.getOrAdd(uri, () -> { String namespace = uri.getNamespace(); + assert namespace != null; String sharedNamespace = namespaceRegistry.get(namespace); if (sharedNamespace == null) { @@ -246,38 +274,26 @@ public synchronized MemIRI getOrCreateMemURI(IRI uri) { } // Create a MemURI and add it to the registry - memURI = new MemIRI(this, namespace, uri.getLocalName()); - boolean wasNew = uriRegistry.add(memURI); - assert wasNew : "Created a duplicate MemURI for URI " + uri; - } + return new MemIRI(this, namespace, uri.getLocalName()); + }); - return memURI; } /** * See {@link #getOrCreateMemValue(Value)} for description. */ - public synchronized MemBNode getOrCreateMemBNode(BNode bnode) { - MemBNode memBNode = getMemBNode(bnode); - - if (memBNode == null) { - memBNode = new MemBNode(this, bnode.getID()); - boolean wasNew = bnodeRegistry.add(memBNode); - assert wasNew : "Created a duplicate MemBNode for bnode " + bnode; - } - - return memBNode; + public MemBNode getOrCreateMemBNode(BNode bnode) { + return bnodeRegistry.getOrAdd(bnode, () -> new MemBNode(this, bnode.getID())); } /** * See {@link #getOrCreateMemValue(Value)} for description. */ - public synchronized MemLiteral getOrCreateMemLiteral(Literal literal) { - MemLiteral memLiteral = getMemLiteral(literal); - - if (memLiteral == null) { + public MemLiteral getOrCreateMemLiteral(Literal literal) { + return literalRegistry.getOrAdd(literal, () -> { String label = literal.getLabel(); IRI datatype = literal.getDatatype(); + MemLiteral memLiteral; if (Literals.isLanguageLiteral(literal)) { memLiteral = new MemLiteral(this, label, literal.getLanguage().get()); @@ -306,98 +322,114 @@ public synchronized MemLiteral getOrCreateMemLiteral(Literal literal) { } } - boolean wasNew = literalRegistry.add(memLiteral); - assert wasNew : "Created a duplicate MemLiteral for literal " + literal; - } - - return memLiteral; + return memLiteral; + }); } @Override - public synchronized IRI createIRI(String uri) { + public IRI createIRI(String uri) { return getOrCreateMemURI(super.createIRI(uri)); } @Override - public synchronized IRI createIRI(String namespace, String localName) { - IRI tempURI = null; + public IRI createIRI(String namespace, String localName) { + return iriRegistry.getOrAdd(Values.iri(namespace, localName), () -> { - // Reuse supplied namespace and local name strings if possible - if (URIUtil.isCorrectURISplit(namespace, localName)) { if (namespace.indexOf(':') == -1) { throw new IllegalArgumentException("Not a valid (absolute) URI: " + namespace + localName); } - tempURI = new MemIRI(null, namespace, localName); - } else { - tempURI = super.createIRI(namespace + localName); - } + String correctNamespace; + String correctLocalName; + + if (!URIUtil.isCorrectURISplit(namespace, localName)) { + IRI iri = super.createIRI(namespace + localName); + correctNamespace = iri.getNamespace(); + correctLocalName = iri.getLocalName(); + + } else { + correctNamespace = namespace; + correctLocalName = localName; + } + + String sharedNamespace = namespaceRegistry.get(correctNamespace); + + if (sharedNamespace == null) { + // New namespace, add it to the registry + namespaceRegistry.add(correctNamespace); + sharedNamespace = correctNamespace; + } else { + // Use the shared namespace + sharedNamespace = correctNamespace; + } + + // Create a MemURI and add it to the registry + return new MemIRI(this, sharedNamespace, correctLocalName); + + }); - return getOrCreateMemURI(tempURI); } @Override - public synchronized BNode createBNode(String nodeID) { + public BNode createBNode(String nodeID) { return getOrCreateMemBNode(super.createBNode(nodeID)); } @Override - public synchronized Literal createLiteral(String value) { + public Literal createLiteral(String value) { return getOrCreateMemLiteral(super.createLiteral(value)); } @Override - public synchronized Literal createLiteral(String value, String language) { + public Literal createLiteral(String value, String language) { return getOrCreateMemLiteral(super.createLiteral(value, language)); } @Override - public synchronized Literal createLiteral(String value, IRI datatype) { + public Literal createLiteral(String value, IRI datatype) { return getOrCreateMemLiteral(super.createLiteral(value, datatype)); } @Override - public synchronized Literal createLiteral(boolean value) { + public Literal createLiteral(boolean value) { MemLiteral newLiteral = new BooleanMemLiteral(this, value); return getSharedLiteral(newLiteral); } @Override - public synchronized Literal createLiteral(XMLGregorianCalendar calendar) { + public Literal createLiteral(XMLGregorianCalendar calendar) { MemLiteral newLiteral = new CalendarMemLiteral(this, calendar); return getSharedLiteral(newLiteral); } private Literal getSharedLiteral(MemLiteral newLiteral) { - MemLiteral sharedLiteral = literalRegistry.get(newLiteral); - - if (sharedLiteral == null) { - boolean wasNew = literalRegistry.add(newLiteral); - assert wasNew : "Created a duplicate MemLiteral for literal " + newLiteral; - sharedLiteral = newLiteral; - } - - return sharedLiteral; + return literalRegistry.getOrAdd(newLiteral, () -> newLiteral); } /** * See {@link #getOrCreateMemValue(Value)} for description. */ - private synchronized MemTriple getOrCreateMemTriple(Triple triple) { + private MemTriple getOrCreateMemTriple(Triple triple) { MemTriple memTriple = getMemTriple(triple); if (memTriple == null) { // Create a MemTriple and add it to the registry - memTriple = new MemTriple(this, getOrCreateMemResource(triple - .getSubject()), + MemTriple newMemTriple = new MemTriple(this, getOrCreateMemResource(triple.getSubject()), getOrCreateMemURI(triple.getPredicate()), getOrCreateMemValue(triple.getObject())); - boolean wasNew = tripleRegistry.add(memTriple); + boolean wasNew = tripleRegistry.add(newMemTriple); + + if (!wasNew) { + return tripleRegistry.getOrAdd(triple, () -> newMemTriple); + } else { + return newMemTriple; + } + } else { + return memTriple; } - return memTriple; } - private synchronized MemTriple getMemTriple(Triple triple) { + private MemTriple getMemTriple(Triple triple) { if (isOwnMemValue(triple)) { return (MemTriple) triple; } else { diff --git a/core/sail/memory/src/main/java/org/eclipse/rdf4j/sail/memory/model/WeakObjectRegistry.java b/core/sail/memory/src/main/java/org/eclipse/rdf4j/sail/memory/model/WeakObjectRegistry.java index 4a5c043cdc7..c1f9e8ea5f2 100644 --- a/core/sail/memory/src/main/java/org/eclipse/rdf4j/sail/memory/model/WeakObjectRegistry.java +++ b/core/sail/memory/src/main/java/org/eclipse/rdf4j/sail/memory/model/WeakObjectRegistry.java @@ -9,10 +9,16 @@ import java.lang.ref.WeakReference; import java.util.AbstractSet; +import java.util.Arrays; import java.util.Collection; import java.util.Iterator; import java.util.Map; import java.util.WeakHashMap; +import java.util.concurrent.locks.StampedLock; +import java.util.function.Supplier; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** * An object registry that uses weak references to keep track of the stored objects. The registry can be used to @@ -22,6 +28,8 @@ */ public class WeakObjectRegistry extends AbstractSet { + private static final Logger logger = LoggerFactory.getLogger(WeakObjectRegistry.class); + /*-----------* * Variables * *-----------*/ @@ -29,7 +37,8 @@ public class WeakObjectRegistry extends AbstractSet { /** * The hash map that is used to store the objects. */ - private final Map> objectMap = new WeakHashMap<>(); + private final Map>[] objectMap; + private final StampedLock[] locks; /*--------------* * Constructors * @@ -40,6 +49,17 @@ public class WeakObjectRegistry extends AbstractSet { */ public WeakObjectRegistry() { super(); + int concurrency = Runtime.getRuntime().availableProcessors() * 2; + + objectMap = new WeakHashMap[concurrency]; + for (int i = 0; i < objectMap.length; i++) { + objectMap[i] = new WeakHashMap<>(); + } + + locks = new StampedLock[objectMap.length]; + for (int i = 0; i < locks.length; i++) { + locks[i] = new StampedLock(); + } } /** @@ -64,54 +84,220 @@ public WeakObjectRegistry(Collection c) { * @return A stored object that is equal to the supplied key, or null if no such object was found. */ public E get(Object key) { - WeakReference weakRef = objectMap.get(key); + if (key == null) { + return null; + } + + int index = getIndex(key); + long readLock = locks[index].readLock(); + try { + Map> weakReferenceMap = objectMap[index]; + + WeakReference weakRef = weakReferenceMap.get(key); + if (weakRef != null) { + return weakRef.get(); // may be null + } else { + return null; + } - if (weakRef != null) { - return weakRef.get(); + } finally { + locks[index].unlockRead(readLock); } - return null; + } + + private int getIndex(Object key) { + int i = Math.abs(key.hashCode()); + return i % objectMap.length; + } + + public CloseableIterator closeableIterator() { + return new WeakObjectRegistryIterator<>(objectMap, locks); } @Override public Iterator iterator() { - return objectMap.keySet().iterator(); + logger.warn("This method is not thread safe! Use closeableIterator() instead."); + return new WeakObjectRegistryIterator(objectMap, null); + } + + private static class WeakObjectRegistryIterator implements CloseableIterator { + + private final Iterator>> iterator; + private final StampedLock[] locks; + + Iterator currentIterator; + Long[] readLocks; + boolean init = false; + + public WeakObjectRegistryIterator(Map>[] objectMap, StampedLock[] locks) { + this.iterator = Arrays.asList(objectMap).iterator(); + this.locks = locks; + } + + public void init() { + if (!init) { + init = true; + if (locks != null) { + readLocks = new Long[locks.length]; + for (int i = 0; i < locks.length; i++) { + readLocks[i] = locks[i].readLock(); + } + } + currentIterator = iterator.next().keySet().iterator(); + } + } + + @Override + public boolean hasNext() { + init(); + if (currentIterator == null) { + return false; + } + while (currentIterator != null) { + if (currentIterator.hasNext()) { + return true; + } else { + currentIterator = null; + if (iterator.hasNext()) { + currentIterator = iterator.next().keySet().iterator(); + } + } + } + + return false; + } + + @Override + public E next() { + init(); + return currentIterator.next(); + } + + @Override + public void close() { + if (init) { + if (locks != null) { + for (int i = 0; i < locks.length; i++) { + if (readLocks[i] != 0) { + locks[i].unlockRead(readLocks[i]); + readLocks[i] = 0L; + } + } + } + } + } + } @Override public int size() { - return objectMap.size(); + int size = 0; + for (Map> weakReferenceMap : objectMap) { + size += weakReferenceMap.size(); + } + return size; } @Override - public boolean contains(Object o) { - return get(o) != null; + public boolean contains(Object key) { + return get(key) != null; } @Override public boolean add(E object) { - WeakReference ref = new WeakReference<>(object); + int index = getIndex(object); + long writeLock = locks[index].writeLock(); + try { + Map> weakReferenceMap = objectMap[index]; + WeakReference ref = new WeakReference<>(object); - ref = objectMap.put(object, ref); + ref = weakReferenceMap.put(object, ref); - if (ref != null && ref.get() != null) { - // A duplicate was added which replaced the existing object. Undo this - // operation. - objectMap.put(ref.get(), ref); - return false; + if (ref != null) { + E e = ref.get(); + if (e != null) { + // A duplicate was added which replaced the existing object. Undo this operation. + weakReferenceMap.put(e, ref); + return false; + } + } + + return true; + + } finally { + locks[index].unlockWrite(writeLock); + } + + } + + public E getOrAdd(Object key, Supplier supplier) { + int index = getIndex(key); + Map> weakReferenceMap = objectMap[index]; + + long readLock = locks[index].readLock(); + try { + WeakReference ref = weakReferenceMap.get(key); + if (ref != null) { + E e = ref.get(); + if (e != null) { + // we found the object + return e; + } + } + } finally { + locks[index].unlockRead(readLock); + } + + // we could not find the object, so we will use the supplier to create a new object and add that + long writeLock = locks[index].writeLock(); + try { + E object = supplier.get(); + WeakReference ref = weakReferenceMap.put(object, new WeakReference<>(object)); + if (ref != null) { + E e = ref.get(); + if (e != null) { + // Between releasing the read-lock and acquiring the write-lock another thread put the object in the + // weakReferenceMap. We need to put back the one that was there before and return that one to the + // user. + weakReferenceMap.put(e, ref); + return e; + } + } + assert object != null; + return object; + + } finally { + locks[index].unlockWrite(writeLock); } - return true; } @Override - public boolean remove(Object o) { - WeakReference ref = objectMap.remove(o); - return ref != null && ref.get() != null; + public boolean remove(Object object) { + int index = getIndex(object); + long writeLock = locks[index].writeLock(); + try { + Map> weakReferenceMap = objectMap[index]; + WeakReference ref = weakReferenceMap.remove(object); + return ref != null && ref.get() != null; + + } finally { + locks[index].unlockWrite(writeLock); + } } @Override public void clear() { - objectMap.clear(); + + for (int i = 0; i < objectMap.length; i++) { + long writeLock = locks[i].writeLock(); + try { + objectMap[i].clear(); + } finally { + locks[i].unlockWrite(writeLock); + } + } + } } diff --git a/core/sail/shacl/src/test/java/org/eclipse/rdf4j/sail/shacl/TransactionalIsolationSlowIT.java b/core/sail/shacl/src/test/java/org/eclipse/rdf4j/sail/shacl/TransactionalIsolationSlowIT.java index 61ae4d8ece4..d0069bda71f 100644 --- a/core/sail/shacl/src/test/java/org/eclipse/rdf4j/sail/shacl/TransactionalIsolationSlowIT.java +++ b/core/sail/shacl/src/test/java/org/eclipse/rdf4j/sail/shacl/TransactionalIsolationSlowIT.java @@ -34,9 +34,7 @@ public class TransactionalIsolationSlowIT { @Test public void testIsolation2_multithreaded_READ_COMMITTED() throws Throwable { - for (int i = 0; i < 1000; i++) { - ShaclSail shaclSail = new ShaclSail(new MemoryStore()); SailRepository sailRepository = new SailRepository(shaclSail); @@ -172,9 +170,7 @@ public void testIsolation2_multithreaded_READ_COMMITTED() throws Throwable { @Test public void testIsolation2_multithreaded_SNAPSHOT() throws Throwable { - for (int i = 0; i < 1000; i++) { - ShaclSail shaclSail = new ShaclSail(new MemoryStore()); SailRepository sailRepository = new SailRepository(shaclSail);