Le dim. 1 mars 2020 à 21:00, Mark Struberg <strub...@yahoo.de.invalid> a écrit :
> We do not leak if the ApplicationBoundary is setup correctly. > We do, start/stop/start. You leaked all proxies of the first run. And nowadays one almost always restarts the full container, not a single > webapp anyway. > The opposite since se api is there actually. It is not uncommon to reuse the jvm. Clearly not mainstream but exists. Also quite very common in tests - with ds, arquillian or evzn our own junit5 module - so we allocate too much mem (even if out of the heap these days, still affects CI pods). Not my immediate concern but shows a design issue or at least potential enhancement. > I dig the compile time part. > Thks, ping me if needed. One goal is to make geronimo arthur supporting owb. For ref started https://github.com/rmannibucau/geronimo-arthur/commit/5316053bdb866ac20b2c1f6a5f6f1c3995af4592 some months ago. > LieGrue, > strub > > > Am 01.03.2020 um 14:25 schrieb Romain Manni-Bucau <rmannibu...@gmail.com > >: > > > > Le dim. 1 mars 2020 à 13:36, Mark Struberg <strub...@yahoo.de.invalid> a > > écrit : > > > >>>> Am I missing something? > >> > >> Yes, the problem is imo the name. > >> What we might try is to have a long string where we concatenate all > >> criterias together, and then do a md5 over it. > >> That means there is a low chance of clashes but a high chance of the > being > >> able to re-create the same class name. > >> Although once we hit the field of CDI Extension modifications it is not > >> 100% anymore. > >> > > > > We can also do it like weld: a generic proxy for everything so single > proxy > > per class per classloader. Would be a bit less optimized but would be > > better if you start/stop containers in the same classloader, today we > leak > > until you configure the class unloading. > > > > > >> That was the reason why we bound the generated procy classes to their > >> beans. What's wrong with this approach? > >> Why don't you like that? > >> > > > > I didnt say I dont like that, I said it doesnt work some times. > > Point is we must today enable a new use case: build time proxy generation > > (2-3 sub use cases: 1. Graalvm/aot, 2. optimized startup/cloud (we > already > > have optims for other area) and 3. better support of future jvm where > > unsafe can be broken before we fix its replacement (intermediate built > time > > replacement of defineClass by generating proxies). > > > > High level, it is about scanning at build time somehow (can even be > > explicit class or beans listing but i expect a mvn plugin hooked through > > our metadata discovery), generate all proxy .class and be able to load > them > > by name instead of generate them. > > A trivial option can be to have a mapping file, bean#id -> proxyname > maybe? > > > > A spi plugged into abstractproxyfactory is fine but having something > built > > in sounds better since it is not really about changing the behavior but > > more being ~reentrant. > > > > Hope it is clearer. > > > > > > > >> LieGrue, > >> strub > >> > >>> Am 01.03.2020 um 08:16 schrieb Romain Manni-Bucau < > rmannibu...@gmail.com > >>> : > >>> > >>> Up, I'd like to get it - worse case with a spi - for next release to at > >>> least enable to become native even if not yet trivial. > >>> > >>> Romain Manni-Bucau > >>> @rmannibucau <https://twitter.com/rmannibucau> | Blog > >>> <https://rmannibucau.metawerx.net/> | Old Blog > >>> <http://rmannibucau.wordpress.com> | Github < > >> https://github.com/rmannibucau> | > >>> LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book > >>> < > >> > https://www.packtpub.com/application-development/java-ee-8-high-performance > >>> > >>> > >>> > >>> Le lun. 11 nov. 2019 à 20:40, Romain Manni-Bucau < > rmannibu...@gmail.com> > >> a > >>> écrit : > >>> > >>>> @Mark: so it means my proposal work, right? ;). Joke apart my proposal > >> was > >>>> not to have 1 proxy per type but 1 stable name per proxy without > >> changing > >>>> the number of classes and reuse the existing one if it already exists > in > >>>> the classloader. > >>>> The only part which can fail is this last one since you can start > twice > >> a > >>>> container in the same classloader and change the meta so the proxy but > >>>> since the proxy contains the intercepted method markers in its name > the > >>>> reuse is safe. > >>>> > >>>> So I think it is ok even if the naming convention can likely be > refined > >> a > >>>> lot, no? Am I missing something? > >>>> > >>>> Romain Manni-Bucau > >>>> @rmannibucau <https://twitter.com/rmannibucau> | Blog > >>>> <https://rmannibucau.metawerx.net/> | Old Blog > >>>> <http://rmannibucau.wordpress.com> | Github > >>>> <https://github.com/rmannibucau> | LinkedIn > >>>> <https://www.linkedin.com/in/rmannibucau> | Book > >>>> < > >> > https://www.packtpub.com/application-development/java-ee-8-high-performance > >>> > >>>> > >>>> > >>>> Le lun. 11 nov. 2019 à 20:28, Mark Struberg <strub...@yahoo.de.invalid > > > >> a > >>>> écrit : > >>>> > >>>>> Sure! > >>>>> > >>>>> Think about having an interface and 3 producer methods. Each with > >>>>> different proxy configuration. Means 3 different bytecode > >> optimisations for > >>>>> the same class. > >>>>> Or having an interface in an EAR and 4 WARs which have their own > impls. > >>>>> Still the proxy gets generated and loaded via the shared > >> EarClassLoader for > >>>>> class visibility reasons. > >>>>> > >>>>> So there are scenarios where you end up with n proxies - each with > >>>>> different bytecode - for the same class. > >>>>> > >>>>> LieGrue, > >>>>> strub > >>>>> > >>>>>> Am 11.11.2019 um 20:16 schrieb Romain Manni-Bucau < > >>>>> rmannibu...@gmail.com>: > >>>>>> > >>>>>> @Mark: not sure what you mean and which case you have in mind. Can > you > >>>>>> precise your answer please and what prevents my patch to work? > >>>>>> > >>>>>> Le lun. 11 nov. 2019 à 20:13, Mark Struberg > <strub...@yahoo.de.invalid > >>> > >>>>> a > >>>>>> écrit : > >>>>>> > >>>>>>> no we still need it imo > >>>>>>> > >>>>>>>> Am 09.11.2019 um 16:30 schrieb Romain Manni-Bucau < > >>>>> rmannibu...@gmail.com > >>>>>>>> : > >>>>>>>> > >>>>>>>> Hi everyone, > >>>>>>>> > >>>>>>>> I wonder if we can drop the loop > >>>>>>>> in > >>>>> > org.apache.webbeans.proxy.AbstractProxyFactory#getUnusedProxyClassName > >>>>>>>> to ensure we have stable names for proxies. > >>>>>>>> The rational behind that is to: > >>>>>>>> > >>>>>>>> 1. Stop creating the same class again and again when we > >>>>>>>> start/stop/restart/restop/... a container in the same classloader > >>>>>>>> 2. Ensure we have deterministic names for the same proxy > (currently, > >>>>> when > >>>>>>>> the name are conflicting, it depends what was executed before, > even > >> in > >>>>>>> the > >>>>>>>> same app!). > >>>>>>>> > >>>>>>>> From a quick review we only need to mark the intercepted methods > in > >>>>> the > >>>>>>>> proxy name to ensure we can reuse the existing class. Therefore a > >>>>> quick > >>>>>>>> proposal for the proxy name suffix is: > >>>>>>>> > >>>>>>>> + protected String getProxySuffix(final Method[] > >>>>> interceptedMethods, > >>>>>>>> + final Method[] > >>>>>>> nonInterceptedMethods) { > >>>>>>>> + return "__i_" + getMethodsId(interceptedMethods); > >>>>>>>> + } > >>>>>>>> + > >>>>>>>> + private String getMethodsId(final Method[] methods, final > >>>>>>>> Predicate<Method> filter) { > >>>>>>>> + return Stream.of(methods) > >>>>>>>> + .filter(filter) > >>>>>>>> + .map(m -> m.getName() + > >>>>>>>> + > >>>>>>>> > >>>>>>> > >>>>> > >> > Stream.of(m.getParameterTypes()).map(Class::getSimpleName).collect(joining())) > >>>>>>>> + .collect(joining("_")); > >>>>>>>> + } > >>>>>>>> > >>>>>>>> Full patch is available here: > >>>>>>>> > >> https://gist.github.com/rmannibucau/6f6cdf8d605387ac8820dcbc76e67909 > >>>>>>>> > >>>>>>>> Wdyt? > >>>>>>>> > >>>>>>>> If not desired, should I > >>>>>>>> enrich org.apache.webbeans.spi.DefiningClassService with that > >> feature? > >>>>>>>> > >>>>>>>> > >>>>>>>> Romain Manni-Bucau > >>>>>>>> @rmannibucau <https://twitter.com/rmannibucau> | Blog > >>>>>>>> <https://rmannibucau.metawerx.net/> | Old Blog > >>>>>>>> <http://rmannibucau.wordpress.com> | Github < > >>>>>>> https://github.com/rmannibucau> | > >>>>>>>> LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book > >>>>>>>> < > >>>>>>> > >>>>> > >> > https://www.packtpub.com/application-development/java-ee-8-high-performance > >>>>>>>> > >>>>>>> > >>>>>>> > >>>>> > >>>>> > >> > >> > >
