Login  Register

Re: ExtendedPlugInFilter, GenericDialog and synchronization

Posted by ctrueden on Jun 30, 2015; 9:58pm
URL: http://imagej.273.s1.nabble.com/ExtendedPlugInFilter-GenericDialog-and-synchronization-tp5013333p5013369.html

Hi everyone,

I can't speak to the complexities of ImageJ1's threading when it comes to
ExtendedPluginFilters, but... I did just add some new examples to the
ImageJ2 tutorials, which demonstrate the "ImageJ2 way" of creating
previewable commands [1].

Basically, you just implement the Previewable interface, with two methods:
preview() and cancel(). Then the framework does the rest.

The two new examples illustrate a simple command that alters the title of
an ij.ImagePlus with automatic preview, as well as with preview contingent
on the state of a "Preview" checkbox.

As always, happy to answer any questions about it.

Regards,
Curtis

[1]
https://github.com/imagej/imagej-tutorials/tree/d382b0a2eb2455c921ddc94a90525254b9483bf2/commands-with-preview/src/main/java

On Tue, Jun 30, 2015 at 1:44 PM, Michael Schmid <[hidden email]>
wrote:

> Hi Adrian,
>
> of course, in an ExtendedPlugInFilter, during preview, the class variables
> set by the GenericDialog in dialogItemChanged change asynchronously for the
> run(ip) method.
>
> Usually, this is not a problem, however: If dialogItemChanged is called
> during preview, the result of the run(ip) will be discarded, the image will
> be reverted to the previous state (snapshot) and the run(ip) method will be
> started again. So the user can never see a result where parameters have
> changed asynchronously.
> (On dialogItemChanged the previewThread gets an interrupted condition, so
> it can abort a lengthy calculation, but checking for being interrupted is
> needed only for better responsiveness; you can omit it for very fast
> operations).
>
> When run(ip) gets started again, I am pretty sure that it won't use any
> previous value of class variables, even if they are not declared volatile.
> First, any realistic run(ip) is long enough not to be inlined, so it will
> have to read all class variables from memory; it won't keep them in CPU
> registers.
> Also, in the PlugInFilterRunner, the preview thread will always pass
> through a 'synchronized' block between modification of a variable by
> dialogItemChanged and its actual use in run(ip), so it would be strange if
> it would keep the previous state of a class variable even if not declared
> volatile.
>
> Java experts out there, you agree?
>
> I am aware of only one case where you really have to care about
> asynchronous modification of variables: If the run(ip) method can crash
> (cause an exception) in case of asynchronous modification of variables.
> E.g. if you create an array with its length depending on some dialog
> parameter, and you access values in that array based on the dialog
> parameter. When the dialog parameter changes asynchronously, you can get an
> ArrayIndexOutOfBoundsException.
> In such a case, make sure that you pass all the parameters to a separate
> method that uses only its private copies for every further action. That's
> what I meant with the 'doFiltering' method.
>
> There is one more point to care about: In dialogItemChanged you can check
> the input for consistency and return false in case of inconsistent input.
> This disables preview and the OK button. You should never rely on this; it
> is only for user-friendlyness. Due to asynchronous modification, you may
> nevertheless see an inconsistent state of the dialog parameters during
> preview. Also when running your ExtendedPlugInFilter with IJ.run(..) or a
> macro's run(..) command, it won't care about the return value of
> dialogItemChanged. So you have to recheck the consistency of the parameters
> again before using them (if inconsistent parameters can cause an exception,
> do the check on local copies that will be also used for the rest of the
> code, so you can't have an asynchronous modification later - see the
> doFiltering suggestion).
>
> So I am not aware of any scenario where the programmer of an
> ExtendedPlugInFilter would need a 'synchronized' block for am
> ExtendedPlugInFilter, except if the inner workings of the filter need it.
>
> But I am not a Java expert and not a computer scientist either, maybe
> someone else can prove me wrong...
>
> Michael
> ________________________________________________________________
> On Jun 30, 2015, at 19:35, Adrian Daerr wrote:
>
> > Hi Michael,
> >
> > Thanks a lot for your detailed message and the code design
> recommendations. Your description of the inner workings of the
> PlugInFilterRunner goes way beyond what's in the API documentation and was
> a usefull read.
> >
> > After reading your mail and browsing the source code however I get the
> feeling that, contrary to what you say ("So in general, no real precautions
> are necessary for multithreading in ExtendedPlugInfilters, with very few
> exceptions"), synchronisation is entirely the responsibility of the plugin
> author and not optional at all. Correct me if I am wrong, but if parameters
> are changed in the dialog and read into (non-volatile, non-synchronised)
> variables by the GUI-thread, there is no guarantee that the run(ip) thread
> will *ever* see the change. The JVM specification states that the only way
> to establish a happens-before relationship between actions in different
> threads is through acquisition and release of locks.
> >
> > For making sure that run(ip) sees the latest parameters this can be
> achieved e.g. by setting variables by a dialogItemChanged() method within a
> synchronised(){} statement, and reading the variables from run(ip) in a
> block synchronized on the same monitor.
> >
> > For the other way round, I do not see a way to instantaneously modify
> the dialog from within run(ip): even if I call methods on the dialog and
> sync on a monitor, I have to wait for the gui thread to enter some code I
> control (e.g. the dialogItemChanged() method) so that I can synch on the
> same monitor and make sure that the changes are now visible to the gui
> thread.
> >
> > Of course everything *might* go well without all this synchronisation
> stuff on given JVMs, but explanations like the following suggest this might
> just be a lucky bet:
> >
> > http://www.cs.umd.edu/~pugh/java/memoryModel/jsr-133-faq.html
> >
> > I'd be happy to see comments on this,
> > cheers,
> > Adrian
> >
> >
> >
> >
> >
> >
> > On 06/28/2015 12:15 PM, Michael Schmid wrote:
> >> Hi Adrian,
> >>
> >> the parameters from the showDialog method (GenericDialog) are usually
> >> passed via class variables (=global variables). So they can change any
> >> time during preview time, while run(ip) is active. In addition, if
> >> parameters are changed, the thread executing run(ip) will get an
> >> interrupted condition, and after finishing (either because it checks for
> >> being interrupted and then exits or after finishing), run(ip) will be
> >> started again in the same thread (the previewing thread).
> >>
> >> If the run(ip) method can have a problem with its parameters being
> changed
> >> asynchronously (other than producing unusable output), in run(ip) you
> can
> >> call a method with passing all parameters. Such a method, when declared
> >> public, might be also nice for people who want to use the plugin
> directly
> >> on an ImageProcessor:
> >>
> >> public class ... implements ExtendedPlugInFilter, DialogListener {
> >>    double paramN;   //filter parameters = dialog parameters
> >>    boolean praremB;
> >>
> >>
> >>    public boolean dialogItemChanged(GenericDialog gd, AWTEvent e) {
> >>        paramN = gd.getNextNumber();
> >>        paramB = gd.getNextBoolean();
> >>        ...
> >>    }
> >>
> >>    public void run(ImageProcessor ip) {
> >>        doFiltering(ip, paramN, paramB);
> >>    }
> >>
> >>    public static void doFiltering(ImageProcessor ip, double paramN,
> >> boolean paramB) {
> >>         // here the filtering is really done.
> >>         ...
> >>    }
> >> }
> >>
> >> The doFiltering method can be static if you don't access any class
> >> variables. E.g. if you want to display a progress bar and you process
> >> stacks or RGB images with the CONVERT_TO_FLOAT flag, you need to know
> the
> >> number of passes, so it can't be static any more.
> >>
> >> After preview, there is no danger of a asynchronous modification of
> >> parameters any more.
> >>
> >> ---
> >>
> >> PARALLELIZE_IMAGES for processing an image with several threads: This
> >> happens already during preview, so you can have asynchronous
> modification
> >> of parameters also while only one image part is processed. This does not
> >> cause an additional problem if you use the 'doFiltering' approach above;
> >> each thread will get a different ROI with a different range of y
> >> coordinates.
> >>
> >> There is another point unrelated to preview: With PARALLELIZE_IMAGES,
> you
> >> have to be aware that other threads may modify other parts of the image.
> >> If your code needs to read pixels from a row different from the one it
> >> writes to (i.e., if the neighborhood of a pixel affects the result), use
> >> the snapshot to get the original data (SNAPSHOT flag).
> >>
> >> ---
> >>
> >> There can be an asynchronous modification of nPasses: If you process a
> >> stack and specify KEEP_PREVIEW and PARALLELIZE_STACKS, the preview
> thread
> >> may still work on the currently visible stack slice while the user
> presses
> >> <OK> and ImageJ calls setNPasses with the number of stack slices to
> >> process the stack (it will omit the slice already handled by the preview
> >> thread).
> >>
> >> So the code should not use setNPasses for anything critical.
> >> setNPasses is meant for showing a progress bar. It won't hurt if the
> >> progress bar is inaccurate in a few cases. Typical code looks like the
> >> following:
> >>
> >>    double progressDone = 0.0;
> >>    int nPasses;
> >>
> >>
> >>    public void setNPasses (int nPasses) {
> >>        this.nPasses = nPasses;
> >>        progressDone = 0.0;
> >>    }
> >>
> >>    public void doFiltering(...) {
> >>        for (y=0; y<height; y++) {
> >>            progressDone += 1.0/(height*nPasses); //not thread safe, but
> >> don't care
> >>            if (y%10==0) { //from time to time, update the progress
> bar...
> >>                IJ.showProgress(progressDone);
> >>                // ...and check whether preview got interrupted:
> >>                if (Thread.currentThread().isInterrupted()) return;
> >>            }
> >>            ...
> >>        }
> >>    }
> >>
> >> So in general, no real precautions are necessary for multithreading in
> >> ExtendedPlugInfilters, with very few exceptions:
> >> - Use a separate doFiltering method with a local set of parameters if
> >> processing could crash in case of asynchronous modification of
> parameters.
> >> - Do not use setNPasses for anything critical.
> >> - As usual in multithreaded programming, in code that may run
> >> asynchronously, don't modify class variables unless you understand what
> >> you are doing. In practice, it makes sense to start coding with a static
> >> doFiltering method, where the compiler checks that you don't
> accidentally
> >> use class variables. Get all coding and testing done like this. At the
> end
> >> make it non-static to add the progress bar.
> >>
> >> ---
> >>
> >> ImageJ2: I am not familiar with the current status; it's pretty long ago
> >> that I had a look into this and at that time preview was not supported
> by
> >> the ImageJ2 way of defining plugins. Others will be more knowledgeable.
> >>
> >>
> >> Michael
> >> ______________________________________________________________
> >>
> >>
> >> On Sat, June 27, 2015 17:28, Adrian Daerr wrote:
> >>> Hello,
> >>>
> >>> I want to write an ExtendedPlugInFilter with a preview checkbox. For
> >>> preview, the API documentation of that interface at
> >>>  http://imagej.nih.gov/ij/developer/api/
> >>> says that "a separate thread may call setNPasses(nPasses) and run(ip)
> >>> while the dialog is displayed". It does not say anything about
> >>> synchronization: is this something I need to worry about ?
> >>>
> >>> More specifically, how does the preview thread get its parameters in
> >>> run(ip) ? If the GenericDialog is queried from the preview thread, do
> >>> I have a guarantee that the values are the same as those seen by the
> >>> dialog handling thread ? Otherwise, what is the recommended way of
> >>> synchronizing ?
> >>>
> >>> A similar issue arises when plugin returns the PARALLELIZE_IMAGES /
> >>> PARALLELIZE_STACKS flags, although one may argue that the dialog has
> >>> probably been quit by the time the processing takes places (but the
> >>> processing threads might stem from a worker pool created before the
> >>> dialog, and have a different view of the memory than the gui threadq
> >>> ?). Is it up to the plugin author to properly synchronize stuff ? How
> >>> is this best done ?
> >>>
> >>> Multithreaded programming always gives me a headache, and as there is
> >>> no mention of thread-safety in the docs I figured I'd ask.
> >>>
> >>> Is the situation clearer with ImageJ2 ?
> >>>
> >>> cheers,
> >>> Adrian
> >>>
> >>> --
> >>> ImageJ mailing list: http://imagej.nih.gov/ij/list.html
> >>>
> >>
> >> --
> >> ImageJ mailing list: http://imagej.nih.gov/ij/list.html
> >>
>
> --
> ImageJ mailing list: http://imagej.nih.gov/ij/list.html
>

--
ImageJ mailing list: http://imagej.nih.gov/ij/list.html