ExtendedPlugInFilter, GenericDialog and synchronization

[Adrian Daerr-2]

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

[Michael Schmid]

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

[Adrian Daerr-2]

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

[Michael Schmid]

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

[ctrueden]

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

[Adrian Daerr-2]

Hallo Michael,

On Tue, 30 Jun 2015 20:44:43 +0200
  Michael Schmid <[hidden email]> wrote:
> of course, in an ExtendedPlugInFilter, during preview, the class
>variables set by the GenericDialog in dialogItemChanged change
>asynchronously for the run(ip) method.

Yes, that is of course a possibility, but it is not the problem I
am worried about, which is that there is the possibility that the
variable values do not change at all in run(ip) even if
dialogItemChanged() writes new values.

> 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.

You seem to be somewhat assuming that CPU register caching is the
only way (or at least the most likely way) variables can get out
of sync between threads. First of all, the Java specification
does not make any statements on inlining or caching. In
attempting to specify a behaviour that is independent of the
hard- and software(JVM) implementation details of the platform on
which the program will run, it only states what the Java Virtual
Machine must guarantee. The implementation is then free to use
register variables, the cache management of the CPU(s), code
transformations and so on as it pleases, as long as the
observable behaviour sticks to the java memory model.

And that guarantee, with respect to concurrent programming, boils
down to the following statements (this list is quoted from the
jsr133 FAQ [1]):

[1] http://www.cs.umd.edu/~pugh/java/memoryModel/jsr-133-faq.html

1) Each action in a thread happens before every action in that
    thread that comes later in the program's order.

2) An unlock on a monitor happens before every subsequent lock on that
    same monitor.

3) A write to a volatile field happens before every subsequent read of
    that same volatile.

4) A call to start() on a thread happens before any actions in the
    started thread.

5) All actions in a thread happen before any other thread
    successfully returns from a join() on that thread.

Point one is frequently termed the guarantee of 'as-if-serial
semantics' for each thread, meaning that code transformations
(such as changing the execution order of two statements) are
permitted only as long as the (observable) result is
indistinguishable from in-order-execution.

Points 2-5 are what really concerns us here, because they specify
under what circumstances, and how, the happens-before relation is
extended across concurrent threads.

Now the source of PlugInFilterRunner tells us that the preview
thread is started on the first dialogItemChanged call where the
preview checkbox is ticked. That very first time, point 4)
guarantees that the thread sees all values read from the dialog
up to that point. Perfect for the first preview.

On subsequent dialogItemChanged calls however the previewThread
is merely interrupted, so neither 4) nor 5) apply. The waiting
thread may be in a synchronized(pfr){} block when this occurs, or
later go through it, but dialogItemChanged() never synchronizes
on the same monitor (the PlugInFilterRunner instance pfr), so 2)
does not hold either [actually dialogItemChanged() does not
synchronize on any monitor at all]. There is no volatile field in
play anywhere, so 3) never matters.

Am I wrong to conclude that there is no synchronization between the
preview and the dialogItemChanged threads ?

It is important to note that if none of 2-5 occur, two threads A
and B may be compiled, optimized and executed as if they were
running on different machines. After its creation and start,
thread B may *actually* be serialized along with its context,
transferred to another computer in a cloud and be executed there.
In particular, to get back to your remark on a "realistic
run(ip)" having to "read all class variables from memory": there
is no obligation to take into account what happend to a given
variable in thread A, thread B can use the same (possibly stale)
value without limit, because *from the point of view of B*, the
variable value never appears to change. It is not just a matter
of register caching of variables: The compiler can even optimize
away all reads of a variable: if the calculation in our run(ip)
depends on a test "if
(frobnicate) {...}", where "frobnicate" is only ever modified in
dialogItemChanged() (e.g. "frobnicate = gd.getNextBoolean()"),
and if frobnicate happened to be false at the time the thread
running run(ip) was started, then the compiler is allowed to drop
all the corresponding "if..." code block (including the read of
frobnicate). For the rest of the life of the preview thread, if
none of the conditions 2-5 above are ever fulfilled.

> 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.

That's a good point, but synchronization only guarantees a memory
barrier when the threads lock on the *same* monitor. But
dialogItemChanged() does not contain any synchronized block, even
less a synchronized block on the /same/ monitor.

> Java experts out there, you agree?

Yes, the opinion of a Java expert would really be appreciated
here. I am definitely *not* a Java expert.

> 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

Very good advice

cheers
Adrian



On Tue, 30 Jun 2015 20:44:43 +0200
  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

--
http://www.msc.univ-paris-diderot.fr/~daerr/

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

[Adrian Daerr-2]

Hello Curtis,

> I did just add some new examples to the ImageJ2 tutorials, which
> demonstrate the "ImageJ2 way" of creating previewable commands [1].
> [1]
> https://github.com/imagej/imagej-tutorials/tree/d382b0a2eb2455c921ddc94a90525254b9483bf2/commands-with-preview/src/main/java

That is great, I will look at that !
thanks,
Adrian

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

[Michael Schmid]

Hi Adrian,

ok, I see, my view of computing probably dates back too much to the old days of assembler programming ;-)

Fortunately, so far I have never seen an indication that preview would use an 'old' version of the dialog parameters. But my experience is limited to mostly Java 1.6 (and older) and dual-core processors.

So this means, the class variables for dialog parameters have to be declared volatile? That would be easy to do.

Michael
________________________________________________________________
On Jul 1, 2015, at 00:14, Adrian Daerr wrote:

> Hallo Michael,
>
> On Tue, 30 Jun 2015 20:44:43 +0200
> Michael Schmid <[hidden email]> wrote:
>> of course, in an ExtendedPlugInFilter, during preview, the class variables set by the GenericDialog in dialogItemChanged change asynchronously for the run(ip) method.
>
> Yes, that is of course a possibility, but it is not the problem I
> am worried about, which is that there is the possibility that the
> variable values do not change at all in run(ip) even if
> dialogItemChanged() writes new values.
>
>> 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.
>
> You seem to be somewhat assuming that CPU register caching is the
> only way (or at least the most likely way) variables can get out
> of sync between threads. First of all, the Java specification
> does not make any statements on inlining or caching. In
> attempting to specify a behaviour that is independent of the
> hard- and software(JVM) implementation details of the platform on
> which the program will run, it only states what the Java Virtual
> Machine must guarantee. The implementation is then free to use
> register variables, the cache management of the CPU(s), code
> transformations and so on as it pleases, as long as the
> observable behaviour sticks to the java memory model.
>
> And that guarantee, with respect to concurrent programming, boils
> down to the following statements (this list is quoted from the
> jsr133 FAQ [1]):
>
> [1] http://www.cs.umd.edu/~pugh/java/memoryModel/jsr-133-faq.html
>
> 1) Each action in a thread happens before every action in that
>   thread that comes later in the program's order.
>
> 2) An unlock on a monitor happens before every subsequent lock on that
>   same monitor.
>
> 3) A write to a volatile field happens before every subsequent read of
>   that same volatile.
>
> 4) A call to start() on a thread happens before any actions in the
>   started thread.
>
> 5) All actions in a thread happen before any other thread
>   successfully returns from a join() on that thread.
>
> Point one is frequently termed the guarantee of 'as-if-serial
> semantics' for each thread, meaning that code transformations
> (such as changing the execution order of two statements) are
> permitted only as long as the (observable) result is
> indistinguishable from in-order-execution.
>
> Points 2-5 are what really concerns us here, because they specify
> under what circumstances, and how, the happens-before relation is
> extended across concurrent threads.
>
> Now the source of PlugInFilterRunner tells us that the preview
> thread is started on the first dialogItemChanged call where the
> preview checkbox is ticked. That very first time, point 4)
> guarantees that the thread sees all values read from the dialog
> up to that point. Perfect for the first preview.
>
> On subsequent dialogItemChanged calls however the previewThread
> is merely interrupted, so neither 4) nor 5) apply. The waiting
> thread may be in a synchronized(pfr){} block when this occurs, or
> later go through it, but dialogItemChanged() never synchronizes
> on the same monitor (the PlugInFilterRunner instance pfr), so 2)
> does not hold either [actually dialogItemChanged() does not
> synchronize on any monitor at all]. There is no volatile field in
> play anywhere, so 3) never matters.
>
> Am I wrong to conclude that there is no synchronization between the
> preview and the dialogItemChanged threads ?
>
> It is important to note that if none of 2-5 occur, two threads A
> and B may be compiled, optimized and executed as if they were
> running on different machines. After its creation and start,
> thread B may *actually* be serialized along with its context,
> transferred to another computer in a cloud and be executed there.
> In particular, to get back to your remark on a "realistic
> run(ip)" having to "read all class variables from memory": there
> is no obligation to take into account what happend to a given
> variable in thread A, thread B can use the same (possibly stale)
> value without limit, because *from the point of view of B*, the
> variable value never appears to change. It is not just a matter
> of register caching of variables: The compiler can even optimize
> away all reads of a variable: if the calculation in our run(ip)
> depends on a test "if
> (frobnicate) {...}", where "frobnicate" is only ever modified in
> dialogItemChanged() (e.g. "frobnicate = gd.getNextBoolean()"),
> and if frobnicate happened to be false at the time the thread
> running run(ip) was started, then the compiler is allowed to drop
> all the corresponding "if..." code block (including the read of
> frobnicate). For the rest of the life of the preview thread, if
> none of the conditions 2-5 above are ever fulfilled.
>
>> 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.
>
> That's a good point, but synchronization only guarantees a memory
> barrier when the threads lock on the *same* monitor. But
> dialogItemChanged() does not contain any synchronized block, even
> less a synchronized block on the /same/ monitor.
>
>> Java experts out there, you agree?
>
> Yes, the opinion of a Java expert would really be appreciated
> here. I am definitely *not* a Java expert.
>
>> 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
>
> Very good advice
>
> cheers
> Adrian
>
>
>
> On Tue, 30 Jun 2015 20:44:43 +0200
> 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
>
> --
> http://www.msc.univ-paris-diderot.fr/~daerr/
>
> --
> ImageJ mailing list: http://imagej.nih.gov/ij/list.html

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