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