[ImageJ-devel] [fiji-devel] Status reporting in ImgLib Algorithms

[Johannes Schindelin]

Hi Tobias,

On Mon, 5 Aug 2013, Tobias Pietzsch wrote:

> First of all, I am very reluctant to add any dependency to
> imglib2-algorithms core.

Oh really? ;-)

> I admit that I did not really look into scijava-common so far, so I
> cannot really comment on how severe I think this would be.

I think it might make a lot of sense to look into scijava-common because
it addresses a lot of very general issues with Java programming.

For example, we do often have the problem of inversion of control (IoC):
you will want to let consumers of your code override certain aspects, but
still provide your own, default implementation.

The status service is actually an excellent example for this: lengthy
operations will need to be able to report their progress. For the ImgLib2
developers, stderr is probably not only sufficient, but even desirable.

For most users, stderr is completely inacceptable, especially on Windows
where Java's stderr is re-routed to NUL: by default. Gone. Never to be
seen.

The best way I can think of to address this issue is to have an interface
that describes progress updates, a default implementation writing that to
stderr, and a way to inject different implementations for those clicky
clicky user interfaces.

The way scijava-common addresses this issue is to offer the StatusService
interface:

https://github.com/scijava/scijava-common/blob/master/src/main/java/org/scijava/app/StatusService.java

along with a default implementation:

https://github.com/scijava/scijava-common/blob/master/src/main/java/org/scijava/app/DefaultStatusService.java

And if you want to override it, you can easily do so by implementing the
StatusService, annotating the implementing class with the

	@Plugin(type = StatusService.class)

annotation.

Of course, you are free to reinvent the wheel in ImgLib2 if you so desire,
but please do not ask me to be a fan of that idea.

> I had a quick look and saw that scijava-common is itself not without
> dependencies. I found it depends on org.bushe.eventbus. On the eventbus
> website I found the notice:
>
> Update 3/2013: The EventBus has gone stale and is unlikely to be
> extended to other platforms. Suitable, though more primitive,
> replacements can be found for GWT (GWT EventBus), Javascript (TIBCO
> PageBus - an OpenAjax Hub implementation) and Flex (Parsley's Messaging)
> So would that mean dragging in a dead project as a transitive dependency?

Two points about this:

1) EventBus is very stable and we can easily extend it should we ever need
that. We now even have a Git-SVN mirror for easier debugging, making it
safe to rely on said component:

	https://github.com/scijava/eventbus

2) EventBus is -- as you pointed out -- a *transitive* dependency. No user
of scijava-common needs to use it directly. Actually, no user of
scijava-common needs to use it *full stop*. The point of a
transitive dependency, after all, is that the dependency can switch to any
other way to provide the functionality and your project still runs fine.
And in the case of the eventbus dependency: as I laid out before, the
inversion-of-control principle does even allow you to exclude eventbus.jar
from the classpath completely, given a replacement implementation of the
EventService interface.

> As far as I can tell, the net.imglib2.multithreading stuff is a leftover
> from imglib1 times and I think I have mentioned several times that I
> would like to get rid of it… :-)
>
> I guess that scijava-common ThreadService is more thought-through than
> the SimpleMultiThreading class. So replacing that I would see as an
> advantage.

Indeed, also from the Open Source point of view where you do not only use
software libraries but also contribute improvements back, for a mutual
benefit.

> In general, I think ExecutorServices are a good fit for multithreading
> pixel processing algorithms, along the lines of submitting a runnable
> for every line/plane/.
> 
> I see that you have a need for status reporting in ImageJ2 but I'm not
> sure that it should be in imglib-algorithms.

As I said, if you want to implement the very same functionality in
imglib2-algorithms, go ahead. We need that functionality, as do virtually
all developers having any kind of graphical or web frontend.

I would strongly encourage you to save the time, though, and use and
improve what is there in scijava-common.

> I would favour a way where we could keep the imglib2 code focused on
> processing pixels, not caring about framework issues.

Too late. ImgLib2 *is* used, and the users *need* status updates. Any way
you turn it, we need that functionality.

> In principle, status reporting can be performed before entering and
> after leaving a imglib2 "work-unit".

Not without that "work-unit" knowing about the status reporting, because
it needs to report the numbers based on internal state.

> If fine grained reporting is required, then this would mean splitting up
> the work into smaller units and reporting in between.
> 
> In any case, even if we add scijava-common as a dependency, I strongly
> suggest to also keep non-reporting versions of the algorithms.

I thought the point of ImgLib2 was to prevent having to duplicate code.

> If noone is listening to the status reports thats just wasted
> computation time. I think the overhead will be quite high in some cases.

As your benchmarks pointed out: if the code path through an interface
always lands at the same implementation, the JIT is quite good at
inlining the code.

So if you override the StatusService with an implementation that has only
empty, final methods, I would be highly surprised if your earlier
benchmarks would be contradicted by the overhead you suspect.

> For instance in your forked ComputeMinMax algorithm you do some work on
> updating status for every single pixel, which I think is definitely
> overdoing it.

That is a problem of the ComputeMinMax algorithm, I agree. But nothing
that can serve as a counterargument against using scijava-common.

> As a compromise I could image putting an empty protected method report()
> that is called periodically by the core algorithm implementation.

But that will be insufficient, don't you think? How would the report()
function know 1) about internal state of the consumer, and 2) about the
percentage of the progress?

> The JIT would hopefully just optimize that away if you use the core
> algorithm. Then for the reporting version you would have to extend the
> core algorithm and override report().

As I pointed out above, the JIT has been consistent in your benchmarks in
inlining short, final methods. It would be more than just surprising if
that would not work for empty methods.

The point is that ImgLib2 should cater for the common case. And the common
case *is* a user interface with some sort of progress report.

If you have a better idea, let's hear it.

Ciao,
Dscho