Skip to content

Commit

Permalink
Parallellize w. DefaultChunker
Browse files Browse the repository at this point in the history
  • Loading branch information
@rimadoma
rimadoma committed Apr 18, 2019
1 parent 03a8364 commit dd5a3e3
Showing 1 changed file with 65 additions and 28 deletions.
93 changes: 65 additions & 28 deletions ...main/java/net/imagej/ops/topology/eulerCharacteristic/EulerCharacteristic26NFloating.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,16 @@

import net.imagej.ops.Contingent;
import net.imagej.ops.Ops;
import net.imagej.ops.Parallel;
import net.imagej.ops.special.hybrid.AbstractUnaryHybridCF;
import net.imagej.ops.thread.chunker.CursorBasedChunk;
import net.imagej.ops.thread.chunker.DefaultChunker;
import net.imglib2.Cursor;
import net.imglib2.RandomAccess;
import net.imglib2.RandomAccessibleInterval;
import net.imglib2.type.BooleanType;
import net.imglib2.type.numeric.real.DoubleType;

import net.imglib2.view.ExtendedRandomAccessibleInterval;
import net.imglib2.view.Views;
import org.scijava.plugin.Plugin;

Expand Down Expand Up @@ -76,7 +79,7 @@
public class EulerCharacteristic26NFloating
<B extends BooleanType<B>>
extends AbstractUnaryHybridCF<RandomAccessibleInterval<B>, DoubleType>
implements Ops.Topology.EulerCharacteristic26NFloating, Contingent {
implements Ops.Topology.EulerCharacteristic26NFloating, Contingent, Parallel {
/**
* Δχ(v) for all configurations of a 2x2x2 voxel neighborhood
*/
Expand Down Expand Up @@ -223,24 +226,23 @@ public boolean conforms() {
}

@Override
public void compute(RandomAccessibleInterval<B> interval, DoubleType output) {
long[] position = new long[3];
int[] neighborhood = new int[8];

int sumDeltaEuler = 0;
RandomAccess<B> access = Views.extendZero(interval).randomAccess();
final Cursor<B> cursor = createFloatingCursor(interval);

while (cursor.hasNext()) {
cursor.fwd();
cursor.localize(position);
fillNeighborhood(access, neighborhood, position);
sumDeltaEuler += deltaEuler(neighborhood);
}
public void compute(final RandomAccessibleInterval<B> interval, final DoubleType output) {
final ExtendedRandomAccessibleInterval<B, RandomAccessibleInterval<B>> extendedInterval =
Views.extendZero(interval);
final long x = interval.dimension(0) + 1;
final long y = interval.dimension(1) + 1;
final long z = interval.dimension(2) + 1;
final long extendedSize = x * y * z;
ops().run(DefaultChunker.class, new EulerChunk<>(extendedInterval, output), extendedSize);
output.set(output.get() / 8.0);
}

output.set(sumDeltaEuler / 8.0);
@Override
public DoubleType createOutput(final RandomAccessibleInterval<B> input) {
return new DoubleType(0.0);
}


/**
* Creates a cursor that traverses an interval one pixel per dimension bigger than the input.
* <p>
Expand All @@ -250,19 +252,16 @@ public void compute(RandomAccessibleInterval<B> interval, DoubleType output) {
* </p>
*
* @param interval the input interval.
* @return a larger "floating" interval.
* @return a cursor of a larger "floating" interval.
* @see #fillNeighborhood(RandomAccess, int[], long[])
*/
private Cursor<B> createFloatingCursor(RandomAccessibleInterval<B> interval) {
private static <B extends BooleanType<B>> Cursor<B> createFloatingCursor(
final RandomAccessibleInterval<B> interval) {
final long x = interval.dimension(0) + 1;
final long y = interval.dimension(1) + 1;
final long z = interval.dimension(2) + 1;
return Views.offsetInterval(interval, new long[]{0, 0, 0}, new long[]{x, y, z}).cursor();
}

@Override
public DoubleType createOutput(RandomAccessibleInterval<B> input) {
return new DoubleType(0.0);
return Views.offsetInterval(interval, new long[]{0, 0, 0}, new long[]{x, y, z})
.cursor();
}

private static int deltaEuler(final int[] mask) {
Expand All @@ -280,7 +279,7 @@ private static int deltaEuler(final int[] mask) {
return EULER_LUT[index];
}

private static int findMostSignificantNeighbor(int[] neighborhood) {
private static int findMostSignificantNeighbor(final int[] neighborhood) {

This comment has been minimized.

Sign in to view

Copy link
@mdoube

mdoube Apr 23, 2019

Contributor

Is a Stream here as efficient as low level ifs and arrays?
return IntStream.range(0, neighborhood.length).map(i -> neighborhood.length - 1 - i)
.filter(i -> neighborhood[i] == 1).findFirst().orElse(-1);
}
Expand Down Expand Up @@ -310,8 +309,9 @@ private static <B extends BooleanType<B>> void fillNeighborhood(final RandomAcce
neighborhood[7] = getAtLocation(access, position, 0, 0, 0);
}

private static <B extends BooleanType<B>> int getAtLocation(RandomAccess<B> access,
long[] position, long... offsets) {
private static <B extends BooleanType<B>> int getAtLocation(final RandomAccess<B> access,
final long[] position,
final long... offsets) {
long x = position[0] + offsets[0];
long y = position[1] + offsets[1];
long z = position[2] + offsets[2];
Expand All @@ -320,4 +320,41 @@ private static <B extends BooleanType<B>> int getAtLocation(RandomAccess<B> acce
access.setPosition(z, 2);
return (int) access.get().getRealDouble();
}


private static class EulerChunk<B extends BooleanType<B>> extends CursorBasedChunk {

private final ExtendedRandomAccessibleInterval<B, RandomAccessibleInterval<B>> input;

private final DoubleType output;

private EulerChunk(ExtendedRandomAccessibleInterval<B, RandomAccessibleInterval<B>> in,
DoubleType out) {
input = in;
output = out;
}

@Override
public void execute(int startIndex, int stepSize, int numSteps) {
final long[] position = new long[3];
final int[] neighborhood = new int[8];
final RandomAccess<B> access = input.randomAccess();
final Cursor<B> cursor = createFloatingCursor(input.getSource());
int steps = 0;
double sumDeltaEuler = 0;

setToStart(cursor, startIndex);
while (steps < numSteps) {
cursor.localize(position);
fillNeighborhood(access, neighborhood, position);
sumDeltaEuler += deltaEuler(neighborhood);
cursor.jumpFwd(stepSize);
steps++;
}

synchronized (this) {
output.set(output.get() + sumDeltaEuler);
}
}
}
}

4 comments on commit dd5a3e3

@rimadoma
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addresses issue #600

@mdoube
Copy link
Contributor

@mdoube mdoube commented on dd5a3e3 Apr 23, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

It's multithreaded but still much slower than BoneJ1.

CPU trace from BoneJ1:
Screenshot from 2019-04-23 16-33-22

CPU trace from BoneJ2:
EDIT: It's not usually quite this slow, about half the time, after a restart. Still super slow though.
Screenshot from 2019-04-23 16-29-40

BoneJ2 is doing orders of magnitude more work (CPU cycles) to get to the same result.

@mdoube
Copy link
Contributor

@mdoube mdoube commented on dd5a3e3 Apr 23, 2019

Choose a reason for hiding this comment

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

VisualVM CPU profiling suggests the problem is in:
fillNeighborhood() (but not really getAtLocation(), surprisingly; maybe it is being inlined)
findMostSignificantNeighbor() of which
java.util.stream.IntPipeline.findFirst(), .filter() and java.util.stream.IntStream.range() are the main culprits (longhand array logic could be much faster & more readable)

image

@mdoube
Copy link
Contributor

@mdoube mdoube commented on dd5a3e3 Apr 24, 2019

Choose a reason for hiding this comment

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

The multithreaded version is not appreciably faster and maybe slower than a single threaded version, which suggests a problem with the multithreading logic (e.g. way too much synchronisation). This is the CPU trace of my single-threaded version at 64be042

image

Interestingly, profiling reveals that a full second (≃ 10%) is spent determining whether the image is binary or not with org.bonej.utilities.isColorsBinary() (it spends all the time in HashSet.add()). The rest of the time (≃ 80%) is in getAtLocation(), which leans heavily on imglib2's RandomAccess. I wonder if there is a more efficient way to get pixel values, and to determine binary-ness?

    private boolean getAtLocation(final RandomAccess<B> access, final long x, final long y, final long z) {
        access.setPosition(x, 0);
        access.setPosition(y, 1);
        access.setPosition(z, 2);
        return access.get().get();
    }

image

Please sign in to comment.