<div dir="ltr"><div>Hmm, m_numaMask points to different sized memory chunks on Win/Linux - node/cpu bit vectors.<br><br></div>For Windows, the right thing would be to malloc a DWORD and free it in the destructor.<br></div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Sep 9, 2015 at 11:27 AM, Pradeep Ramachandran <span dir="ltr"><<a href="mailto:pradeep@multicorewareinc.com" target="_blank">pradeep@multicorewareinc.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">But we're storing a reference to the m_winCpuMask variable. If it were a local variable, it won't work, right?</div><div class="gmail_extra"><br clear="all"><div><div><div dir="ltr"><div><div dir="ltr"><div><div dir="ltr"><div><div dir="ltr">Pradeep Ramachandran, PhD<div>Solution Architect,</div><div>Multicoreware Inc.</div><div>Ph: +91 99627 82018</div></div></div></div></div></div></div></div></div></div><div><div class="h5">
<br><div class="gmail_quote">On Wed, Sep 9, 2015 at 11:12 AM, Deepthi Nandakumar <span dir="ltr"><<a href="mailto:deepthi@multicorewareinc.com" target="_blank">deepthi@multicorewareinc.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote"><div><div>On Tue, Sep 8, 2015 at 4:56 PM, <span dir="ltr"><<a href="mailto:pradeep@multicorewareinc.com" target="_blank">pradeep@multicorewareinc.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"># HG changeset patch<br>
# User Pradeep Ramachandran <<a href="mailto:pradeep@multicorewareinc.com" target="_blank">pradeep@multicorewareinc.com</a>><br>
# Date 1441710481 -19800<br>
# Tue Sep 08 16:38:01 2015 +0530<br>
# Node ID 314dbb79621f74a18108439fb49cc6f10070bcca<br>
# Parent e1adac00dce8e5641cbe9aec3d50a72261c308d9<br>
bug: build warnings removal for linux, correct threading masks for windows.<br>
<br>
diff -r e1adac00dce8 -r 314dbb79621f source/common/threadpool.cpp<br>
--- a/source/common/threadpool.cpp Thu Sep 03 14:41:06 2015 +0530<br>
+++ b/source/common/threadpool.cpp Tue Sep 08 16:38:01 2015 +0530<br>
@@ -378,8 +378,16 @@<br>
X265_CHECK(numThreads <= MAX_POOL_THREADS, "a single thread pool cannot have more than MAX_POOL_THREADS threads\n");<br>
<br>
#if defined(_WIN32_WINNT) && _WIN32_WINNT >= _WIN32_WINNT_WIN7<br>
- m_winNodemask = nodeMask & ~(0x1 << getNumaNodeCount());<br>
- m_numaNodeMask = &m_winNodemask;<br>
+ m_winCpuMask = 0x0;<br></blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+ GROUP_AFFINITY groupAffinity;<br>
+ for (int i = 0; i < getNumaNodeCount(); i++)<br>
+ {<br>
+ int numaNode = ((nodeMask >> i) & 0x1U) ? i : -1;<br>
+ if (numaNode != -1)<br>
+ if (GetNumaNodeProcessorMaskEx((USHORT)numaNode, &groupAffinity))<br>
+ m_winCpuMask |= groupAffinity.Mask;<br>
+ }<br>
+ m_numaMask = &m_winCpuMask;<br>
#elif HAVE_LIBNUMA<br>
if (numa_available() >= 0)<br>
{<br>
@@ -387,11 +395,13 @@<br>
if (nodemask)<br>
{<br>
*(nodemask->maskp) = nodeMask;<br>
- m_numaNodeMask = nodemask;<br>
+ m_numaMask = nodemask;<br>
}<br>
else<br>
x265_log(NULL, X265_LOG_ERROR, "unable to get NUMA node mask for %lx\n", nodeMask);<br>
}<br>
+#else<br>
+ (void)nodeMask;<br>
#endif<br>
<br>
m_numWorkers = numThreads;<br>
@@ -449,33 +459,35 @@<br>
X265_FREE(m_jpTable);<br>
<br>
#if HAVE_LIBNUMA<br>
- if(m_numaNodeMask)<br>
- numa_free_nodemask((struct bitmask*)m_numaNodeMask);<br>
+ if(m_numaMask)<br>
+ numa_free_nodemask((struct bitmask*)m_numaMask);<br>
#endif<br>
}<br>
<br>
void ThreadPool::setCurrentThreadAffinity()<br>
{<br>
- setThreadNodeAffinity(m_numaNodeMask);<br>
+ setThreadNodeAffinity(m_numaMask);<br>
}<br>
<br>
/* static */<br>
-void ThreadPool::setThreadNodeAffinity(void *numaNodeMask)<br>
+void ThreadPool::setThreadNodeAffinity(void *numaMask)<br>
{<br>
#if defined(_WIN32_WINNT) && _WIN32_WINNT >= _WIN32_WINNT_WIN7<br>
- if (SetThreadAffinityMask(GetCurrentThread(), (DWORD_PTR)(*((DWORD*)numaNodeMask))))<br>
+ if (SetThreadAffinityMask(GetCurrentThread(), (DWORD_PTR)(*((DWORD*)numaMask))))<br>
return;<br>
else<br>
x265_log(NULL, X265_LOG_ERROR, "unable to set thread affinity for NUMA node mask\n");<br>
#elif HAVE_LIBNUMA<br>
if (numa_available() >= 0)<br>
{<br>
- numa_run_on_node_mask((struct bitmask*)numaNodeMask);<br>
- numa_set_interleave_mask((struct bitmask*)numaNodeMask);<br>
+ numa_run_on_node_mask((struct bitmask*)numaMask);<br>
+ numa_set_interleave_mask((struct bitmask*)numaMask);<br>
numa_set_localalloc();<br>
return;<br>
}<br>
x265_log(NULL, X265_LOG_ERROR, "unable to set thread affinity for NUMA node mask\n");<br>
+#else<br>
+ (void)numaMask;<br>
#endif<br>
return;<br>
}<br>
diff -r e1adac00dce8 -r 314dbb79621f source/common/threadpool.h<br>
--- a/source/common/threadpool.h Thu Sep 03 14:41:06 2015 +0530<br>
+++ b/source/common/threadpool.h Tue Sep 08 16:38:01 2015 +0530<br>
@@ -83,9 +83,9 @@<br>
sleepbitmap_t m_sleepBitmap;<br>
int m_numProviders;<br>
int m_numWorkers;<br>
- void* m_numaNodeMask;<br>
+ void* m_numaMask; // node mask in linux, cpu mask in windows<br>
#if defined(_WIN32_WINNT) && _WIN32_WINNT >= _WIN32_WINNT_WIN7<br>
- DWORD m_winNodemask;<br>
+ DWORD m_winCpuMask;<br>
#endif<br></blockquote></div></div><div><br><div></div>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). <br><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span>
bool m_isActive;<br>
<br>
@@ -106,7 +106,7 @@<br>
<br>
static int getCpuCount();<br>
static int getNumaNodeCount();<br>
- static void setThreadNodeAffinity(void *numaNodeMask);<br>
+ static void setThreadNodeAffinity(void *numaMask);<br>
};<br>
<br>
/* Any worker thread may enlist the help of idle worker threads from the same<br></span>
_______________________________________________<br>
x265-devel mailing list<br>
<a href="mailto:x265-devel@videolan.org" target="_blank">x265-devel@videolan.org</a><br>
<a href="https://mailman.videolan.org/listinfo/x265-devel" rel="noreferrer" target="_blank">https://mailman.videolan.org/listinfo/x265-devel</a><br>
</blockquote></div><br></div></div>
<br>_______________________________________________<br>
x265-devel mailing list<br>
<a href="mailto:x265-devel@videolan.org" target="_blank">x265-devel@videolan.org</a><br>
<a href="https://mailman.videolan.org/listinfo/x265-devel" rel="noreferrer" target="_blank">https://mailman.videolan.org/listinfo/x265-devel</a><br>
<br></blockquote></div><br></div></div></div>
<br>_______________________________________________<br>
x265-devel mailing list<br>
<a href="mailto:x265-devel@videolan.org">x265-devel@videolan.org</a><br>
<a href="https://mailman.videolan.org/listinfo/x265-devel" rel="noreferrer" target="_blank">https://mailman.videolan.org/listinfo/x265-devel</a><br>
<br></blockquote></div><br></div>