RE: ServerResource is currently incompatible with OSGi
Hi Tal and Tim,
Thanks for the discussion. I have added a clearCache() method which is
invoked each time an application is stopped (there is no
Application#release() method). The minor issue with an application specific
cache is that resource classes could be shared by several applications.
I have also entered a RFE to keep track of the need for a more optimal
solution in the future:
Improve cleaning of annotation descriptors
http://restlet.tigris.org/issues/show_bug.cgi?id=784
Best regards,
Jerome Louvel
--
Restlet ~ Founder and Lead developer ~ http://www.restlet.org/
http://www.restlet.org
Noelios Technologies ~ Co-founder ~ http://www.noelios.com/
http://www.noelios.com
_
De : Tal Liron [mailto:tal.li...@threecrickets.com]
Envoyé : dimanche 12 avril 2009 17:39
À : discuss@restlet.tigris.org
Objet : Re: ServerResource is currently incompatible with OSGi
Tim... you seem to be agreeing with me, then, that this does have something
to do with the Application, and you are also allowing for some redundancy in
calculation and memory use, so, again, why not just store it in the
Application's Context, where all this is taken care of automatically? I
think Jerome's first instinct here may have been the most straightforward.
Barring that, how about calling the clearCache in the Application's
release(), rather than stop()?
-Tal
Tim Peierls wrote:
I wouldn't recommend RRWL for this.
Since it's harmless to recompute the AnnotationInfos, I think just having an
AnnotationUtils.clearCache method would work well. Call it whenever an
Application stops, say. It's worth a little redundant computation to avoid
having to deal with shared locks and liveness.
--tim
On Thu, Apr 9, 2009 at 2:22 PM, Tal Liron tal.li...@threecrickets.com
wrote:
Oh, I didn't mean synchronizing the whole map. I actually didn't mean much
beyond suggesting that we brainstorm a way to use WeakReferences as keys in
this particular case. I still don't have a complete solution in mind.
Another idea: Perhaps we can use a ReentrantReadWrite lock around a simple
WeakHashMap? Since this is a 99% read situation, most requests won't need
to acquire locks.
(ConcurrentHashMap is actually a very complicated and delicately tuned
class, and it would be anything but trivial for us to try to re-implement it
using WeakReferences.)
I know there are generic 3rd party solutions to this, but I know we'd all
prefer to use what Java 5 offers us right now. I'm sure we can cobble
together something that performs well!
-Tal
Tim Peierls wrote:
Did you read my subsequent e-mail that talked about opening an issue to make
this more robust?
There is currently no standard concurrent map with weak keys, although
MapMaker in Google Collections provides this and much more. An effort to put
something like this into Java 7 is under way, but there's no telling whether
it will actually happen.
Wrapping a WeakHashMap with synchronization is likely to be a performance
bottleneck: every request has to grab a global lock.
--tim
On Wed, Apr 8, 2009 at 2:06 PM, Tal Liron tal.li...@threecrickets.com
wrote:
Hmm. I disagree that this has nothing to do with the Application. For
example, if an Application is unloaded, this cached information will remain
in the static field. In fact, there is no mechanism in place right now clean
this cache, and in dynamic environments (possibly OSGi) it may accumulate
cruft. Not a huge problem, but this is the stuff memory leaks are made
of...
Here's a thought: can we use a synchronized WeakHashMap here? That way, the
AnnotationInfo would be discarded when the class is unloaded.
-Tal
Jerome Louvel wrote:
Hi all,
Beautiful! This new solution is available in SVN trunk.
Thanks Dave for the report and Tim for the clever fix!
Best regards,
Jerome Louvel
--
Restlet ~ Founder and Lead developer ~ http://www.restlet.org/
http://www.restlet.org
Noelios Technologies ~ Co-founder ~ http://www.noelios.com
http://www.noelios.com
_
De : tpeie...@gmail.com [mailto:tpeie...@gmail.com] De la part de Tim
Peierls
Envoyé : mercredi 8 avril 2009 00:23
À : discuss@restlet.tigris.org
Objet : Re: ServerResource is currently incompatible with OSGi
Why involve Context at all? The AnnotationInfo associated with a Class?
extends UniformResource does not depend on Context. You could just add a
method to AnnotationUtils:
public static AnnotationInfo getAnnotationDescriptor(Class? extends
UniformResource resourceClass) {
AnnotationInfo result = cache.get(resourceClass);
if (result == null) {
result = computeAnnotationDescriptor(resourceClass); // use code from
existing getAnnotationDescriptors for this
AnnotationInfo prev = cache.putIfAbsent(resourceClass, result);
if (prev != null) result = prev;
}
return result;
}
private static final ConcurrentMapClass? extends Resource,
AnnotationInfo cache =
new ConcurrentHashMapClass? extends Resource, AnnotationInfo();
Then you don't need
Re: ServerResource is currently incompatible with OSGi
I wouldn't recommend RRWL for this.
Since it's harmless to recompute the AnnotationInfos, I think just having an
AnnotationUtils.clearCache method would work well. Call it whenever an
Application stops, say. It's worth a little redundant computation to avoid
having to deal with shared locks and liveness.
--tim
On Thu, Apr 9, 2009 at 2:22 PM, Tal Liron tal.li...@threecrickets.comwrote:
Oh, I didn't mean synchronizing the whole map. I actually didn't mean
much beyond suggesting that we brainstorm a way to use WeakReferences as
keys in this particular case. I still don't have a complete solution in
mind.
Another idea: Perhaps we can use a ReentrantReadWrite lock around a simple
WeakHashMap? Since this is a 99% read situation, most requests won't need
to acquire locks.
(ConcurrentHashMap is actually a very complicated and delicately tuned
class, and it would be anything but trivial for us to try to re-implement it
using WeakReferences.)
I know there are generic 3rd party solutions to this, but I know we'd all
prefer to use what Java 5 offers us right now. I'm sure we can cobble
together something that performs well!
-Tal
Tim Peierls wrote:
Did you read my subsequent e-mail that talked about opening an issue to
make this more robust?
There is currently no standard concurrent map with weak keys, although
MapMaker in Google Collections provides this and much more. An effort to put
something like this into Java 7 is under way, but there's no telling whether
it will actually happen.
Wrapping a WeakHashMap with synchronization is likely to be a performance
bottleneck: every request has to grab a global lock.
--tim
On Wed, Apr 8, 2009 at 2:06 PM, Tal Liron tal.li...@threecrickets.comwrote:
Hmm. I disagree that this has *nothing* to do with the Application. For
example, if an Application is unloaded, this cached information will remain
in the static field. In fact, there is no mechanism in place right now clean
this cache, and in dynamic environments (possibly OSGi) it may accumulate
cruft. Not a huge problem, but this is the stuff memory leaks are made
of...
Here's a thought: can we use a synchronized WeakHashMap here? That way,
the AnnotationInfo would be discarded when the class is unloaded.
-Tal
Jerome Louvel wrote:
Hi all,
Beautiful! This new solution is available in SVN trunk.
Thanks Dave for the report and Tim for the clever fix!
Best regards,
Jerome Louvel
--
Restlet ~ Founder and Lead developer ~ http://www.restlet.org
Noelios Technologies ~ Co-founder ~ http://www.noelios.com
--
*De :* tpeie...@gmail.com [mailto:tpeie...@gmail.com tpeie...@gmail.com]
*De la part de* Tim Peierls
*Envoyé :* mercredi 8 avril 2009 00:23
*À :* discuss@restlet.tigris.org
*Objet :* Re: ServerResource is currently incompatible with OSGi
Why involve Context at all? The AnnotationInfo associated with a Class?
extends UniformResource does not depend on Context. You could just add a
method to AnnotationUtils:
public static AnnotationInfo getAnnotationDescriptor(Class? extends
UniformResource resourceClass) {
AnnotationInfo result = cache.get(resourceClass);
if (result == null) {
result = computeAnnotationDescriptor(resourceClass); // use code from
existing getAnnotationDescriptors for this
AnnotationInfo prev = cache.putIfAbsent(resourceClass, result);
if (prev != null) result = prev;
}
return result;
}
private static final ConcurrentMapClass? extends Resource,
AnnotationInfo cache =
new ConcurrentHashMapClass? extends Resource, AnnotationInfo();
Then you don't need getAnnotationDescriptors. You probably want a way to
clear cache entries if a class is reloaded, say.
You could get more fancy, with Futures and such, but I think that would
be wasted effort.
--tim
On Tue, Apr 7, 2009 at 1:20 PM, Tal Liron tal.li...@threecrickets.comwrote:
Ah, I can see your point now.
I would recommend, then, that a Context would have two separate
ConcurrentHashMaps, something like this:
1. Context.getConfiguration() -- this would be for the user
2. Context.getImplementationCache() -- this would be for things like
AnnotationUtils, and the user should not normally look at this, except
perhaps for debugging
-Tal
David Fogel wrote:
Hi Tal-
I think what you suggested about appending a classloader id (perhaps
System.identityHashCode(cl) ?) would work.
Regarding the Restlet Context and this cache being cruft: You say
the Context is a reasonable place for information that applies to the
Application, but this doesn't have annotation cache doesn't have
anything to do with my Application, it's related to a hidden
_implementation_ detail. As an end-user, we're not meant to be using
this annotion info cache directly. Yet the Context, as a very
prominent and unavoidable part of the Restlet public API is meant to
be used and understood. That's why I think it isn't a good place to
stash
Re: ServerResource is currently incompatible with OSGi
Tim... you seem to be agreeing with me, then, that
this does have something to do with the Application, and you are also
allowing for some redundancy in calculation and memory use, so, again,
why not just store it in the Application's Context, where all this is
taken care of automatically? I think Jerome's first instinct here may
have been the most straightforward.
Barring that, how about calling the clearCache in the Application's
release(), rather than stop()?
-Tal
Tim Peierls wrote:
I wouldn't recommend RRWL for this.
Since it's harmless to recompute the AnnotationInfos, I think
just having an AnnotationUtils.clearCache method would work well. Call
it whenever an Application stops, say. It's worth a little redundant
computation to avoid having to deal with shared locks and liveness.
--tim
On Thu, Apr 9, 2009 at 2:22 PM, Tal Liron tal.li...@threecrickets.com
wrote:
Oh, I didn't mean synchronizing the whole map.
I
actually didn't mean much beyond suggesting that we brainstorm a way to
use WeakReferences as keys in this particular case. I still don't have
a complete solution in mind.
Another idea: Perhaps we can use a ReentrantReadWrite lock around a
simple WeakHashMap? Since this is a "99% read" situation, most requests
won't need to acquire locks.
(ConcurrentHashMap is actually a very
complicated and delicately tuned class, and it would be anything but
trivial for us to try to re-implement it using WeakReferences.)
I know there are generic 3rd party solutions to this, but I know we'd
all prefer to use what Java 5 offers us right now. I'm sure we can
cobble together something that performs well!
-Tal
Tim Peierls wrote:
Did you read my subsequent e-mail that
talked about
opening an issue to make this more robust?
There is currently no standard concurrent map with weak
keys,
although MapMaker in Google Collections provides this and much more. An
effort to put something like this into Java 7 is under way, but there's
no telling whether it will actually happen.
Wrapping a WeakHashMap with synchronization is likely to be
a
performance bottleneck: every request has to grab a global lock.
--tim
On Wed, Apr 8, 2009 at 2:06 PM, Tal
Liron tal.li...@threecrickets.com
wrote:
Hmm.
I disagree that this has nothing to do with the Application.
For example, if an Application is unloaded, this cached information
will remain in the static field. In fact, there is no mechanism in
place right now clean this cache, and in dynamic environments (possibly
OSGi) it may accumulate "cruft." Not a huge problem, but this is the
stuff memory leaks are made of...
Here's
a thought: can we use a synchronized WeakHashMap here? That way, the
AnnotationInfo would be discarded when the class is unloaded.
-Tal
Jerome Louvel wrote:
Hi
all,
Beautiful!
This new solution is available in SVN
trunk.
Thanks Dave for
the report and Tim for the clever fix!
Best
regards,
Jerome Louvel
--
Restlet ~ Founder and Lead developer ~ http://www.restlet.org
Noelios Technologies ~
Co-founder~ http://www.noelios.com
De: tpeie...@gmail.com
[mailto:tpeie...@gmail.com] De la part de
Tim Peierls
Envoy: mercredi 8 avril 2009 00:23
: discuss@restlet.tigris.org
Objet: Re: ServerResource is currently incompatible
with
OSGi
Why involve Context at all? The AnnotationInfo associated with a
Class? extends UniformResource does not depend on Context. You
could just add a method to AnnotationUtils:
public static AnnotationInfo
getAnnotationDescriptor(Class?
extends UniformResource resourceClass) {
AnnotationInfo result = cache.get(resourceClass);
if (result == null) {
result = computeAnnotationDescriptor(resourceClass); //
use
code
from existing getAnnotationDescriptors for this
AnnotationInfo prev = cache.putIfAbsent(resourceClass,
result);
if (prev != null) result = prev;
}
return result;
}
private static final ConcurrentMapClass? extends
Resource, AnnotationInfo cache =
new ConcurrentHashMapClass? extends
Resource,
AnnotationInfo();
Then you don't need getAnnotationDescriptors. You
probably
want
a way to clear cache entries if a class is reloaded, say.
You could get more fancy, with Futures and such, but I
think
that would be wasted effort.
--tim
On Tue, Apr 7, 2009 at 1:20
Re: ServerResource is currently incompatible with OSGi
Did you read my subsequent e-mail that talked about opening an issue to make
this more robust?
There is currently no standard concurrent map with weak keys, although
MapMaker in Google Collections provides this and much more. An effort to put
something like this into Java 7 is under way, but there's no telling whether
it will actually happen.
Wrapping a WeakHashMap with synchronization is likely to be a performance
bottleneck: every request has to grab a global lock.
--tim
On Wed, Apr 8, 2009 at 2:06 PM, Tal Liron tal.li...@threecrickets.comwrote:
Hmm. I disagree that this has *nothing* to do with the Application. For
example, if an Application is unloaded, this cached information will remain
in the static field. In fact, there is no mechanism in place right now clean
this cache, and in dynamic environments (possibly OSGi) it may accumulate
cruft. Not a huge problem, but this is the stuff memory leaks are made
of...
Here's a thought: can we use a synchronized WeakHashMap here? That way,
the AnnotationInfo would be discarded when the class is unloaded.
-Tal
Jerome Louvel wrote:
Hi all,
Beautiful! This new solution is available in SVN trunk.
Thanks Dave for the report and Tim for the clever fix!
Best regards,
Jerome Louvel
--
Restlet ~ Founder and Lead developer ~ http://www.restlet.org
Noelios Technologies ~ Co-founder ~ http://www.noelios.com
--
*De :* tpeie...@gmail.com [mailto:tpeie...@gmail.com tpeie...@gmail.com]
*De la part de* Tim Peierls
*Envoyé :* mercredi 8 avril 2009 00:23
*À :* discuss@restlet.tigris.org
*Objet :* Re: ServerResource is currently incompatible with OSGi
Why involve Context at all? The AnnotationInfo associated with a Class?
extends UniformResource does not depend on Context. You could just add a
method to AnnotationUtils:
public static AnnotationInfo getAnnotationDescriptor(Class? extends
UniformResource resourceClass) {
AnnotationInfo result = cache.get(resourceClass);
if (result == null) {
result = computeAnnotationDescriptor(resourceClass); // use code from
existing getAnnotationDescriptors for this
AnnotationInfo prev = cache.putIfAbsent(resourceClass, result);
if (prev != null) result = prev;
}
return result;
}
private static final ConcurrentMapClass? extends Resource,
AnnotationInfo cache =
new ConcurrentHashMapClass? extends Resource, AnnotationInfo();
Then you don't need getAnnotationDescriptors. You probably want a way to
clear cache entries if a class is reloaded, say.
You could get more fancy, with Futures and such, but I think that would
be wasted effort.
--tim
On Tue, Apr 7, 2009 at 1:20 PM, Tal Liron tal.li...@threecrickets.comwrote:
Ah, I can see your point now.
I would recommend, then, that a Context would have two separate
ConcurrentHashMaps, something like this:
1. Context.getConfiguration() -- this would be for the user
2. Context.getImplementationCache() -- this would be for things like
AnnotationUtils, and the user should not normally look at this, except
perhaps for debugging
-Tal
David Fogel wrote:
Hi Tal-
I think what you suggested about appending a classloader id (perhaps
System.identityHashCode(cl) ?) would work.
Regarding the Restlet Context and this cache being cruft: You say
the Context is a reasonable place for information that applies to the
Application, but this doesn't have annotation cache doesn't have
anything to do with my Application, it's related to a hidden
_implementation_ detail. As an end-user, we're not meant to be using
this annotion info cache directly. Yet the Context, as a very
prominent and unavoidable part of the Restlet public API is meant to
be used and understood. That's why I think it isn't a good place to
stash implementation-related data like this annotation stuff.
-Dave
--http://restlet.tigris.org/ds/viewMessage.do?dsForumId=4447dsMessageId=1572499
--
http://restlet.tigris.org/ds/viewMessage.do?dsForumId=4447dsMessageId=1602917
Re: ServerResource is currently incompatible with OSGi
On Wed, Apr 8, 2009 at 9:42 AM, Jerome Louvel jerome.lou...@noelios.comwrote: Hi all, Beautiful! This new solution is available in SVN trunk. Thanks Dave for the report and Tim for the clever fix! You're welcome. ;-) This fix will do for now, but it could be made more robust. Having strong references to the Class objects makes me uncomfortable: what if these resource classes are being generated dynamically for one shot uses? They'll never go away. I suggest creating an issue to track alternative implementations. For example, Google Collections has a MapMaker class that would make it trivial to implement a much more robust solution. --tim -- http://restlet.tigris.org/ds/viewMessage.do?dsForumId=4447dsMessageId=1598681
Re: ServerResource is currently incompatible with OSGi
Oh, I didn't mean synchronizing the whole map. I
actually didn't mean much beyond suggesting that we brainstorm a way to
use WeakReferences as keys in this particular case. I still don't have
a complete solution in mind.
Another idea: Perhaps we can use a ReentrantReadWrite lock around a
simple WeakHashMap? Since this is a "99% read" situation, most requests
won't need to acquire locks.
(ConcurrentHashMap is actually a very
complicated and delicately tuned class, and it would be anything but
trivial for us to try to re-implement it using WeakReferences.)
I know there are generic 3rd party solutions to this, but I know we'd
all prefer to use what Java 5 offers us right now. I'm sure we can
cobble together something that performs well!
-Tal
Tim Peierls wrote:
Did you read my subsequent e-mail that talked about
opening an issue to make this more robust?
There is currently no standard concurrent map with weak keys,
although MapMaker in Google Collections provides this and much more. An
effort to put something like this into Java 7 is under way, but there's
no telling whether it will actually happen.
Wrapping a WeakHashMap with synchronization is likely to be a
performance bottleneck: every request has to grab a global lock.
--tim
On Wed, Apr 8, 2009 at 2:06 PM, Tal Liron tal.li...@threecrickets.com
wrote:
Hmm.
I disagree that this has nothing to do with the Application.
For example, if an Application is unloaded, this cached information
will remain in the static field. In fact, there is no mechanism in
place right now clean this cache, and in dynamic environments (possibly
OSGi) it may accumulate "cruft." Not a huge problem, but this is the
stuff memory leaks are made of...
Here's
a thought: can we use a synchronized WeakHashMap here? That way, the
AnnotationInfo would be discarded when the class is unloaded.
-Tal
Jerome Louvel wrote:
Hi
all,
Beautiful!
This new solution is available in SVN
trunk.
Thanks Dave for
the report and Tim for the clever fix!
Best regards,
Jerome Louvel
--
Restlet ~ Founder and Lead developer ~ http://www.restlet.org
Noelios Technologies ~ Co-founder~ http://www.noelios.com
De:
tpeie...@gmail.com [mailto:tpeie...@gmail.com]
De la part de
Tim Peierls
Envoy: mercredi 8 avril 2009 00:23
: discuss@restlet.tigris.org
Objet: Re: ServerResource is currently incompatible with
OSGi
Why involve Context at all? The AnnotationInfo associated with a
Class? extends UniformResource does not depend on Context. You
could just add a method to AnnotationUtils:
public static AnnotationInfo
getAnnotationDescriptor(Class?
extends UniformResource resourceClass) {
AnnotationInfo result = cache.get(resourceClass);
if (result == null) {
result = computeAnnotationDescriptor(resourceClass); // use
code
from existing getAnnotationDescriptors for this
AnnotationInfo prev = cache.putIfAbsent(resourceClass,
result);
if (prev != null) result = prev;
}
return result;
}
private static final ConcurrentMapClass? extends
Resource, AnnotationInfo cache =
new ConcurrentHashMapClass? extends Resource,
AnnotationInfo();
Then you don't need getAnnotationDescriptors. You probably
want
a way to clear cache entries if a class is reloaded, say.
You could get more fancy, with Futures and such, but I think
that would be wasted effort.
--tim
On Tue, Apr 7, 2009 at 1:20 PM, Tal
Liron tal.li...@threecrickets.com
wrote:
Ah, I can see your point now.
I would recommend, then, that a Context would have two separate
ConcurrentHashMaps, something like this:
1. Context.getConfiguration() -- this would be for the user
2. Context.getImplementationCache() -- this would be for things like
AnnotationUtils, and the user should not normally look at this, except
perhaps for debugging
-Tal
David Fogel wrote:
Hi Tal-
I think what you suggested about appending a classloader id (perhaps
System.identityHashCode(cl) ?) would work.
Regarding the Restlet Context and this cache being "cruft": You say
the Context is a reasonable place for information that applies to the
Application, but this doesn't have annotation cache doesn't have
anything to do with my Application, it's related to a hidden
_implementation_ detail. As an end-user, we're not meant to be using
this annotion info cache directly. Yet the Context, as a very
prominent and unavoidable part of the Restlet public API is meant to
be used and understood. That's why I think it isn't a good place to
stash implementation-related data like
Re: ServerResource is currently incompatible with OSGi
Why involve Context at all? The AnnotationInfo associated with a Class?
extends UniformResource does not depend on Context. You could just add a
method to AnnotationUtils:
public static AnnotationInfo getAnnotationDescriptor(Class?
extends UniformResource resourceClass) {
AnnotationInfo result = cache.get(resourceClass);
if (result == null) {
result = computeAnnotationDescriptor(resourceClass); // use code
from existing getAnnotationDescriptors for this
AnnotationInfo prev = cache.putIfAbsent(resourceClass, result);
if (prev != null) result = prev;
}
return result;
}
private static final ConcurrentMapClass? extends Resource,
AnnotationInfo cache =
new ConcurrentHashMapClass? extends Resource, AnnotationInfo();
Then you don't need getAnnotationDescriptors. You probably want a way to
clear cache entries if a class is reloaded, say.
You could get more fancy, with Futures and such, but I think that would be
wasted effort.
--tim
On Tue, Apr 7, 2009 at 1:20 PM, Tal Liron tal.li...@threecrickets.comwrote:
Ah, I can see your point now.
I would recommend, then, that a Context would have two separate
ConcurrentHashMaps, something like this:
1. Context.getConfiguration() -- this would be for the user
2. Context.getImplementationCache() -- this would be for things like
AnnotationUtils, and the user should not normally look at this, except
perhaps for debugging
-Tal
David Fogel wrote:
Hi Tal-
I think what you suggested about appending a classloader id (perhaps
System.identityHashCode(cl) ?) would work.
Regarding the Restlet Context and this cache being cruft: You say
the Context is a reasonable place for information that applies to the
Application, but this doesn't have annotation cache doesn't have
anything to do with my Application, it's related to a hidden
_implementation_ detail. As an end-user, we're not meant to be using
this annotion info cache directly. Yet the Context, as a very
prominent and unavoidable part of the Restlet public API is meant to
be used and understood. That's why I think it isn't a good place to
stash implementation-related data like this annotation stuff.
-Dave
--http://restlet.tigris.org/ds/viewMessage.do?dsForumId=4447dsMessageId=1572499
--
http://restlet.tigris.org/ds/viewMessage.do?dsForumId=4447dsMessageId=1584755
RE: ServerResource is currently incompatible with OSGi
Hi all,
Beautiful! This new solution is available in SVN trunk.
Thanks Dave for the report and Tim for the clever fix!
Best regards,
Jerome Louvel
--
Restlet ~ Founder and Lead developer ~ http://www.restlet.org/
http://www.restlet.org
Noelios Technologies ~ Co-founder ~ http://www.noelios.com
http://www.noelios.com
_
De : tpeie...@gmail.com [mailto:tpeie...@gmail.com] De la part de Tim
Peierls
Envoyé : mercredi 8 avril 2009 00:23
À : discuss@restlet.tigris.org
Objet : Re: ServerResource is currently incompatible with OSGi
Why involve Context at all? The AnnotationInfo associated with a Class?
extends UniformResource does not depend on Context. You could just add a
method to AnnotationUtils:
public static AnnotationInfo getAnnotationDescriptor(Class? extends
UniformResource resourceClass) {
AnnotationInfo result = cache.get(resourceClass);
if (result == null) {
result = computeAnnotationDescriptor(resourceClass); // use code from
existing getAnnotationDescriptors for this
AnnotationInfo prev = cache.putIfAbsent(resourceClass, result);
if (prev != null) result = prev;
}
return result;
}
private static final ConcurrentMapClass? extends Resource,
AnnotationInfo cache =
new ConcurrentHashMapClass? extends Resource, AnnotationInfo();
Then you don't need getAnnotationDescriptors. You probably want a way to
clear cache entries if a class is reloaded, say.
You could get more fancy, with Futures and such, but I think that would be
wasted effort.
--tim
On Tue, Apr 7, 2009 at 1:20 PM, Tal Liron tal.li...@threecrickets.com
wrote:
Ah, I can see your point now.
I would recommend, then, that a Context would have two separate
ConcurrentHashMaps, something like this:
1. Context.getConfiguration() -- this would be for the user
2. Context.getImplementationCache() -- this would be for things like
AnnotationUtils, and the user should not normally look at this, except
perhaps for debugging
-Tal
David Fogel wrote:
Hi Tal-
I think what you suggested about appending a classloader id (perhaps
System.identityHashCode(cl) ?) would work.
Regarding the Restlet Context and this cache being cruft: You say
the Context is a reasonable place for information that applies to the
Application, but this doesn't have annotation cache doesn't have
anything to do with my Application, it's related to a hidden
_implementation_ detail. As an end-user, we're not meant to be using
this annotion info cache directly. Yet the Context, as a very
prominent and unavoidable part of the Restlet public API is meant to
be used and understood. That's why I think it isn't a good place to
stash implementation-related data like this annotation stuff.
-Dave
--
http://restlet.tigris.org/ds/viewMessage.do?dsForumId=4447
http://restlet.tigris.org/ds/viewMessage.do?dsForumId=4447dsMessageId=1572
499 dsMessageId=1572499
--
http://restlet.tigris.org/ds/viewMessage.do?dsForumId=4447dsMessageId=1596661
Re: ServerResource is currently incompatible with OSGi
Hmm.
I disagree that this has nothing to do with the Application.
For example, if an Application is unloaded, this cached information
will remain in the static field. In fact, there is no mechanism in
place right now clean this cache, and in dynamic environments (possibly
OSGi) it may accumulate "cruft." Not a huge problem, but this is the
stuff memory leaks are made of...
Here's
a thought: can we use a synchronized WeakHashMap here? That way, the
AnnotationInfo would be discarded when the class is unloaded.
-Tal
Jerome Louvel wrote:
Hi all,
Beautiful! This new solution is available in SVN
trunk.
Thanks Dave for
the report and Tim for the clever fix!
Best regards,
Jerome Louvel
--
Restlet ~ Founder and Lead developer ~ http://www.restlet.org
Noelios Technologies ~ Co-founder~ http://www.noelios.com
De:
tpeie...@gmail.com [mailto:tpeie...@gmail.com] De la part de
Tim Peierls
Envoy: mercredi 8 avril 2009 00:23
: discuss@restlet.tigris.org
Objet: Re: ServerResource is currently incompatible with OSGi
Why involve Context at all? The AnnotationInfo associated with a
Class? extends UniformResource does not depend on Context. You
could just add a method to AnnotationUtils:
public static AnnotationInfo getAnnotationDescriptor(Class?
extends UniformResource resourceClass) {
AnnotationInfo result = cache.get(resourceClass);
if (result == null) {
result = computeAnnotationDescriptor(resourceClass); // use code
from existing getAnnotationDescriptors for this
AnnotationInfo prev = cache.putIfAbsent(resourceClass, result);
if (prev != null) result = prev;
}
return result;
}
private static final ConcurrentMapClass? extends
Resource, AnnotationInfo cache =
new ConcurrentHashMapClass? extends Resource,
AnnotationInfo();
Then you don't need getAnnotationDescriptors. You probably want
a way to clear cache entries if a class is reloaded, say.
You could get more fancy, with Futures and such, but I think
that would be wasted effort.
--tim
On Tue, Apr 7, 2009 at 1:20 PM, Tal Liron tal.li...@threecrickets.com
wrote:
Ah, I can see your point now.
I would recommend, then, that a Context would have two separate
ConcurrentHashMaps, something like this:
1. Context.getConfiguration() -- this would be for the user
2. Context.getImplementationCache() -- this would be for things like
AnnotationUtils, and the user should not normally look at this, except
perhaps for debugging
-Tal
David Fogel wrote:
Hi Tal-
I think what you suggested about appending a classloader id (perhaps
System.identityHashCode(cl) ?) would work.
Regarding the Restlet Context and this cache being "cruft": You say
the Context is a reasonable place for information that applies to the
Application, but this doesn't have annotation cache doesn't have
anything to do with my Application, it's related to a hidden
_implementation_ detail. As an end-user, we're not meant to be using
this annotion info cache directly. Yet the Context, as a very
prominent and unavoidable part of the Restlet public API is meant to
be used and understood. That's why I think it isn't a good place to
stash implementation-related data like this annotation stuff.
-Dave
--
http://restlet.tigris.org/ds/viewMessage.do?dsForumId=4447dsMessageId=1572499
Re: ServerResource is currently incompatible with OSGi
Hi Tal- I think what you suggested about appending a classloader id (perhaps System.identityHashCode(cl) ?) would work. Regarding the Restlet Context and this cache being cruft: You say the Context is a reasonable place for information that applies to the Application, but this doesn't have annotation cache doesn't have anything to do with my Application, it's related to a hidden _implementation_ detail. As an end-user, we're not meant to be using this annotion info cache directly. Yet the Context, as a very prominent and unavoidable part of the Restlet public API is meant to be used and understood. That's why I think it isn't a good place to stash implementation-related data like this annotation stuff. -Dave -- http://restlet.tigris.org/ds/viewMessage.do?dsForumId=4447dsMessageId=1572499
Re: ServerResource is currently incompatible with OSGi
There's a lot of precedent -- look at all the weird stuff that ends up in System.properties() on startup of some JVMs, or the values hanging out in ServletContext or PageContext in every JEE server I know. Doesn't mean it thrills me. Thanks for spotting this, David ... would have been a stopper for me as well. I agree Tal's short term solution should work with minimal changes to the implementation. On Tue, Apr 7, 2009 at 2:51 AM, David Fogel carrotsa...@gmail.com wrote: Yet the Context, as a very prominent and unavoidable part of the Restlet public API is meant to be used and understood. That's why I think it isn't a good place to stash implementation-related data like this annotation stuff. -- http://restlet.tigris.org/ds/viewMessage.do?dsForumId=4447dsMessageId=1576284
Re: ServerResource is currently incompatible with OSGi
Ah, I can see your point now. I would recommend, then, that a Context would have two separate ConcurrentHashMaps, something like this: 1. Context.getConfiguration() -- this would be for the user 2. Context.getImplementationCache() -- this would be for things like AnnotationUtils, and the user should not normally look at this, except perhaps for debugging -Tal David Fogel wrote: Hi Tal- I think what you suggested about appending a classloader id (perhaps System.identityHashCode(cl) ?) would work. Regarding the Restlet Context and this cache being "cruft": You say the Context is a reasonable place for information that applies to the Application, but this doesn't have annotation cache doesn't have anything to do with my Application, it's related to a hidden _implementation_ detail. As an end-user, we're not meant to be using this annotion info cache directly. Yet the Context, as a very prominent and unavoidable part of the Restlet public API is meant to be used and understood. That's why I think it isn't a good place to stash implementation-related data like this annotation stuff. -Dave -- http://restlet.tigris.org/ds/viewMessage.do?dsForumId=4447dsMessageId=1572499
Re: ServerResource is currently incompatible with OSGi
I've noticed this, too, but couldn't immediately think of an application where this would be a problem... thanks for bringing up OSGi. An immediate solution could be to append the class's ClassLoader instance into the key: org.restlet.resource.ServerResource.annotations.classloader instance ID.canonical class name here I generally think the application Context is the right place to cache anything that applies to the application. That's not cruft. However, this is a case in which the information being cached is not specific to the application, but rather to the loaded class instance itself. For example, we might have multiple Applications running in the same Component, which all use the same resource class. In the current implementation, the annotations would be computed and cached all over again for each loaded class, even though they come from the same ClassLoader. This is a tiny price to pay in redundancy, though, and only happens when the class is instantiated for the first time per application. Still, it offends my sense of order. :) I wish Java provided us with a context within the ClassLoader to store things like this, just as we can store things in the Thread context. Unfortunately, creating such a mechanism on our own is not so trivial, specifically because we would have to do it via classes loaded by ClassLoaders. :) It involves created a hierarchy of meta-ClassLoaders which exist in a higher context than the running application. There, you can place a ConcurrentMap of attributes per ClassLoader. There are applications that absolutely need this, and there are solutions around (for example, daemon wrappers such as Tanuki's Service Wrapper or YAJSW). If we're going to see a lot of this annotation stuff going on in Restlet, it may be worth looking into a more comprehensive solution to per-class-instance contexts. In the meantime, I think my suggested solution would work. -Tal David Fogel wrote: Hi Jerome, all- I've been experimenting with the new ServerResource, and I've found that in its current form it is incompatible with being deployed in an OSGi environment. The current implementation uses reflection to scan all the annotations on a particular ServerResource subclass. ServerResource code calls into the AnnotationUtils class to get this done. Presumably because this is an expensive operation that always returns the same result, this code caches the scanned annotation info, stuffing it into the Context's attribute map under the key "org.restlet.resource.ServerResource.annotations.canonical class name here". The problem with this is that in an OSGi context, the ServerResource subclass may be loaded from a different bundle than the Restlet Component (and it's root Context). This means that if the ServerResource subclass's bundle is refreshed (in OSGi this means that the bundle jar is reloaded, into a new classloader), it will have the SAME CANONICAL NAME, but will have a DIFFERENT Class instance, and DIFFERENT java.lang.reflect.Method instances. As a result, when an instance of the ServerResource subclass is next created to handle a request, it tries to call Method.invoke() on an instance of Method taken from the annotation info cache, but passes it an instance of the resource which is not the same type as the original Method object was created from. This results in the following Exception: java.lang.IllegalArgumentException: object is not an instance of declaring class While it seems reasonable to cache this annotation data, you must do it in a way that depends on actual Class instances, rather than CanonicalNames. This is currently a blocking issue for us with the new ServerResource, since we have an application architecture that makes use of this kind of reloading parts of a web service frequently. Also, in general it seems to me that the Context attributes map is not the best place for this data anyhow- the Context seems to be increasingly used as a general catch-all storage for implementation details, which seems crufty. -Dave Fogel -- http://restlet.tigris.org/ds/viewMessage.do?dsForumId=4447dsMessageId=1571498
