[FFmpeg-devel] [PATCH] Don't needlessly reinitialize ff_cos_## tables.

[wm4]

On Sat, 24 Oct 2015 04:13:18 +0200
Michael Niedermayer <michael at niedermayer.cc> wrote:

> On Thu, Oct 22, 2015 at 09:48:30PM -0700, Dale Curtis wrote:
> > On Wed, Oct 21, 2015 at 6:52 PM, Dale Curtis <dalecurtis at chromium.org>
> > wrote:
> >   
> > > On Tue, Oct 20, 2015 at 11:50 PM, Michael Niedermayer <  
> > > michael at niedermayer.cc> wrote:
> > >>
> > >> the last element to be written should be checked, so that if
> > >> initialization is done by 2 threads at the same time, neither can
> > >> return from this function without initialization having finished
> > >>
> > >> also the race detectors are broken if they complain about cases where
> > >> a variable that has value a is set to value a, that cannot be part of
> > >> a race, not even if a is written byte per byte instead of atomically
> > >> unless theres something iam missing
> > >> Is this something that can be fixed or disabled on the side of the
> > >> race detectors?
> > >> It might reduce false positives in FFmpeg and potentially other
> > >> tools.
> > >>  
> > >
> > > We can suppress it, which I think is more reasonable then the overhead
> > > it'd take to make this "race" go away. I notice the sin tables are
> > > initialized within the fft context so there's no "race." Is there a reason
> > > the cosine tables aren't done this way?
> > >  
> > 
> > Actually it looks like sin may suffer from the same issues -- it doesn't
> > make a copy like I was thinking it did. One of the TSAN folk detailed why
> > suppression isn't a good idea here  
> 
> > https://codereview.chromium.org/1412123007/#msg7 -- which roughly says we
> > can't rely on the compiler to do the right thing here.  
> 
> quite independant of the sin/cos discussion here, This statement is
> IMHO somewhat problematic
> It may be strictly true in a language lawyer sense but
> 
> Lets assume that compilers did add random writes into writable arrays
> as long as they restore it before the next function call or return.
> like it seems assumed in the linked discussion
> 
> Frame multithreading has one thread decoding a frame and the next
> using that frame after it has been written, the individual accesses
> are not protected by mutexes nor could they, it would defeat the
> idea of parallel decoding ...
> Now if the compiler could randomly add writes that exist nowhere in
> the C code as long as the thread itself doesnt access it at that time,
> then this would break.

All what matters is if there are memory barriers between the accesses.
So for example if one thread is done with decoding a frame, and is
handed off to other threads as reference frame, then the
synchronization that takes care of this "handing off" will perform the
required memory barrier. Or am I getting this wrong?