Merge pull request #34 from saalfeldlab/perf/memoryImprovements

Perf/memory improvements

src/main/java/net/imglib2/type/label/FromIntegerTypeConverter.java

@@ -28,11 +28,8 @@ public static VolatileLabelMultisetType getAppropriateVolatileType() {
 	@Override
 	public void convert(final I input, final LabelMultisetType output) {
 
+
 		final long newVal = input.getIntegerLong();
-		final long[] data = getListData(output).data;
-		if (ByteUtils.getLong(data, Integer.BYTES) != newVal) {
-			ByteUtils.putLong(newVal, data, Integer.BYTES);
-			output.getAccess().setArgMax(output.getIndex(), newVal);
-		}
+		output.set(newVal, 1);
 	}
 }

src/main/java/net/imglib2/type/label/LabelMultisetEntryList.java

@@ -9,9 +9,9 @@
 import java.util.Set;
 
 /**
- * List of LabelMultisetEntries backed by a LongArray. Should ALWAYS remain sorted by ID. It is sorted after construction, and
+ * List of LabelMultisetEntries backed by a LongArray. Should ALWAYS remain sorted by ID, ascending. It is sorted after construction, and
  * all add operations should insert appropriately to remain sorted. If `sortByCount` is ever called (or the order is manually changed)
- * it is the developers responsiblity to `sortById()` prior to any additional calls.
+ * it is the developers responsibility to `sortById()` prior to any additional calls.
  */
 public class LabelMultisetEntryList extends MappedObjectArrayList<LabelMultisetEntry, LongMappedAccess> {
 
@@ -224,7 +224,10 @@ public void sortByCount() {
 		sort((o1, o2) -> {
 			final int i1 = o1.getCount();
 			final int i2 = o2.getCount();
-			return Integer.compare(i1, i2);
+			final int countCompare = Integer.compare(i1, i2);
+			if (countCompare == 0)
+				return Long.compare(o2.getId(), o1.getId());
+			return countCompare;
 		});
 	}
 

src/main/java/net/imglib2/type/label/LabelMultisetType.java

@@ -90,6 +90,51 @@ public void releaseRef(LabelMultisetEntry ref) {
 					super.releaseRef(ref);
 				}
 			}
+
+			@Override
+			public RefIterator<LabelMultisetEntry> iterator() {
+
+				return new RefIterator<LabelMultisetEntry>() {
+
+					private LabelMultisetEntry ref = reference != null ? reference : createRef();
+
+					private int i = 0;
+
+					@Override
+					public boolean hasNext() {
+
+						if (i < size())
+							return true;
+						else {
+							release();
+							return false;
+						}
+					}
+
+					@Override
+					public LabelMultisetEntry next() {
+
+						return get(i++, ref);
+					}
+
+					@Override
+					public void release() {
+
+						if (reference == null && ref != null) {
+							releaseRef(ref);
+							ref = null;
+						}
+					}
+
+					@Override
+					public void reset() {
+
+						if (reference == null && ref == null)
+							ref = createRef();
+						i = 0;
+					}
+				};
+			}
 		};
 
 		this.img = img;
@@ -120,7 +165,6 @@ public void release() {
 					if (reference == null) {
 						it.release();
 					}
-
 				}
 
 				@Override
@@ -161,11 +205,15 @@ public void add(final long id, final int count) {
 
 	public void add(LabelMultisetEntry entry) {
 
-		final Label id = entry.id;
-		final LabelMultisetEntryList lmel = labelMultisetEntries();
-		lmel.add(entry);
-		if (count(id) > count(argMax()))
-			updateArgMax(id.id());
+		final Label label = entry.id;
+		final LabelMultisetEntryList entryList = labelMultisetEntries();
+		entryList.add(entry);
+
+		final long argMax = argMax();
+		final long id = label.id();
+		if (id == argMax) return;
+		if (entryList.size() == 1 || count(id) > count(argMax))
+			updateArgMax(id);
 	}
 
 	public void addAll(Collection<? extends LabelMultisetEntry> entries) {
@@ -422,7 +470,8 @@ LabelMultisetEntryList labelMultisetEntries() {
 	}
 
 	private void updateEntriesLocation() {
-
+		if (access.getCurrentStorageArray().length == 0)
+			return;
 		access.getValue(i.get(), entries);
 	}
 

src/main/java/net/imglib2/type/label/LabelMultisetTypeDownscaler.java

@@ -32,7 +32,7 @@ public static VolatileLabelMultisetArray createDownscaledCell(
 			final int[] downscaleFactor,
 			final int maxNumEntriesPerPixel) {
 
-		final RandomAccess<LabelMultisetType> randomAccess = source.randomAccess();
+		final RandomAccess<LabelMultisetType> sourceAccess = source.randomAccess();
 
 		final int numDim = source.numDimensions();
 		final long[] sourceShape = new long[numDim];
@@ -59,7 +59,12 @@ public static VolatileLabelMultisetArray createDownscaledCell(
 
 		final TLongArrayList argMax = new TLongArrayList();
 
-		final LabelMultisetEntry addAllRef = list.createRef();
+		final LabelMultisetEntry emptyZeroEntry = new LabelMultisetEntry(0, 0);
+
+		final LabelMultisetEntry addRef = list.createRef();
+		final LabelMultisetEntry entrySetRef = list.createRef();
+
+		int nonEmptyListCount = 0;
 
 		for (int d = 0; d < numDim; ) {
 
@@ -70,9 +75,16 @@ public static VolatileLabelMultisetArray createDownscaledCell(
 			// populate list with all entries from all types
 			for (int g = 0; g < numDim; ) {
 
-				final LabelMultisetType lmt = randomAccess.setPositionAndGet(totalOffset);
+				final LabelMultisetType lmt = sourceAccess.setPositionAndGet(totalOffset);
+				if ( lmt.getAccess().getListDataUsedSizeInBytes() == 0) {
+					/* this cell is non-existent, skip to the next cell */
+					break;
+				}
+
+
+				for (final LabelMultisetType.Entry<Label> labelEntry : lmt.entrySetWithRef(entrySetRef))
+					list.add((LabelMultisetEntry)labelEntry, addRef);
 
-				list.addAll(lmt.labelMultisetEntries(), addAllRef);
 
 				/* This controls movement within the cell*/
 				for (g = 0; g < numDim; g++) {
@@ -85,18 +97,28 @@ public static VolatileLabelMultisetArray createDownscaledCell(
 				}
 			}
 
-			// sort by count and restrict to maxNumEntriesPerPixel (if
-			// applicable)
-
-			if (maxNumEntriesPerPixel > 0 && list.size() > maxNumEntriesPerPixel) {
-				// change order of e2, e1 for decreasing sort by count
+			/* Empty lists are fine if the whole block is empty, since we delete it.
+			 * But empty lists in a block that has other non-empty lists results in no label
+			 * over the empty list areas. We could do this (actually, I think it's a good idea)
+			 * but historically, empty/missing data was represented as ID `0`, or Label.BACKGROUND
+			 *
+			 *
+			 * NOTE: I think it's better if this behavior is changed in the future to allow empty lists which
+			 * map to Label.INVALID, so that Label.BACKGROUND isn't ambiguous whether intentional or missing. */
+			if (list.isEmpty()){
+				list.add(emptyZeroEntry);
+			} else{
+				nonEmptyListCount++;
+
+				// change order of e2, e1 for increasing sort by count
 				list.sortByCount();
-				list.limitSize(maxNumEntriesPerPixel);
+				if (maxNumEntriesPerPixel > 0 && list.size() > maxNumEntriesPerPixel)
+					list.limitSize(maxNumEntriesPerPixel);
 				final long max = list.get(list.size() - 1).getId();
 				argMax.add(max);
 				list.sortById();
-			} else
-				argMax.add(LabelUtils.getArgMax(list));
+			}
+
 
 			boolean makeNewList = true;
 			final int hash = list.hashCode();
@@ -118,9 +140,7 @@ public static VolatileLabelMultisetArray createDownscaledCell(
 			if (makeNewList) {
 				writeInt(listEntryOffsets, nextListOffset, ByteOrder.BIG_ENDIAN);
 				listOffsets.add(nextListOffset);
-				nextListOffset += list.getSizeInBytes();
-
-				// add entry with max count
+				nextListOffset += (int)list.getSizeInBytes();
 			}
 
 			/* this controls movement between cells */
@@ -133,6 +153,10 @@ public static VolatileLabelMultisetArray createDownscaledCell(
 				}
 			}
 		}
+
+		if (nonEmptyListCount == 0 )
+			return new VolatileLabelMultisetArray(0, true, new long[]{Label.INVALID});
+
 		final IntBuffer intBuffer = ByteBuffer.wrap(listEntryOffsets.toByteArray()).asIntBuffer();
 		final int[] listOffsets = new int[intBuffer.limit()];
 		intBuffer.rewind();

src/main/java/net/imglib2/type/label/LabelUtils.java

@@ -63,8 +63,12 @@ public static byte[] serializeLabelMultisetTypes(
 
 		int nextListOffset = 0;
 
+		int nonEmptyListCount = 0;
+
 		final ByteArrayOutputStream entryList = new ByteArrayOutputStream();
 		for (final LabelMultisetType lmt : lmts) {
+			if (!(lmt.isEmpty() || (lmt.size() == 0) ))
+				nonEmptyListCount++;
 			final int listHash = lmt.listHashCode();
 			int listOffset = listOffsets.get(listHash);
 			if (listOffset != listOffsets.getNoEntryValue()) {
@@ -81,6 +85,9 @@ public static byte[] serializeLabelMultisetTypes(
 			}
 		}
 
+		if (nonEmptyListCount == 0)
+			return null;
+
 		final byte[] entryListBytes = entryList.toByteArray();
 		dataBuffer.write(entryListBytes, 0, entryListBytes.length);
 		return dataBuffer.toByteArray();

src/test/java/net/imglib2/type/label/LabelMultisetTypeDownscalerTest.java

@@ -27,15 +27,16 @@ public class LabelMultisetTypeDownscalerTest {
 	public static final String DATASET = "sourceImg";
 
 	private static RandomAccessibleInterval<LabelMultisetType> sourceImg;
+	private static RandomAccessibleInterval<LabelMultisetType> emptyImg;
 	private static byte[] expectedDownScaledBytes;
 	private static VolatileLabelMultisetArray expectedDownScaledVlma;
 
 	@BeforeClass
 	public static void setUpBeforeClass() throws Exception {
 
-		N5FSReader n5Reader = new N5FSReader(DOWNSCALE_TEST_N5);
+		N5FSReader n5 = new N5FSReader(DOWNSCALE_TEST_N5);
 		/* Load expected data*/
-		sourceImg = N5Utils.open(n5Reader, DATASET);
+		sourceImg = N5Utils.open(n5, DATASET);
 		expectedDownScaledBytes = Files.readAllBytes(Paths.get(DOWNSCALE_TEST_SERIALIZED_BYTES));
 		expectedDownScaledVlma = LabelUtils.fromBytes(expectedDownScaledBytes, DOWNSCALED_NUM_ELEMENTS);
 		final byte[] sanityCheck = new byte[expectedDownScaledBytes.length];
@@ -61,27 +62,6 @@ public void testSerialization() {
 		VolatileLabelMultisetArrayTest.assertVolatileLabelMultisetArrayEquality(test, deserialized);
 	}
 
-	private static RandomAccessibleInterval<LabelMultisetType> generateRandomLabelMultisetImg() {
-
-		final Random rnd = new Random(10);
-
-		final int numElements = (int) Intervals.numElements(DIMENSIONS);
-		final List<LabelMultisetType> typeElements = new ArrayList<>();
-		LabelMultisetEntry obj = new LabelMultisetEntry(0, 1);
-		LabelMultisetEntry tmpObj = new LabelMultisetEntry(0, 1);
-		for (int i = 0; i < numElements; ++i) {
-			final int numEntries = rnd.nextInt(10);
-			final LabelMultisetEntryList entries = new LabelMultisetEntryList(numEntries);
-			for (int j = 0; j < numEntries; ++j) {
-				obj.setId(rnd.nextInt(100));
-				obj.setCount(rnd.nextInt(100));
-				entries.add(obj, tmpObj);
-			}
-			typeElements.add(new LabelMultisetType(entries));
-		}
-		return new ListImg<>(typeElements, DIMENSIONS);
-	}
-
 	@Test
 	public void copyOverImg() {
 		final LabelMultisetType atTen = sourceImg.getAt(10, 10, 10);
@@ -111,9 +91,7 @@ public void copyOverImg() {
 		assertNotEquals(atTen.entrySet(), copy.entrySet());
 
 
-
-
-		/* NOTE: In general its not really safe to increment the index manually.
+		/* NOTE: In general it's not really safe to increment the index manually.
 		*   When moving the index, you also should trigger the `updateContainer` method,
 		*   but to do that you need to pass in the object to update against, which
 		*   is not available internally to the LabelMultisetType.
@@ -135,4 +113,26 @@ public void copyOverImg() {
 		assertNotEquals(atEleven.index().get(), copy.index().get());
 		assertNotEquals(atEleven.entrySet(), copy.entrySet());
 	}
+
+
+	private static RandomAccessibleInterval<LabelMultisetType> generateRandomLabelMultisetImg() {
+
+		final Random rnd = new Random(10);
+
+		final int numElements = (int) Intervals.numElements(DIMENSIONS);
+		final List<LabelMultisetType> typeElements = new ArrayList<>();
+		LabelMultisetEntry obj = new LabelMultisetEntry(0, 1);
+		LabelMultisetEntry tmpObj = new LabelMultisetEntry(0, 1);
+		for (int i = 0; i < numElements; ++i) {
+			final int numEntries = rnd.nextInt(10);
+			final LabelMultisetEntryList entries = new LabelMultisetEntryList(numEntries);
+			for (int j = 0; j < numEntries; ++j) {
+				obj.setId(rnd.nextInt(100));
+				obj.setCount(rnd.nextInt(100));
+				entries.add(obj, tmpObj);
+			}
+			typeElements.add(new LabelMultisetType(entries));
+		}
+		return new ListImg<>(typeElements, DIMENSIONS);
+	}
 }

src/test/java/net/imglib2/type/label/LabelMultisetTypeTest.java

@@ -296,5 +296,7 @@ public void testArgMax() {
 		assertEquals(3, LabelUtils.getArgMax(entries));
 		final LabelMultisetType lmt = new LabelMultisetType(entries);
 		assertEquals(3, lmt.argMax());
+		entries.sortByCount();
+		assertEquals(3, entries.get(entries.size() - 1).getId());
 	}
 }