Skip to content

Commit

Permalink
Replace synchronized in PersistentStringIndexer with `ReentrantLo…
Browse files Browse the repository at this point in the history
…ck`.

The contention on `PersistentStringIndexer` increased a lot when building with many more concurrent actions, with virtual threads.

This CL makes it invisible in the profile.

PiperOrigin-RevId: 698395733
Change-Id: Id0ed988a110cdd3312a07359acf0682f737f3ac9
  • Loading branch information
coeuvre authored and copybara-github committed Nov 20, 2024
1 parent 0dc9bba commit 8a56158
Showing 1 changed file with 40 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.atomic.AtomicReferenceArray;
import java.util.concurrent.locks.ReentrantLock;
import javax.annotation.Nullable;

/**
Expand Down Expand Up @@ -70,6 +71,8 @@ static PersistentStringIndexer create(Path dataPath, Clock clock) throws IOExcep
return new PersistentStringIndexer(stringToInt, intToString);
}

private final ReentrantLock lock = new ReentrantLock();

// These two fields act similarly to a (synchronized) BiMap. Mutating operations are performed in
// synchronized blocks. Reads are done lock-free.
private final PersistentIndexMap stringToInt;
Expand All @@ -82,9 +85,14 @@ private PersistentStringIndexer(
}

@Override
public synchronized void clear() {
stringToInt.clear();
intToString = new AtomicReferenceArray<>(INITIAL_CAPACITY);
public void clear() {
lock.lock();
try {
stringToInt.clear();
intToString = new AtomicReferenceArray<>(INITIAL_CAPACITY);
} finally {
lock.unlock();
}
}

@Override
Expand All @@ -99,7 +107,8 @@ public Integer getOrCreateIndex(String s) {
return i;
}
s = s.intern();
synchronized (this) {
lock.lock();
try {
i = stringToInt.size();
Integer existing = stringToInt.putIfAbsent(s, i);
if (existing != null) {
Expand All @@ -111,6 +120,8 @@ public Integer getOrCreateIndex(String s) {
}
intToString.set(i, s);
return i;
} finally {
lock.unlock();
}
}

Expand Down Expand Up @@ -140,23 +151,38 @@ public String getStringForIndex(Integer i) {
}

/** Saves index data to the file. */
synchronized long save() throws IOException {
return stringToInt.save();
long save() throws IOException {
lock.lock();
try {
return stringToInt.save();
} finally {
lock.unlock();
}
}

/** Flushes the journal. */
synchronized void flush() {
stringToInt.flush();
void flush() {
lock.lock();
try {
stringToInt.flush();
} finally {
lock.unlock();
}
}

@Override
public synchronized String toString() {
StringBuilder builder = new StringBuilder();
builder.append("size = ").append(size()).append("\n");
for (Map.Entry<String, Integer> entry : stringToInt.entrySet()) {
builder.append(entry.getKey()).append(" <==> ").append(entry.getValue()).append("\n");
public String toString() {
lock.lock();
try {
StringBuilder builder = new StringBuilder();
builder.append("size = ").append(size()).append("\n");
for (Map.Entry<String, Integer> entry : stringToInt.entrySet()) {
builder.append(entry.getKey()).append(" <==> ").append(entry.getValue()).append("\n");
}
return builder.toString();
} finally {
lock.unlock();
}
return builder.toString();
}

/**
Expand Down

0 comments on commit 8a56158

Please sign in to comment.