Re: [cp-patches] FYI: Implemented an ObjectPool

[Robert Schuster]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Ok,
I have seen this class was removed now.

So forget about this mail :)

cu
Robert

Robert Schuster wrote:
> Hi,
> shouldn't the pooling class use a WeakHashMap?
> 
> And then I want to admit that the Apidoc should really explain when to
> use a pooled object and when it is not good to use it. Eg. for all kind
> of MouseEvent I think it is not a good idea to use the pool because the
> user can access the Point instance and store it somewhere.
> 
> Putting such an object back in the pool (even if it was untouched in
> Classpath API code) would sooner or later cause havoc (when the values
> in the object are overwritten and the user did not expect that).
> 
> And now an example to get to know whether I have understood how to use
> the object pool correctly:
> In JTable I would change all instantiations of Point to
> ObjectPool.getPoint(). When new values for these internal objects are
> available I set them with begin.x, begin.y and so on.
> 
> When the JTable instance is freed (finalize) I return the Point
> instances to the ObjectPool.
> 
> Done right?
> 
> cu
> Robert
> 
> Roman Kennke wrote:
> 
>>>Hi,
>>>
>>>
>>>
>>>>>I implemented and added an ObjectPool class.
>>>>
>>>>Please measure! Don't assume that this will lead to better performance.
>>>>I do follow your reasoning. But ultimately this is the job of the
>>>>allocator/garbage collector.
>>>
>>>
>>>I completely agree with you that ideally this should be done in the GC.
>>>This is why I hestitate to actually introduce some 'optimizations'. I
>>>don't know about the current situation in free VMs Garbage Collectors
>>>and how the impact would be. It would be helpful if some VM hackers
>>>could comment on the issue.
>>>
>>>
>>>
>>>>Please make it really easy to turn on/off caching so people can do real
>>>>measurements of the performance impact.
>>>
>>>
>>>I have added a flag in the ObjectPool that can turn off the cache. As
>>>well as some counters that count how many objects are
>>>requested/returned/created/pooled. This is not a very clever benchmark,
>>>since it does not actually measure performance (time-wise).
>>>
>>>
>>>
>>>>And only introduce it when it is
>>>>a clear win.
>>>
>>>
>>>Seeing that it is quite difficult to find the right spots where objects
>>>should actually be returned into the pool, I strongly hesitate now to
>>>introduce anything.
>>>
>>>
>>>
>>>>What seems to happen a lot in larger projects is that someone introduces
>>>>some pooling/caching of certain constructs and when some implementation
>>>>detail somewhere else in the stack changes the pooling/caching isn't
>>>>revised/remeasured to see if it still makes sense (or that it actually
>>>>decreases performance given the new circumstances!).
>>>>Don't let that happen in this case.
>>>
>>>
>>>I won't. It would be nice if some folks would play a little and comment
>>>on this issue. Note that the pool isn't actually in use in Classpath, it
>>>only sits around now for testing. I committed the following changes that
>>>could help benchmarking:
>>>
>>>2005-07-01  Roman Kennke  <address@hidden>
>>>
>>>        * gnu/classpath/ObjectPool.java:
>>>        Introduced flag for turning on/off caching.
>>>        (getInstance): Synchronized access to this method.
>>>        (borrowObject): Synchronized access to the pool.
>>>        Added some benchmarking statements.
>>>        (returnObject): Synchronized access to the pool.
>>>        Added some benchmarking statements.
>>>        (createObject): Synchronized access to the pool.
>>>        Added some benchmarking statements.
>>>        (printStats): New method. Prints out some stats about the pool
>>>     usage.
>>>
>>>/Roman
>>>
>>>
>>>
>>>------------------------------------------------------------------------
>>>
>>>Index: gnu/classpath/ObjectPool.java
>>>===================================================================
>>>RCS file: /cvsroot/classpath/classpath/gnu/classpath/ObjectPool.java,v
>>>retrieving revision 1.2
>>>diff -u -r1.2 ObjectPool.java
>>>--- gnu/classpath/ObjectPool.java    1 Jul 2005 10:58:07 -0000       1.2
>>>+++ gnu/classpath/ObjectPool.java    1 Jul 2005 13:04:39 -0000
>>>@@ -77,6 +77,12 @@
>>> public final class ObjectPool
>>> {
>>> 
>>>+  /** The maximum number of instances that we keep for each type. */
>>>+  private static final int MAX_POOL_SIZE = 128;
>>>+
>>>+  /** This flag turns on/off caching (for benchmarking purposes). */
>>>+  private static final boolean IS_CACHING = true;
>>>+
>>>   /** The only instance of ObjectPool. */
>>>   private static ObjectPool instance;
>>> 
>>>@@ -87,6 +93,14 @@
>>>    * requested type is in the pool.
>>>    */
>>>   private HashMap pool;
>>>+  
>>>+  /**
>>>+   * Collect some stats in this fields. TODO: Can be removed later.
>>>+   */
>>>+  int created = 0;
>>>+  int requested = 0;
>>>+  int returned = 0;
>>>+  int pooled = 0;
>>> 
>>>   /**
>>>    * Creates a new instance of ObjectPool. This constructor is made private
>>>@@ -102,7 +116,7 @@
>>>    *
>>>    * @return an ObjectPool instance ready for use
>>>    */
>>>-  public static ObjectPool getInstance()
>>>+  public static synchronized ObjectPool getInstance()
>>>   {
>>>     if (instance == null)
>>>       instance = new ObjectPool();
>>>@@ -123,15 +137,32 @@
>>>    */
>>>   public Object borrowObject(Class type)
>>>   {
>>>+    // This is only here for benchmarking purposes.
>>>+    if (!IS_CACHING)
>>>+      return createObject(type);
>>>+    // Counts the requested objects. This is only here for benchmarking
>>>+    // purposes.
>>>+    requested++;
>>>+    if (requested % 10000 == 0)
>>>+      printStats();
>>>+
>>>+
>>>     Object object = null;
>>>-    Stack pooledInstances = (Stack) pool.get(type);
>>>+    Stack pooledInstances = null;
>>>+    synchronized (this)
>>>+      {
>>>+    pooledInstances = (Stack) pool.get(type);
>>>+      }
>>>     if (pooledInstances == null)
>>>       object = createObject(type);
>>>     else
>>>       if (pooledInstances.size() == 0)
>>>     object = createObject(type);
>>>       else
>>>-    object = pooledInstances.pop();
>>>+    synchronized (this)
>>>+      {
>>>+        object = pooledInstances.pop();
>>>+      }
>>>     return object;
>>>   }
>>> 
>>>@@ -142,14 +173,36 @@
>>>    */
>>>   public void returnObject(Object object)
>>>   {
>>>+    // This is only here for benchmarking purposes.
>>>+    if (!IS_CACHING)
>>>+      return;
>>>+    // Count the returned objects. This is only here for benchmarking 
>>>purposes.
>>>+    returned++;
>>>+
>>>     Class type = object.getClass();
>>>-    Stack pooledInstances = (Stack) pool.get(type);
>>>+    Stack pooledInstances = null;
>>>+    synchronized (this)
>>>+      {
>>>+    pooledInstances = (Stack) pool.get(type);
>>>+      }
>>>     if (pooledInstances == null)
>>>       {
>>>     pooledInstances = new Stack();
>>>-    pool.put(type, pooledInstances);
>>>       }
>>>-    pooledInstances.push(object);
>>>+    if (pooledInstances.size() < MAX_POOL_SIZE)
>>>+      synchronized (this)
>>>+    {
>>>+      pool.put(type, pooledInstances);
>>>+
>>>+      // Count the objects that are actually pooled. This is only
>>>+      // here for benchmarking purposes.
>>>+      pooled++;
>>>+    }
>>>+
>>>+    synchronized (this)
>>>+      {
>>>+    pooledInstances.push(object);
>>>+      }
>>>   }
>>> 
>>>   /**
>>>@@ -162,6 +215,11 @@
>>>    */
>>>   private Object createObject(Class type)
>>>   {
>>>+    // Counts the objects that are created here. This is only here for
>>>+    // benchmarking purposes.
>>>+    created++;
>>>+
>>>+
>>>     Object object = null;
>>>     try
>>>       {
>>>@@ -177,4 +235,16 @@
>>>       }
>>>     return object;
>>>   }
>>>+
>>>+  /**
>>>+   * This method prints out some stats about the object pool. This gives
>>>+   * an indication on how efficiently the pool is used.
>>>+   */
>>>+  void printStats()
>>>+  {
>>>+    System.err.println("Requested Objects: " + requested);
>>>+    System.err.println("Returned Objects: " + returned);
>>>+    System.err.println("Created Objects: " + created);
>>>+    System.err.println("Pooled Objects: " + pooled);
>>>+  }
>>> }
>>>
>>>
>>>------------------------------------------------------------------------
>>>
>>>_______________________________________________
>>>Classpath-patches mailing list
>>>address@hidden
>>>http://lists.gnu.org/mailman/listinfo/classpath-patches
> 

_______________________________________________
Classpath-patches mailing list
address@hidden
http://lists.gnu.org/mailman/listinfo/classpath-patches


-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.1 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFCxcNKG9cfwmwwEtoRAuO8AJ9QwLDpMqHVQXInSujOd2Id6aHofwCeLHTr
UQXNdVn/4U0J+Afs2PGpcIE=
=t70q
-----END PGP SIGNATURE-----