search

imglib / imglib2-ij

Translation between ImgLib & ImageJ data structures (both 1.x and 2)
Other
4 stars 8 forks source link

Unnecessary type conversion when wrapping to ImagePlus #16

Closed ctrueden closed 5 years ago

ctrueden commented 6 years ago

Consider the following Groovy script:

#@ DatasetIOService datasetIO
#@ UIService ui
bigCellImg = datasetIO.open("big&pixelType=uint8&axes=X,Y,Z,Time&lengths=3000,4000,5000,6000.fake")
ui.show(bigCellImg)

This pops up the huge image very quickly, which is awesome.

But now put a breakpoint at org.scijava.ui.DefaultUIService line 381 of org.scijava:scijava-common:2.73.1; this will be the line ui.show(display) inside onEvent(DisplayCreated). Run the same script above, and it will break there. Then STEP instead of CONTINUE. The step now hangs, with IterableIntervalProjector2D.map taking forever to complete. (I waited minutes, and it still was not done.)

Here are some example stack traces of what is going on during the step, retrieved from the CLI using ctrl+\:

"AWT-EventQueue-0" #21 prio=6 os_prio=31 tid=0x00007fd158950000 nid=0x6e23 runnable [0x00007000066d7000]
   java.lang.Thread.State: RUNNABLE
    at net.imglib2.type.numeric.integer.UnsignedByteType.get(UnsignedByteType.java:151)
    at net.imglib2.type.numeric.integer.UnsignedByteType.getIntegerLong(UnsignedByteType.java:168)
    at net.imglib2.type.numeric.integer.AbstractIntegerType.getRealDouble(AbstractIntegerType.java:62)
    at net.imglib2.img.display.imagej.ImageJVirtualStackUnsignedByte$ByteConverter.convert(ImageJVirtualStackUnsignedByte.java:79)
    at net.imglib2.img.display.imagej.ImageJVirtualStackUnsignedByte$ByteConverter.convert(ImageJVirtualStackUnsignedByte.java:72)
    at net.imglib2.display.projector.IterableIntervalProjector2D.map(IterableIntervalProjector2D.java:151)
    at net.imglib2.img.display.imagej.ImageJVirtualStack.getSlice(ImageJVirtualStack.java:148)
    at net.imglib2.img.display.imagej.ImageJVirtualStack.getPixels(ImageJVirtualStack.java:155)
    at net.imglib2.img.display.imagej.AbstractVirtualStack.getProcessor(AbstractVirtualStack.java:85)
    at ij.ImagePlus.setStack(ImagePlus.java:683)
    at ij.ImagePlus.<init>(ImagePlus.java:154)
    at net.imglib2.img.display.imagej.ImgToVirtualStack.wrap(ImgToVirtualStack.java:100)
    at net.imglib2.img.display.imagej.ImgToVirtualStack.wrap(ImgToVirtualStack.java:80)
    at net.imagej.legacy.translate.ImagePlusCreator.createImagePlus(ImagePlusCreator.java:121)
    at net.imagej.legacy.translate.ImagePlusCreator.createLegacyImage(ImagePlusCreator.java:98)
    at net.imagej.legacy.translate.ImagePlusCreator.createLegacyImage(ImagePlusCreator.java:87)
    at net.imagej.legacy.translate.ImageTranslator.createLegacyImage(ImageTranslator.java:74)
    at net.imagej.legacy.LegacyImageMap.registerDisplay(LegacyImageMap.java:267)
    at net.imagej.legacy.LegacyImageMap.registerDisplay(LegacyImageMap.java:254)
    at net.imagej.legacy.display.LegacyImageDisplayViewer.view(LegacyImageDisplayViewer.java:121)
    at org.scijava.ui.viewer.DisplayViewer.view(DisplayViewer.java:86)
    at net.imagej.legacy.ui.LegacyUI$1.run(LegacyUI.java:197)
    at org.scijava.thread.DefaultThreadService.invoke(DefaultThreadService.java:114)
    at net.imagej.legacy.ui.LegacyUI.show(LegacyUI.java:193)
    at org.scijava.ui.DefaultUIService.onEvent(DefaultUIService.java:381)

"AWT-EventQueue-0" #21 prio=6 os_prio=31 tid=0x00007fd158950000 nid=0x6e23 runnable [0x00007000066d7000]
   java.lang.Thread.State: RUNNABLE
    at java.lang.Double.doubleToRawLongBits(Native Method)
    at java.lang.Math.copySign(Math.java:1766)
    at java.lang.Math.signum(Math.java:1525)
    at net.imglib2.util.Util.round(Util.java:499)
    at net.imglib2.type.numeric.integer.AbstractIntegerType.setReal(AbstractIntegerType.java:74)
    at net.imglib2.img.display.imagej.ImageJVirtualStackUnsignedByte$ByteConverter.convert(ImageJVirtualStackUnsignedByte.java:84)
    at net.imglib2.img.display.imagej.ImageJVirtualStackUnsignedByte$ByteConverter.convert(ImageJVirtualStackUnsignedByte.java:72)
    at net.imglib2.display.projector.IterableIntervalProjector2D.map(IterableIntervalProjector2D.java:151)
    at net.imglib2.img.display.imagej.ImageJVirtualStack.getSlice(ImageJVirtualStack.java:148)
    at net.imglib2.img.display.imagej.ImageJVirtualStack.getPixels(ImageJVirtualStack.java:155)
    at net.imglib2.img.display.imagej.AbstractVirtualStack.getProcessor(AbstractVirtualStack.java:85)
    at ij.ImagePlus.setStack(ImagePlus.java:683)
    at ij.ImagePlus.<init>(ImagePlus.java:154)
    at net.imglib2.img.display.imagej.ImgToVirtualStack.wrap(ImgToVirtualStack.java:100)
    at net.imglib2.img.display.imagej.ImgToVirtualStack.wrap(ImgToVirtualStack.java:80)
    at net.imagej.legacy.translate.ImagePlusCreator.createImagePlus(ImagePlusCreator.java:121)
    at net.imagej.legacy.translate.ImagePlusCreator.createLegacyImage(ImagePlusCreator.java:98)
    at net.imagej.legacy.translate.ImagePlusCreator.createLegacyImage(ImagePlusCreator.java:87)
    at net.imagej.legacy.translate.ImageTranslator.createLegacyImage(ImageTranslator.java:74)
    at net.imagej.legacy.LegacyImageMap.registerDisplay(LegacyImageMap.java:267)
    at net.imagej.legacy.LegacyImageMap.registerDisplay(LegacyImageMap.java:254)
    at net.imagej.legacy.display.LegacyImageDisplayViewer.view(LegacyImageDisplayViewer.java:121)
    at org.scijava.ui.viewer.DisplayViewer.view(DisplayViewer.java:86)
    at net.imagej.legacy.ui.LegacyUI$1.run(LegacyUI.java:197)
    at org.scijava.thread.DefaultThreadService.invoke(DefaultThreadService.java:114)
    at net.imagej.legacy.ui.LegacyUI.show(LegacyUI.java:193)
    at org.scijava.ui.DefaultUIService.onEvent(DefaultUIService.java:381)

"AWT-EventQueue-0" #16 prio=6 os_prio=31 tid=0x00007fb4c4388000 nid=0xda27 runnable [0x000070000dc18000]
   java.lang.Thread.State: RUNNABLE
    at net.imglib2.view.MixedRandomAccess.get(MixedRandomAccess.java:364)
    at net.imglib2.display.projector.IterableIntervalProjector2D.map(IterableIntervalProjector2D.java:151)
    at net.imglib2.img.display.imagej.ImageJVirtualStack.getSlice(ImageJVirtualStack.java:148)
    at net.imglib2.img.display.imagej.ImageJVirtualStack.getPixels(ImageJVirtualStack.java:155)
    at net.imglib2.img.display.imagej.AbstractVirtualStack.getProcessor(AbstractVirtualStack.java:85)
    at ij.ImagePlus.setStack(ImagePlus.java:683)
    at ij.ImagePlus.<init>(ImagePlus.java:154)
    at net.imglib2.img.display.imagej.ImgToVirtualStack.wrap(ImgToVirtualStack.java:100)
    at net.imglib2.img.display.imagej.ImgToVirtualStack.wrap(ImgToVirtualStack.java:80)
    at net.imagej.legacy.translate.ImagePlusCreator.createImagePlus(ImagePlusCreator.java:121)
    at net.imagej.legacy.translate.ImagePlusCreator.createLegacyImage(ImagePlusCreator.java:98)
    at net.imagej.legacy.translate.ImagePlusCreator.createLegacyImage(ImagePlusCreator.java:87)
    at net.imagej.legacy.translate.ImageTranslator.createLegacyImage(ImageTranslator.java:74)
    at net.imagej.legacy.LegacyImageMap.registerDisplay(LegacyImageMap.java:267)
    at net.imagej.legacy.LegacyImageMap.registerDisplay(LegacyImageMap.java:254)
    at net.imagej.legacy.display.LegacyImageDisplayViewer.view(LegacyImageDisplayViewer.java:121)
    at org.scijava.ui.viewer.DisplayViewer.view(DisplayViewer.java:86)
    at net.imagej.legacy.ui.LegacyUI$1.run(LegacyUI.java:197)
    at org.scijava.thread.DefaultThreadService.invoke(DefaultThreadService.java:114)
    at net.imagej.legacy.ui.LegacyUI.show(LegacyUI.java:193)
    at org.scijava.ui.DefaultUIService.onEvent(DefaultUIService.java:381)

"AWT-EventQueue-0" #16 prio=6 os_prio=31 tid=0x00007fb4c4388000 nid=0xda27 runnable [0x000070000dc18000]
   java.lang.Thread.State: RUNNABLE
    at net.imglib2.img.array.AbstractArrayCursor.get(AbstractArrayCursor.java:123)
    at net.imglib2.img.array.AbstractArrayCursor.get(AbstractArrayCursor.java:53)
    at net.imglib2.display.projector.IterableIntervalProjector2D.map(IterableIntervalProjector2D.java:151)
    at net.imglib2.img.display.imagej.ImageJVirtualStack.getSlice(ImageJVirtualStack.java:148)
    at net.imglib2.img.display.imagej.ImageJVirtualStack.getPixels(ImageJVirtualStack.java:155)
    at net.imglib2.img.display.imagej.AbstractVirtualStack.getProcessor(AbstractVirtualStack.java:85)
    at ij.ImagePlus.setStack(ImagePlus.java:683)
    at ij.ImagePlus.<init>(ImagePlus.java:154)
    at net.imglib2.img.display.imagej.ImgToVirtualStack.wrap(ImgToVirtualStack.java:100)
    at net.imglib2.img.display.imagej.ImgToVirtualStack.wrap(ImgToVirtualStack.java:80)
    at net.imagej.legacy.translate.ImagePlusCreator.createImagePlus(ImagePlusCreator.java:121)
    at net.imagej.legacy.translate.ImagePlusCreator.createLegacyImage(ImagePlusCreator.java:98)
    at net.imagej.legacy.translate.ImagePlusCreator.createLegacyImage(ImagePlusCreator.java:87)
    at net.imagej.legacy.translate.ImageTranslator.createLegacyImage(ImageTranslator.java:74)
    at net.imagej.legacy.LegacyImageMap.registerDisplay(LegacyImageMap.java:267)
    at net.imagej.legacy.LegacyImageMap.registerDisplay(LegacyImageMap.java:254)
    at net.imagej.legacy.display.LegacyImageDisplayViewer.view(LegacyImageDisplayViewer.java:121)
    at org.scijava.ui.viewer.DisplayViewer.view(DisplayViewer.java:86)
    at net.imagej.legacy.ui.LegacyUI$1.run(LegacyUI.java:197)
    at org.scijava.thread.DefaultThreadService.invoke(DefaultThreadService.java:114)
    at net.imagej.legacy.ui.LegacyUI.show(LegacyUI.java:193)
    at org.scijava.ui.DefaultUIService.onEvent(DefaultUIService.java:381)

"AWT-EventQueue-0" #16 prio=6 os_prio=31 tid=0x00007fb4c4388000 nid=0xda27 runnable [0x000070000dc18000]
   java.lang.Thread.State: RUNNABLE
    at java.lang.Math.signum(Math.java:1525)
    at net.imglib2.util.Util.round(Util.java:499)
    at net.imglib2.type.numeric.integer.AbstractIntegerType.setReal(AbstractIntegerType.java:74)
    at net.imglib2.img.display.imagej.ImageJVirtualStackUnsignedByte$ByteConverter.convert(ImageJVirtualStackUnsignedByte.java:84)
    at net.imglib2.img.display.imagej.ImageJVirtualStackUnsignedByte$ByteConverter.convert(ImageJVirtualStackUnsignedByte.java:72)
    at net.imglib2.display.projector.IterableIntervalProjector2D.map(IterableIntervalProjector2D.java:151)
    at net.imglib2.img.display.imagej.ImageJVirtualStack.getSlice(ImageJVirtualStack.java:148)
    at net.imglib2.img.display.imagej.ImageJVirtualStack.getPixels(ImageJVirtualStack.java:155)
    at net.imglib2.img.display.imagej.AbstractVirtualStack.getProcessor(AbstractVirtualStack.java:85)
    at ij.ImagePlus.setStack(ImagePlus.java:683)
    at ij.ImagePlus.<init>(ImagePlus.java:154)
    at net.imglib2.img.display.imagej.ImgToVirtualStack.wrap(ImgToVirtualStack.java:100)
    at net.imglib2.img.display.imagej.ImgToVirtualStack.wrap(ImgToVirtualStack.java:80)
    at net.imagej.legacy.translate.ImagePlusCreator.createImagePlus(ImagePlusCreator.java:121)
    at net.imagej.legacy.translate.ImagePlusCreator.createLegacyImage(ImagePlusCreator.java:98)
    at net.imagej.legacy.translate.ImagePlusCreator.createLegacyImage(ImagePlusCreator.java:87)
    at net.imagej.legacy.translate.ImageTranslator.createLegacyImage(ImageTranslator.java:74)
    at net.imagej.legacy.LegacyImageMap.registerDisplay(LegacyImageMap.java:267)
    at net.imagej.legacy.LegacyImageMap.registerDisplay(LegacyImageMap.java:254)
    at net.imagej.legacy.display.LegacyImageDisplayViewer.view(LegacyImageDisplayViewer.java:121)
    at org.scijava.ui.viewer.DisplayViewer.view(DisplayViewer.java:86)
    at net.imagej.legacy.ui.LegacyUI$1.run(LegacyUI.java:197)
    at org.scijava.thread.DefaultThreadService.invoke(DefaultThreadService.java:114)
    at net.imagej.legacy.ui.LegacyUI.show(LegacyUI.java:193)
    at org.scijava.ui.DefaultUIService.onEvent(DefaultUIService.java:381)

The gist is that IterableIntervalProjector2D.map uses a ImageJVirtualStackUnsignedByte$ByteConverter to do an identity conversion from UnsignedByteType to UnsignedByteType—i.e. no value adjustment. I am guessing that when stepping in the debugger, we do not gain the full benefits of JIT/inlining, so end up looping over all samples. Even then, it seems odd it is so slow... but regardless, this code path is highly suboptimal. Would be much better not to use the converter at all in circumstances where the source type already matches.

tpietzsch commented 6 years ago

@maarzt Can you look into this?

maarzt commented 6 years ago

My thoughts on this:

  1. There is a tremendous performance improvement possible. DatasetIOService returns a SCIFIOCellImg with planar cells. I can add a wrapper SCIFIOCellImg to VirtualStack which will eliminate the need to copy data for unsigned byte, unsigned short and float images.
  2. When debugging java code: STEP is very slow. It's often way faster to put a breakpoint into the next source code line and use CONTINUE. I'm not surprised that a step over a slightly bigger copy loop takes ages. It's more a performance issue of the java debugger.
  3. @ctrueden you are right. There is an unnecessary type conversion. ImageJVirtualStackUnsignedByte$ByteConverter converts from RealType to UnsignedByteType. I can add a special converter that simply copies UnsignedByteType to UnsignedByteType with no extra conversion.

It's on my todo list.