Hi! Stefan Steiniger schrieb: > Similar to Landon I have never touched threading nor heared any lectures > or read books on it. Thus .. i dont know if it turns out to be good or bad. > But i know that making things more clean is good objective. Thus, i > support the changes. It would be good when we finally change the code, > that you also put into the code some docs or references where you > outline why somethings has changed. (actually I currently would opt to > leave the old code as comment and not to remove entirely)
The ThreadQueue is JavaDoc'ed but I can add some design notes as well. Keeping old things as comments negates the existence of version control systems. CVS is exactly the time machine you for this. As I pointed out earlier having a ChangeLog would be nice to document motivations and backgrounds for a change. Simply generating a ChangeLog out of the commit messages is convenient but it does not uncover the true potential of such a 'log' file. > Due to my lack of knowledge i forwarded your email to a colleague who > developed a web-service based on JUMP (which runs on a server, see [1]), > an ajax client and recently did some multiprocessor experiments for the > web-services processing. I look forward to his comments. > I hope 1) he will look on your proposal and 2) you can wait some days > until he, Larry and Michael got an opinion. @Michael: You ask what to change. For the moment just replace ThreadQueue.java with one I've sent last. The first patches are addressing the problem with ThreadQueue limited to one Thread at a time (defaultRenderingThreadQueue in RenderingManger). You can apply these patches too, but they are not needed any longer. With next patch I'll send the this second thread queue will be removed from RenderingManager entirely. What to test? Doing your day-to-day is the best thing you can do. The most important thing is that the new one behaves exactly like the old one. The magic word is compatibility. I don't want to break the show. Use plug-ins, use many layers, use less layer, use db and WMS layers. Do all the fancy stuff you like to do. If the OJ freezes or something other looks odd just tell me. :-) - Sascha > Michaël Michaud schrieb: >> Sascha L. Teichmann a écrit : >> >>> Really back to topic: >>> >>> Find attached a replacement for ThreadQueue [1]. >>> To use it just overwrite the original one. >>> >>> >> Hi Sascha : >> >> I think that trying to have a cleaner and more simple code is an >> excellent goal, and I'd like to help, but I'm not sure I can understand >> all these thread issues. >> If you tell me exactly which classes I must replace (on ly ThreadQueue >> or also the pieces of code from your previous mail) and what kind of >> tests I should do (rendering different kind of layers ? mixing different >> kind of layers), I'll try to make some more tests on my desktop. >> >> Thanks, for the hard work >> >> Michaël >> >>> This one works for the real parallel case of >>> layer rendering too. Each time a thread finished >>> executing its Runnable it looks into the queue >>> if they are more jobs to do. This prevents unnecessary >>> thread creations/shutdowns. If the queue is empty >>> the worker thread is kept alive for 5 seconds waiting >>> for new jobs. This results in a kind of thread pooling. >>> >>> @Larry: I've isolated my implementation and the OJ >>> ThreadQueue and done a synthetic benchmark with a >>> larger number of jobs (10,000+). My implementation >>> works about two orders faster than the OJ one. >>> But this is of little meaning because OJ only >>> has to handle a number of jobs equal the number >>> of layers. This will hardly hit 10,000+ ;-) >>> But again: My mission is improve the structure not >>> primarily the speed. >>> >>> I've tested the new ThreadQueue to some extent but >>> I'am male(tm) after all ... potentially making mistakes. >>> It would be very kind if someone test it too. >>> >>> My next step will be some clean up in the RenderingManager [2]. >>> I'am not sure that it is really needed to have two ThreadQueues >>> there. The effect of the single tread one can be easily >>> simulated with a data structure like the RunnableArrayList which >>> I've posted already. >>> >>> Any comments? >>> >>> Yours, >>> Sascha >>> >>> [1] com.vividsolutions.jump.workbench.ui.renderer.ThreadQueue >>> [2] com.vividsolutions.jump.workbench.ui.renderer.RenderingManager >>> >>> Sunburned Surveyor schrieb: >>> >>> >>>> Sascha, >>>> >>>> Please accept my sincerest aopologies. I'm afriad my American >>>> ignorance of other cultures is more than just a little obvious at >>>> times. >>>> >>>> I believe I have made the same mistake with Jan. :] >>>> >>>> Please be patient with me as I learn the details of cultures across >>>> the Pacific and Atalantic Oceans! >>>> >>>> The Sunburned Surveyor >>>> >>>> On 5/24/07, Sascha L. Teichmann <[EMAIL PROTECTED]> wrote: >>>> >>>> >>>>> TNX, but for the records 'he' would be more suited in my case. >>>>> >>>>> 'Sascha' is basically a Russian term of endearment for the >>>>> boys name 'Alexander' but it's also used as a girls name. >>>>> >>>>> BTW: 'Jan' is a girls name in the US too, isn't? ;-) >>>>> >>>>> - Sascha >>>>> >>>>> >>>>> Sunburned Surveyor schrieb: >>>>> >>>>> >>>>>> Sascha and Larry, >>>>>> >>>>>> I must admit that I am way over my head here. I haven't done much >>>>>> thread programming in Java. (Hopefully Stefan has!) >>>>>> >>>>>> Sascha wrote: "My primary goal is to simplify the >>>>>> threading code to make it more reliable in terms of time." >>>>>> >>>>>> This seems like an admirable goal to me. If Larry, or a similar >>>>>> programmer of his experience, agrees that this changes would be >>>>>> beneficial, I say we give Sascha a shot at it. It sounds like she has >>>>>> considered her changes carefully. >>>>>> >>>>>> Just my two cents. >>>>>> >>>>>> The Sunburned Surveyor >>>>>> >>>>>> >>>>>> >>>>>> On 5/24/07, Sascha L. Teichmann <[EMAIL PROTECTED]> wrote: >>>>>> >>>>>> >>>>>>> Hi Larry, >>>>>>> >>>>>>> short answer first: No, I don't have any benchmarks, yet. >>>>>>> >>>>>>> The long answer: My primary goal is to simplify the >>>>>>> threading code to make it more reliable in terms of time. >>>>>>> Gaining performance improvements would be a neat side effect. >>>>>>> >>>>>>> Background: Multi-threading may be fine for slow layers >>>>>>> which arrives later on the screen but for exporting >>>>>>> the data (to print/layout e.g) it would be nice to have >>>>>>> them arriving one after the other. >>>>>>> >>>>>>> My final goal is to have a simple switch between the normal >>>>>>> and the serial mode. To archive that I try to carefully >>>>>>> refactor the system doing little patches step by step >>>>>>> not to break it. Refactoring the ThreadQueue with it's >>>>>>> 'flaws' seems a to be a good starting point to me. >>>>>>> >>>>>>> One reason for this change is the fact that you are able >>>>>>> to figure out if the default thread runs empty. But there >>>>>>> is no way to figure out when the last thread ends. That >>>>>>> are different things. A mechanism for this is planned. >>>>>>> >>>>>>> Sorry, if I've shadowed my true intentions to much. Maybe >>>>>>> I should discuss more before I send patches. ;-) >>>>>>> >>>>>>> Back to the technical side: >>>>>>> >>>>>>> The patch needs some testing but I don't expect too much >>>>>>> performance improvement. >>>>>>> >>>>>>> A less intrusive alternative to bind the Runnables that go to the >>>>>>> default ThreadQueue into one thread is to create a container >>>>>>> which self is Runnable (see e.g. RunnableArrayList attached) >>>>>>> and put them into an instance of this class. >>>>>>> This container is put into multiRendererThreadQueue as a Runnable. >>>>>>> With this modification the defaultRendererThreadQueue can >>>>>>> be removed (multiRendererThreadQueue renamed to >>>>>>> defaultRendererThreadQueue). Only an idea ... I'm in discussion >>>>>>> mode now. ;-) >>>>>>> >>>>>>> - Sascha >>>>>>> >>>>>>> >>>>>>> Larry Becker schrieb: >>>>>>> >>>>>>> >>>>>>>> Hi Sascha, >>>>>>>> >>>>>>>> I read your comments and look at your code with interest. It appears >>>>>>>> to be an improved ThreadQueue implementation, but will require a lot of >>>>>>>> testing to verify. Before I invest this time, I would like to know >>>>>>>> what >>>>>>>> problem it is solving. I see your dislikes a - e, but these are not >>>>>>>> really problems, only architectural critiques. Have you done any >>>>>>>> benchmarks that show that the new SingleThreadQueue speeds up >>>>>>>> rendering? Your logical argument that it should be more efficient is >>>>>>>> persuasive, but I have been surprised by Java before. >>>>>>>> >>>>>>>> respectfully, >>>>>>>> Larry Becker >>>>>>>> >>>>>>>> On 5/23/07, *Sascha L. Teichmann* <[EMAIL PROTECTED] >>>>>>>> <mailto:[EMAIL PROTECTED]>> wrote: >>>>>>>> >>>>>>>> Hi together, >>>>>>>> >>>>>>>> as some of you may already know i have my dislikes against >>>>>>>> ThreadQueue [1] (Hi, Larry!) see my mail [2] >>>>>>>> >>>>>>>> a - It forks a new thread for any Runnable it processes. >>>>>>>> b - It has an ugly busy wait loop inside. >>>>>>>> c - The event listener for empty queue fires to often. >>>>>>>> d - The default ThreadQueue is some kind of thread serializer. >>>>>>>> e - The DB/WMS ThreadQueue has no public access. >>>>>>>> >>>>>>>> Now I've written a sub class of ThreadQueue: SingleThreadQueue >>>>>>>> (see attachment). This one deals with the issues a, b and d. >>>>>>>> I also attached a patch against RenderingManager [3] to handle e. >>>>>>>> >>>>>>>> The new class (to be placed in package >>>>>>>> com.vividsolutions.jump.workbench.ui.renderer) is a drop-in >>>>>>>> replacement for the default ThreadQueue in RenderingManager. >>>>>>>> Not for the ThreadQueue that handles the DB/WMS layers. >>>>>>>> >>>>>>>> Because Jon limited the number of parallel threads in default >>>>>>>> queue to 1 I see no reason why to fork a new thread for each >>>>>>>> Runnable it processes. Thread creation/shutdown is fairly >>>>>>>> expensive. Instead a single background thread is started >>>>>>>> which processes the Runnables one by one. If the thread >>>>>>>> is idle for 30 secs it shuts itself down. If you have a lot >>>>>>>> of (non-WMS/BB) layers this should improve performance >>>>>>>> and save some resources. The processing itself is done >>>>>>>> with a monitor (synchronized/wait/notify) so there is no >>>>>>>> busy wait any more. >>>>>>>> >>>>>>>> The DB/WMS ThreadQueue (real parallel threads) is left untouched >>>>>>>> for the moment. Depending on my personal schedule I will send >>>>>>>> a patch against this one too. Preliminary code with thread pooling >>>>>>>> exists but it needs a bit more testing. >>>>>>>> >>>>>>>> Find attached the new class and patches against RenderingManager and >>>>>>>> the old ThreadQueue to bring it to work. >>>>>>>> >>>>>>>> Comments are very welcome. :-) >>>>>>>> >>>>>>>> Kind regrads, >>>>>>>> Sascha >>>>>>>> >>>>>>>> [1] com.vividsolutions.jump.workbench.ui.renderer.ThreadQueue >>>>>>>> [2] >>>>>>>> >>>>>>>> http://sourceforge.net/mailarchive/message.php?msg_name=4653389E.6000706%40intevation.de >>>>>>>> [3] com.vividsolutions.jump.workbench.ui.renderer.RenderingManager >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> ------------------------------------------------------------------------ >>>>>>>> >>>>>>>> /* >>>>>>>> * The Unified Mapping Platform (JUMP) is an extensible, interactive >>>>>>>> GUI >>>>>>>> * for visualizing and manipulating spatial features with geometry and >>>>>>>> attributes. >>>>>>>> * >>>>>>>> * Copyright (C) 2003 Vivid Solutions >>>>>>>> * Copyright (C) 2007 Intevation GmbH >>>>>>>> * >>>>>>>> * This program is free software; you can redistribute it and/or >>>>>>>> * modify it under the terms of the GNU General Public License >>>>>>>> * as published by the Free Software Foundation; either version 2 >>>>>>>> * of the License, or (at your option) any later version. >>>>>>>> * >>>>>>>> * This program is distributed in the hope that it will be useful, >>>>>>>> * but WITHOUT ANY WARRANTY; without even the implied warranty of >>>>>>>> * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >>>>>>>> * GNU General Public License for more details. >>>>>>>> * >>>>>>>> * You should have received a copy of the GNU General Public License >>>>>>>> * along with this program; if not, write to the Free Software >>>>>>>> * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA >>>>>>>> 02111-1307, USA. >>>>>>>> * >>>>>>>> * Suite #1A >>>>>>>> * 2328 Government Street >>>>>>>> * Victoria BC V8T 5G5 >>>>>>>> * Canada >>>>>>>> * >>>>>>>> * (250)385-6040 >>>>>>>> * www.vividsolutions.com >>>>>>>> */ >>>>>>>> package com.vividsolutions.jump.workbench.ui.renderer; >>>>>>>> >>>>>>>> import java.util.LinkedList; >>>>>>>> import java.util.ArrayList; >>>>>>>> >>>>>>>> /** >>>>>>>> * This thread queue executes at maximum N Runnables in parallel >>>>>>>> * were N is a given number of worker threads that should be used. >>>>>>>> * If N threads are running and busy each further incoming >>>>>>>> * Runnable is queued until one of the threads has finished its current >>>>>>>> job. >>>>>>>> * If a worker thread becomes idle (no more job in the queue) >>>>>>>> * it is hold alive for 5 seconds. If during this period of time >>>>>>>> * no new Runnable is enqueued the worker thread dies. >>>>>>>> * >>>>>>>> * @author Sascha L. Teichmann ([EMAIL PROTECTED]) >>>>>>>> */ >>>>>>>> public class ThreadQueue >>>>>>>> { >>>>>>>> /** The time a worker thread stays alive if idle */ >>>>>>>> public static final long WORKER_STAY_ALIVE_TIME = 5000L; >>>>>>>> >>>>>>>> /** >>>>>>>> * Worker thread. Fetches Runnable from the surrounding >>>>>>>> * ThreadQueue instance. >>>>>>>> */ >>>>>>>> protected class Worker >>>>>>>> extends Thread >>>>>>>> { >>>>>>>> public void run() { >>>>>>>> try { >>>>>>>> for (;;) { >>>>>>>> Runnable runnable; >>>>>>>> >>>>>>>> synchronized (queuedRunnables) { >>>>>>>> if >>>>>>>> (queuedRunnables.isEmpty()) { >>>>>>>> >>>>>>>> ++waitingThreads; >>>>>>>> try { >>>>>>>> >>>>>>>> queuedRunnables.wait(WORKER_STAY_ALIVE_TIME); >>>>>>>> } >>>>>>>> catch >>>>>>>> (InterruptedException ie) { >>>>>>>> } >>>>>>>> finally { >>>>>>>> >>>>>>>> --waitingThreads; >>>>>>>> } >>>>>>>> >>>>>>>> // if still >>>>>>>> empty -> die! >>>>>>>> if >>>>>>>> (queuedRunnables.isEmpty()) >>>>>>>> break; >>>>>>>> } >>>>>>>> if (disposed) >>>>>>>> break; >>>>>>>> runnable = >>>>>>>> (Runnable)queuedRunnables.remove(); >>>>>>>> } // synchronized >>>>>>>> queuedRunnables >>>>>>>> >>>>>>>> try { >>>>>>>> runnable.run(); >>>>>>>> } >>>>>>>> catch (Exception e) { >>>>>>>> e.printStackTrace(); >>>>>>>> } >>>>>>>> } // for (;;) >>>>>>>> } >>>>>>>> finally { // guarantee that counter goes down >>>>>>>> boolean allRunningThreadsFinished; >>>>>>>> synchronized (runningThreads) { >>>>>>>> allRunningThreadsFinished = >>>>>>>> --runningThreads[0] == 0; >>>>>>>> } >>>>>>>> if (allRunningThreadsFinished) >>>>>>>> fireAllRunningThreadsFinished(); >>>>>>>> } >>>>>>>> } >>>>>>>> } // class Worker >>>>>>>> >>>>>>>> /** >>>>>>>> * If the number of running threads goes down to zero >>>>>>>> * implementations of this interface are able to be informed. >>>>>>>> */ >>>>>>>> public interface Listener { >>>>>>>> void allRunningThreadsFinished(); >>>>>>>> } // interface Listener >>>>>>>> >>>>>>>> /** Number of running threads */ >>>>>>>> protected int [] runningThreads = new int[1]; >>>>>>>> >>>>>>>> /** max. Number of threads running parallel */ >>>>>>>> protected int maxRunningThreads; >>>>>>>> >>>>>>>> /** Number of threads that are currently idle */ >>>>>>>> protected int waitingThreads; >>>>>>>> >>>>>>>> /** The queue of Runnables jobs waiting to be run */ >>>>>>>> protected LinkedList queuedRunnables; >>>>>>>> >>>>>>>> /** Singals that the ThreadQueue is going to quit */ >>>>>>>> protected boolean disposed; >>>>>>>> >>>>>>>> /** List of Listeners */ >>>>>>>> protected ArrayList listeners = new ArrayList(); >>>>>>>> >>>>>>>> /** >>>>>>>> * Creates a ThreadQueue with one worker thread. >>>>>>>> */ >>>>>>>> public ThreadQueue() { >>>>>>>> this(1); >>>>>>>> } >>>>>>>> >>>>>>>> /** Creates a ThreadQueue with a given number of worker threads. >>>>>>>> * @param maxRunningThreads the max. number of threads to be >>>>>>>> run parallel. >>>>>>>> */ >>>>>>>> public ThreadQueue(int maxRunningThreads) { >>>>>>>> this.maxRunningThreads = Math.max(1, maxRunningThreads); >>>>>>>> queuedRunnables = new LinkedList(); >>>>>>>> } >>>>>>>> >>>>>>>> /** >>>>>>>> * Adds a Listener to this ThreadQueue. >>>>>>>> * @param listener the listener to add. >>>>>>>> */ >>>>>>>> public synchronized void add(Listener listener) { >>>>>>>> if (listener != null) >>>>>>>> listeners.add(listener); >>>>>>>> } >>>>>>>> >>>>>>>> /** >>>>>>>> * Removes a Listener from this ThreadQueue. >>>>>>>> * @param listener the listener to be removed. >>>>>>>> */ >>>>>>>> public synchronized void remove(Listener listener) { >>>>>>>> if (listener != null) >>>>>>>> listeners.add(listener); >>>>>>>> } >>>>>>>> >>>>>>>> /** >>>>>>>> * Informs Listeners of the fact that the number of running >>>>>>>> threads >>>>>>>> * went to zero. >>>>>>>> */ >>>>>>>> protected void fireAllRunningThreadsFinished() { >>>>>>>> ArrayList copy; >>>>>>>> synchronized (this) { copy = new ArrayList(listeners); } >>>>>>>> for (int i = copy.size()-1; i >= 0; --i) >>>>>>>> >>>>>>>> ((Listener)copy.get(i)).allRunningThreadsFinished(); >>>>>>>> } >>>>>>>> >>>>>>>> >>>>>>>> /** >>>>>>>> * The number of currently running worker threads. >>>>>>>> * @return number of currently running worker threads. >>>>>>>> */ >>>>>>>> public int runningThreads() { >>>>>>>> synchronized (runningThreads) { >>>>>>>> return runningThreads[0]; >>>>>>>> } >>>>>>>> } >>>>>>>> >>>>>>>> /** >>>>>>>> * The number of currently waiting Runnables. >>>>>>>> * @return number of currently waiting Runnables. >>>>>>>> */ >>>>>>>> public int waitingRunnables() { >>>>>>>> synchronized (runningThreads) { >>>>>>>> return queuedRunnables.size(); >>>>>>>> } >>>>>>>> } >>>>>>>> >>>>>>>> /** >>>>>>>> * The number of currently idle worker threads. >>>>>>>> * @return number of currently idle worker threads. >>>>>>>> */ >>>>>>>> public int waitingThreads() { >>>>>>>> synchronized (queuedRunnables) { >>>>>>>> return waitingThreads; >>>>>>>> } >>>>>>>> } >>>>>>>> >>>>>>>> /** >>>>>>>> * Adds a Runnables to the queue. It will be run in one >>>>>>>> * of the worker threads. >>>>>>>> * @param runnable The Runnables to add >>>>>>>> */ >>>>>>>> public void add(Runnable runnable) { >>>>>>>> int waiting; >>>>>>>> synchronized (queuedRunnables) { >>>>>>>> if (disposed) >>>>>>>> return; >>>>>>>> waiting = waitingThreads; >>>>>>>> queuedRunnables.add(runnable); >>>>>>>> queuedRunnables.notify(); >>>>>>>> } // synchronized (queuedRunnables) >>>>>>>> >>>>>>>> synchronized (runningThreads) { >>>>>>>> >>>>>>>> // if waitingThreads == 1 then >>>>>>>> // the queuedRunnables.notify() should have >>>>>>>> waked it up. >>>>>>>> >>>>>>>> if (waitingThreads < 2 && runningThreads[0] < >>>>>>>> maxRunningThreads) { >>>>>>>> ++runningThreads[0]; >>>>>>>> Worker w = new Worker(); >>>>>>>> w.setDaemon(true); >>>>>>>> w.start(); >>>>>>>> } >>>>>>>> } // synchronized (runningThreads) >>>>>>>> } >>>>>>>> >>>>>>>> /** >>>>>>>> * Empties the queue of waiting Runnables. >>>>>>>> */ >>>>>>>> public void clear() { >>>>>>>> synchronized (queuedRunnables) { >>>>>>>> queuedRunnables.clear(); >>>>>>>> } >>>>>>>> } >>>>>>>> >>>>>>>> /** >>>>>>>> * Shuts down the ThreadQueue. >>>>>>>> */ >>>>>>>> public void dispose() { >>>>>>>> synchronized (queuedRunnables) { >>>>>>>> disposed = true; >>>>>>>> queuedRunnables.clear(); >>>>>>>> // wakeup idle threads >>>>>>>> queuedRunnables.notifyAll(); >>>>>>>> } >>>>>>>> synchronized (this) { >>>>>>>> listeners.clear(); >>>>>>>> } >>>>>>>> } >>>>>>>> } >>>>>>>> // end of file >>>>>>>> >>>>>>>> >>>>>>>> ------------------------------------------------------------------------ >>>>>>>> >>>>>>>> ------------------------------------------------------------------------- >>>>>>>> This SF.net email is sponsored by DB2 Express >>>>>>>> Download DB2 Express C - the FREE version of DB2 express and take >>>>>>>> control of your XML. No limits. Just data. Click to get it now. >>>>>>>> http://sourceforge.net/powerbar/db2/ >>>>>>>> >>>>>>>> ------------------------------------------------------------------------ >>>>>>>> >>>>>>>> _______________________________________________ >>>>>>>> Jump-pilot-devel mailing list >>>>>>>> Jump-pilot-devel@lists.sourceforge.net >>>>>>>> https://lists.sourceforge.net/lists/listinfo/jump-pilot-devel >>>>>>>> >>>>>>>> >> >> ------------------------------------------------------------------------- >> This SF.net email is sponsored by DB2 Express >> Download DB2 Express C - the FREE version of DB2 express and take >> control of your XML. No limits. Just data. Click to get it now. >> http://sourceforge.net/powerbar/db2/ >> _______________________________________________ >> Jump-pilot-devel mailing list >> Jump-pilot-devel@lists.sourceforge.net >> https://lists.sourceforge.net/lists/listinfo/jump-pilot-devel >> >> > > ------------------------------------------------------------------------- > This SF.net email is sponsored by DB2 Express > Download DB2 Express C - the FREE version of DB2 express and take > control of your XML. No limits. Just data. Click to get it now. > http://sourceforge.net/powerbar/db2/ > _______________________________________________ > Jump-pilot-devel mailing list > Jump-pilot-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/jump-pilot-devel ------------------------------------------------------------------------- This SF.net email is sponsored by DB2 Express Download DB2 Express C - the FREE version of DB2 express and take control of your XML. No limits. Just data. Click to get it now. http://sourceforge.net/powerbar/db2/ _______________________________________________ Jump-pilot-devel mailing list Jump-pilot-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/jump-pilot-devel
