Changeset 1966

Timestamp:

12/12/07 19:44:06 (12 months ago)

Author:

callan

Message:

• ThumbnailService rework which removes the dependence on the rendering bean; we're now using the renderer directly. (#848)
  
• Passivation has now been turned back on for the stateful beans, this should clear up any lingering memory leaks. (#769)
  

Location:

trunk/components

Files:

• rendering/src/omeis/providers/re/Renderer.java (modified) (5 diffs)
• server/resources/ome/services/service-ome.api.IThumb.xml (modified) (1 diff)
• server/src/ome/api/local/LocalQuery.java (modified) (1 diff)
• server/src/ome/logic/PixelsImpl.java (modified) (1 diff)
• server/src/ome/logic/QueryImpl.java (modified) (1 diff)
• server/src/ome/services/RenderingBean.java (modified) (21 diffs)
• server/src/ome/services/ThumbnailBean.java (modified) (16 diffs)

trunk/components/rendering/src/omeis/providers/re/Renderer.java

@@ -534 +534 @@
         RGBBuffer img = renderingStrategy.render(this, pd);
         stats.stop();
-        log.info(stats.getStats());
-        // TODO: is this the right place to log??? We want to have as little
-        // impact on performance as possible.
+        // TODO: Commenting this out for now. -- callan
+        //log.info(stats.getStats());
         return img;
     }
@@ -569 +568 @@
         RGBIntBuffer img = renderingStrategy.renderAsPackedInt(this, pd);
         stats.stop();
-        log.info(stats.getStats());
-        // TODO: is this the right place to log??? We want to have as little
-        // impact on performance as possible.
+        // TODO: Commenting this out for now. -- callan
+        //log.info(stats.getStats());
         return img.getDataBuffer();
     }
@@ -918 +916 @@
                         "Unable to find default rendering model in enumerated list.");
             }
-            def.setModel(defaultModel);
-        
+            
+            // Unload the rendering model to avoid transactional headaches
+            RenderingModel unloadedModel = new RenderingModel();
+            unloadedModel.setId(defaultModel.getId());
+            unloadedModel.unload();
+            def.setModel(unloadedModel);
+
             // Quantization settings
             QuantumDef quantumDef = def.getQuantization();
@@ -926 +929 @@
             quantumDef.setBitResolution(QuantumFactory.DEPTH_8BIT);
             def.setQuantization(quantumDef);
-        
+
             // Reset the channel bindings
             resetChannelBindings(def, pixels, quantumFactory, buffer);
@@ -964 +967 @@
         r.setQuantization(new QuantumDef());
         r.setWaveRendering(createNewChannelBindings(p));
-        r.setPixels(p);
+        
+        // Unload the pixels object to avoid transactional headaches
+        Pixels unloadedPixels = new Pixels();
+        unloadedPixels.setId(p.getId());
+        unloadedPixels.unload();
+        r.setPixels(unloadedPixels);
         return r;
     }

trunk/components/server/resources/ome/services/service-ome.api.IThumb.xml

@@ -25 +25 @@
         <constructor-arg><value>true</value></constructor-arg>
     <property name="ioService" ref="/OMERO/Thumbs"/>
-    <property name="renderingEngine"
-        ref="internal:omeis.providers.re.RenderingEngine"/>
+    <property name="IPixels" ref="internal:ome.api.IPixels"/>
+    <property name="pixelDataService" ref="/OMERO/Pixels"/>
     <property name="scaleService" ref="internal:ome.api.IScale"/>
     <property name="compressionService" ref="internal:ome.api.ICompress"/>

trunk/components/server/src/ome/api/local/LocalQuery.java

@@ -73 +73 @@
     void evict(Object object);
 
+        void clear();
+
     /**
      * Uses the Hibernate static method <code>initialize</code> to prepare an

trunk/components/server/src/ome/logic/PixelsImpl.java

@@ -121 +121 @@
     @Transactional(readOnly = false)
     public void saveRndSettings(RenderingDef rndSettings) {
-        iUpdate.saveObject(rndSettings);
+        iUpdate.saveAndReturnObject(rndSettings);
     }
 

trunk/components/server/src/ome/logic/QueryImpl.java

@@ -99 +99 @@
     public void evict(Object obj) {
         getHibernateTemplate().evict(obj);
+    }
+
+    @RolesAllowed("user")
+    @Transactional(readOnly = false)
+    public void clear() {
+        getHibernateTemplate().clear();
     }
 

trunk/components/server/src/ome/services/RenderingBean.java

@@ -12 +12 @@
 import java.io.ByteArrayOutputStream;
 import java.io.IOException;
-import java.io.PrintWriter;
 import java.io.Serializable;
-import java.io.StringWriter;
-import java.io.Writer;
 import java.util.ArrayList;
 import java.util.List;
@@ -39 +36 @@
 import org.jboss.annotation.ejb.RemoteBinding;
 import org.jboss.annotation.ejb.RemoteBindings;
-import org.jboss.annotation.ejb.cache.Cache;
 import org.jboss.annotation.security.SecurityDomain;
-import org.jboss.ejb3.cache.NoPassivationCache;
 import org.springframework.transaction.annotation.Transactional;
 
@@ -47 +42 @@
 import ome.annotations.RevisionDate;
 import ome.annotations.RevisionNumber;
-import ome.api.ICompress;
 import ome.api.IPixels;
 import ome.api.ServiceInterface;
@@ -58 +52 @@
 import ome.io.nio.PixelsService;
 import ome.logic.AbstractLevel2Service;
-import ome.logic.SimpleLifecycle;
 import ome.model.IObject;
 import ome.model.core.Channel;
@@ -86 +79 @@
  * <p>
  * The multi-threaded design of this component is based on dynamic locking and
- * confinement techiniques. All access to the component's internal parts happens
+ * confinement techniques. All access to the component's internal parts happens
  * through a <code>RenderingEngineImpl</code> object, which is fully
  * synchronized. Internal parts are either never leaked out or given away only
@@ -118 +111 @@
 @LocalBinding(jndiBinding = "omero/local/omeis.providers.re.RenderingEngine")
 @SecurityDomain("OmeroSecurity")
-@Cache(NoPassivationCache.class)
 @TransactionManagement(TransactionManagementType.BEAN)
 @Transactional(readOnly = true)
@@ -152 +144 @@
 
     /** The pixels set to the rendering engine is for. */
-    private transient Pixels pixelsObj;
+    private Pixels pixelsObj;
 
     /** The rendering settings associated to the pixels set. */
-    private transient RenderingDef rendDefObj;
+    private RenderingDef rendDefObj;
 
     /** Reference to the service used to retrieve the pixels data. */
@@ -171 +163 @@
      */
     private transient ReentrantReadWriteLock rwl = new ReentrantReadWriteLock();
+
+    /** Notification that the bean has just returned from passivation. */
+    private transient boolean wasPassivated = false;
     
     /**
@@ -202 +197 @@
      * Delegates to super class
      */
-    @PostActivate
     @PostConstruct
     public void create() {
@@ -208 +202 @@
     }
 
-    /** lifecycle method -- {@link PrePassivate}. Disallows all passivation. */
+    /** lifecycle method -- {@link PostPassivate}. */
+    @PostActivate
+    public void postPassivate() {
+        log.info("***** Returning from passivation... ******");
+        create();
+        wasPassivated = true;
+        rwl = new ReentrantReadWriteLock();
+    }
+
+    /** lifecycle method -- {@link PrePassivate}. */
     @PrePassivate
     public void passivate() {
-        closeRenderer();
-        getBeanHelper().passivationNotAllowed();
+        log.info("***** Passivating... ******");
+        closeRenderer();
+        renderer = null;
     }
 
@@ -253 +257 @@
      */
     @RolesAllowed("user")
+    @Transactional(readOnly = false)
     public void lookupPixels(long pixelsId) {
-        rwl.writeLock().lock();
-
-        try {
-            this.pixelsObj = pixMetaSrv.retrievePixDescription(pixelsId);
-            closeRenderer();
-            this.renderer = null;
-
-            if (pixelsObj == null) {
-                throw new ValidationException("Pixels object with id "
-                        + pixelsId + " not found.");
-            }
-        } finally {
-            rwl.writeLock().unlock();
-        }
-
-        if (log.isDebugEnabled()) {
-            log.debug("lookupPixels for id " + pixelsId + " succeeded: "
-                    + this.pixelsObj);
-        }
+        rwl.writeLock().lock();
+
+        try {
+                pixelsObj = pixMetaSrv.retrievePixDescription(pixelsId);
+                closeRenderer();
+                renderer = null;
+
+                if (pixelsObj == null) {
+                        throw new ValidationException("Pixels object with id "
+                                        + pixelsId + " not found.");
+                }
+        } finally {
+                rwl.writeLock().unlock();
+        }
+
+        if (log.isDebugEnabled()) {
+                log.debug("lookupPixels for id " + pixelsId + " succeeded: "
+                                + this.pixelsObj);
+        }
     }
 
@@ -281 +286 @@
      */
     @RolesAllowed("user")
+    @Transactional(readOnly = false)
     public boolean lookupRenderingDef(long pixelsId) {
-        rwl.writeLock().lock();
-
-        try {
-            rendDefObj = pixMetaSrv.retrieveRndSettings(pixelsId);
-            closeRenderer();
-            renderer = null;
-
-            if (rendDefObj == null) {
-                // We've been initialized on a pixels set that has no rendering
-                // definition for the given user. In order to maintain the
-                // proper state and ensure that we avoid transactional problems
-                // we're going to notify the caller instead of performing *any*
-                // magic that would require a database update.
-                // *** Ticket #564 -- Chris Allan <callan@blackcat.ca> ***
-                return false;
-            }
-        } finally {
-            rwl.writeLock().unlock();
-        }
-
-        if (log.isDebugEnabled()) {
-            log.debug("lookupRenderingDef for Pixels=" + pixelsId
-                    + " succeeded: " + this.rendDefObj);
-        }
-        return true;
+        rwl.writeLock().lock();
+
+        try {
+                rendDefObj = pixMetaSrv.retrieveRndSettings(pixelsId);
+                closeRenderer();
+                renderer = null;
+
+                if (rendDefObj == null) {
+                        // We've been initialized on a pixels set that has no rendering
+                        // definition for the given user. In order to maintain the
+                        // proper state and ensure that we avoid transactional problems
+                        // we're going to notify the caller instead of performing *any*
+                        // magic that would require a database update.
+                        // *** Ticket #564 -- Chris Allan <callan@blackcat.ca> ***
+                        return false;
+                }
+        } finally {
+                rwl.writeLock().unlock();
+        }
+
+        if (log.isDebugEnabled()) {
+                log.debug("lookupRenderingDef for Pixels=" + pixelsId
+                                + " succeeded: " + this.rendDefObj);
+        }
+        return true;
     }
 
@@ -318 +324 @@
         rwl.writeLock().lock();
 
+        
         try {
             errorIfNullPixels();
+            
+                // Ensure that we do not have "dirty" pixels or rendering settings
+            // left around in the Hibernate session cache.
+                iQuery.clear();
 
             /*
@@ -401 +412 @@
      */
     @RolesAllowed("user")
-    public int[] renderAsPackedInt(PlaneDef pd) throws ResourceError,
-            ValidationException {
-        rwl.readLock().lock();
-
-        try {
-            errorIfInvalidState();
-            return renderer.renderAsPackedInt(pd);
-        } catch (IOException e) {
-            ResourceError re = new ResourceError("IO error while rendering:\n"
-                    + e.getMessage());
-            re.initCause(e);
-            throw re;
-        } catch (QuantizationException e) {
-            InternalException ie = new InternalException(
-                    "QuantizationException while rendering:\n" + e.getMessage());
-            ie.initCause(e);
-            throw ie;
-        } finally {
-            rwl.readLock().unlock();
-        }
+    public int[] renderAsPackedInt(PlaneDef pd)
+        throws ResourceError, ValidationException {
+        rwl.writeLock().lock();
+
+        try {
+                errorIfInvalidState();
+                return renderer.renderAsPackedInt(pd);
+        } catch (IOException e) {
+                ResourceError re = new ResourceError("IO error while rendering:\n"
+                                + e.getMessage());
+                re.initCause(e);
+                throw re;
+        } catch (QuantizationException e) {
+                InternalException ie = new InternalException(
+                                "QuantizationException while rendering:\n" + e.getMessage());
+                ie.initCause(e);
+                throw ie;
+        } finally {
+                rwl.writeLock().unlock();
+        }
     }
     
@@ -481 +492 @@
             // Ensure that we haven't just been called before 
             // lookupRenderingDef().
-            if (rendDefObj == null
-                && pixMetaSrv.retrieveRndSettings(pixelsId) == null)
+            if (rendDefObj == null)
             {
+                                RenderingDef def = pixMetaSrv.retrieveRndSettings(pixelsId); 
+                                if (def != null)
+                                {
+                                        errorIfInvalidState();
+                                }
                 List<Family> families =
                         pixMetaSrv.getAllEnumerations(Family.class);
@@ -490 +505 @@
                 QuantumFactory quantumFactory = new QuantumFactory(families);
                         PixelBuffer buffer = pixDataSrv.getPixelBuffer(pixelsObj);
-                        RenderingDef def = Renderer.createNewRenderingDef(pixelsObj);
+                        def = Renderer.createNewRenderingDef(pixelsObj);
                         Renderer.resetDefaults(def, pixelsObj, quantumFactory,
                                                renderingModels, buffer);
@@ -515 +530 @@
             rwl.writeLock().unlock();
         }
-        iUpdate.flush();
     }
     
@@ -573 +587 @@
             errorIfNullRenderingDef();
             pixMetaSrv.saveRndSettings(rendDefObj);
-        } finally {
-            rwl.writeLock().unlock();
-        }
-        iUpdate.flush();
+            rendDefObj = reload(rendDefObj);
+            iQuery.clear();
+        } finally {
+            rwl.writeLock().unlock();
+        }
     }
 
@@ -1133 +1148 @@
     // =========================================================================
 
-    protected final static String NULL_RENDERER = "RenderingEngine not ready: renderer is null."
+    protected final static String NULL_RENDERER = "RenderingEngine not ready: renderer is null.\n"
             + "This method can only be called "
             + "after the renderer is properly "
-            + "initialized (not-null). \n"
+            + "initialized (not-null).\n"
             + "Try lookup and/or use methods.";
 
@@ -1154 +1169 @@
 
     protected void errorIfNullRenderingDef() {
-        if (rendDefObj == null) {
-            throw new ApiUsageException(
-                    "RenderingEngine not ready: RenderingDef object not set.");
-        }
-
-    }
-
-    protected void errorIfNullRenderer() {
-        if (renderer == null) {
-            throw new ApiUsageException(NULL_RENDERER);
-        }
-
+        if (rendDefObj == null) {
+                throw new ApiUsageException(
+                                "RenderingEngine not ready: RenderingDef object not set.");
+        }
+    }
+
+    protected void errorIfNullRenderer()
+    {
+        if (renderer == null && wasPassivated)
+        {
+                load();
+        }
+        else if (renderer == null)
+        {
+                throw new ApiUsageException(NULL_RENDERER);
+        }
     }
 
@@ -1235 +1254 @@
     }
 
+    /**
+     * Reloads an {@link Pixels} or {@link RenderingDef} object following a
+     * <pre>clear()</pre> being called on the Hibernate session.
+     * @param t The object to reload.
+     * @return A reloaded object of the same type as that passed in.
+     */
+    @SuppressWarnings("unchecked")
+    private <T extends IObject> T reload(T t)
+    {
+        if (t != null)
+        {
+                if (t instanceof RenderingDef)
+                {
+                        return (T) pixMetaSrv.retrieveRndSettings(pixelsObj.getId());
+                }
+                else if (t instanceof Pixels)
+                {
+                        return (T) pixMetaSrv.retrievePixDescription(t.getId());
+                }
+                else
+                {
+                        throw new RuntimeException(
+                                        "Unknown reload class: " + t.getClass());
+                }
+        }
+        return t;
+    }
 }

trunk/components/server/src/ome/services/ThumbnailBean.java

@@ -9 +9 @@
 
 // Java imports
-import java.awt.Point;
 import java.awt.image.BufferedImage;
-import java.awt.image.ColorModel;
-import java.awt.image.DataBuffer;
-import java.awt.image.DataBufferInt;
-import java.awt.image.DirectColorModel;
-import java.awt.image.SinglePixelPackedSampleModel;
-import java.awt.image.WritableRaster;
 import java.io.ByteArrayOutputStream;
 import java.io.FileOutputStream;
@@ -36 +29 @@
 import javax.interceptor.Interceptors;
 
-import ome.api.ICompress;
+import ome.api.IPixels;
 import ome.api.IRepositoryInfo;
 import ome.api.IScale;
@@ -43 +36 @@
 import ome.api.local.LocalCompress;
 import ome.conditions.ApiUsageException;
+import ome.conditions.InternalException;
 import ome.conditions.ResourceError;
+import ome.conditions.ValidationException;
+import ome.io.nio.PixelBuffer;
+import ome.io.nio.PixelsService;
 import ome.io.nio.ThumbnailService;
 import ome.logic.AbstractLevel2Service;
+import ome.model.IObject;
 import ome.model.core.Pixels;
+import ome.model.display.RenderingDef;
 import ome.model.display.Thumbnail;
+import ome.model.enums.Family;
+import ome.model.enums.RenderingModel;
 import ome.parameters.Parameters;
 import ome.services.util.OmeroAroundInvoke;
@@ -53 +54 @@
 import ome.system.SimpleEventContext;
 import ome.util.ImageUtil;
-import omeis.providers.re.RenderingEngine;
+import omeis.providers.re.Renderer;
 import omeis.providers.re.data.PlaneDef;
+import omeis.providers.re.quantum.QuantizationException;
+import omeis.providers.re.quantum.QuantumFactory;
 
 import org.apache.commons.logging.Log;
@@ -61 +64 @@
 import org.jboss.annotation.ejb.RemoteBinding;
 import org.jboss.annotation.ejb.RemoteBindings;
+import org.jboss.annotation.ejb.cache.tree.CacheConfig;
 import org.jboss.annotation.security.SecurityDomain;
 import org.springframework.transaction.annotation.Transactional;
-
-import sun.awt.image.IntegerInterleavedRaster;
 
 /**
@@ -100 +102 @@
     private transient static Log log = LogFactory.getLog(ThumbnailBean.class);
 
-    /** The rendering engine that this service uses for thumbnail creation. */
-    private transient RenderingEngine re;
-
-    /** The scaling service that will be used to scale buffered images. */
+    /** The renderer that this service uses for thumbnail creation. */
+    private transient Renderer renderer;
+
+    /** The scaling service will be used to scale buffered images. */
     private transient IScale iScale;
+    
+    /** The pixels service, will be used to load pixels and settings. */
+    private transient IPixels iPixels;
+    
+    /** The service used to retrieve the pixels data. */
+    private transient PixelsService pixelDataService;
 
     /** The ROMIO thumbnail service. */
@@ -120 +128 @@
     /** The pixels instance that the service is currently working on. */
     private Pixels pixels;
-
-    /** The id of the pixels instance. */
-    private Long pixelsId;
-    
-    /** Only set after a passivated bean is activated. */
-    private transient Long resetPix = null;
-
-    /** Currently unused. */
-    private transient Long resetRE = null;
-    
-    /** The id of the rendering definition instance. */
-    private Long renderingDefId;
+    
+    /** The rendering settings that the service is currently working with. */
+    private RenderingDef settings;
 
     /** The default X-width for a thumbnail. */
@@ -145 +144 @@
     public static final String DEFAULT_MIME_TYPE = "image/jpeg";
 
+    /** Notification that the bean has just returned from passivation. */