You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@camel.apache.org by Hadrian Zbarcea <hz...@gmail.com> on 2011/09/01 03:16:48 UTC
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/BeanComponentCustomCreateEndpointTest.java
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,<ha...@apache.org> wrote:
>> Author: hadrian
>> Date: Wed Aug 31 17:10:53 2011
>> New Revision: 1163705
>>
>> URL: http://svn.apache.org/viewvc?rev=1163705&view=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=1163705&r1=1163704&r2=1163705&view=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 Map<Class, Class> misses = new LRUSoftCache<Class, Class>(1000);
>> + private final Map<Class<?>, Class<?>> misses = new LRUSoftCache<Class<?>, Class<?>>(1000);
>> // we don't anticipate so many property editors so we have unbounded map
>> - private final Map<Class, PropertyEditor> cache = new HashMap<Class, PropertyEditor>();
>> + private final Map<Class<?>, PropertyEditor> cache = new HashMap<Class<?>, PropertyEditor>();
>>
>> public<T> T convertTo(Class<T> 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(Class<T> 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=1163705&r1=1163704&r2=1163705&view=diff
>> ==============================================================================
>> --- camel/trunk/camel-core/src/test/java/org/apache/camel/component/bean/BeanComponentCustomCreateEndpointTest.java (original)
>> +++ camel/trunk/camel-core/src/test/java/org/apache/camel/component/bean/BeanComponentCustomCreateEndpointTest.java Wed Aug 31 17:10:53 2011
>> @@ -47,11 +47,11 @@ public class BeanComponentCustomCreateEn
>>
>> public void testCreateEndpointUri() throws Exception {
>> BeanComponent bc = context.getComponent("bean", BeanComponent.class);
>> - ProcessorEndpoint pe = bc.createEndpoint(new MyFooBean(), "cheese");
>> + ProcessorEndpoint pe = bc.createEndpoint(new MyFooBean(), "bean:cheese");
>> assertNotNull(pe);
>>
>> String uri = pe.getEndpointUri();
>> - assertEquals("cheese", uri);
>> + assertEquals("bean:cheese", uri);
>>
>> Producer producer = pe.createProducer();
>> Exchange exchange = producer.createExchange();
>>
>>
>>
>
>
>
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/BeanComponentCustomCreateEndpointTest.java
Posted by Claus Ibsen <cl...@gmail.com>.
On Thu, Sep 1, 2011 at 3:16 AM, Hadrian Zbarcea <hz...@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,<ha...@apache.org> wrote:
>>>
>>> Author: hadrian
>>> Date: Wed Aug 31 17:10:53 2011
>>> New Revision: 1163705
>>>
>>> URL: http://svn.apache.org/viewvc?rev=1163705&view=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=1163705&r1=1163704&r2=1163705&view=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 Map<Class, Class> misses = new LRUSoftCache<Class,
>>> Class>(1000);
>>> + private final Map<Class<?>, Class<?>> misses = new
>>> LRUSoftCache<Class<?>, Class<?>>(1000);
>>> // we don't anticipate so many property editors so we have unbounded
>>> map
>>> - private final Map<Class, PropertyEditor> cache = new HashMap<Class,
>>> PropertyEditor>();
>>> + private final Map<Class<?>, PropertyEditor> cache = new
>>> HashMap<Class<?>, PropertyEditor>();
>>>
>>> public<T> T convertTo(Class<T> 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(Class<T> 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=1163705&r1=1163704&r2=1163705&view=diff
>>>
>>> ==============================================================================
>>> ---
>>> camel/trunk/camel-core/src/test/java/org/apache/camel/component/bean/BeanComponentCustomCreateEndpointTest.java
>>> (original)
>>> +++
>>> camel/trunk/camel-core/src/test/java/org/apache/camel/component/bean/BeanComponentCustomCreateEndpointTest.java
>>> Wed Aug 31 17:10:53 2011
>>> @@ -47,11 +47,11 @@ public class BeanComponentCustomCreateEn
>>>
>>> public void testCreateEndpointUri() throws Exception {
>>> BeanComponent bc = context.getComponent("bean",
>>> BeanComponent.class);
>>> - ProcessorEndpoint pe = bc.createEndpoint(new MyFooBean(),
>>> "cheese");
>>> + ProcessorEndpoint pe = bc.createEndpoint(new MyFooBean(),
>>> "bean:cheese");
>>> assertNotNull(pe);
>>>
>>> String uri = pe.getEndpointUri();
>>> - assertEquals("cheese", uri);
>>> + assertEquals("bean:cheese", uri);
>>>
>>> Producer producer = pe.createProducer();
>>> Exchange exchange = producer.createExchange();
>>>
>>>
>>>
>>
>>
>>
>
--
Claus Ibsen
-----------------
FuseSource
Email: cibsen@fusesource.com
Web: http://fusesource.com
Twitter: davsclaus, fusenews
Blog: http://davsclaus.blogspot.com/
Author of Camel in Action: http://www.manning.com/ibsen/