Le dim. 19 juil. 2020 à 23:25, Mark Struberg <strub...@yahoo.de.invalid> a écrit :
> > > > Am 19.07.2020 um 06:42 schrieb Romain Manni-Bucau <rmannibu...@gmail.com > >: > > > > Le dim. 19 juil. 2020 à 00:47, Mark Struberg <strub...@yahoo.de.invalid> > a > > écrit : > > > >> Yes, it seems that the recursive call is just from using an openjpa > class > >> when the first javaagent transformer starts it's work. > >> That means before any openjpa code is actually used. This _could_ be > >> prevented if the premain code in the agent would touch all the necessary > >> classes. But of course this is error prone as well. > >> > > > > Not in EE case where enhancer classes will be loaded by the transformer > and > > never the agent. > > Also can not be with a custom openjpa spi loading classes (if we exclude > > this case, im for it, we should write it in our doc somewhere probably > just > > to not forget again). > > That's just a question where we place it. > > > > > >> For any entity etc it will not be called recursively. The > >> java.lang.instrument.ClassFileTransformer#transform method just returns > a > >> byte[] with the new bytecode. But it does not interpret that bytecode > >> during transformation. Thus no recursive call. > >> > > > > Well it can with asm but using tmploader so until you do like me in > > embedded case (or optim in tomee), apploader=tmploader + cds, to save a > lot > > of time, then it works. > > > > We could use a ThreadLocal<AtomicBoolean> and even set it to null after > the > >> first run. Because once openjpa is initted we should not see any > recursive > >> invocations anymore. Something like that. > >> > > > > Also means we dont need that and can just exclude openjpa stack as i > > proposed whicj makes the method "pure" which is saner and like as fast in > > general case (in terms of complexity). > > So sounds saner. > > This is the same problem like with the pre-init: which classes exactly? > The one of the PR? ;) Actually a bit less is needed but the pr is done in a way which does not hurt perfs and avoids to leak a state between calls which is very nasty (as proven by this thread) and uncontrollable - strictly speaking names test should happen before the threadlocal, then next test etc....blacklisting avoids it cleanly. We can refine thz classes graph if needed but packages are more than enough. > In your case we have to exclude that classes from transformation. > In the other case we load those classes before the PCClassFileTransformer > gets installed. > Both cases have the downside that you need to exactly know which classes > will get loaded. > > After going through all the pros and cons I am in favour for the > ThreadLocal. > This could really already also hit any Tomcat installation which enables > parallel war loading and has openjpa in the shared classloader, right? > Even a plain standalone app. > LieGrue, > strub > > > > > > > >> LieGrue, > >> strub > >> > >>> Am 18.07.2020 um 18:18 schrieb Romain Manni-Bucau < > rmannibu...@gmail.com > >>> : > >>> > >>> Oh just got it, you need to extend openjpa (MDR for ex) otherwise it > does > >>> only happen if openjpa is not loaded during the enhancement, which > means > >>> the javaagent is there, the entity is loaded but the EMF not yet there, > >> so > >>> 2 cases (even if one is unlikely but can happen due to our spi). > >>> > >>> 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 sam. 18 juil. 2020 à 18:07, Mark Struberg <strub...@yahoo.de.invalid > > > >> a > >>> écrit : > >>> > >>>> no, didn't work. > >>>> > >>>> LieGrue, > >>>> strub > >>>> > >>>> > >>>>> Am 18.07.2020 um 18:00 schrieb Romain Manni-Bucau < > >> rmannibu...@gmail.com > >>>>> : > >>>>> > >>>>> Not sure I got the question but I suspect the case can be reproduced > >>>> with a > >>>>> plain new MyEntity() before your Persistence.createEMF. > >>>>> > >>>>> 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 sam. 18 juil. 2020 à 17:57, Mark Struberg > <strub...@yahoo.de.invalid > >>> > >>>> a > >>>>> écrit : > >>>>> > >>>>>> Found it yesterday when reviewing my sample with Romain: > >>>>>> > >>>>>> I apparently did copy the following property over from another > >> example: > >>>>>> <property name="openjpa.InitializeEagerly" value="true"/> > >>>>>> And this did lead to already trigger the class transformation while > >>>>>> createEntityManagerFactory. > >>>>>> > >>>>>> I did create a few samples, but did not manage to trigger any > >> subsequent > >>>>>> classloading at all. So no wonder in which case this could hit us > yet. > >>>>>> Before we introduce another expensive mechanism I would love to > >>>> understand > >>>>>> when the situation occurs at all. > >>>>>> > >>>>>> LieGrue, > >>>>>> strub > >>>>>> > >>>>>> > >>>>>> > >>>>>> > >>>>>> > >>>>>>> Am 17.07.2020 um 12:03 schrieb Romain Manni-Bucau < > >>>> rmannibu...@gmail.com > >>>>>>> : > >>>>>>> > >>>>>>> Le ven. 17 juil. 2020 à 11:27, Mark Struberg > >> <strub...@yahoo.de.invalid > >>>>>> <mailto:strub...@yahoo.de.invalid>> a > >>>>>>> écrit : > >>>>>>> > >>>>>>>> answers inside. > >>>>>>>> > >>>>>>>> LieGrue, > >>>>>>>> strub > >>>>>>>> > >>>>>>>>> Am 17.07.2020 um 11:14 schrieb Romain Manni-Bucau < > >>>>>> rmannibu...@gmail.com > >>>>>>>>> : > >>>>>>>>> > >>>>>>>>> Le ven. 17 juil. 2020 à 10:45, Mark Struberg > >>>> <strub...@yahoo.de.invalid > >>>>>>> > >>>>>>>> a > >>>>>>>>> écrit : > >>>>>>>>> > >>>>>>>>>> Both prepareUnenhancedClasses and loadAgent already run in > >>>>>>>>>> createEntityManagerFactory. > >>>>>>>>>> > >>>>>>>>> > >>>>>>>>> Not in general, depends the conf and by default i run in none of > >> both > >>>>>>>> with > >>>>>>>>> a standard config for ex for EE. > >>>>>>>> > >>>>>>>> Let me rephrase this: both those methods will get called in > >>>>>>>> createEntityManagerFactory already. If they actually perform some > >>>>>> bytecode > >>>>>>>> enhancement is up to configuration and whether the classes have > been > >>>>>>>> enhanced otherwise before ;) > >>>>>>>> > >>>>>>>> > >>>>>>>>> > >>>>>>>>> > >>>>>>>>>> crateEntityManager does not trigger enhancement afaict. > >>>>>>>>>> > >>>>>>>>> > >>>>>>>>> createEMF does not since the creation of openjpa is lazy so the > >> first > >>>>>>>>> createEM does (in general case one again). > >>>>>>>> > >>>>>>>> Nope, it really gets called already by createEntityManagerFactory. > >> You > >>>>>> can > >>>>>>>> debug into this if you want. > >>>>>>>> > >>>>>>> > >>>>>>> Did and I can guarantee you it is lazy if your conf does not > enforce > >> it > >>>>>>> somehow. > >>>>>>> > >>>>>>> > >>>>>>>> > >>>>>>>> LieGrue, > >>>>>>>> strub > >>>>>>>> > >>>>>>>> > >>>>>>>>> > >>>>>>>>>> > >>>>>>>>>> LieGrue, > >>>>>>>>>> strub > >>>>>>>>>> > >>>>>>>>>>> Am 17.07.2020 um 10:11 schrieb Romain Manni-Bucau < > >>>>>>>> rmannibu...@gmail.com > >>>>>>>>>>> : > >>>>>>>>>>> > >>>>>>>>>>> Hi Mark, > >>>>>>>>>>> > >>>>>>>>>>> If it helps: > >>>>>>>>>>> > >>>>>>>>>>> 1. > >>>>>> org.apache.openjpa.kernel.AbstractBrokerFactory#postCreationCallback > >>>>>>>>>>> only triggers the prepareUnenhancedClasses if preload=true > (never > >>>> the > >>>>>>>>>> case > >>>>>>>>>>> right? ;)) > >>>>>>>>>>> 2. Normally classes are loaded in > >>>>>>>>>>> there > >>>>>> org.apache.openjpa.kernel.AbstractBrokerFactory#initializeBroker > >>>>>>>>>>> *when creating an em, not an emf* so better to have loaded ran > >> the > >>>>>>>>>>> enhancement early (loadAgent) > >>>>>>>>>>> 3. Not sure it is done but it can happen 2 has a custom MDR not > >>>>>>>> returning > >>>>>>>>>>> right the classes - seems code is ok with that. I don't know if > >> it > >>>>>> is a > >>>>>>>>>>> case we want to handle, I don't think it is needed. > >>>>>>>>>>> 4. There is another case you didn't list where the EMF can be > >> there > >>>>>> but > >>>>>>>>>> no > >>>>>>>>>>> EM and enhancement should happen ASAP: deserialization ;) > >>>>>>>>>>> 5. Finally there is a subtle difference between SE and EE case > >>>> where > >>>>>>>> one > >>>>>>>>>>> will trigger the agent and the other will just add the > >> transformer > >>>> in > >>>>>>>> the > >>>>>>>>>>> classloader through PersistenUnitInfo (and container should > >> handle > >>>>>> this > >>>>>>>>>>> additional properly), I don't know if our agent should be > >>>> compatible > >>>>>>>> with > >>>>>>>>>>> it for apploader (jvm one). > >>>>>>>>>>> > >>>>>>>>>>> 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 ven. 17 juil. 2020 à 09:35, Mark Struberg > >>>>>> <strub...@yahoo.de.invalid > >>>>>>>>> > >>>>>>>>>> a > >>>>>>>>>>> écrit : > >>>>>>>>>>> > >>>>>>>>>>>> hi folks! > >>>>>>>>>>>> > >>>>>>>>>>>> I'm right now cleaning up old code in our enhancer. Like > >> removing > >>>>>>>> java5 > >>>>>>>>>>>> hacks, etc > >>>>>>>>>>>> This is also triggered by our recent discussion about > _transform > >>>> and > >>>>>>>>>> other > >>>>>>>>>>>> pieces. > >>>>>>>>>>>> Slowly getting my head into the right space for it. This is > way > >>>> more > >>>>>>>>>>>> complex than I remembered ;) > >>>>>>>>>>>> > >>>>>>>>>>>> So here is my next puzzle: > >>>>>>>>>>>> > >>>>>>>>>>>> I've created a new example with 2 classes: Person and Employee > >>>>>> extends > >>>>>>>>>>>> Person > >>>>>>>>>>>> Plus a persistence.xml with enlisting those 2 classes (makes a > >>>>>>>>>>>> difference!) and essentially > >>>>>>>>>>>> <property name="openjpa.DynamicEnhancementAgent" > value="true"/> > >>>>>>>>>>>> <property name="openjpa.RuntimeUnenhancedClasses" > >>>>>> value="supported"/> > >>>>>>>>>>>> The reason I switched on RuntimeUnenhancedClasses is that this > >>>> check > >>>>>>>> is > >>>>>>>>>>>> performed when the BrokerFactory gets created. > >>>>>>>>>>>> And if the classes are not build-time enhanced, then it fails > in > >>>>>>>>>>>> > >>>>>>>>>> > >>>>>>>> > >>>>>> > >>>> > >> > org.apache.openjpa.enhance.ManagedClassSubclasser#prepareUnenhancedClasses > >>>>>>>>>>>> if (conf.getRuntimeUnenhancedClassesConstant() != > >>>>>>>>>>>> RuntimeUnenhancedClassesModes.SUPPORTED) { .. > >>>>>>>>>>>> And if you switch to supported, then it will create a new > >> subclass > >>>>>> and > >>>>>>>>>>>> replaces field access to the getters/setters. Sadly > >>>>>>>>>>>> java.lang.instrument.Instrumentation#retransform cannot be > used > >> to > >>>>>> add > >>>>>>>>>> new > >>>>>>>>>>>> fields or methods. Otherwise we could do a direct full > >> replacement > >>>>>> of > >>>>>>>>>> this > >>>>>>>>>>>> class. > >>>>>>>>>>>> > >>>>>>>>>>>> Fun thing is that later on in > >>>>>>>>>>>> > >>>>>>>>>> > >>>>>>>> > >>>>>> > >>>> > >> > org.apache.openjpa.persistence.PersistenceProviderImpl#createEntityManagerFactory(java.lang.String, > >>>>>>>>>>>> java.lang.String, java.util.Map) > >>>>>>>>>>>> we call loadAgent(factory); > >>>>>>>>>>>> But what is it supposed to do? Which cases does it serve? > >>>>>>>>>>>> If the classes did not have been already enhanced at build > time > >>>> nor > >>>>>>>> via > >>>>>>>>>>>> javaagent they will now be subclassed by > ManagedClassSubclasser. > >>>>>>>>>>>> And if we do not set RuntimeUnenhancedClasses=supported then > we > >>>> will > >>>>>>>>>> never > >>>>>>>>>>>> make it so far but blow up way earlier. > >>>>>>>>>>>> > >>>>>>>>>>>> Also: it is nice to have that agent attached. But why? All the > >>>>>> entity > >>>>>>>>>>>> classe already have been loaded already during BrokerFactory > >>>>>> startup. > >>>>>>>>>> There > >>>>>>>>>>>> is no more entity class left over which needs enhancement, > >> isn't? > >>>>>>>>>>>> > >>>>>>>>>>>> This is the log I get which reflects what I mean. Happy to > share > >>>> the > >>>>>>>>>> code > >>>>>>>>>>>> in a playground branch. > >>>>>>>>>>>> > >>>>>>>>>>>> 1261 test-classtransform INFO [main] openjpa.Enhance - > >>>> Creating > >>>>>>>>>>>> subclass and redefining methods for "[class > >>>>>>>>>>>> org.apache.openjpa.examples.dynenhance.Employee, class > >>>>>>>>>>>> org.apache.openjpa.examples.dynenhance.Person]". This means > that > >>>>>> your > >>>>>>>>>>>> application will be less efficient than it would if you ran > the > >>>>>>>> OpenJPA > >>>>>>>>>>>> enhancer. > >>>>>>>>>>>> 1412 test-classtransform INFO [main] openjpa.Runtime - > >> OpenJPA > >>>>>>>>>>>> dynamically loaded the class enhancer. Any classes that were > not > >>>>>>>>>> enhanced > >>>>>>>>>>>> at build time will be enhanced when they are loaded by the > JVM. > >>>>>>>>>>>> > >>>>>>>>>>>> > >>>>>>>>>>>> LieGrue, > >>>>>>>>>>>> strub > >>>>>> > >>>>>> > >>>> > >>>> > >> > >> > >
