[FFmpeg-devel] [PATCH] Avoid creating unecessary dependencies on thread libraries.

[wm4]

On Mon, 21 Nov 2016 16:23:03 +0000
Gregory J Wolfe <gregory.wolfe at kodakalaris.com> wrote:

> > -----Original Message-----
> > From: ffmpeg-devel [mailto:ffmpeg-devel-bounces at ffmpeg.org] On
> > Behalf Of wm4
> > Sent: Monday, November 21, 2016 10:05 AM
> > To: ffmpeg-devel at ffmpeg.org
> > Subject: Re: [FFmpeg-devel] [PATCH] Avoid creating unecessary
> > dependencies on thread libraries.
> > 
> > On Mon, 21 Nov 2016 09:58:33 -0500
> > "Gregory J. Wolfe" <gregory.wolfe at kodakalaris.com> wrote:
> >   
> > > (1) Multi-threading support requires knowing the number of CPUs  
> > available.  
> > > When building with MinGW on a Windows system, both gcc and  
> > Windows run  
> > > time functions are available to get this information.  However, when  
> > Windows  
> > > threading has been selected, the Windows function should be used,  
> > not the  
> > > gcc function.  This avoids creating an unnecessary dependency on  
> > the gcc  
> > > thread library.
> > >
> > > (2) When ALL threading support is disabled, the build should not  
> > create a  
> > > dependency on ANY thread library.
> > >
> > > Signed-off-by: Gregory J. Wolfe <gregory.wolfe at kodakalaris.com>
> > > ---
> > >  libavutil/cpu.c | 8 +++++++-
> > >  1 file changed, 7 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/libavutil/cpu.c b/libavutil/cpu.c
> > > index f5785fc..3843778 100644
> > > --- a/libavutil/cpu.c
> > > +++ b/libavutil/cpu.c
> > > @@ -258,10 +258,15 @@ int av_cpu_count(void)
> > >      static volatile int printed;
> > >
> > >      int nb_cpus = 1;
> > > +#if HAVE_THREADS
> > >  #if HAVE_WINRT
> > >      SYSTEM_INFO sysinfo;
> > >  #endif
> > > -#if HAVE_SCHED_GETAFFINITY && defined(CPU_COUNT)
> > > +    // if HAVE_W32THREADS and HAVE_GETPROCESSAFFINITYMASK,  
> > we will use  
> > > +    // Windows GetProcessAffinityMask() instead of gcc library  
> > function  
> > > +    // sched_getaffinity().  This avoids creating a dependency on the  
> > gcc  
> > > +    // thread library that we don't need/want.
> > > +#if HAVE_SCHED_GETAFFINITY && defined(CPU_COUNT) &&  
> > !(HAVE_W32THREADS && HAVE_GETPROCESSAFFINITYMASK)  
> > >      cpu_set_t cpuset;
> > >
> > >      CPU_ZERO(&cpuset);
> > > @@ -286,6 +291,7 @@ int av_cpu_count(void)
> > >      GetNativeSystemInfo(&sysinfo);
> > >      nb_cpus = sysinfo.dwNumberOfProcessors;
> > >  #endif
> > > +#endif
> > >
> > >      if (!printed) {
> > >          av_log(NULL, AV_LOG_DEBUG, "detected %d logical cores\n",  
> > nb_cpus);
> > 
> > Wouldn't it be better to move the HAVE_GETPROCESSAFFINITYMASK
> > case
> > above HAVE_SCHED_GETAFFINITY, and avoid making the ifdeffery
> > mess
> > worse? GetProcessAffinityMask is available on every supported
> > Windows
> > and could as well be replaced by "ifdef _WIN32" instead.
> >   
> 
> Yeah, I had the same thought.  For the first go around I opted for the
> solution that moved around the least code, not knowing for sure if
> I might cause some undesirable side effects by reordering the existing
> code.  I can easily redo the patch if we're in agreement on this point.

Depends if there are any cargo cult people who are against it.

> > Also, technically you might have to stop the configure script to stop
> > linking to libpthread. I'm not sure what exactly stops it from doing
> > that in your case.  
>  
> The result I observed was that the resulting avutil.dll did NOT have
> a dependency on the gcc run time.  My interpretation of this is that
> if you link against a library, but don't reference any symbols in it, then
> it's as if you never linked against it in the first place.

This could still be caused by certain link flag (forgot which one). In
general, linking against a dynamic library without using any of its
functions, could still have side effects, such as registering plugins
in the DLL init functions and such.