2016-08-19 16:48 GMT+02:00 Mark Struberg <[email protected]>: > 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. > > I'd would have loved to fail, it is just aligned on the RI behavior which is this one and we need to keep that
> 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? > > Depends the environment, in a plain tomcat yes but in other containers/contexts the classloader can ensure to force this hierarchy. Typically in TomEE we handle it for several spec. Using this cached instance would break it very very badly. > 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. > > Anyway guys you seems to ignore the point that the fix is trivial and there is no actual performance issue once correctly coded so not sure it is a real issue. > 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 <[email protected] > >: > > > > > > > > 2016-08-19 15:40 GMT+02:00 Clebert Suconic <[email protected]>: > > 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 <[email protected]> > 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" <[email protected]> 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 > > > > > >
