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
> >>>>>>
> >>>>>>
> >>>>
> >>>>
> >>
> >>
>
>

Reply via email to