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.

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

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