From a8bc4575bce8c4d2d170a890c83c2cda72127ab9 Mon Sep 17 00:00:00 2001 From: David Gillingham Date: Tue, 22 Jan 2013 18:12:00 -0600 Subject: [PATCH] Issue #1515: Move blocking VCParm calls off of the ParmListener notification threads. Change-Id: I1c3c46dde051802fb79b7ead6fb63d766d1d00a6 Former-commit-id: 88e518725e268058bcf0428f4688bfd8abb9a91a [formerly bf6230441fc8986f356a9cdae0f00be274086e9d [formerly 9cd87997f7ae7b51608ac56fcdf4d282339526aa]] Former-commit-id: bf6230441fc8986f356a9cdae0f00be274086e9d Former-commit-id: eab83c0f63d671f52d8af5becd8b62ec998e2e57 --- .../core/internal/AbstractParmManager.java | 7 +- .../raytheon/viz/gfe/core/parm/VCParm.java | 159 +++++++++++------- .../viz/gfe/core/parm/vcparm/VCModule.java | 7 +- 3 files changed, 103 insertions(+), 70 deletions(-) diff --git a/cave/com.raytheon.viz.gfe/src/com/raytheon/viz/gfe/core/internal/AbstractParmManager.java b/cave/com.raytheon.viz.gfe/src/com/raytheon/viz/gfe/core/internal/AbstractParmManager.java index 821357e690..9510f79367 100644 --- a/cave/com.raytheon.viz.gfe/src/com/raytheon/viz/gfe/core/internal/AbstractParmManager.java +++ b/cave/com.raytheon.viz.gfe/src/com/raytheon/viz/gfe/core/internal/AbstractParmManager.java @@ -32,9 +32,7 @@ import java.util.Iterator; import java.util.List; import java.util.ListIterator; import java.util.Set; -import java.util.SortedSet; import java.util.TimeZone; -import java.util.TreeSet; import org.eclipse.core.runtime.ListenerList; @@ -65,7 +63,6 @@ import com.raytheon.viz.gfe.GFEServerException; import com.raytheon.viz.gfe.PythonPreferenceStore; import com.raytheon.viz.gfe.core.DataManager; import com.raytheon.viz.gfe.core.IParmManager; -import com.raytheon.viz.gfe.core.griddata.IGridData; import com.raytheon.viz.gfe.core.internal.NotificationRouter.AbstractGFENotificationObserver; import com.raytheon.viz.gfe.core.msgs.IAvailableSourcesChangedListener; import com.raytheon.viz.gfe.core.msgs.IDisplayedParmListChangedListener; @@ -101,6 +98,8 @@ import com.raytheon.viz.gfe.core.parm.vcparm.VCModuleJobPool; * execution. * 08/20/2012 #1082 randerso Moved calcStepTimes to AbstractParmManager for * use in PngWriter + * 01/22/2013 #1515 dgilling Increase default size of VCModule thread pool + * to decrease UI hang-ups waiting for results. * * * @@ -281,7 +280,7 @@ public abstract class AbstractParmManager implements IParmManager { // Get virtual parm definitions vcModules = initVirtualCalcParmDefinitions(); vcModulePool = new VCModuleJobPool("GFE Virtual ISC Python executor", - this.dataManager, vcModules.size(), Boolean.TRUE); + this.dataManager, vcModules.size() + 2, Boolean.TRUE); PythonPreferenceStore prefs = Activator.getDefault() .getPreferenceStore(); diff --git a/cave/com.raytheon.viz.gfe/src/com/raytheon/viz/gfe/core/parm/VCParm.java b/cave/com.raytheon.viz.gfe/src/com/raytheon/viz/gfe/core/parm/VCParm.java index 74dae96982..eacbf16f61 100644 --- a/cave/com.raytheon.viz.gfe/src/com/raytheon/viz/gfe/core/parm/VCParm.java +++ b/cave/com.raytheon.viz.gfe/src/com/raytheon/viz/gfe/core/parm/VCParm.java @@ -24,9 +24,8 @@ import java.util.Arrays; import java.util.Collection; import java.util.Date; import java.util.List; - -import org.eclipse.core.runtime.IStatus; -import org.eclipse.core.runtime.Status; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; import com.raytheon.uf.common.dataplugin.gfe.GridDataHistory; import com.raytheon.uf.common.dataplugin.gfe.db.objects.ParmID; @@ -40,7 +39,6 @@ import com.raytheon.uf.common.status.UFStatus; import com.raytheon.uf.common.status.UFStatus.Priority; import com.raytheon.uf.common.time.TimeRange; import com.raytheon.uf.common.util.RWLArrayList; -import com.raytheon.viz.gfe.Activator; import com.raytheon.viz.gfe.core.DataManager; import com.raytheon.viz.gfe.core.griddata.AbstractGridData; import com.raytheon.viz.gfe.core.griddata.IGridData; @@ -74,6 +72,10 @@ import com.raytheon.viz.gfe.core.parm.vcparm.VCModule.VCInventory; * Jun 25, 2012 #766 dgilling Cleanup error logging so we * don't spam alertViz in practice * mode. + * Jan 22, 2013 #1515 dgilling Handle Parm notifications on + * separate thread to prevent backup + * in ParmListener's notification job + * pool. * * * @@ -93,6 +95,8 @@ public class VCParm extends VParm implements IParmListChangedListener, private List vcInventory; + private final ExecutorService notificationWorkers; + /** * Constructor for Virtual Calculated Parm. * @@ -111,18 +115,15 @@ public class VCParm extends VParm implements IParmListChangedListener, // Need to check that the above call to mod.getGpi() did not fail if (!mod.isValid()) { - //statusHandler.handle(Priority.EVENTB, "Can't get GPI: ", - // this.mod.getErrorString()); - Activator - .getDefault() - .getLog() - .log(new Status(IStatus.INFO, Activator.PLUGIN_ID, - "Can't get GPI: " + this.mod.getErrorString())); + statusHandler.handle(Priority.EVENTB, "Can't get GPI: ", + this.mod.getErrorString()); } // set the parm type // setParmType(Parm::VCPARM); + notificationWorkers = Executors.newSingleThreadExecutor(); + // Determine dependent parms, and register for their ParmClient // notifications, determine initial inventory registerParmClients(dataMgr.getParmManager().getAllParms(), false); @@ -141,6 +142,8 @@ public class VCParm extends VParm implements IParmListChangedListener, for (Parm reg : registeredParms()) { unregisterPC(reg); } + + notificationWorkers.shutdownNow(); } /* @@ -153,41 +156,51 @@ public class VCParm extends VParm implements IParmListChangedListener, */ @Override public void gridDataChanged(final ParmID parmId, final TimeRange validTime) { - synchronized (this) { - if (disposed) { - return; - } - } + Runnable onNotificationTask = new Runnable() { - // statusHandler.handle(Priority.DEBUG, "gridDataChanged for: " + parmId - // + " " + validTime); + @Override + public void run() { + synchronized (VCParm.this) { + if (disposed) { + return; + } + } - for (VCInventory inv : this.vcInventory) { - for (DepParmInv dpi : inv.getDepParmInv()) { - if (dpi.getParmID().equals(parmId)) { - for (TimeRange tr : dpi.getTimes()) { - if (tr.getStart().equals(validTime.getStart())) { - this.grids.acquireReadLock(); - try { - for (IGridData grid : this.grids) { - if (inv.getGridTimeRange().equals( - grid.getGridTime())) { - grid.depopulate(); - gridDataHasChanged(grid, - getDisplayAttributes() - .getDisplayMask(), - false); - return; + // statusHandler.handle(Priority.DEBUG, "gridDataChanged for: " + // + parmId + // + " " + validTime); + + for (VCInventory inv : vcInventory) { + for (DepParmInv dpi : inv.getDepParmInv()) { + if (dpi.getParmID().equals(parmId)) { + for (TimeRange tr : dpi.getTimes()) { + if (tr.getStart().equals(validTime.getStart())) { + grids.acquireReadLock(); + try { + for (IGridData grid : grids) { + if (inv.getGridTimeRange().equals( + grid.getGridTime())) { + grid.depopulate(); + gridDataHasChanged( + grid, + getDisplayAttributes() + .getDisplayMask(), + false); + return; + } + } + } finally { + grids.releaseReadLock(); } } - } finally { - this.grids.releaseReadLock(); } } } } } - } + }; + + notificationWorkers.submit(onNotificationTask); } /* @@ -202,22 +215,31 @@ public class VCParm extends VParm implements IParmListChangedListener, @Override public void parmListChanged(final Parm[] parms, Parm[] deletions, Parm[] additions) { + Runnable onNotificationTask = new Runnable() { - // forcing access to the disposed variable and subsequent - // registration/unregistation of listeners through this synchronized - // block seems to prevent VCParm objects being leaked through outdated - // listener list copies - synchronized (this) { - if (disposed) { - return; + @Override + public void run() { + // forcing access to the disposed variable and subsequent + // registration/unregistation of listeners through this + // synchronized + // block seems to prevent VCParm objects being leaked through + // outdated + // listener list copies + synchronized (VCParm.this) { + if (disposed) { + return; + } + + // statusHandler.handle(Priority.DEBUG, + // "ParmListChangedMsg received: "); + // System.out.println("ParmListChangedMsg received: " + // + getParmID().toString()); + registerParmClients(parms, true); + } } + }; - // statusHandler.handle(Priority.DEBUG, - // "ParmListChangedMsg received: "); - // System.out.println("ParmListChangedMsg received: " - // + getParmID().toString()); - registerParmClients(parms, true); - } + notificationWorkers.submit(onNotificationTask); } /* @@ -239,13 +261,21 @@ public class VCParm extends VParm implements IParmListChangedListener, // + getParmID().toString()); // System.out.println("ParmInventoryChanged notification for: " // + getParmID().toString()); - synchronized (this) { - if (disposed) { - return; - } - } + Runnable onNotificationTask = new Runnable() { - recalcInventory(true); + @Override + public void run() { + synchronized (VCParm.this) { + if (disposed) { + return; + } + } + + recalcInventory(true); + } + }; + + notificationWorkers.submit(onNotificationTask); } /* @@ -464,14 +494,15 @@ public class VCParm extends VParm implements IParmListChangedListener, // get list of dependent parms List args = new ArrayList(mod.dependentParms()); if (!mod.isValid()) { - //statusHandler.handle(Priority.EVENTB, - // "Error getting dependent WeatherElements: ", - // mod.getErrorString()); - Activator - .getDefault() - .getLog() - .log(new Status(IStatus.INFO, Activator.PLUGIN_ID, - "Error getting dependent WeatherElements: " + this.mod.getErrorString())); + statusHandler.handle(Priority.EVENTB, + "Error getting dependent WeatherElements: ", + mod.getErrorString()); + // Activator + // .getDefault() + // .getLog() + // .log(new Status(IStatus.INFO, Activator.PLUGIN_ID, + // "Error getting dependent WeatherElements: " + // + this.mod.getErrorString())); } // get list of currently registered parms diff --git a/cave/com.raytheon.viz.gfe/src/com/raytheon/viz/gfe/core/parm/vcparm/VCModule.java b/cave/com.raytheon.viz.gfe/src/com/raytheon/viz/gfe/core/parm/vcparm/VCModule.java index a50bc048c2..2dcce03fc3 100644 --- a/cave/com.raytheon.viz.gfe/src/com/raytheon/viz/gfe/core/parm/vcparm/VCModule.java +++ b/cave/com.raytheon.viz.gfe/src/com/raytheon/viz/gfe/core/parm/vcparm/VCModule.java @@ -72,6 +72,8 @@ import com.raytheon.viz.gfe.core.parm.Parm; * Oct 17, 2011 dgilling Initial creation * Jun 20, 2012 #766 dgilling Refactor to improve * performance. + * Jan 22, 2013 #1515 dgilling Fix ClassCastException in + * getMethodArgs(). * * * @@ -189,8 +191,9 @@ public class VCModule { parmMgr.getVCModulePool().enqueue(req); Object result = req.getResult(); - String[] argNames = (String[]) result; - return Arrays.asList(argNames); + @SuppressWarnings("unchecked") + List argNames = (List) result; + return argNames; } public Collection dependentParms() {