Skip to content

Commit

Permalink
perf: improve compareTo of LabelMultisetEntryList
Browse files Browse the repository at this point in the history
refactor: Comparable<LabelMultisetEntryList> organization

the `put` operation of `TreeMap` is expensive, since `compareTo` is expensive. So we want to reduce the number of calls to this. To that end we don't want to check equality BEFORE calling `put` since both will calculate `compareTo`. So we `putIfAbsent` on the list. However, the list at this point is a reference that can change, so we need to copy the data. But we don't want to always copy, since most lists (in most cases) are not unique, and we only need a single copy to reference in the `TreeMap`. So only copy the data `via addAll` when the list is a newly added one.

In addition, move the `Comparable` implementation to a package-private subclass that also reuses a single ref from construction to do the comparison.

smarter way of detecting when all entries are empty (`size()` was expensive, since it requires iterating over all entries)
  • Loading branch information
cmhulbert committed Sep 19, 2024
1 parent 7cae5c2 commit a36f2d8
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 47 deletions.
31 changes: 4 additions & 27 deletions src/main/java/net/imglib2/type/label/LabelMultisetEntryList.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
import net.imglib2.type.label.LabelMultisetType.Entry;
import org.apache.commons.lang3.builder.HashCodeBuilder;

import java.util.Arrays;
import java.util.Collection;
import java.util.Iterator;
import java.util.Set;
Expand All @@ -13,7 +12,7 @@
* all add operations should insert appropriately to remain sorted. If `sortByCount` is ever called (or the order is manually changed)
* it is the developers responsibility to `sortById()` prior to any additional calls.
*/
public class LabelMultisetEntryList extends MappedObjectArrayList<LabelMultisetEntry, LongMappedAccess> implements Comparable<LabelMultisetEntryList> {
public class LabelMultisetEntryList extends MappedObjectArrayList<LabelMultisetEntry, LongMappedAccess> {

public LabelMultisetEntryList() {

Expand Down Expand Up @@ -43,7 +42,9 @@ public boolean equals(Object o) {
if (!(o instanceof LabelMultisetEntryList)) return false;
final LabelMultisetEntryList other = (LabelMultisetEntryList)o;
if (other.size() != size()) return false;
return compareTo(other) == 0;
final ComparableLabelMultisetEntryList thisComparable = new ComparableLabelMultisetEntryList(this);
final ComparableLabelMultisetEntryList otherComparable = new ComparableLabelMultisetEntryList(other);
return thisComparable.compareTo(otherComparable) == 0;
}

public LabelMultisetEntryList(final int capacity) {
Expand Down Expand Up @@ -394,28 +395,4 @@ public void mergeWith(final Set<Entry<Label>> entrySet) {
}
releaseRef(e1);
}

@Override public int compareTo(LabelMultisetEntryList o) {

final int size = size();

final int sizeCompare = Integer.compare(size, o.size());
if (sizeCompare != 0) return sizeCompare;

final RefIterator<LabelMultisetEntry> thisIter = iterator();
final RefIterator<LabelMultisetEntry> otherIter = o.iterator();

for (int i = 0; i < size; i++) {
final LabelMultisetEntry thisNext = thisIter.next();
final LabelMultisetEntry otherNext = otherIter.next();

final int idCompare = Long.compare(thisNext.getId(), otherNext.getId());
if (idCompare != 0) return idCompare;

final int countCompare = Long.compare(thisNext.getCount(), otherNext.getCount());
if (countCompare != 0) return countCompare;
}

return 0;
}
}
7 changes: 4 additions & 3 deletions src/main/java/net/imglib2/type/label/LabelMultisetType.java
Original file line number Diff line number Diff line change
Expand Up @@ -287,8 +287,7 @@ public LabelMultisetType copy() {
access.isValid(),
access.argMaxCopy());
/* get a new type instance */
final LabelMultisetType that = new LabelMultisetType(null, accessCopy);
return that;
return new LabelMultisetType(null, accessCopy);
}
}

Expand Down Expand Up @@ -709,7 +708,9 @@ public int compareTo(final LabelMultisetType arg0) {
final int countCompare = Long.compare(count(ours), count(theirs));
if (countCompare != 0) return countCompare;

return labelMultisetEntries().compareTo(arg0.labelMultisetEntries());
final ComparableLabelMultisetEntryList thisComparable = new ComparableLabelMultisetEntryList(labelMultisetEntries());
final ComparableLabelMultisetEntryList otherComparable = new ComparableLabelMultisetEntryList(arg0.labelMultisetEntries());
return thisComparable.compareTo(otherComparable);
}

@Override
Expand Down
37 changes: 20 additions & 17 deletions src/main/java/net/imglib2/type/label/LabelUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,13 @@
import java.nio.ByteBuffer;
import java.nio.ByteOrder;
import java.util.Collection;
import java.util.Set;
import java.util.TreeMap;

import static gnu.trove.impl.Constants.DEFAULT_CAPACITY;
import static gnu.trove.impl.Constants.DEFAULT_LOAD_FACTOR;
import static net.imglib2.type.label.AbstractLabelMultisetLoader.argMaxListSizeInBytes;
import static net.imglib2.type.label.AbstractLabelMultisetLoader.listOffsetsSizeInBytes;
import static net.imglib2.type.label.ByteUtils.INT_SIZE;

public class LabelUtils {

Expand Down Expand Up @@ -51,44 +51,47 @@ public static byte[] serializeLabelMultisetTypes(

final LabelMultisetEntry entryReference = new LabelMultisetEntry(0, 1);

final int argMaxSize = INT_SIZE; // in reality, the int value is always `0`, with size of 4 bytes
final int offsetListSize = INT_SIZE * numElements;
final ByteArrayOutputStream dataBuffer = new ByteArrayOutputStream();
/* No longer serialized out ArgMax; we do this by specifying the ArgMax size as 0;
* It's now calculated during deserializtaion instead.*/
writeInt(dataBuffer, 0, ByteOrder.BIG_ENDIAN);

final TreeMap<LabelMultisetEntryList, Integer> listOffsets = new TreeMap<>();
final TreeMap<ComparableLabelMultisetEntryList, Integer> listOffsets = new TreeMap<>();

int nextListOffset = 0;

int nonEmptyListCount = 0;
boolean allListsEmpty = true;

final ByteArrayOutputStream entryList = new ByteArrayOutputStream();
ComparableLabelMultisetEntryList listRef = new ComparableLabelMultisetEntryList();
for (final LabelMultisetType lmt : lmts) {

// It's not isEmpty() and size() are not equivalent in this case.
// size() refers to the sum of the count() of each entry.
// entries can have a count of `0`
//noinspection SizeReplaceableByIsEmpty
if (!(lmt.isEmpty() || (lmt.size() == 0) ))
nonEmptyListCount++;
final Integer listOffset = listOffsets.get(lmt.labelMultisetEntries());
lmt.getAccess().getValue(lmt.index().get(), listRef);
final Integer listOffset = listOffsets.putIfAbsent(listRef, nextListOffset);
if (listOffset != null) {
writeInt(dataBuffer, listOffset, ByteOrder.BIG_ENDIAN);
} else {
writeInt(entryList, lmt.entrySet().size(), ByteOrder.LITTLE_ENDIAN);
for (final Entry<Label> entry : lmt.entrySetWithRef(entryReference)) {
/* deep copy the list (in-place) ONLY if we are a new list just added to the treeView */
final ComparableLabelMultisetEntryList swap = new ComparableLabelMultisetEntryList();
swap.addAll(listRef);
listRef.referToDataAt(swap.data, swap.getBaseOffset());
listRef = swap;

final Set<Entry<Label>> entries = lmt.entrySetWithRef(entryReference);
writeInt(entryList, entries.size(), ByteOrder.LITTLE_ENDIAN);
for (final Entry<Label> entry : entries) {
writeLong(entryList, entry.getElement().id(), ByteOrder.LITTLE_ENDIAN);
writeInt(entryList, entry.getCount(), ByteOrder.LITTLE_ENDIAN);
final int count = entry.getCount();
writeInt(entryList, count, ByteOrder.LITTLE_ENDIAN);

allListsEmpty = allListsEmpty && count == 0;
}
listOffsets.put(lmt.copy().labelMultisetEntries(), nextListOffset);
writeInt(dataBuffer, nextListOffset, ByteOrder.BIG_ENDIAN);
nextListOffset = entryList.size(); //Another quirk to maintain size compatibility, see list NOTE above.
}
}

if (nonEmptyListCount == 0)
if (allListsEmpty)
return null;

final byte[] entryListBytes = entryList.toByteArray();
Expand Down

0 comments on commit a36f2d8

Please sign in to comment.