[Insight-developers] RE: Threaded dilate/erode

[Miller, James V (Research)]

Jerome, 
 
// the devil part
    bool bIsInBound;
    for( itIndex = indexDifferenceSet.begin(); itIndex != indexDifferenceSet.end(); ++itIndex )
      {
      onit.SetPixel( *itIndex, foregroundValue, bIsInBound );
      }

This is the part of the code that I was worried about.  As Gaetan suggested, we are trying to verify the problem by running with a high number of threads.  Unfortunately, the problems are rare. Sometimes things like this may only cause a problem in 1 in 1000 runs.
 
On Friday, Bill and I discussed the same options that you came up with: cacheing the pixels that could in contention between threads and placing a mutex around the critical section.  The section of code in question takes about 25% of the algorithms processing time.  So maybe running this section as a single threaded operation would not be too bad.
 
Jim
 
 

-----Original Message-----
From: SCHMID, Jerome [mailto:jeromeschmid at surgery.cuhk.edu.hk]
Sent: Sunday, November 20, 2005 11:15 PM
To: Miller, James V (Research); Gaetan Lehmann; Insight-developers (E-mail)
Subject: RE: Threaded dilate/erode



Miller, James V (Research) wrote:

>Jerome, Gaetan,
>
>We have integrated the threaded versions of the binary dilate/erode into the ITK repository.  The use of the ProgressReporter needed to be changed slightly in the multithreaded versions to be thread safe.  One of the arguments to the ProgressReporter is the threadId.  Thread 0 is only one that outputs information by the ProgressReporter.  This argument was set to 0 for all threads, causing some intermittent errors.
>
Hi Jim,

Glad to see that you have interest on the filter implementation. BTW, just by curiosity, what is the effect of :
"(void)ThreadId;" in the .txx code?

>
>I question whether stage 3 is really thread safe.  It looks like this loop can write to pixels that are outside the outputRegionForThread.  This could result in two threads writing to the same memory location at the same time.  This could result in corrupted memory and in some cases crashes.  Can you verify whether or not stage 3 could write to pixels that are outside the outputRegionForThread.
>

Well I have a deeper look and it doesnt't seem thread-safe... However I am not an expert in iterators/multi-threading so let's see the hazardous points:

// Paint SE     --> "( BORDER(X) (+) B )" Part:

// the dangerous iterator ^_^
 NeighborhoodIterator<OutputImageType> onit;
  onit = NeighborhoodIterator<OutputImageType>( kernel.GetRadius(), output,
                                                outputRegionForThread );

....

// the devil part
    bool bIsInBound;
    for( itIndex = indexDifferenceSet.begin(); itIndex != indexDifferenceSet.end(); ++itIndex )
      {
      onit.SetPixel( *itIndex, foregroundValue, bIsInBound );
      }

So the pb is: the neighborhoodIterator will write in is "neighboor band" that will overlap the close region of an other thread. The pb is that we need this writing if we want to have a correct result so it cannot be supressed.


The last part doesn't cause pb because the output iterator is in the requestedRegionFroThread and the input iterator is in the requestedRegionFroThread padded but in READ mode, which should not cause pb, isn'it?

So How to deal with the previous pb?

Well I don't know exactly, I am not a thread expert...:-( But I think a mutex/barrier lock may become necessary only on the pixels belonging to the neighborhood that are outside the requested region of the iterator. This may require a new iterator writing...

Another solution, maybe not so easy to implement, would be to record these pixels to be writing and problematic and to store them in a structure without performing the writing. These pixels can be identified before the writing. Then, if itk offers this possibility, write these pixels in a *non* threaded way (AfterGenerateThreadedData() function or similar).


>
>Minor point: ITK style guide requires that TABS be replaced with spaces.  It would make integrating contributed code into ITK smoother if your submissions used spaces for indentation instead of TABS.
>

Yes you already told me but the threaded code was submitted a long time ago...:-) BTW I do not submit often !

Tell me your thinking and we can try to find the best way. BTW I am surprised that there are no filter in itk with the same problematic...

Best Regards,

Jerome Schmid

-----------------------------------
Jerome SCHMID
Project Manager/ Engineer
Augmented and Virtual Reality
MIS Centre
Prince of Wales Hospital
Chinese University Of Hong-Kong
-----------------------------------



-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://www.itk.org/mailman/private/insight-developers/attachments/20051121/25c0194a/attachment.htm