Skip to content

Commit

Permalink
Avoid using ConcurrentSkipListMap.size().
Browse files Browse the repository at this point in the history
The performance of ConcurrentSkipListMap degrades as the size of the map
grows. Favor a volatile integer containing the size of the map for
performance reasons.

Before: Tests run: 1, ..., Time elapsed: 780.74 sec
After:  Tests run: 1, ..., Time elapsed: 48.716 sec
  • Loading branch information
Philip K. Warren committed Aug 19, 2013
1 parent a1e73f5 commit a5579fa
Showing 1 changed file with 11 additions and 7 deletions.
18 changes: 11 additions & 7 deletions src/main/java/com/cscotta/recordinality/Recordinality.java
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ public class Recordinality {
private final AtomicLong cachedMin = new AtomicLong(Long.MIN_VALUE);
private final ConcurrentSkipListMap<Long, Element> kMap =
new ConcurrentSkipListMap<Long, Element>();
private volatile int kMapSize = 0;

/*
* Initializes a new Recordinality instance with a configurable 'k'-size.
Expand Down Expand Up @@ -87,24 +88,27 @@ private boolean insertIfFits(String element) {
long hashedValue = hash.hashString(element).asLong();

// Short-circuit if our k-set is saturated. Common case.
if (hashedValue < cachedMin.get() && kMap.size() >= sampleSize)
if (hashedValue < cachedMin.get() && kMapSize >= sampleSize)
return false;

synchronized (this) {
int mapSize = kMap.size();
assert(mapSize <= sampleSize);
assert(kMapSize <= sampleSize);

if (mapSize < sampleSize || hashedValue >= cachedMin.get()) {
if (kMapSize < sampleSize || hashedValue >= cachedMin.get()) {
Element existing = kMap.get(hashedValue);
if (existing != null) {
existing.count.incrementAndGet();
return false;
} else {
long lowestKey = (mapSize > 0) ? kMap.firstKey() : Long.MIN_VALUE;
if (hashedValue > lowestKey || mapSize < sampleSize) {
long lowestKey = (kMapSize > 0) ? kMap.firstKey() : Long.MIN_VALUE;
if (hashedValue > lowestKey || kMapSize < sampleSize) {
kMap.put(hashedValue, new Element(element));
kMapSize++;
cachedMin.set(lowestKey);
if (kMap.size() > sampleSize) kMap.remove(lowestKey);
if (kMapSize > sampleSize) {
kMap.remove(lowestKey);
kMapSize = sampleSize;
}
return true;
} else {
return false;
Expand Down

1 comment on commit a5579fa

@jdmaturen
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤘

Please sign in to comment.