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

[Deepthi Nandakumar]

Hmm, m_numaMask points to different sized memory chunks on Win/Linux -
node/cpu bit vectors.

For Windows, the right thing would be to malloc a DWORD and free it in the
destructor.

On Wed, Sep 9, 2015 at 11:27 AM, Pradeep Ramachandran <
pradeep at multicorewareinc.com> wrote:

> 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
>>
>>
>
> _______________________________________________
> 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/8bc0ab04/attachment-0001.html>