geronimo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Mark Struberg <strub...@yahoo.de>
Subject Re: PR Merge?
Date Fri, 19 Aug 2016 14:48:44 GMT
I’m with Clebert here.

The current implementation in json-1.0 is a manual java.util.ServiceLoader (for java5 backward
compat I guess).

But it does not do a loop and tries to find the ‚closest’ one, but just takes the first
service it finds.

Consider now you have 2 impls. The first in the tomcat/lib and the other in your web app.

There is no guarantee that you always get the one from your webapp. It’s just random, isn’t?

In that case we need to come up with a better approach anyway. OR we execute this only once
so the container can force it’s own impl.

LieGrue,
strub

PS: this seems to be a complex topic. Each spec does it different. And each of them has achileus
heals. E.g. the CDI.setCdiProvider :(


> Am 19.08.2016 um 15:44 schrieb Romain Manni-Bucau <rmannibucau@gmail.com>:
> 
> 
> 
> 2016-08-19 15:40 GMT+02:00 Clebert Suconic <clebert.suconic@gmail.com>:
> The patch is caching it with a soft cache. Meaning the impl will go away when memory
is needed and a class loader gone.  
> 
> 
> Leaking was not in term of memory but in term of impl (the API should enable you to change
the impl at each request if the classloader/app/bundle changed)
>  
> 
> The performance of having to load the class on every operation is really heavy.  
> 
> True but in an app you never do it multiple times using Json.xxxx but you rely on jsonXXXFactory
which is looked up only once. (that's how johnzon mapper is implemented for instance)
>  
> 
> 
> On Friday, August 19, 2016, Romain Manni-Bucau <rmannibucau@gmail.com> wrote:
> Hi John
> 
> I implement it this way to work in all cases and deployment without leaking an impl (this
patch does)
> 
> The fix is in artemis I suspect: Json.write/read shouldnt be used bit they should go
through the factory.
> 
> Hope it helps.
> 
> Le 19 août 2016 04:11, "John D. Ament" <johndament@apache.org> a écrit :
> Hi,
> 
> I was wondering, would anyone be able to review this PR and perhaps merge it?  After
I ported Artemis to use Johnzon, Clebert noticed some performance issues where the service
was being looked up always.  His PR is to help resolve that.
> 
> https://github.com/apache/geronimo-specs/pull/4
> 
> John
> 
> 
> -- 
> Clebert Suconic
> 
> 


Mime
View raw message