[x265] [PATCH] bug: build warnings removal for linux, correct threading masks for windows

[Pradeep Ramachandran]

But we're storing a reference to the m_winCpuMask variable. If it were a
local variable, it won't work, right?

Pradeep Ramachandran, PhD
Solution Architect,
Multicoreware Inc.
Ph:   +91 99627 82018

On Wed, Sep 9, 2015 at 11:12 AM, Deepthi Nandakumar <
deepthi at multicorewareinc.com> wrote:

>
>
> On Tue, Sep 8, 2015 at 4:56 PM, <pradeep at multicorewareinc.com> wrote:
>
>> # HG changeset patch
>> # User Pradeep Ramachandran <pradeep at multicorewareinc.com>
>> # Date 1441710481 -19800
>> #      Tue Sep 08 16:38:01 2015 +0530
>> # Node ID 314dbb79621f74a18108439fb49cc6f10070bcca
>> # Parent  e1adac00dce8e5641cbe9aec3d50a72261c308d9
>> bug: build warnings removal for linux, correct threading masks for
>> windows.
>>
>> diff -r e1adac00dce8 -r 314dbb79621f source/common/threadpool.cpp
>> --- a/source/common/threadpool.cpp      Thu Sep 03 14:41:06 2015 +0530
>> +++ b/source/common/threadpool.cpp      Tue Sep 08 16:38:01 2015 +0530
>> @@ -378,8 +378,16 @@
>>      X265_CHECK(numThreads <= MAX_POOL_THREADS, "a single thread pool
>> cannot have more than MAX_POOL_THREADS threads\n");
>>
>>  #if defined(_WIN32_WINNT) && _WIN32_WINNT >= _WIN32_WINNT_WIN7
>> -    m_winNodemask = nodeMask & ~(0x1 << getNumaNodeCount());
>> -    m_numaNodeMask = &m_winNodemask;
>> +    m_winCpuMask = 0x0;
>>
> +    GROUP_AFFINITY groupAffinity;
>> +    for (int i = 0; i < getNumaNodeCount(); i++)
>> +    {
>> +        int numaNode = ((nodeMask >> i) & 0x1U) ? i : -1;
>> +        if (numaNode != -1)
>> +            if (GetNumaNodeProcessorMaskEx((USHORT)numaNode,
>> &groupAffinity))
>> +                m_winCpuMask |= groupAffinity.Mask;
>> +    }
>> +    m_numaMask = &m_winCpuMask;
>>  #elif HAVE_LIBNUMA
>>      if (numa_available() >= 0)
>>      {
>> @@ -387,11 +395,13 @@
>>          if (nodemask)
>>          {
>>              *(nodemask->maskp) = nodeMask;
>> -            m_numaNodeMask = nodemask;
>> +            m_numaMask = nodemask;
>>          }
>>          else
>>              x265_log(NULL, X265_LOG_ERROR, "unable to get NUMA node mask
>> for %lx\n", nodeMask);
>>      }
>> +#else
>> +    (void)nodeMask;
>>  #endif
>>
>>      m_numWorkers = numThreads;
>> @@ -449,33 +459,35 @@
>>      X265_FREE(m_jpTable);
>>
>>  #if HAVE_LIBNUMA
>> -    if(m_numaNodeMask)
>> -        numa_free_nodemask((struct bitmask*)m_numaNodeMask);
>> +    if(m_numaMask)
>> +        numa_free_nodemask((struct bitmask*)m_numaMask);
>>  #endif
>>  }
>>
>>  void ThreadPool::setCurrentThreadAffinity()
>>  {
>> -    setThreadNodeAffinity(m_numaNodeMask);
>> +    setThreadNodeAffinity(m_numaMask);
>>  }
>>
>>  /* static */
>> -void ThreadPool::setThreadNodeAffinity(void *numaNodeMask)
>> +void ThreadPool::setThreadNodeAffinity(void *numaMask)
>>  {
>>  #if defined(_WIN32_WINNT) && _WIN32_WINNT >= _WIN32_WINNT_WIN7
>> -    if (SetThreadAffinityMask(GetCurrentThread(),
>> (DWORD_PTR)(*((DWORD*)numaNodeMask))))
>> +    if (SetThreadAffinityMask(GetCurrentThread(),
>> (DWORD_PTR)(*((DWORD*)numaMask))))
>>          return;
>>      else
>>          x265_log(NULL, X265_LOG_ERROR, "unable to set thread affinity
>> for NUMA node mask\n");
>>  #elif HAVE_LIBNUMA
>>      if (numa_available() >= 0)
>>      {
>> -        numa_run_on_node_mask((struct bitmask*)numaNodeMask);
>> -        numa_set_interleave_mask((struct bitmask*)numaNodeMask);
>> +        numa_run_on_node_mask((struct bitmask*)numaMask);
>> +        numa_set_interleave_mask((struct bitmask*)numaMask);
>>          numa_set_localalloc();
>>          return;
>>      }
>>      x265_log(NULL, X265_LOG_ERROR, "unable to set thread affinity for
>> NUMA node mask\n");
>> +#else
>> +    (void)numaMask;
>>  #endif
>>      return;
>>  }
>> diff -r e1adac00dce8 -r 314dbb79621f source/common/threadpool.h
>> --- a/source/common/threadpool.h        Thu Sep 03 14:41:06 2015 +0530
>> +++ b/source/common/threadpool.h        Tue Sep 08 16:38:01 2015 +0530
>> @@ -83,9 +83,9 @@
>>      sleepbitmap_t m_sleepBitmap;
>>      int           m_numProviders;
>>      int           m_numWorkers;
>> -    void*         m_numaNodeMask;
>> +    void*         m_numaMask; // node mask in linux, cpu mask in windows
>>  #if defined(_WIN32_WINNT) && _WIN32_WINNT >= _WIN32_WINNT_WIN7
>> -    DWORD         m_winNodemask;
>> +    DWORD         m_winCpuMask;
>>  #endif
>>
>
> Thanks, I will push this - but you could clean this up further by making
> m_winCpuMask a local variable (doesnt need to be a threadpool class
> member).
>
>      bool          m_isActive;
>>
>> @@ -106,7 +106,7 @@
>>
>>      static int  getCpuCount();
>>      static int  getNumaNodeCount();
>> -    static void setThreadNodeAffinity(void *numaNodeMask);
>> +    static void setThreadNodeAffinity(void *numaMask);
>>  };
>>
>>  /* Any worker thread may enlist the help of idle worker threads from the
>> same
>> _______________________________________________
>> x265-devel mailing list
>> x265-devel at videolan.org
>> https://mailman.videolan.org/listinfo/x265-devel
>>
>
>
> _______________________________________________
> x265-devel mailing list
> x265-devel at videolan.org
> https://mailman.videolan.org/listinfo/x265-devel
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/x265-devel/attachments/20150909/2aebb4ca/attachment.html>