On Sat, 2003-09-27 at 12:01, Brandon Low wrote: > Here is a proposed patch, tell me what you think. > > (note this also goes back to wait(1000) once every loop of the creation > thread, I did this to ensure that one call to awaken() will _always_ put > the thread in creation mode. > > --Brandon
Here is my proposed clarification. I deleted the createThreads and deleteThreads functions (which are called only from one place) and put the code into the creator thread's loop. I also moved the awaken() code into getThread(). Look at this to see if you still think changes are needed. [from the earlier message] > What happens if ... the QThreadFactory's creator thread is > able to keep calling createThread repeatedly but NONE of > those threads that are created ever manage to get the > countLock? Then all of a sudden we have tons of extra > threads around. With createThread moved into the text of the loop, it becomes clear that there is no way for available to be off by more than one. It doesn't matter if available says the stack is empty when it has one free thread, but the reverse is a mess -- an NPE or messy code to avoid it. I agree that the code you removed is unnecessary and removal makes the remaining code clearer: this patch removes it too. -- Ed Huff
Index: QThreadFactory.java
===================================================================
RCS file: /cvsroot/freenet/freenet/src/freenet/thread/QThreadFactory.java,v
retrieving revision 1.35
diff -c -r1.35 QThreadFactory.java
*** QThreadFactory.java 27 Sep 2003 07:37:16 -0000 1.35
--- QThreadFactory.java 27 Sep 2003 17:03:16 -0000
***************
*** 73,85 ****
}
while ( available < required ) {
doLog = true;
! createThread();
! synchronized(countLock) {
// required must not increase.
required =
Math.min(required,
Math.max((int) (active * IDEAL_AVAILABLE_RATIO),
2 * MINIMUM_AVAILABLE_ABS));
if ( available > required &&
available > active * MINIMUM_AVAILABLE_RATIO ) {
try {
--- 73,99 ----
}
while ( available < required ) {
doLog = true;
! // createThread();
! QThread newThread;
! long newThreadNumber;
! synchronized(numLock) {
! newThreadNumber = ++threadNumber;
! }
! newThread = new QThread(newThreadNumber);
! synchronized(headLock) {
! newThread.next = headerThread;
! headerThread = newThread;
! }
! synchronized(countLock) {
! available++; // never an overestimate.
// required must not increase.
required =
Math.min(required,
Math.max((int) (active * IDEAL_AVAILABLE_RATIO),
2 * MINIMUM_AVAILABLE_ABS));
+ // Wait after each thread creation to see if we still need it.
+ // This limits the rate of thread creation. Some active
+ // threads might finish.
if ( available > required &&
available > active * MINIMUM_AVAILABLE_RATIO ) {
try {
***************
*** 120,132 ****
}
while ( available > allowed ) {
doLog = true;
! destroyThread();
! synchronized(countLock) {
// allowed must not decrease.
allowed =
Math.max((int) (active * IDEAL_AVAILABLE_RATIO),
Math.max(allowed,
2 * MINIMUM_AVAILABLE_ABS));
if ( available < allowed &&
available < active * MAXIMUM_AVAILABLE_RATIO ) {
try {
--- 134,160 ----
}
while ( available > allowed ) {
doLog = true;
! synchronized(countLock) {
! // Remove a thread from the stack and signal it to die.
! // But if all of the threads have meanwhile started jobs,
! // then do nothing.
! if (available >= 1) {
! available--; // never an overestimate.
! QThread dyingThread;
! synchronized(headLock) {
! dyingThread = headerThread;
! headerThread = headerThread.next;
! }
! dyingThread.die();
! }
// allowed must not decrease.
allowed =
Math.max((int) (active * IDEAL_AVAILABLE_RATIO),
Math.max(allowed,
2 * MINIMUM_AVAILABLE_ABS));
+ // Wait after each thread destruction to see if we
+ // still want to destroy more. This limits the rate
+ // of thread destruction. More work might come in.
if ( available < allowed &&
available < active * MAXIMUM_AVAILABLE_RATIO ) {
try {
***************
*** 236,280 ****
thread.job = job;
thread.start();
- awaken();
- return thread;
- }
-
- /**
- * Creates a thread and adds it to the stack.
- */
- private final void createThread() {
- QThread newThread;
- long newThreadNumber;
- synchronized(numLock) {
- newThreadNumber = ++threadNumber;
- }
- newThread = new QThread(newThreadNumber);
- synchronized(headLock) {
- newThread.next = headerThread;
- headerThread = newThread;
- }
synchronized(countLock) {
! available++; // never an overestimate.
! }
! }
!
! /**
! * Removes a thread from the stack and signals it to die.
! * But if all of the threads have meanwhile started jobs,
! * then does nothing.
! */
! private final void destroyThread() {
! QThread dyingThread;
! synchronized(countLock) {
! if (available < 1) return;
! available--; // never an overestimate.
! }
! synchronized(headLock) {
! dyingThread = headerThread;
! headerThread = headerThread.next;
}
! dyingThread.die();
}
/**
--- 264,278 ----
thread.job = job;
thread.start();
synchronized(countLock) {
! if ( ( available < MINIMUM_AVAILABLE_ABS) ||
! ( available < active * MINIMUM_AVAILABLE_RATIO) ||
! ( ( available > (3 * MINIMUM_AVAILABLE_ABS)) &&
! ( available > active * MAXIMUM_AVAILABLE_RATIO))) {
! countLock.notifyAll();
! }
}
! return thread;
}
/**
***************
*** 288,304 ****
synchronized(countLock) {
active--;
available++; // never an overestimate
- }
- }
-
- private final void awaken() {
- synchronized(countLock) {
- if ( ( available < MINIMUM_AVAILABLE_ABS) ||
- ( available < active * MINIMUM_AVAILABLE_RATIO) ||
- ( ( available > (3 * MINIMUM_AVAILABLE_ABS)) &&
- ( available > active * MAXIMUM_AVAILABLE_RATIO))) {
- countLock.notifyAll();
- }
}
}
--- 286,291 ----
signature.asc
Description: This is a digitally signed message part
_______________________________________________ Devl mailing list [EMAIL PROTECTED] http://dodo.freenetproject.org/cgi-bin/mailman/listinfo/devl
