From 6ca79049a0429a6b3df531d275484905e4770d32 Mon Sep 17 00:00:00 2001 From: Andy Seaborne Date: Wed, 1 Aug 2018 21:02:34 +0100 Subject: [PATCH 1/2] Add sync to _close(). Try out auto-strip trailing whitepace. --- .../transaction/DatasetGraphTransaction.java | 36 +++++++++---------- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/jena-tdb/src/main/java/org/apache/jena/tdb/transaction/DatasetGraphTransaction.java b/jena-tdb/src/main/java/org/apache/jena/tdb/transaction/DatasetGraphTransaction.java index 859ca1196d2..2bc1721f147 100644 --- a/jena-tdb/src/main/java/org/apache/jena/tdb/transaction/DatasetGraphTransaction.java +++ b/jena-tdb/src/main/java/org/apache/jena/tdb/transaction/DatasetGraphTransaction.java @@ -39,13 +39,13 @@ /** * A transactional {@code DatasetGraph} that allows one active transaction per thread. - * + * * {@link DatasetGraphTxn} holds the {@link Transaction} object. * - * This is analogous to a "connection" in JDBC. + * This is analogous to a "connection" in JDBC. * It is a holder of a {@link StoreConnection} combined with the machinary from - * {@link DatasetGraphTrackActive}. - * + * {@link DatasetGraphTrackActive}. + * * Not considered to be in the public API. */ @@ -54,7 +54,7 @@ public class DatasetGraphTransaction extends DatasetGraphTrackActive implements * Initially, the app can use this DatasetGraph non-transactionally. But as * soon as it starts a transaction, the dataset can only be used inside * transactions. - * + * * There are two per-thread state variables: txn: ThreadLocalTxn -- the * transactional , one time use dataset isInTransactionB: ThreadLocalBoolean * -- flags true between begin and commit/abort, and end for read @@ -73,7 +73,7 @@ public DatasetGraphTransaction(Location location) { } public DatasetGraphTransaction(StoreConnection sConn) { - this.sConn = sConn; + this.sConn = sConn; } public Location getLocation() { @@ -99,12 +99,12 @@ public DatasetGraphTDB getBaseDatasetGraph() { TxnType txnType = dsgTxn.getTransaction().getTxnType(); Promote mode; switch(txnType) { - case READ : - throw new JenaTransactionException("Attempt to update in a read transaction"); + case READ : + throw new JenaTransactionException("Attempt to update in a read transaction"); case WRITE : // Impossible. We're in read-mode. throw new TDBException("Internal inconsistency: read-mode write transaction"); - case READ_PROMOTE : + case READ_PROMOTE : mode = Promote.ISOLATED; break; case READ_COMMITTED_PROMOTE : @@ -125,7 +125,7 @@ public DatasetGraphTDB getBaseDatasetGraph() { } return super.getW() ; } - + /** Get the current DatasetGraphTDB */ @Override public DatasetGraphTDB get() { @@ -168,13 +168,13 @@ public boolean isInTransaction() { checkNotClosed() ; return inTransaction.get() ; } - + @Override public ReadWrite transactionMode() { checkNotClosed() ; if ( ! isInTransaction() ) return null; - return dsgtxn.get().getTransaction().getTxnMode(); + return dsgtxn.get().getTransaction().getTxnMode(); } @Override @@ -193,7 +193,7 @@ public void syncIfNotTransactional() { if ( !sConn.haveUsedInTransaction() ) sConn.getBaseDataset().sync() ; } - + @Override public Graph getDefaultGraph() { return new GraphTxnTDB(this, null); @@ -203,7 +203,7 @@ public Graph getDefaultGraph() { public Graph getUnionGraph() { return getGraph(Quad.unionGraph); } - + @Override public Graph getGraph(Node graphNode) { return new GraphTxnTDB(this, graphNode); @@ -229,7 +229,7 @@ protected boolean _promote(Promote promoteMode) { dsgtxn.set(dsgTxn2) ; return true; } - + @Override protected void _commit() { checkNotClosed() ; @@ -265,10 +265,10 @@ protected void _end() { @Override public boolean supportsTransactions() { return true ; } - + @Override public boolean supportsTransactionAbort() { return true ; } - + @Override public String toString() { try { @@ -284,7 +284,7 @@ public String toString() { } @Override - protected void _close() { + protected synchronized void _close() { if ( isClosed ) return ; if ( sConn.haveUsedInTransaction() ) { From 8627e79deb4c58337e49be0e524e423b3789b102 Mon Sep 17 00:00:00 2001 From: Andy Seaborne Date: Wed, 1 Aug 2018 21:15:41 +0100 Subject: [PATCH 2/2] JENA-1581: Don't use a global buffer. --- .../java/org/apache/jena/tdb/lib/NodeLib.java | 46 +++++++++++-------- .../tdb/store/nodetable/NodeTableNative.java | 24 ++++------ .../jena/tdb/store/nodetable/NodecSSE.java | 5 +- 3 files changed, 40 insertions(+), 35 deletions(-) diff --git a/jena-tdb/src/main/java/org/apache/jena/tdb/lib/NodeLib.java b/jena-tdb/src/main/java/org/apache/jena/tdb/lib/NodeLib.java index 3925b522310..284f286c4b2 100644 --- a/jena-tdb/src/main/java/org/apache/jena/tdb/lib/NodeLib.java +++ b/jena-tdb/src/main/java/org/apache/jena/tdb/lib/NodeLib.java @@ -48,34 +48,44 @@ public class NodeLib { private static Nodec nodec = new NodecSSE() ; - // Characters in IRIs that are illegal and cause SSE problems, but we wish to keep. - final private static char MarkerChar = '_' ; - final private static char[] invalidIRIChars = { MarkerChar , ' ' } ; - final private static int SIZE = 1024; - // Marshalling space. - // This buffer is used in encodeStore in a single threaded fashion. - // Callers of encodeStore must ensure writing is not concurrent. - final private static ByteBuffer workspace = ByteBuffer.allocate(SIZE); - /** * Encode and write a {@link Node} to the {@link ObjectFile}. Returns the location, * suitable for use with {@link #fetchDecode}. - *

- * Callers must synchronize to ensure writing is not concurrent. */ public static long encodeStore(Node node, ObjectFile file) { + return encodeStore(node, file, null); + } + + /** + * Encode and write a {@link Node} to the {@link ObjectFile}. + * Uses the given {@link ByteBuffer} for encoding space if possible. + * Returns the location, suitable for use with {@link #fetchDecode}. + */ + public static long encodeStore(Node node, ObjectFile file, ByteBuffer bb) { int maxSize = nodec.maxSize(node); - ByteBuffer bb = workspace; - if ( maxSize >= SIZE ) - // Large object. Special buffer. - bb = ByteBuffer.allocate(maxSize); - else - bb.clear(); + if ( bb == null ) + return allocEncodeWrite(node, file, maxSize); + if ( bb.capacity() < maxSize ) + // Buffer may not be big enough. + return allocEncodeWrite(node, file, maxSize); + // Use buffer provided. + bb.clear(); + return encodeWrite(node, file, bb); + } + + /** Encode and write, allocating space as needed */ + private static long allocEncodeWrite(Node node, ObjectFile file, int maxSize) { + ByteBuffer bb = ByteBuffer.allocate(maxSize); + return encodeWrite(node, file, bb); + } + + /** Encode and write, using the space provided which is assumed to be large enough. */ + private static long encodeWrite(Node node, ObjectFile file, ByteBuffer bb) { int len = nodec.encode(node, bb, null); long x = file.write(bb); return x; } - + /** * Read and decode a {@link Node} from the {@link ObjectFile}. The {@code id} must * have originally been generated by {@link #encodeStore}. diff --git a/jena-tdb/src/main/java/org/apache/jena/tdb/store/nodetable/NodeTableNative.java b/jena-tdb/src/main/java/org/apache/jena/tdb/store/nodetable/NodeTableNative.java index 9974c5d997a..2148bbda3cc 100644 --- a/jena-tdb/src/main/java/org/apache/jena/tdb/store/nodetable/NodeTableNative.java +++ b/jena-tdb/src/main/java/org/apache/jena/tdb/store/nodetable/NodeTableNative.java @@ -38,12 +38,9 @@ /** A concrete NodeTable based on native storage (string file and an index) */ public class NodeTableNative implements NodeTable { - // TODO Split into a general accessor (get and put (node,NodeId) pairs) - // Abstracts the getAllocateNodeId requirements. - protected ObjectFile objects ; - protected Index nodeHashToId ; // hash -> int - private boolean syncNeeded = false ; + protected Index nodeHashToId ; // hash -> int + private boolean syncNeeded = false ; // Non-transactional mode sync. // Delayed construction - must call init explicitly. protected NodeTableNative() {} @@ -95,10 +92,10 @@ public boolean containsNodeId(NodeId nodeId) { // accessIndex and readNodeFromTable // Cache around this class further out in NodeTableCache are synchronized - // to maintain cache validatity which indirectly sync access to the NodeTable. + // to maintain cache validity which indirectly sync access to the NodeTable. // But to be sure, we provide MRSW guarantees on this class. // (otherwise if no cache => disaster) - // synchonization happens in accessIndex() and readNodeByNodeId + // Synchronization happens in accessIndex() and readNodeByNodeId() // NodeId to Node worker. private Node _retrieveNodeByNodeId(NodeId id) @@ -120,13 +117,11 @@ private NodeId _idForNode(Node node, boolean allocate) { if ( node == Node.ANY ) return NodeId.NodeIdAny ; - - // synchronized in accessIndex NodeId nodeId = accessIndex(node, allocate) ; return nodeId ; } - protected final NodeId accessIndex(Node node, boolean create) + private final NodeId accessIndex(Node node, boolean create) { Hash hash = new Hash(nodeHashToId.getRecordFactory().keyLength()) ; setHash(hash, node) ; @@ -163,19 +158,20 @@ protected final NodeId accessIndex(Node node, boolean create) } // -------- NodeId<->Node + // Workspace for "normal" sized nodes. + // Null means allocate a fresh buffer each time. + private final ByteBuffer writeBuffer = ByteBuffer.allocate(1024); // Synchronization: // write: in accessIndex // read: synchronized here. // Only places for accessing the StringFile. - + private final NodeId writeNodeToTable(Node node) { syncNeeded = true ; - // Synchronized in accessIndex - long x = NodeLib.encodeStore(node, getObjects()) ; + long x = NodeLib.encodeStore(node, getObjects(), writeBuffer) ; return NodeId.create(x); } - private final Node readNodeFromTable(NodeId id) { diff --git a/jena-tdb/src/main/java/org/apache/jena/tdb/store/nodetable/NodecSSE.java b/jena-tdb/src/main/java/org/apache/jena/tdb/store/nodetable/NodecSSE.java index 1b7febb67e5..3a3284e3975 100644 --- a/jena-tdb/src/main/java/org/apache/jena/tdb/store/nodetable/NodecSSE.java +++ b/jena-tdb/src/main/java/org/apache/jena/tdb/store/nodetable/NodecSSE.java @@ -41,10 +41,9 @@ public class NodecSSE implements Nodec { - private static boolean SafeChars = false ; // Characters in IRIs that are illegal and cause SSE problems, but we wish to keep. - final private static char MarkerChar = '_' ; - final private static char[] invalidIRIChars = { MarkerChar , ' ' } ; + private final static char MarkerChar = '_' ; + private final static char[] invalidIRIChars = { MarkerChar , ' ' } ; public NodecSSE() {}