Moderators: hgm, Rebel, chrisw
Sorry for late reply, I have seen your question, but I wanted to answer during the week end because is not a trivial question and I want to have a bit of time....more later !hcyrano wrote:ok i see
return a object Splitpoint clean.
@+
If I remember correctly, I came to conclusion that this could be safely removed. For some reason the change never entered the codebase...hcyrano wrote:hi,
in void ThreadsManager::idle_loop(int threadID, SplitPoint* sp);
can you explain this lines:
// Because sp->slaves[] is reset under lock protection,
// be sure sp->lock has been released before to return.
lock_grab(&(sp->lock));
lock_release(&(sp->lock));
i don't see real utility
thx a lot
I agree, but I think it is useful to have an object splitPoint clean after using.the point is of course to have clean splitPoint object, but I think I came to conclusion that this isn't absolute necessity.
, because i can use std::vector<int> slaves and not a fix table volatile int slaves[].Yes but you have an additional shared variable. Here the aim is to reduce the shared vairables to the bare minimum to keep things easier.hcyrano wrote:.I agree, but I think it is useful to have an object splitPoint clean after using.the point is of course to have clean splitPoint object, but I think I came to conclusion that this isn't absolute necessity.
this is more satisfying, keep this code.
for info: i prefere "old" code this nCpus
Code: Select all
// Pick the next available split point object from the split point stack
SplitPoint& splitPoint = masterThread.splitPoints[masterThread.activeSplitPoints];
int workersCnt = 1; // At least the master is included
lock_grab(&MPLock);
// Allocate available threads setting state to THREAD_BOOKED
for (unsigned int i = 0; !Fake && i < ActiveThreads; i++) {
splitPoint.slaves[i] = 0;
if (workersCnt < MaxThreadsPerSplitPoint && thread_is_available(i, master))
{
threads[i].state = THREAD_BOOKED;
threads[i].splitPoint = &splitPoint; //initialisation later
splitPoint.slaves[i] = 1;
workersCnt++;
}
}
// We can release the lock because slave threads are already booked and master is not available
lock_release(&MPLock);
if (workerCnt == 1)
return;
masterThread.activeSplitPoints++;
Code: Select all
for(RXSplitPoint* sp = threads[master].splitPoint; sp != NULL; sp = sp->parent) {
if(sp == &(threads[slave].splitPointStack[localActiveSplitPoints-1]))
return true;
}
Code: Select all
--- a/src/search.cpp
+++ b/src/search.cpp
@@ -2578,16 +2578,12 @@ namespace {
int i, master = p.thread();
Thread& masterThread = threads[master];
- lock_grab(&MPLock);
+ // If we are here it means we are not available
+ assert(masterThread.state != THREAD_AVAILABLE);
- // If no other thread is available to help us, or if we have too many
- // active split points, don't split.
- if ( !available_thread_exists(master)
- || masterThread.activeSplitPoints >= MAX_ACTIVE_SPLIT_POINTS)
- {
- lock_release(&MPLock);
+ // If we have too many active split points, don't split
+ if (masterThread.activeSplitPoints >= MAX_ACTIVE_SPLIT_POINTS)
return;
- }
// Pick the next available split point object from the split point stack
SplitPoint& splitPoint = masterThread.splitPoints[masterThread.activeSplitPoints++];
@@ -2612,12 +2608,11 @@ namespace {
masterThread.splitPoint = &splitPoint;
- // If we are here it means we are not available
- assert(masterThread.state != THREAD_AVAILABLE);
-
int workersCnt = 1; // At least the master is included
- // Allocate available threads setting state to THREAD_BOOKED
+ lock_grab(&MPLock);
+
+ // Try to allocate available threads setting state to THREAD_BOOKED
for (i = 0; !Fake && i < ActiveThreads && workersCnt < MaxThreadsPerSplitPoint; i++)
if (thread_is_available(i, master))
{
@@ -2627,11 +2622,16 @@ namespace {
workersCnt++;
}
- assert(Fake || workersCnt > 1);
-
- // We can release the lock because slave threads are already booked and master is not available
+ // We can release the lock because slave threads are already booked
lock_release(&MPLock);
+ // If we fail to book any slave release the split point and return
+ if (!Fake && workersCnt == 1)
+ {
+ masterThread.activeSplitPoints--;
+ return;
+ }
+
// Tell the threads that they have work to do. This will make them leave
// their idle loop. But before copy search stack tail for each thread.
for (i = 0; i < ActiveThreads; i++)
Code: Select all
--- a/src/search.cpp
+++ b/src/search.cpp
@@ -2658,14 +2658,10 @@ namespace {
// We have returned from the idle loop, which means that all threads are
// finished. Update alpha and bestValue, and return.
- lock_grab(&MPLock);
-
*alpha = splitPoint.alpha;
*bestValue = splitPoint.bestValue;
masterThread.activeSplitPoints--;
masterThread.splitPoint = splitPoint.parent;
-
- lock_release(&MPLock);
}
Code: Select all
--- a/src/search.cpp
+++ b/src/search.cpp
@@ -2384,10 +2384,15 @@ namespace {
lock_grab(&(sp->lock));
lock_release(&(sp->lock));
- assert(threads[threadID].state == THREAD_AVAILABLE);
+ lock_grab(&MPLock);
- threads[threadID].state = THREAD_SEARCHING;
- return;
+ if (threads[threadID].state == THREAD_AVAILABLE)
+ {
+ threads[threadID].state = THREAD_SEARCHING;
+ lock_release(&MPLock);
+ return;
+ }
+ lock_release(&MPLock);
}
}
}
helpful master concept : can help only a sub-tree, or here all is finish, so it is not possible taht this thread are booked.and especially this that seems important because fixes a possible race in case someone books us while we are waiting for other threads to finish. I don't know how was possible to miss that for so long, or perhaps I am missing something obvious. But I would like to show you so to have your comment on this:
New macpro can have 24 threads (12 + 12HT)mcostalba wrote:Thanks, this is a possible patch (only slightly tested)
Regarding your proposal is not that we don't love, simply it doesn't seems to improve anything with a QUAD, perhas with an OCTAL we could see something better, but we don't have an OCTAL