From: John B. Matthews on 27 Nov 2009 14:27 In article <cb502594-5385-4d4e-9a45-9646caa99a79(a)j19g2000yqk.googlegroups.com>, Hole <h0leforfun(a)gmail.com> wrote: > May you give me any suggestion for the following? [...] > class SchedulerTask extends SwingWorker<Integer, Void> ... [...] > class BatchTask extends SwingWorker<List<SampledValue>, Void> ... I can't see where you define Void, "the type used for carrying out intermediate results by this SwingWorker's publish and process methods" and symbolized by the generic parameter V in the API [1]. You might look at the examples in the SwingWorker API [1] and this similar database example [2]. In the latter, the generic type V is Double, and individual instances are published from the background thread. Note that the List<Double> received by process() is _not_ the same List<Double> returned by doInBackground(). The former contains intermediate results; the latter is the background thread's working result set. The method doInBackground() can create other threads, but you still have to synchronize their processing, as others have suggested. [1]<http://java.sun.com/javase/6/docs/api/javax/swing/SwingWorker.html> [2]<http://sites.google.com/site/drjohnbmatthews/randomdata> [...] -- John B. Matthews trashgod at gmail dot com <http://sites.google.com/site/drjohnbmatthews>
From: Lew on 27 Nov 2009 14:47 John B. Matthews wrote: > I can't see where you define Void, "the type used for carrying out > intermediate results by this SwingWorker's publish and process methods" > and symbolized by the generic parameter V in the API [1]. > <http://java.sun.com/javase/6/docs/api/java/lang/Void.html> > You might look at the examples in the SwingWorker API [1] and this > similar database example [2]. In the latter, the generic type V is > Double, and individual instances are published from the background > thread. Note that the List<Double> received by process() is _not_ the > same List<Double> returned by doInBackground(). The former contains > intermediate results; the latter is the background thread's working > result set. > > The method doInBackground() can create other threads, but you still have > to synchronize their processing, as others have suggested. > > [1]<http://java.sun.com/javase/6/docs/api/javax/swing/SwingWorker.html> > [2]<http://sites.google.com/site/drjohnbmatthews/randomdata> >
From: markspace on 27 Nov 2009 15:05 John B. Matthews wrote: >> class SchedulerTask extends SwingWorker<Integer, Void> > I can't see where you define Void, As Lew mentioned, it's part of the java.lang package. A trick is to define SwingWorker<Void,Void> and then return "null" from doInBackground(): class ExampleWorker extends SwingWorker<Void, Void> { ... @Override protected Void doInBackground() { ... return null; } I notice the OP is declaring "Integer" but always returning 0. "Void" might actually suit him better. See my example up thread.
From: John B. Matthews on 27 Nov 2009 15:55 In article <hepben$99l$1(a)news.eternal-september.org>, markspace <nospam(a)nowhere.com> wrote: > > John B. Matthews wrote: > > > > > class SchedulerTask extends SwingWorker<Integer, Void> > > > I can't see where you define Void, > At nearly the same time, Lew wrote: > <http://java.sun.com/javase/6/docs/api/java/lang/Void.html> > As Lew mentioned, it's part of the java.lang package. Ah, thank you both. > A trick is to define SwingWorker<Void,Void> and then return "null" from > doInBackground(): > > class ExampleWorker > extends SwingWorker<Void, Void> > { > ... > @Override > protected Void doInBackground() { > ... > return null; > } > > I notice the OP is declaring "Integer" but always returning 0. I wondered about this. > "Void" might actually suit him better. > > See my example up thread. Exemplary! -- John B. Matthews trashgod at gmail dot com <http://sites.google.com/site/drjohnbmatthews>
From: Douwe on 30 Nov 2009 04:54 On 27 nov, 20:01, Lew <l...(a)lewscanon.com> wrote: > > ... Try implementing > > something like the following piece of code > > > private final Object lock = new Object(); > > private volatile int countCompleted; > > pricate volatile int activeProcesses; > > You're controlling these variables with different monitors - sometimes > 'lock', sometimes the internal one due to being 'volatile'. I'm not > convinced that the relationship between these variables is reliably > readable. the countCompleted and activeProcesses do not have to be synchronized via the lock object they just need to be written immediately by the JVM (so no caching). The countCompleted is only used to update the progressbar so if the number would be off (like i.e. 1 to less) for a few milliseconds then nobody is noticing it. On the activeProcesses I agree the volatile can be removed (it is only read by the main thread). > > public void processCompleted(BatchExtractionItem completedItem) { > > //System.out.println("Completed "+completedItem); > > > synchronized(lock) { > > activeProcesses--; > > countCompleted++; > > lock.notifyAll(); // wake up the main thread > > } > > > } > > > enum State { WAITING, RUN_NEXT }; > > > protected Integer doInBackground() throws Exception { State state = State.WAITING; // forgot this line ... > > > List<BatchExtractionItem> queuedItems = new > > ArrayList<BatchExtractionItem>(); > > queuedItems.addAll(items); > > activeProcesses = 0; > > countCompleted = 0; > > boolean keepRunning = true; > > BatchExtractionItem itemToRun = null; > > > while (keep_running) { > > switch(state) { > > The read of 'state' is not synchronized with the write. > > > > case WAITING : { > > synchronized(lock) { > > if (activeProcesses<MAX_ACTIVE_PROCESSES) { > > if (queuedItems.isEmpty()) { > > if (activeProcesses==0) { > > keep_running = false; > > break; > > } > > } else { > > state = FIND_TO_RUN; > > break; > > } > > } > > try { > > lock.wait(20000l); // wait for 20 seconds max > > (or a notify from processCompleted) and then check again > > } catch(Exception ignore) {} > > } > > } break; > > > case RUN_NEXT : { > > BatchExtractionItem item = queuedItems.removeLast(queuedItems); > > You don't synchronize the change to 'queuedItems'. > > > if (!item.getStatus().equals(BatchExtractionItem.COMPLETED) && ! > > or the 'getStatus()' read. > > > > > item.getStatus().equals(BatchExtractionItem.PROCESSING)) { > > item.setStatus(BatchExtractionItem.PROCESSING); > > BatchTask task = new BatchTask(item); > > task.execute(); > > activeProcesses++; > > } else { > > // spitt out a warning > > System.err.println("warn: next item was already processing or has > > completed"); > > // countCompleted++; // add this if it should be counted as a > > completed task > > } > > state = WAITING; > > } break; > > } > > } > > // no further checking needed ... all items have finised processing > > return 0; > > > } > > > @Override > > public void actionPerformed(ActionEvent e) { > > //issued by Timer event > > int progress = countCompleted/items.size(); > > setProgress(progress); > > form.getProgressBar().setValue(progress); > > > } > > I'm having difficulty reasoning about the synchronization the way > you've written all this. I suspect there are subtle threading bugs > there. > > -- > Lew The variable state is only written and read by one thread so there is no need for synchronization. About the part of the getStatus I have to partly agree with you: the getStatus() internally calls the method FutureTask.isDone() which than calls sync.innerIsDone() where the sync is of type Sync (see: http://www.google.com/codesearch/p?hl=en&sa=N&cd=2&ct=rc#atE6BTe41-M/libcore/concurrent/src/main/java/java/util/concurrent/FutureTask.java&q=FutureTask) It seems to me that it is overly synced (but at the time of writing I forgot to check. I assumed hole did the right job there)
First
|
Prev
|
Next
|
Last
Pages: 1 2 3 Prev: BUY CHEAP TEXTBOOKS 24/7 Next: How many illegal character for jdom? |