Skip to content

Commit 0aa8995

Browse files
committed
perf: usage of custom collections/maps in hot paths
Fixed issue #3965
1 parent 1d6d683 commit 0aa8995

17 files changed

Lines changed: 1124 additions & 114 deletions

engine/src/main/java/com/arcadedb/database/LocalTransactionExplicitLock.java

Lines changed: 19 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -23,18 +23,18 @@
2323
import com.arcadedb.index.IndexInternal;
2424
import com.arcadedb.log.LogManager;
2525
import com.arcadedb.schema.DocumentType;
26+
import com.arcadedb.utility.IntHashSet;
2627

27-
import java.util.*;
2828
import java.util.logging.Level;
2929

3030
/**
3131
* Explicit lock on a transaction to lock buckets and types in pessimistic way. This avoids the retry mechanism of default implicit locking.
3232
*
33-
* @author Luca Garulli (l.garulli@arcadedata.com)
33+
* @author Luca Garulli (l.garulli--(at)--arcadedata.com)
3434
*/
3535
public class LocalTransactionExplicitLock implements TransactionExplicitLock {
3636
private final TransactionContext transactionContext;
37-
private final Set<Integer> filesToLock = new HashSet<>();
37+
private final IntHashSet filesToLock = new IntHashSet();
3838

3939
public LocalTransactionExplicitLock(final TransactionContext transactionContext) {
4040
this.transactionContext = transactionContext;
@@ -43,15 +43,13 @@ public LocalTransactionExplicitLock(final TransactionContext transactionContext)
4343
@Override
4444
public LocalTransactionExplicitLock bucket(final String bucketName) {
4545
final Bucket bucket = transactionContext.getDatabase().getSchema().getBucketByName(bucketName);
46-
filesToLock.add(bucket.getFileId());
46+
addNonNegative(bucket.getFileId());
4747
final DocumentType associatedType = transactionContext.getDatabase().getSchema().getInvolvedTypeByBucketId(bucket.getFileId());
4848
if (associatedType != null)
49-
filesToLock.addAll(associatedType.getAllIndexes(true).stream()
50-
.flatMap(i -> Arrays.stream(i.getIndexesOnBuckets()))
51-
.map(b -> b.getFileId())
52-
.toList());
49+
for (final var typeIndex : associatedType.getAllIndexes(true))
50+
for (final IndexInternal idx : typeIndex.getIndexesOnBuckets())
51+
addNonNegative(idx.getFileId());
5352

54-
filesToLock.removeIf((f) -> f < 0); // Remove negative file IDs (e.g., for virtual buckets)
5553
return this;
5654
}
5755

@@ -60,19 +58,18 @@ public LocalTransactionExplicitLock type(final String typeName) {
6058
final DocumentType type = transactionContext.getDatabase().getSchema().getType(typeName);
6159

6260
// Lock all indexes for this type
63-
filesToLock.addAll(type.getAllIndexes(true).stream()
64-
.flatMap(i -> Arrays.stream(i.getIndexesOnBuckets()))
65-
.map(IndexInternal::getFileId)
66-
.toList());
61+
for (final var typeIndex : type.getAllIndexes(true))
62+
for (final IndexInternal idx : typeIndex.getIndexesOnBuckets())
63+
addNonNegative(idx.getFileId());
6764

6865
// Lock all currently involved buckets for this type
69-
filesToLock.addAll(type.getInvolvedBuckets().stream().map(b -> b.getFileId()).toList());
66+
for (final Bucket b : type.getInvolvedBuckets())
67+
addNonNegative(b.getFileId());
7068

7169
// COMPREHENSIVE BUCKET LOCKING: Also lock all polymorphic buckets (includes subtypes)
7270
// This helps handle cases where records might be created in buckets not initially involved
73-
filesToLock.addAll(type.getBuckets(true).stream().map(b -> b.getFileId()).toList());
74-
75-
filesToLock.removeIf((f) -> f < 0); // Remove negative file IDs (e.g., for virtual buckets)
71+
for (final Bucket b : type.getBuckets(true))
72+
addNonNegative(b.getFileId());
7673

7774
LogManager.instance().log(this, Level.FINE,
7875
"Explicit lock for type '%s' will lock %d bucket files (threadId=%d)",
@@ -85,4 +82,9 @@ public LocalTransactionExplicitLock type(final String typeName) {
8582
public void lock() {
8683
transactionContext.explicitLock(filesToLock);
8784
}
85+
86+
private void addNonNegative(final int fileId) {
87+
if (fileId >= 0)
88+
filesToLock.add(fileId);
89+
}
8890
}

engine/src/main/java/com/arcadedb/database/TransactionContext.java

Lines changed: 37 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
import com.arcadedb.index.lsm.LSMTreeIndexAbstract;
3939
import com.arcadedb.log.LogManager;
4040
import com.arcadedb.schema.LocalSchema;
41+
import com.arcadedb.utility.IntHashSet;
4142
import com.arcadedb.utility.RidHashSet;
4243

4344
import java.io.*;
@@ -545,8 +546,11 @@ public void commitFromReplica(final WALFile.WALTransaction buffer,
545546
}
546547

547548
try {
548-
// LOCK FILES (IN ORDER, SO TO AVOID DEADLOCK)
549-
final Set<Integer> modifiedFiles = new HashSet<>();
549+
// LOCK FILES (IN ORDER, SO TO AVOID DEADLOCK).
550+
// IntHashSet (zero-boxing) is used because this set is built on every commit
551+
// and the small Integer boxing churn shows up in young-gen GC pressure under
552+
// high transaction throughput.
553+
final IntHashSet modifiedFiles = new IntHashSet(buffer.pages.length + 4);
550554

551555
for (final WALFile.WALPage p : buffer.pages)
552556
modifiedFiles.add(p.fileId);
@@ -620,7 +624,7 @@ public TransactionPhase1 commit1stPhase(final boolean isLeader) {
620624
((LocalBucket) database.getSchema().getBucketById(rid.getBucketId())).fetchPageInTransaction(rid);
621625
}
622626

623-
final Set<Integer> modifiedFiles = lockFilesFromChanges();
627+
final IntHashSet modifiedFiles = lockFilesFromChanges();
624628

625629
if (explicitLockedFiles != null)
626630
checkExplicitLocks(modifiedFiles);
@@ -866,7 +870,7 @@ public void setStatus(final STATUS status) {
866870
this.status = status;
867871
}
868872

869-
protected void explicitLock(final Set<Integer> filesToLock) {
873+
protected void explicitLock(final IntHashSet filesToLock) {
870874
if (explicitLockedFiles != null)
871875
throw new TransactionException("Explicit lock already acquired");
872876

@@ -879,8 +883,8 @@ protected void explicitLock(final Set<Integer> filesToLock) {
879883
explicitLockedFiles = lockFilesInOrder(filesToLock);
880884
}
881885

882-
private Set<Integer> lockFilesFromChanges() {
883-
final Set<Integer> modifiedFiles = new HashSet<>();
886+
private IntHashSet lockFilesFromChanges() {
887+
final IntHashSet modifiedFiles = new IntHashSet(modifiedPages.size() + 16);
884888

885889
for (final PageId p : modifiedPages.keySet())
886890
modifiedFiles.add(p.getFileId());
@@ -890,15 +894,16 @@ private Set<Integer> lockFilesFromChanges() {
890894

891895
indexChanges.addFilesToLock(modifiedFiles);
892896

893-
modifiedFiles.addAll(newPageCounters.keySet());
897+
for (final Integer fid : newPageCounters.keySet())
898+
modifiedFiles.add(fid);
894899

895900
return modifiedFiles;
896901
}
897902

898-
private List<Integer> lockFilesInOrder(final Set<Integer> files) {
903+
private List<Integer> lockFilesInOrder(final IntHashSet files) {
899904
final long timeout = database.getConfiguration().getValueAsLong(GlobalConfiguration.COMMIT_LOCK_TIMEOUT);
900905

901-
final List<Integer> locked = database.getTransactionManager().tryLockFiles(files, timeout, getRequester());
906+
final List<Integer> locked = database.getTransactionManager().tryLockFiles(files.toArray(), timeout, getRequester());
902907

903908
// CHECK IF ALL THE LOCKED FILES STILL EXIST. FILE MISSING CAN HAPPEN IN CASE OF INDEX COMPACTION OR DROP OF A BUCKET OR AN INDEX
904909
for (Integer f : locked)
@@ -915,9 +920,14 @@ private List<Integer> lockFilesInOrder(final Set<Integer> files) {
915920
return locked;
916921
}
917922

918-
private void checkExplicitLocks(final Set<Integer> modifiedFiles) {
919-
// CHECK THE LOCKED FILES ARE ALL LOCKED ALREADY
920-
if (!explicitLockedFiles.containsAll(modifiedFiles)) {
923+
private void checkExplicitLocks(final IntHashSet modifiedFiles) {
924+
// CHECK THE LOCKED FILES ARE ALL LOCKED ALREADY: every modified file must already be in explicitLockedFiles.
925+
final boolean[] missing = { false };
926+
modifiedFiles.forEach(fid -> {
927+
if (!missing[0] && !explicitLockedFiles.contains(fid))
928+
missing[0] = true;
929+
});
930+
if (missing[0]) {
921931
boolean anyMigration = false;
922932
// CHECK FOR ANY MIGRATED FILES (INDEX COMPACTION)
923933
final List<Integer> migratedFileIds = new ArrayList<>(explicitLockedFiles.size());
@@ -932,13 +942,23 @@ private void checkExplicitLocks(final Set<Integer> modifiedFiles) {
932942
}
933943
}
934944

935-
if (anyMigration && migratedFileIds.containsAll(modifiedFiles))
936-
// FOUND MIGRATED FILE(S), FORCE THE CLIENT TO RETRY THE OPERATION
937-
throw new ConcurrentModificationException(
938-
"Error on commit transaction: some files have been migrated, please retry the operation");
945+
if (anyMigration) {
946+
// Check if EVERY modifiedFiles entry is in migratedFileIds
947+
final HashSet<Integer> migratedSet = new HashSet<>(migratedFileIds);
948+
final boolean[] allMigrated = { true };
949+
modifiedFiles.forEach(fid -> {
950+
if (allMigrated[0] && !migratedSet.contains(fid))
951+
allMigrated[0] = false;
952+
});
953+
if (allMigrated[0])
954+
// FOUND MIGRATED FILE(S), FORCE THE CLIENT TO RETRY THE OPERATION
955+
throw new ConcurrentModificationException(
956+
"Error on commit transaction: some files have been migrated, please retry the operation");
957+
}
939958

940959
// ERROR: NOT ALL THE MODIFIED FILES ARE LOCKED
941-
final HashSet<Integer> left = new HashSet<>(modifiedFiles);
960+
final HashSet<Integer> left = new HashSet<>(modifiedFiles.size());
961+
modifiedFiles.forEach(left::add);
942962
left.removeAll(explicitLockedFiles);
943963

944964
final Set<String> resourceNames = left.stream().map((fileId -> database.getSchema().getFileById(fileId).getName()))

engine/src/main/java/com/arcadedb/database/TransactionIndexContext.java

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
import com.arcadedb.schema.Schema;
3232
import com.arcadedb.serializer.BinaryComparator;
3333
import com.arcadedb.utility.CollectionUtils;
34+
import com.arcadedb.utility.IntHashSet;
3435

3536
import java.util.*;
3637
import java.util.logging.*;
@@ -244,7 +245,7 @@ else if (key.operation == IndexKey.IndexKeyOperation.REPLACE && key.oldRid != nu
244245
indexEntries.clear();
245246
}
246247

247-
public void addFilesToLock(final Set<Integer> modifiedFiles) {
248+
public void addFilesToLock(final IntHashSet modifiedFiles) {
248249
final Schema schema = database.getSchema();
249250

250251
final Set<Index> lockedIndexes = new HashSet<>(indexEntries.size());
@@ -266,7 +267,8 @@ public void addFilesToLock(final Set<Integer> modifiedFiles) {
266267
// LOCK ALL THE FILES IMPACTED BY THE INDEX KEYS TO CHECK FOR UNIQUE CONSTRAINT
267268
// TODO: OPTIMIZE LOCKING IF STRATEGY IS PARTITIONED: LOCK ONLY THE RELEVANT INDEX
268269
final DocumentType type = schema.getType(index.getTypeName());
269-
modifiedFiles.addAll(type.getBucketIds(false));
270+
for (final int bid : type.getBucketIds(false))
271+
modifiedFiles.add(bid);
270272

271273
for (final TypeIndex typeIndex : type.getAllIndexes(true))
272274
for (final IndexInternal idx : typeIndex.getIndexesOnBuckets())

engine/src/main/java/com/arcadedb/engine/TransactionManager.java

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -517,6 +517,41 @@ else if (lock == LockManager.LOCK_STATUS.NO) {
517517
return lockedFiles;
518518
}
519519

520+
/**
521+
* Primitive-array overload to avoid Integer boxing on the per-commit hot path. The
522+
* passed-in {@code fileIds} array is sorted in place to acquire locks in deadlock-safe order.
523+
*/
524+
public List<Integer> tryLockFiles(final int[] fileIds, final long timeout, final Object requester) {
525+
// ORDER THE FILES TO AVOID DEADLOCK
526+
Arrays.sort(fileIds);
527+
528+
final List<Integer> lockedFiles = new ArrayList<>(fileIds.length);
529+
530+
int attemptFileId = -1;
531+
for (final int fileId : fileIds) {
532+
attemptFileId = fileId;
533+
534+
final LockManager.LOCK_STATUS lock = tryLockFile(fileId, timeout, requester);
535+
536+
if (lock == LockManager.LOCK_STATUS.YES)
537+
lockedFiles.add(fileId);
538+
else if (lock == LockManager.LOCK_STATUS.NO) {
539+
// ERROR: UNLOCK LOCKED FILES
540+
unlockFilesInOrder(lockedFiles, requester);
541+
542+
throw new TimeoutException(
543+
"Timeout on locking file " + attemptFileId + " (" + database.getFileManager().getFile(attemptFileId).getFileName()
544+
+ ") during commit (fileIds=" + Arrays.toString(fileIds) + ", timeout=" + timeout + "ms");
545+
}
546+
}
547+
548+
// OK: ALL LOCKED
549+
if (LogManager.instance().isDebugEnabled())
550+
LogManager.instance().log(this, Level.FINE, "Locked files %s (threadId=%d)", null, Arrays.toString(fileIds),
551+
Thread.currentThread().threadId());
552+
return lockedFiles;
553+
}
554+
520555
public void unlockFilesInOrder(final List<Integer> lockedFileIds, final Object requester) {
521556
if (lockedFileIds != null && !lockedFileIds.isEmpty()) {
522557
for (final Integer fileId : lockedFileIds)

engine/src/main/java/com/arcadedb/function/sql/vector/SQLFunctionVectorNeighbors.java

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
import com.arcadedb.query.sql.executor.CommandContext;
3434
import com.arcadedb.query.sql.executor.ResultSet;
3535
import com.arcadedb.schema.DocumentType;
36+
import com.arcadedb.utility.IntHashSet;
3637
import com.arcadedb.utility.Pair;
3738

3839
import java.util.*;
@@ -113,8 +114,10 @@ public Object execute(final Object self, final Identifiable currentRecord, final
113114
"No vector index found on property '" + propertyName + "' for type '" + specifiedTypeName + "' or its parent types");
114115
}
115116

116-
// Get the bucket IDs that belong to the specified type (not polymorphic - just this type's own buckets)
117-
final Set<Integer> allowedBucketIds = new HashSet<>();
117+
// Get the bucket IDs that belong to the specified type (not polymorphic - just this type's own buckets).
118+
// IntHashSet is zero-boxing - the contains() in executeWithTypeIndex runs once per bucket index per
119+
// vector query, and avoiding Integer boxing on every probe matters under sustained ANN workloads.
120+
final IntHashSet allowedBucketIds = new IntHashSet();
118121
for (final Bucket bucket : specifiedType.getBuckets(false)) {
119122
allowedBucketIds.add(bucket.getFileId());
120123
}
@@ -144,7 +147,7 @@ else if (item instanceof String s)
144147
return out;
145148
}
146149

147-
private Object executeWithTypeIndex(final TypeIndex typeIndex, final Set<Integer> allowedBucketIds, final Object key,
150+
private Object executeWithTypeIndex(final TypeIndex typeIndex, final IntHashSet allowedBucketIds, final Object key,
148151
final int limit, final int efSearch, final Set<RID> allowedRIDs, final CommandContext context) {
149152
final var bucketIndexes = typeIndex.getIndexesOnBuckets();
150153
if (bucketIndexes == null || bucketIndexes.length == 0) {

engine/src/main/java/com/arcadedb/graph/GraphBatch.java

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
import com.arcadedb.schema.Property;
3535
import com.arcadedb.serializer.BinarySerializer;
3636
import com.arcadedb.serializer.BinaryTypes;
37+
import com.arcadedb.utility.LongHashSet;
3738

3839
import java.util.ArrayList;
3940
import java.util.Arrays;
@@ -755,15 +756,17 @@ private void batchUpdateVertexHeadChunks() {
755756

756757
final long startNs = System.nanoTime();
757758

758-
// Collect all vertex keys that need updating
759-
final Set<Long> allKeys = new HashSet<>(deferredOutHead.keySet());
760-
allKeys.addAll(deferredInHead.keySet());
759+
// Collect all vertex keys that need updating. Using LongHashSet (zero-boxing,
760+
// open-addressing) instead of HashSet<Long> avoids ~70 bytes/entry of overhead
761+
// and Long boxing on every addAll, which matters during 100K+ entry bulk loads.
762+
final LongHashSet allKeys = new LongHashSet(deferredOutHead.size() + deferredInHead.size());
763+
for (final Long k : deferredOutHead.keySet())
764+
allKeys.add(k);
765+
for (final Long k : deferredInHead.keySet())
766+
allKeys.add(k);
761767

762768
// Sort by vertex key for page locality
763-
final long[] sortedKeys = new long[allKeys.size()];
764-
int ki = 0;
765-
for (final long key : allKeys)
766-
sortedKeys[ki++] = key;
769+
final long[] sortedKeys = allKeys.toArray();
767770
Arrays.parallelSort(sortedKeys);
768771

769772
beginTx();

0 commit comments

Comments
 (0)