Re: svn commit: r1163705 - in /camel/trunk/camel-core/src: main/java/org/apache/camel/impl/converter/PropertyEditorTypeConverter.java test/java/org/apache/camel/component/bean/BeanComponentCustomCreat

2011-09-02 Thread Claus Ibsen
On Thu, Sep 1, 2011 at 3:16 AM, Hadrian Zbarcea hzbar...@gmail.com wrote:
 Yes, I agree with that, reason why Registries that use caches *are* and
 should stay as Services. The PropertyEditorTypeConverter in particular
 doesn't need to be a Service, but the EndpointRegisty, ConsumerCache and
 ProducerCache should be and are Services.

 So you are correct about not leaving memory leaks, etc. Owners of caches
 that hold Service(s) should and do shut them down before the cache clear()
 since that's not the job of the cache itself (plus caches may not hold
 Services in which case all the iteration over, dunno, hundreds, thousands of
 object to check for (item instanceof Service) is kinda useless anyway.
 Services are something that have a life cycle, the LRUCache is a handy data
 structure. All in all I was quite careful about my changes, I hope I didn't
 miss anything. I am sure that if anybody spots something he'll point it out
 and we'll fix it. Needless to say, all tests pass after my changes and I am
 fairly comfortable with the fix.

 Cheers,
 Hadrian


There is still a leaking problem.

For example try run
PropertyEditorTypeConverterIssueTest

And then revert the change on PropertyEditorTypeConverterIssue to the
previous commit (eg its a Service).
And then for example do a System.out in doStop

For example when I do this
Starting org.apache.camel.impl.converter.PropertyEditorTypeConverter@6a8c436b
Stopping org.apache.camel.impl.converter.PropertyEditorTypeConverter@6a8c436b
with 0 in cache and 1 misses

eg I can see that when Camel is shutdown this type converter is as
well, where I then would be able to clear the caches.
And thus not leak resources.





 On 08/31/2011 03:55 PM, Claus Ibsen wrote:

 I wonder if there wont be a problem when you shutdown camel, or run
 Camel in a hot deployment environment.
 As before the service would ensure start|stop callbacks, where we
 could clear the caches when stopping.

 That is now gone, which means we can just hope the GC eventually will
 be able to reclaim the objects.

 So I suggest to use Service for classes that has caches or any map
 store which can hold possible a lot of data.
 Then being able to properly clean/stop those cachen when stopping
 Camel is a good citizen to avoid eating memory or leaking in any way.



 On Wed, Aug 31, 2011 at 7:10 PM,hadr...@apache.org  wrote:

 Author: hadrian
 Date: Wed Aug 31 17:10:53 2011
 New Revision: 1163705

 URL: http://svn.apache.org/viewvc?rev=1163705view=rev
 Log:
 CAMEL-4392. Fix side effect of LRUCache not a service.

 Modified:

  camel/trunk/camel-core/src/main/java/org/apache/camel/impl/converter/PropertyEditorTypeConverter.java

  camel/trunk/camel-core/src/test/java/org/apache/camel/component/bean/BeanComponentCustomCreateEndpointTest.java

 Modified:
 camel/trunk/camel-core/src/main/java/org/apache/camel/impl/converter/PropertyEditorTypeConverter.java
 URL:
 http://svn.apache.org/viewvc/camel/trunk/camel-core/src/main/java/org/apache/camel/impl/converter/PropertyEditorTypeConverter.java?rev=1163705r1=1163704r2=1163705view=diff

 ==
 ---
 camel/trunk/camel-core/src/main/java/org/apache/camel/impl/converter/PropertyEditorTypeConverter.java
 (original)
 +++
 camel/trunk/camel-core/src/main/java/org/apache/camel/impl/converter/PropertyEditorTypeConverter.java
 Wed Aug 31 17:10:53 2011
 @@ -22,11 +22,9 @@ import java.util.HashMap;
  import java.util.Map;

  import org.apache.camel.Exchange;
 -import org.apache.camel.Service;
  import org.apache.camel.TypeConverter;
  import org.apache.camel.util.LRUSoftCache;
  import org.apache.camel.util.ObjectHelper;
 -import org.apache.camel.util.ServiceHelper;
  import org.slf4j.Logger;
  import org.slf4j.LoggerFactory;

 @@ -36,14 +34,14 @@ import org.slf4j.LoggerFactory;
  *
  * @version
  */
 -public class PropertyEditorTypeConverter implements TypeConverter,
 Service {
 +public class PropertyEditorTypeConverter implements TypeConverter {

     private static final Logger LOG =
 LoggerFactory.getLogger(PropertyEditorTypeConverter.class);
     // use a soft bound cache to avoid using too much memory in case a
 lot of different classes
     // is being converted to string
 -    private final MapClass, Class  misses = new LRUSoftCacheClass,
 Class(1000);
 +    private final MapClass?, Class?  misses = new
 LRUSoftCacheClass?, Class?(1000);
     // we don't anticipate so many property editors so we have unbounded
 map
 -    private final MapClass, PropertyEditor  cache = new HashMapClass,
 PropertyEditor();
 +    private final MapClass?, PropertyEditor  cache = new
 HashMapClass?, PropertyEditor();

     publicT  T convertTo(ClassT  type, Object value) {
         // We can't convert null values since we can't figure out a
 property
 @@ -58,14 +56,14 @@ public class PropertyEditorTypeConverter
                 return ObjectHelper.cast(type, value);
             }

 -            Class key = type;
 

Re: svn commit: r1163705 - in /camel/trunk/camel-core/src: main/java/org/apache/camel/impl/converter/PropertyEditorTypeConverter.java test/java/org/apache/camel/component/bean/BeanComponentCustomCreat

2011-08-31 Thread Claus Ibsen
I wonder if there wont be a problem when you shutdown camel, or run
Camel in a hot deployment environment.
As before the service would ensure start|stop callbacks, where we
could clear the caches when stopping.

That is now gone, which means we can just hope the GC eventually will
be able to reclaim the objects.

So I suggest to use Service for classes that has caches or any map
store which can hold possible a lot of data.
Then being able to properly clean/stop those cachen when stopping
Camel is a good citizen to avoid eating memory or leaking in any way.



On Wed, Aug 31, 2011 at 7:10 PM,  hadr...@apache.org wrote:
 Author: hadrian
 Date: Wed Aug 31 17:10:53 2011
 New Revision: 1163705

 URL: http://svn.apache.org/viewvc?rev=1163705view=rev
 Log:
 CAMEL-4392. Fix side effect of LRUCache not a service.

 Modified:
    
 camel/trunk/camel-core/src/main/java/org/apache/camel/impl/converter/PropertyEditorTypeConverter.java
    
 camel/trunk/camel-core/src/test/java/org/apache/camel/component/bean/BeanComponentCustomCreateEndpointTest.java

 Modified: 
 camel/trunk/camel-core/src/main/java/org/apache/camel/impl/converter/PropertyEditorTypeConverter.java
 URL: 
 http://svn.apache.org/viewvc/camel/trunk/camel-core/src/main/java/org/apache/camel/impl/converter/PropertyEditorTypeConverter.java?rev=1163705r1=1163704r2=1163705view=diff
 ==
 --- 
 camel/trunk/camel-core/src/main/java/org/apache/camel/impl/converter/PropertyEditorTypeConverter.java
  (original)
 +++ 
 camel/trunk/camel-core/src/main/java/org/apache/camel/impl/converter/PropertyEditorTypeConverter.java
  Wed Aug 31 17:10:53 2011
 @@ -22,11 +22,9 @@ import java.util.HashMap;
  import java.util.Map;

  import org.apache.camel.Exchange;
 -import org.apache.camel.Service;
  import org.apache.camel.TypeConverter;
  import org.apache.camel.util.LRUSoftCache;
  import org.apache.camel.util.ObjectHelper;
 -import org.apache.camel.util.ServiceHelper;
  import org.slf4j.Logger;
  import org.slf4j.LoggerFactory;

 @@ -36,14 +34,14 @@ import org.slf4j.LoggerFactory;
  *
  * @version
  */
 -public class PropertyEditorTypeConverter implements TypeConverter, Service {
 +public class PropertyEditorTypeConverter implements TypeConverter {

     private static final Logger LOG = 
 LoggerFactory.getLogger(PropertyEditorTypeConverter.class);
     // use a soft bound cache to avoid using too much memory in case a lot of 
 different classes
     // is being converted to string
 -    private final MapClass, Class misses = new LRUSoftCacheClass, 
 Class(1000);
 +    private final MapClass?, Class? misses = new 
 LRUSoftCacheClass?, Class?(1000);
     // we don't anticipate so many property editors so we have unbounded map
 -    private final MapClass, PropertyEditor cache = new HashMapClass, 
 PropertyEditor();
 +    private final MapClass?, PropertyEditor cache = new 
 HashMapClass?, PropertyEditor();

     public T T convertTo(ClassT type, Object value) {
         // We can't convert null values since we can't figure out a property
 @@ -58,14 +56,14 @@ public class PropertyEditorTypeConverter
                 return ObjectHelper.cast(type, value);
             }

 -            Class key = type;
 +            Class? key = type;
             PropertyEditor editor = lookupEditor(key);
             if (editor != null) {
                 editor.setAsText(value.toString());
                 return ObjectHelper.cast(type, editor.getValue());
             }
         } else if (type == String.class) {
 -            Class key = value.getClass();
 +            Class? key = value.getClass();
             PropertyEditor editor = lookupEditor(key);
             if (editor != null) {
                 editor.setValue(value);
 @@ -76,7 +74,7 @@ public class PropertyEditorTypeConverter
         return null;
     }

 -    private PropertyEditor lookupEditor(Class type) {
 +    private PropertyEditor lookupEditor(Class? type) {
         // check misses first
         if (misses.containsKey(type)) {
             LOG.trace(No previously found property editor for type: {}, 
 type);
 @@ -115,14 +113,4 @@ public class PropertyEditorTypeConverter
     public T T mandatoryConvertTo(ClassT type, Exchange exchange, Object 
 value) {
         return convertTo(type, value);
     }
 -
 -    public void start() throws Exception {
 -        ServiceHelper.startService(misses);
 -    }
 -
 -    public void stop() throws Exception {
 -        cache.clear();
 -        ServiceHelper.stopService(misses);
 -    }
 -
  }

 Modified: 
 camel/trunk/camel-core/src/test/java/org/apache/camel/component/bean/BeanComponentCustomCreateEndpointTest.java
 URL: 
 http://svn.apache.org/viewvc/camel/trunk/camel-core/src/test/java/org/apache/camel/component/bean/BeanComponentCustomCreateEndpointTest.java?rev=1163705r1=1163704r2=1163705view=diff
 ==
 --- 
 

Re: svn commit: r1163705 - in /camel/trunk/camel-core/src: main/java/org/apache/camel/impl/converter/PropertyEditorTypeConverter.java test/java/org/apache/camel/component/bean/BeanComponentCustomCreat

2011-08-31 Thread Hadrian Zbarcea
Yes, I agree with that, reason why Registries that use caches *are* and 
should stay as Services. The PropertyEditorTypeConverter in particular 
doesn't need to be a Service, but the EndpointRegisty, ConsumerCache and 
ProducerCache should be and are Services.


So you are correct about not leaving memory leaks, etc. Owners of caches 
that hold Service(s) should and do shut them down before the cache 
clear() since that's not the job of the cache itself (plus caches may 
not hold Services in which case all the iteration over, dunno, hundreds, 
thousands of object to check for (item instanceof Service) is kinda 
useless anyway. Services are something that have a life cycle, the 
LRUCache is a handy data structure. All in all I was quite careful about 
my changes, I hope I didn't miss anything. I am sure that if anybody 
spots something he'll point it out and we'll fix it. Needless to say, 
all tests pass after my changes and I am fairly comfortable with the fix.


Cheers,
Hadrian



On 08/31/2011 03:55 PM, Claus Ibsen wrote:

I wonder if there wont be a problem when you shutdown camel, or run
Camel in a hot deployment environment.
As before the service would ensure start|stop callbacks, where we
could clear the caches when stopping.

That is now gone, which means we can just hope the GC eventually will
be able to reclaim the objects.

So I suggest to use Service for classes that has caches or any map
store which can hold possible a lot of data.
Then being able to properly clean/stop those cachen when stopping
Camel is a good citizen to avoid eating memory or leaking in any way.



On Wed, Aug 31, 2011 at 7:10 PM,hadr...@apache.org  wrote:

Author: hadrian
Date: Wed Aug 31 17:10:53 2011
New Revision: 1163705

URL: http://svn.apache.org/viewvc?rev=1163705view=rev
Log:
CAMEL-4392. Fix side effect of LRUCache not a service.

Modified:

camel/trunk/camel-core/src/main/java/org/apache/camel/impl/converter/PropertyEditorTypeConverter.java

camel/trunk/camel-core/src/test/java/org/apache/camel/component/bean/BeanComponentCustomCreateEndpointTest.java

Modified: 
camel/trunk/camel-core/src/main/java/org/apache/camel/impl/converter/PropertyEditorTypeConverter.java
URL: 
http://svn.apache.org/viewvc/camel/trunk/camel-core/src/main/java/org/apache/camel/impl/converter/PropertyEditorTypeConverter.java?rev=1163705r1=1163704r2=1163705view=diff
==
--- 
camel/trunk/camel-core/src/main/java/org/apache/camel/impl/converter/PropertyEditorTypeConverter.java
 (original)
+++ 
camel/trunk/camel-core/src/main/java/org/apache/camel/impl/converter/PropertyEditorTypeConverter.java
 Wed Aug 31 17:10:53 2011
@@ -22,11 +22,9 @@ import java.util.HashMap;
  import java.util.Map;

  import org.apache.camel.Exchange;
-import org.apache.camel.Service;
  import org.apache.camel.TypeConverter;
  import org.apache.camel.util.LRUSoftCache;
  import org.apache.camel.util.ObjectHelper;
-import org.apache.camel.util.ServiceHelper;
  import org.slf4j.Logger;
  import org.slf4j.LoggerFactory;

@@ -36,14 +34,14 @@ import org.slf4j.LoggerFactory;
  *
  * @version
  */
-public class PropertyEditorTypeConverter implements TypeConverter, Service {
+public class PropertyEditorTypeConverter implements TypeConverter {

 private static final Logger LOG = 
LoggerFactory.getLogger(PropertyEditorTypeConverter.class);
 // use a soft bound cache to avoid using too much memory in case a lot of 
different classes
 // is being converted to string
-private final MapClass, Class  misses = new LRUSoftCacheClass, 
Class(1000);
+private final MapClass?, Class?  misses = new LRUSoftCacheClass?, 
Class?(1000);
 // we don't anticipate so many property editors so we have unbounded map
-private final MapClass, PropertyEditor  cache = new HashMapClass, 
PropertyEditor();
+private final MapClass?, PropertyEditor  cache = new HashMapClass?, 
PropertyEditor();

 publicT  T convertTo(ClassT  type, Object value) {
 // We can't convert null values since we can't figure out a property
@@ -58,14 +56,14 @@ public class PropertyEditorTypeConverter
 return ObjectHelper.cast(type, value);
 }

-Class key = type;
+Class?  key = type;
 PropertyEditor editor = lookupEditor(key);
 if (editor != null) {
 editor.setAsText(value.toString());
 return ObjectHelper.cast(type, editor.getValue());
 }
 } else if (type == String.class) {
-Class key = value.getClass();
+Class?  key = value.getClass();
 PropertyEditor editor = lookupEditor(key);
 if (editor != null) {
 editor.setValue(value);
@@ -76,7 +74,7 @@ public class PropertyEditorTypeConverter
 return null;
 }

-private PropertyEditor lookupEditor(Class type) {
+private PropertyEditor lookupEditor(Class?  type) {