Posted by
Adrian Daerr-2 on
Jun 30, 2015; 10:14pm
URL: http://imagej.273.s1.nabble.com/ExtendedPlugInFilter-GenericDialog-and-synchronization-tp5013333p5013370.html
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.html1) 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