[hibernate-dev] Bytecode enhancement

[Steve Ebersole]

Well it does not keep track of EntityEntry specifically.  It keeps 
track of the replacements.  But that is the point.  We have quite a few 
"context caches" already.

PersistenceContext was intended for this lookup.  It just suffers from 
a bad implementation of how that works atm.    If I understand what you 
are suggesting (though to be honest, its extremely vague, at least to 
me) then whole point is to no longer do entity->EntityEntry resolutions 
via PersistenceContext (through this IdentityMap).  But thats actually 
the same net effect as bytecode enhancement.  On 
StatefulPersistenceContext:

public EntityEntry getEntityEntry(Object entity) {
    if ( isEnhanced( entity ) ) {
        return ( (Enhanced) entity ).getEntityEntry();
    }
    else {
        // this is what hotspots
        return entityEntries.get( entity );
   }
}

On Tue 09 Oct 2012 11:08:42 AM CDT, Emmanuel Bernard wrote:
> On Tue 2012-10-09 10:30, Steve Ebersole wrote:
>> On Tue 09 Oct 2012 09:57:12 AM CDT, Emmanuel Bernard wrote:
>>> On Thu 2012-10-04 10:00, Steve Ebersole wrote:
>>>> See https://hibernate.onjira.com/browse/HHH-7667
>>>>
>>>> I want to investigate expanding Hibernate's support for bytecode
>>>> enhancement.  I can see 3 main fronts to this:
>>>>
>>>> 1) offloading map lookups from the PersistenceContext .  Currently we
>>>> keep state associated with each entity in a map (a special type of map
>>>> called IdentityMap) on the PersistenceContext, keyed by the entity.
>>>> Profiling consistently shows these maps as a hot-spot.  The main thing
>>>> we keep in the PersistenceContext in regards to the entity is called an
>>>> EntityEntry, which is kept in an IdentityMap<Object,EntityEntry> (again,
>>>> the entity is the key).  So the thought here is to invert this from
>>>> `map.get( entity )` to something like `( (Enhanced) entity
>>>> ).getEntityEntry()`
>>>
>>> I was discussing this with Sanne. One "workaround" to the heavy
>>> IdentityMap price is to keep a contextual cache. I am pretty sure these
>>> IndeitityMap lookups are done over and over for a given operation.
>>>
>>> We might be able to:
>>>
>>> - pass along the EntityEntry instead of relying on the lookup several
>>>    times
>>> - use a cache of lookups for either the most recent or for the
>>>    contextual operation.
>>>
>>> The cache would be array based and thus not involve hashcode or lookup.
>>>
>>> for ( int index = 0 ; index < cache.length ; index++ ) {
>>>    if ( searchedO == cache[index] ) {
>>>      entityEntry = entityEntryCache[index];
>>>    }
>>> }
>>> if ( entityEntry == null ) {
>>>    identityMap.get(searchedO);
>>>    //add to cache
>>> }
>>>
>>> Alternatively, I don't remember what is costly, the actual lookup or the
>>> computation of the identity hashcode.
>>> If that's the former, we can take the bet and use the entity hashcode
>>> instead but still use == instead of equals. The risk is more collisions
>>> or in case of a rogue implementation of hashCode not finding the entity.
>>>
>>> These options seems more lightweight than relying on bytecode
>>> enhancement to keep a placeholder.
>>
>> Well technically the hotspot is the calculation of the hashcode.
>> However, the same is true no matter how we calculate the hashcode.
>> Both Object#hashCode and System#identityHashCode have the same
>> characteristic (in fact Object#hashCode simply calls
>> System#identityHashCode, as best we can tell since they are both
>> defined as native calls).  However, the fact that this call stack
>> shows up as a hotspot at all is more due to the poor choice of Map
>> impl here and then the fact that we do lots of lookups.  What
>> happens is that we wrap the keys (the entity instances) in a wrapper
>> object that applies the "hashCode as System#identityHashCode" and
>> "equality as ==" semantics.  They problem with the number of calls
>> then is the number of times we do lookups and the fact that we need
>> to wrap the incoming keys and call identityHashCode to be able to do
>> a map lookup.
>>
>> I do not see how the answer can be "contextual" caches, beyond the
>> set of contextual caches we leverage already (see "merge map", etc).
>
> I am not familiar with this part of the code but if merge maps are used
> even beyond the merge operations and if the merge map walks through an
> underlying array of data without needing the hashcode (ie not map.get()) then you are
> correct, you already have a cache system to prevent hashCode
> computation and my assumption that it would reduce the pressure on
> IdentityMap is not correct.

--
steve at hibernate.org
http://hibernate.org