You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@openjpa.apache.org by Patrick Linskey <pl...@gmail.com> on 2007/07/20 20:42:03 UTC

unenhanced class support

Hi,

Over the last week, I've been working on making OpenJPA work with
unenhanced classes in a variety of different environments. My goal has
been to make the enhancement step (either at build time or via the
javaagent) optional in order to remove a barrier to entry for
newcomers to OpenJPA. There are still reasons why enhancement is a
good thing to do in a running OpenJPA system, but my hope is that we
can make these be mostly quality-of-service issues, and not
functionality issues.

I've been pretty successful so far, but there are a few limitations in
some environments. I'd like to get some feedback about what to do
about these limitations. First, I'll describe my approach:

I started looking into this when Marc told me about the new
retransformation capabilities in Java 6. (Briefly, in Sun's Java 6
impl, it is no longer necessary to specify a javaagent flag in order
to retransform classes.) This led me to investigate retransforming
entity types to add field-tracking code to method blocks without
violating any of the retransformation rules. I combined this with a
subclassing approach so that instances created by OpenJPA (vs. created
by the user with 'new') can be treated as proper persistence-capable
instances.

Since retransformation does not allow fields, methods, or interfaces
to be added to a class, I added a helper class that maintains a static
map of *all* the instances created with 'new' that have been
persisted; this map associates a managed instance with a
ReflectingPersistenceCapable instance, which implements
PersistenceCapable via reflection, as the name implies, and can wrap
unenhanced managed instances for OpenJPA's needs.

So, the only functionality cost in a Java 6 environment is the extra
overhead of occasionally looking up the PersistenceCapable for a given
unenhanced managed instance. There is still a bit of a performance
cost, since I needed to do reflection in some places instead of direct
field access because of details of the class hierarchy and field
access restrictions.

In a Java 5 environment, if you specify a new javaagent class, then
the situation is exactly the same. IMO this improves on our current
javaagent in that it releases the restriction that persistent types
must be listed in the first persistence unit in persistence.xml, and
because the redefinition logic is never invoked against any
non-managed types.

If you do not specify a javaagent class in Java 5, then OpenJPA cannot
do redefinition. However, it can still create subclasses with logic in
overridden setters and getters. So, in Java 5 without a javaagent but
with property access, we only lose dirty tracking and lazy loading for
instances created by  the user code. This only matters for cases where
an instance so created is flushed (for dirty tracking) or cleared (for
lazy loading).

In Java 5 without a javaagent and with field access, things are a bit
worse -- OpenJPA can't do any dirty tracking or lazy loading, either
for instances created by the user code or for instances created by
OpenJPA.

We could improve the situation a bit for field access and post-flush
property access in Java 5 with no javaagent. This would require state
comparison, though, which is not the fastest thing in the world, and
requires that we hold hard references to all applicable instances for
the duration of the EM's life (all user-created property access
instances, and all field access instances regardless of how they were
created).


So, I'm looking for answers to the following questions in particular:

1. what should we do about { Java 5, no javaagent, field access }?
Should we support this configuration, including the corresponding
extra overhead, or should we require either property access or a
javaagent specified in these configurations?

2. what should we do about { Java 5, no javaagent, property access,
flushed | cleared instances }? There is a much lower impact to doing
the dirty tracking in these configurations, since the scope is
narrower. However, we might also be able to just not allow flush or
clear or multiple sequential transactions if the persistence context
has references to unenhanced, unredefined user-created instances.

Thoughts?

(Oh, and the code is still a work in progress -- everything I
discussed above is working, but serialization and cloning and
detachment are untested, and will probably require some tweaks to get
working correctly.)

-Patrick

-- 
Patrick Linskey
202 669 5907

Re: unenhanced class support

Posted by Michael Dick <mi...@gmail.com>.
It's a good feature, I don't have a problem committing the changes so far.

It'll make it easier / more likely to get some testing too :-).

If we're voting this is a +1.

-Mike

On 7/25/07, Craig L Russell <Cr...@sun.com> wrote:
>
> +1 to commit it if it works "at all".
>
> Failing the TCK with unenhanced classes is not a big issue for me.
> There's a pretty big cost to keeping large (> 50 LOC) changes and
> having to synchronize commits with the trunk. And it appears like
> we're committed to this direction.
>
> Craig
>
> On Jul 25, 2007, at 8:05 PM, Marc Prud'hommeaux wrote:
>
> >>> > 1. One of the big TODOs seems to be support for compound
> >>> primary keys
> >>> > (e.g., implementing
> >>> > ReflectingPersistenceCapable.pcCopyKeyFieldsToObjectId()).
> >>>
> >>> Actually, I think that things might work as-is with compound PKs. We
> >>
> >> Nope, it fails.
> >>
> >> Ok. So the TCK does not pass with my patch and without
> >> enhancement. My
> >> inclination is to get it committed so that it's in there, and then we
> >> can work on the TCK failures in parallel. If others agree with this
> >> strategy, I'll commit it probably sometime tomorrow. I'll make sure
> >> that whatever I commit passes the TCK with enhancement on, so there
> >> won't be any regression for people using enhancement. Thoughts?
> >>
> >> -Patrick
> >>
>
> Craig Russell
> Architect, Sun Java Enterprise System http://java.sun.com/products/jdo
> 408 276-5638 mailto:Craig.Russell@sun.com
> P.S. A good JDO? O, Gasp!
>
>
>

Re: unenhanced class support

Posted by Craig L Russell <Cr...@Sun.COM>.
+1 to commit it if it works "at all".

Failing the TCK with unenhanced classes is not a big issue for me.  
There's a pretty big cost to keeping large (> 50 LOC) changes and  
having to synchronize commits with the trunk. And it appears like  
we're committed to this direction.

Craig

On Jul 25, 2007, at 8:05 PM, Marc Prud'hommeaux wrote:

>>> > 1. One of the big TODOs seems to be support for compound  
>>> primary keys
>>> > (e.g., implementing
>>> > ReflectingPersistenceCapable.pcCopyKeyFieldsToObjectId()).
>>>
>>> Actually, I think that things might work as-is with compound PKs. We
>>
>> Nope, it fails.
>>
>> Ok. So the TCK does not pass with my patch and without  
>> enhancement. My
>> inclination is to get it committed so that it's in there, and then we
>> can work on the TCK failures in parallel. If others agree with this
>> strategy, I'll commit it probably sometime tomorrow. I'll make sure
>> that whatever I commit passes the TCK with enhancement on, so there
>> won't be any regression for people using enhancement. Thoughts?
>>
>> -Patrick
>>

Craig Russell
Architect, Sun Java Enterprise System http://java.sun.com/products/jdo
408 276-5638 mailto:Craig.Russell@sun.com
P.S. A good JDO? O, Gasp!


Re: unenhanced class support

Posted by Marc Prud'hommeaux <mp...@apache.org>.
I'm in favor of committing it. If others have any reservations,  
perhaps we should turn it into a full-blown vote.



On Jul 25, 2007, at 7:18 PM, Patrick Linskey wrote:

>> > 1. One of the big TODOs seems to be support for compound primary  
>> keys
>> > (e.g., implementing
>> > ReflectingPersistenceCapable.pcCopyKeyFieldsToObjectId()).
>>
>> Actually, I think that things might work as-is with compound PKs. We
>
> Nope, it fails.
>
> Ok. So the TCK does not pass with my patch and without enhancement. My
> inclination is to get it committed so that it's in there, and then we
> can work on the TCK failures in parallel. If others agree with this
> strategy, I'll commit it probably sometime tomorrow. I'll make sure
> that whatever I commit passes the TCK with enhancement on, so there
> won't be any regression for people using enhancement. Thoughts?
>
> -Patrick
>
> On 7/25/07, Patrick Linskey <pl...@gmail.com> wrote:
>> > 1. One of the big TODOs seems to be support for compound primary  
>> keys
>> > (e.g., implementing
>> > ReflectingPersistenceCapable.pcCopyKeyFieldsToObjectId()).
>>
>> Actually, I think that things might work as-is with compound PKs. We
>> definitely need to test, but the ReflectingPC methods only get called
>> in certain contexts, and some of the methods in it are never used,
>> since the subclass versions are used instead. (Subclass instances are
>> put into PCRegistry; ReflectingPC is only used to control instances
>> created by the user.)
>>
>> > 2. Should ImplHelper._unenhancedInstanceMap be some sort of
>> > IdentityMap? If the user overrides MyEntity.equals() to return true
>> > if, say, their names are the same, we could get into trouble by  
>> doing
>> > just a simple map lookup.
>>
>> Nice catch.
>>
>> > 5. I don't completely understand what is happening with the
>> > subclassing and enhancement. When we are redefining the classes, do
>> > we change the methods internally to do something like change String
>> > getName() { return name; } to String getName()
>> > { ((MyGeneratedSubclass) this).pcProvideField(nameFieldIndex); }? I
>> > don't see any issue with doing this, but if it does turn out to  
>> be a
>> > problem, we could alternately have some lookup that does String
>> > getName() { ImplHelper.getPCProxyFor(this).pcProvideField
>> > (nameFieldIndex); }
>>
>> Basically, the subclasses are used whenever OpenJPA loads a type.  
>> What
>> it does depends on a bunch of factors, but the short story is thus:
>>
>> 1. if available, we redefine user-defined methods to call
>> RedefinitionHelper.accessingField() or
>> RedefinitionHelper.settingField(), either using field-access or
>> property-access rules as appropriate.
>>
>> 2. the subclass basically just implements PC, and does some work for
>> serialization and cloning.
>>
>> So the subclasses don't really have proxies -- they actually  
>> implement
>> PersistenceCapable. The only place where we do any proxying /
>> delegating is in ReflectingPC, to support instances created by the
>> user.
>>
>> In both reflecting and redefinition / subclassing, there's a lot of
>> delegating to ImplHelper.getPC(), which will do the map lookup if
>> necessary, but is optimized to support subclass instances, which
>> implement PC, so don't need entries in the map.
>>
>> So, in a field-access redefinition world, the only cost is that
>> associated with maintaining the map of instance => PC for
>> newly-created instances.
>>
>> In a property-access redefinition world, some of the PC methods have
>> additional costs, since they need to reflect to access fields
>> directly, since we've inserted tracking code directly into the
>> superclass. So, some of the PC-related operations will end up using
>> reflection. Additionally, we require that property-access redefined
>> types are implemented with a single backing field per setter/getter
>> pair. We auto-detect the name of the backing field (assuming
>> relatively simple bytecode in the setter / getter pair), so that
>> shouldn't be that burdensome for normal usage. But it does mean that
>> you can't do prop access + redefinition + store everything in a Map
>> internally.
>>
>> -Patrick
>>
>> On 7/25/07, Marc Prud'hommeaux <mp...@apache.org> wrote:
>> > Patrick-
>> >
>> > It looks very good to me! A few notes/questions on the patch:
>> >
>> > 1. One of the big TODOs seems to be support for compound primary  
>> keys
>> > (e.g., implementing
>> > ReflectingPersistenceCapable.pcCopyKeyFieldsToObjectId()).
>> >
>> > 2. Should ImplHelper._unenhancedInstanceMap be some sort of
>> > IdentityMap? If the user overrides MyEntity.equals() to return true
>> > if, say, their names are the same, we could get into trouble by  
>> doing
>> > just a simple map lookup.
>> >
>> > 3. It's pretty nice that we only need 54 invocations of
>> > ImplHelper.toPersistenceCapable().
>> >
>> > 4. InstrumentationFactory is scary with all the implementation
>> > dependent assumptions it makes, and the creation of temporary files
>> > and whatnot. If anyone knows of any way to cheat and obtain an
>> > InstrumentationFactory without having to create a jar file with the
>> > appropriately-formatted manifest, it'd be great.
>> >
>> > 5. I don't completely understand what is happening with the
>> > subclassing and enhancement. When we are redefining the classes, do
>> > we change the methods internally to do something like change String
>> > getName() { return name; } to String getName()
>> > { ((MyGeneratedSubclass) this).pcProvideField(nameFieldIndex); }? I
>> > don't see any issue with doing this, but if it does turn out to  
>> be a
>> > problem, we could alternately have some lookup that does String
>> > getName() { ImplHelper.getPCProxyFor(this).pcProvideField
>> > (nameFieldIndex); }
>> >
>> >
>> > Overall, it looks very good! I'm very excited to be able to start
>> > working towards eliminating the enhancement steps that people have
>> > found so cumbersome in the past.
>> >
>> >
>> >
>> > On Jul 24, 2007, at 4:15 PM, Patrick Linskey wrote:
>> >
>> > > Hi,
>> > >
>> > > I attached my current patch to OPENJPA-293. There are a number  
>> of open
>> > > issues in the patch marked by '#####' marks -- I'd appreciate  
>> another
>> > > set of eyes on those items in particular, and on the patch in  
>> general.
>> > >
>> > > Aside from that, I'm planning to run the CTS without  
>> enhancement on at
>> > > some point in the next day or so, and want to get these changes
>> > > committed soon, so that we can start figuring out whether or  
>> not it
>> > > works.
>> > >
>> > > -Patrick
>> > >
>> > > On 7/22/07, Craig L Russell <Cr...@sun.com> wrote:
>> > >> Hi Patrick,
>> > >>
>> > >> On Jul 22, 2007, at 12:51 PM, Patrick Linskey wrote:
>> > >>
>> > >> >> I think that no other implementation will have much of a  
>> better
>> > >> >> solution. So I don't see that we should try to exclude user
>> > >> options
>> > >> >> or a possible solution just because it's a poor performer.
>> > >> >
>> > >> > What about eviction? My feeling is that wherever OpenJPA would
>> > >> > normally clear state (eviction, certain state transitions), we
>> > >> should
>> > >> > keep the state available instead when we can't intercept and
>> > >> reload on
>> > >> > demand.
>> > >>
>> > >> Yes.
>> > >>
>> > >> Craig
>> > >> >
>> > >> > -Patrick
>> > >> >
>> > >> > On 7/21/07, Craig L Russell <Cr...@sun.com> wrote:
>> > >> >>
>> > >> >> On Jul 20, 2007, at 11:42 AM, Patrick Linskey wrote:
>> > >> >>
>> > >> >> > So, I'm looking for answers to the following questions in
>> > >> >> particular:
>> > >> >> >
>> > >> >> > 1. what should we do about { Java 5, no javaagent, field
>> > >> access }?
>> > >> >> > Should we support this configuration, including the
>> > >> corresponding
>> > >> >> > extra overhead, or should we require either property  
>> access or a
>> > >> >> > javaagent specified in these configurations?
>> > >> >>
>> > >> >> I think we should do EAGER fetching of fields just like  
>> the other
>> > >> >> implementations have to do.
>> > >> >> >
>> > >> >> > 2. what should we do about { Java 5, no javaagent, property
>> > >> access,
>> > >> >> > flushed | cleared instances }? There is a much lower  
>> impact to
>> > >> >> doing
>> > >> >> > the dirty tracking in these configurations, since the  
>> scope is
>> > >> >> > narrower. However, we might also be able to just not allow
>> > >> flush or
>> > >> >> > clear or multiple sequential transactions if the  
>> persistence
>> > >> >> context
>> > >> >> > has references to unenhanced, unredefined user-created
>> > >> instances.
>> > >> >>
>> > >> >> I think that no other implementation will have much of a  
>> better
>> > >> >> solution. So I don't see that we should try to exclude user
>> > >> options
>> > >> >> or a possible solution just because it's a poor performer.
>> > >> >>
>> > >> >> Craig
>> > >> >>
>> > >> >> Craig Russell
>> > >> >> Architect, Sun Java Enterprise System http://java.sun.com/
>> > >> products/
>> > >> >> jdo
>> > >> >> 408 276-5638 mailto:Craig.Russell@sun.com
>> > >> >> P.S. A good JDO? O, Gasp!
>> > >> >>
>> > >> >>
>> > >> >>
>> > >> >
>> > >> >
>> > >> > --
>> > >> > Patrick Linskey
>> > >> > 202 669 5907
>> > >>
>> > >> Craig Russell
>> > >> Architect, Sun Java Enterprise System http://java.sun.com/ 
>> products/
>> > >> jdo
>> > >> 408 276-5638 mailto:Craig.Russell@sun.com
>> > >> P.S. A good JDO? O, Gasp!
>> > >>
>> > >>
>> > >>
>> > >
>> > >
>> > > --
>> > > Patrick Linskey
>> > > 202 669 5907
>> >
>> >
>>
>>
>> --
>> Patrick Linskey
>> 202 669 5907
>>
>
>
> -- 
> Patrick Linskey
> 202 669 5907


Re: unenhanced class support

Posted by Patrick Linskey <pl...@gmail.com>.
> > 1. One of the big TODOs seems to be support for compound primary keys
> > (e.g., implementing
> > ReflectingPersistenceCapable.pcCopyKeyFieldsToObjectId()).
>
> Actually, I think that things might work as-is with compound PKs. We

Nope, it fails.

Ok. So the TCK does not pass with my patch and without enhancement. My
inclination is to get it committed so that it's in there, and then we
can work on the TCK failures in parallel. If others agree with this
strategy, I'll commit it probably sometime tomorrow. I'll make sure
that whatever I commit passes the TCK with enhancement on, so there
won't be any regression for people using enhancement. Thoughts?

-Patrick

On 7/25/07, Patrick Linskey <pl...@gmail.com> wrote:
> > 1. One of the big TODOs seems to be support for compound primary keys
> > (e.g., implementing
> > ReflectingPersistenceCapable.pcCopyKeyFieldsToObjectId()).
>
> Actually, I think that things might work as-is with compound PKs. We
> definitely need to test, but the ReflectingPC methods only get called
> in certain contexts, and some of the methods in it are never used,
> since the subclass versions are used instead. (Subclass instances are
> put into PCRegistry; ReflectingPC is only used to control instances
> created by the user.)
>
> > 2. Should ImplHelper._unenhancedInstanceMap be some sort of
> > IdentityMap? If the user overrides MyEntity.equals() to return true
> > if, say, their names are the same, we could get into trouble by doing
> > just a simple map lookup.
>
> Nice catch.
>
> > 5. I don't completely understand what is happening with the
> > subclassing and enhancement. When we are redefining the classes, do
> > we change the methods internally to do something like change String
> > getName() { return name; } to String getName()
> > { ((MyGeneratedSubclass) this).pcProvideField(nameFieldIndex); }? I
> > don't see any issue with doing this, but if it does turn out to be a
> > problem, we could alternately have some lookup that does String
> > getName() { ImplHelper.getPCProxyFor(this).pcProvideField
> > (nameFieldIndex); }
>
> Basically, the subclasses are used whenever OpenJPA loads a type. What
> it does depends on a bunch of factors, but the short story is thus:
>
> 1. if available, we redefine user-defined methods to call
> RedefinitionHelper.accessingField() or
> RedefinitionHelper.settingField(), either using field-access or
> property-access rules as appropriate.
>
> 2. the subclass basically just implements PC, and does some work for
> serialization and cloning.
>
> So the subclasses don't really have proxies -- they actually implement
> PersistenceCapable. The only place where we do any proxying /
> delegating is in ReflectingPC, to support instances created by the
> user.
>
> In both reflecting and redefinition / subclassing, there's a lot of
> delegating to ImplHelper.getPC(), which will do the map lookup if
> necessary, but is optimized to support subclass instances, which
> implement PC, so don't need entries in the map.
>
> So, in a field-access redefinition world, the only cost is that
> associated with maintaining the map of instance => PC for
> newly-created instances.
>
> In a property-access redefinition world, some of the PC methods have
> additional costs, since they need to reflect to access fields
> directly, since we've inserted tracking code directly into the
> superclass. So, some of the PC-related operations will end up using
> reflection. Additionally, we require that property-access redefined
> types are implemented with a single backing field per setter/getter
> pair. We auto-detect the name of the backing field (assuming
> relatively simple bytecode in the setter / getter pair), so that
> shouldn't be that burdensome for normal usage. But it does mean that
> you can't do prop access + redefinition + store everything in a Map
> internally.
>
> -Patrick
>
> On 7/25/07, Marc Prud'hommeaux <mp...@apache.org> wrote:
> > Patrick-
> >
> > It looks very good to me! A few notes/questions on the patch:
> >
> > 1. One of the big TODOs seems to be support for compound primary keys
> > (e.g., implementing
> > ReflectingPersistenceCapable.pcCopyKeyFieldsToObjectId()).
> >
> > 2. Should ImplHelper._unenhancedInstanceMap be some sort of
> > IdentityMap? If the user overrides MyEntity.equals() to return true
> > if, say, their names are the same, we could get into trouble by doing
> > just a simple map lookup.
> >
> > 3. It's pretty nice that we only need 54 invocations of
> > ImplHelper.toPersistenceCapable().
> >
> > 4. InstrumentationFactory is scary with all the implementation
> > dependent assumptions it makes, and the creation of temporary files
> > and whatnot. If anyone knows of any way to cheat and obtain an
> > InstrumentationFactory without having to create a jar file with the
> > appropriately-formatted manifest, it'd be great.
> >
> > 5. I don't completely understand what is happening with the
> > subclassing and enhancement. When we are redefining the classes, do
> > we change the methods internally to do something like change String
> > getName() { return name; } to String getName()
> > { ((MyGeneratedSubclass) this).pcProvideField(nameFieldIndex); }? I
> > don't see any issue with doing this, but if it does turn out to be a
> > problem, we could alternately have some lookup that does String
> > getName() { ImplHelper.getPCProxyFor(this).pcProvideField
> > (nameFieldIndex); }
> >
> >
> > Overall, it looks very good! I'm very excited to be able to start
> > working towards eliminating the enhancement steps that people have
> > found so cumbersome in the past.
> >
> >
> >
> > On Jul 24, 2007, at 4:15 PM, Patrick Linskey wrote:
> >
> > > Hi,
> > >
> > > I attached my current patch to OPENJPA-293. There are a number of open
> > > issues in the patch marked by '#####' marks -- I'd appreciate another
> > > set of eyes on those items in particular, and on the patch in general.
> > >
> > > Aside from that, I'm planning to run the CTS without enhancement on at
> > > some point in the next day or so, and want to get these changes
> > > committed soon, so that we can start figuring out whether or not it
> > > works.
> > >
> > > -Patrick
> > >
> > > On 7/22/07, Craig L Russell <Cr...@sun.com> wrote:
> > >> Hi Patrick,
> > >>
> > >> On Jul 22, 2007, at 12:51 PM, Patrick Linskey wrote:
> > >>
> > >> >> I think that no other implementation will have much of a better
> > >> >> solution. So I don't see that we should try to exclude user
> > >> options
> > >> >> or a possible solution just because it's a poor performer.
> > >> >
> > >> > What about eviction? My feeling is that wherever OpenJPA would
> > >> > normally clear state (eviction, certain state transitions), we
> > >> should
> > >> > keep the state available instead when we can't intercept and
> > >> reload on
> > >> > demand.
> > >>
> > >> Yes.
> > >>
> > >> Craig
> > >> >
> > >> > -Patrick
> > >> >
> > >> > On 7/21/07, Craig L Russell <Cr...@sun.com> wrote:
> > >> >>
> > >> >> On Jul 20, 2007, at 11:42 AM, Patrick Linskey wrote:
> > >> >>
> > >> >> > So, I'm looking for answers to the following questions in
> > >> >> particular:
> > >> >> >
> > >> >> > 1. what should we do about { Java 5, no javaagent, field
> > >> access }?
> > >> >> > Should we support this configuration, including the
> > >> corresponding
> > >> >> > extra overhead, or should we require either property access or a
> > >> >> > javaagent specified in these configurations?
> > >> >>
> > >> >> I think we should do EAGER fetching of fields just like the other
> > >> >> implementations have to do.
> > >> >> >
> > >> >> > 2. what should we do about { Java 5, no javaagent, property
> > >> access,
> > >> >> > flushed | cleared instances }? There is a much lower impact to
> > >> >> doing
> > >> >> > the dirty tracking in these configurations, since the scope is
> > >> >> > narrower. However, we might also be able to just not allow
> > >> flush or
> > >> >> > clear or multiple sequential transactions if the persistence
> > >> >> context
> > >> >> > has references to unenhanced, unredefined user-created
> > >> instances.
> > >> >>
> > >> >> I think that no other implementation will have much of a better
> > >> >> solution. So I don't see that we should try to exclude user
> > >> options
> > >> >> or a possible solution just because it's a poor performer.
> > >> >>
> > >> >> Craig
> > >> >>
> > >> >> Craig Russell
> > >> >> Architect, Sun Java Enterprise System http://java.sun.com/
> > >> products/
> > >> >> jdo
> > >> >> 408 276-5638 mailto:Craig.Russell@sun.com
> > >> >> P.S. A good JDO? O, Gasp!
> > >> >>
> > >> >>
> > >> >>
> > >> >
> > >> >
> > >> > --
> > >> > Patrick Linskey
> > >> > 202 669 5907
> > >>
> > >> Craig Russell
> > >> Architect, Sun Java Enterprise System http://java.sun.com/products/
> > >> jdo
> > >> 408 276-5638 mailto:Craig.Russell@sun.com
> > >> P.S. A good JDO? O, Gasp!
> > >>
> > >>
> > >>
> > >
> > >
> > > --
> > > Patrick Linskey
> > > 202 669 5907
> >
> >
>
>
> --
> Patrick Linskey
> 202 669 5907
>


-- 
Patrick Linskey
202 669 5907

Re: unenhanced class support

Posted by Patrick Linskey <pl...@gmail.com>.
> 1. One of the big TODOs seems to be support for compound primary keys
> (e.g., implementing
> ReflectingPersistenceCapable.pcCopyKeyFieldsToObjectId()).

Actually, I think that things might work as-is with compound PKs. We
definitely need to test, but the ReflectingPC methods only get called
in certain contexts, and some of the methods in it are never used,
since the subclass versions are used instead. (Subclass instances are
put into PCRegistry; ReflectingPC is only used to control instances
created by the user.)

> 2. Should ImplHelper._unenhancedInstanceMap be some sort of
> IdentityMap? If the user overrides MyEntity.equals() to return true
> if, say, their names are the same, we could get into trouble by doing
> just a simple map lookup.

Nice catch.

> 5. I don't completely understand what is happening with the
> subclassing and enhancement. When we are redefining the classes, do
> we change the methods internally to do something like change String
> getName() { return name; } to String getName()
> { ((MyGeneratedSubclass) this).pcProvideField(nameFieldIndex); }? I
> don't see any issue with doing this, but if it does turn out to be a
> problem, we could alternately have some lookup that does String
> getName() { ImplHelper.getPCProxyFor(this).pcProvideField
> (nameFieldIndex); }

Basically, the subclasses are used whenever OpenJPA loads a type. What
it does depends on a bunch of factors, but the short story is thus:

1. if available, we redefine user-defined methods to call
RedefinitionHelper.accessingField() or
RedefinitionHelper.settingField(), either using field-access or
property-access rules as appropriate.

2. the subclass basically just implements PC, and does some work for
serialization and cloning.

So the subclasses don't really have proxies -- they actually implement
PersistenceCapable. The only place where we do any proxying /
delegating is in ReflectingPC, to support instances created by the
user.

In both reflecting and redefinition / subclassing, there's a lot of
delegating to ImplHelper.getPC(), which will do the map lookup if
necessary, but is optimized to support subclass instances, which
implement PC, so don't need entries in the map.

So, in a field-access redefinition world, the only cost is that
associated with maintaining the map of instance => PC for
newly-created instances.

In a property-access redefinition world, some of the PC methods have
additional costs, since they need to reflect to access fields
directly, since we've inserted tracking code directly into the
superclass. So, some of the PC-related operations will end up using
reflection. Additionally, we require that property-access redefined
types are implemented with a single backing field per setter/getter
pair. We auto-detect the name of the backing field (assuming
relatively simple bytecode in the setter / getter pair), so that
shouldn't be that burdensome for normal usage. But it does mean that
you can't do prop access + redefinition + store everything in a Map
internally.

-Patrick

On 7/25/07, Marc Prud'hommeaux <mp...@apache.org> wrote:
> Patrick-
>
> It looks very good to me! A few notes/questions on the patch:
>
> 1. One of the big TODOs seems to be support for compound primary keys
> (e.g., implementing
> ReflectingPersistenceCapable.pcCopyKeyFieldsToObjectId()).
>
> 2. Should ImplHelper._unenhancedInstanceMap be some sort of
> IdentityMap? If the user overrides MyEntity.equals() to return true
> if, say, their names are the same, we could get into trouble by doing
> just a simple map lookup.
>
> 3. It's pretty nice that we only need 54 invocations of
> ImplHelper.toPersistenceCapable().
>
> 4. InstrumentationFactory is scary with all the implementation
> dependent assumptions it makes, and the creation of temporary files
> and whatnot. If anyone knows of any way to cheat and obtain an
> InstrumentationFactory without having to create a jar file with the
> appropriately-formatted manifest, it'd be great.
>
> 5. I don't completely understand what is happening with the
> subclassing and enhancement. When we are redefining the classes, do
> we change the methods internally to do something like change String
> getName() { return name; } to String getName()
> { ((MyGeneratedSubclass) this).pcProvideField(nameFieldIndex); }? I
> don't see any issue with doing this, but if it does turn out to be a
> problem, we could alternately have some lookup that does String
> getName() { ImplHelper.getPCProxyFor(this).pcProvideField
> (nameFieldIndex); }
>
>
> Overall, it looks very good! I'm very excited to be able to start
> working towards eliminating the enhancement steps that people have
> found so cumbersome in the past.
>
>
>
> On Jul 24, 2007, at 4:15 PM, Patrick Linskey wrote:
>
> > Hi,
> >
> > I attached my current patch to OPENJPA-293. There are a number of open
> > issues in the patch marked by '#####' marks -- I'd appreciate another
> > set of eyes on those items in particular, and on the patch in general.
> >
> > Aside from that, I'm planning to run the CTS without enhancement on at
> > some point in the next day or so, and want to get these changes
> > committed soon, so that we can start figuring out whether or not it
> > works.
> >
> > -Patrick
> >
> > On 7/22/07, Craig L Russell <Cr...@sun.com> wrote:
> >> Hi Patrick,
> >>
> >> On Jul 22, 2007, at 12:51 PM, Patrick Linskey wrote:
> >>
> >> >> I think that no other implementation will have much of a better
> >> >> solution. So I don't see that we should try to exclude user
> >> options
> >> >> or a possible solution just because it's a poor performer.
> >> >
> >> > What about eviction? My feeling is that wherever OpenJPA would
> >> > normally clear state (eviction, certain state transitions), we
> >> should
> >> > keep the state available instead when we can't intercept and
> >> reload on
> >> > demand.
> >>
> >> Yes.
> >>
> >> Craig
> >> >
> >> > -Patrick
> >> >
> >> > On 7/21/07, Craig L Russell <Cr...@sun.com> wrote:
> >> >>
> >> >> On Jul 20, 2007, at 11:42 AM, Patrick Linskey wrote:
> >> >>
> >> >> > So, I'm looking for answers to the following questions in
> >> >> particular:
> >> >> >
> >> >> > 1. what should we do about { Java 5, no javaagent, field
> >> access }?
> >> >> > Should we support this configuration, including the
> >> corresponding
> >> >> > extra overhead, or should we require either property access or a
> >> >> > javaagent specified in these configurations?
> >> >>
> >> >> I think we should do EAGER fetching of fields just like the other
> >> >> implementations have to do.
> >> >> >
> >> >> > 2. what should we do about { Java 5, no javaagent, property
> >> access,
> >> >> > flushed | cleared instances }? There is a much lower impact to
> >> >> doing
> >> >> > the dirty tracking in these configurations, since the scope is
> >> >> > narrower. However, we might also be able to just not allow
> >> flush or
> >> >> > clear or multiple sequential transactions if the persistence
> >> >> context
> >> >> > has references to unenhanced, unredefined user-created
> >> instances.
> >> >>
> >> >> I think that no other implementation will have much of a better
> >> >> solution. So I don't see that we should try to exclude user
> >> options
> >> >> or a possible solution just because it's a poor performer.
> >> >>
> >> >> Craig
> >> >>
> >> >> Craig Russell
> >> >> Architect, Sun Java Enterprise System http://java.sun.com/
> >> products/
> >> >> jdo
> >> >> 408 276-5638 mailto:Craig.Russell@sun.com
> >> >> P.S. A good JDO? O, Gasp!
> >> >>
> >> >>
> >> >>
> >> >
> >> >
> >> > --
> >> > Patrick Linskey
> >> > 202 669 5907
> >>
> >> Craig Russell
> >> Architect, Sun Java Enterprise System http://java.sun.com/products/
> >> jdo
> >> 408 276-5638 mailto:Craig.Russell@sun.com
> >> P.S. A good JDO? O, Gasp!
> >>
> >>
> >>
> >
> >
> > --
> > Patrick Linskey
> > 202 669 5907
>
>


-- 
Patrick Linskey
202 669 5907

Re: unenhanced class support

Posted by Marc Prud'hommeaux <mp...@apache.org>.
Patrick-

It looks very good to me! A few notes/questions on the patch:

1. One of the big TODOs seems to be support for compound primary keys  
(e.g., implementing  
ReflectingPersistenceCapable.pcCopyKeyFieldsToObjectId()).

2. Should ImplHelper._unenhancedInstanceMap be some sort of  
IdentityMap? If the user overrides MyEntity.equals() to return true  
if, say, their names are the same, we could get into trouble by doing  
just a simple map lookup.

3. It's pretty nice that we only need 54 invocations of  
ImplHelper.toPersistenceCapable().

4. InstrumentationFactory is scary with all the implementation  
dependent assumptions it makes, and the creation of temporary files  
and whatnot. If anyone knows of any way to cheat and obtain an  
InstrumentationFactory without having to create a jar file with the  
appropriately-formatted manifest, it'd be great.

5. I don't completely understand what is happening with the  
subclassing and enhancement. When we are redefining the classes, do  
we change the methods internally to do something like change String  
getName() { return name; } to String getName()  
{ ((MyGeneratedSubclass) this).pcProvideField(nameFieldIndex); }? I  
don't see any issue with doing this, but if it does turn out to be a  
problem, we could alternately have some lookup that does String  
getName() { ImplHelper.getPCProxyFor(this).pcProvideField 
(nameFieldIndex); }


Overall, it looks very good! I'm very excited to be able to start  
working towards eliminating the enhancement steps that people have  
found so cumbersome in the past.



On Jul 24, 2007, at 4:15 PM, Patrick Linskey wrote:

> Hi,
>
> I attached my current patch to OPENJPA-293. There are a number of open
> issues in the patch marked by '#####' marks -- I'd appreciate another
> set of eyes on those items in particular, and on the patch in general.
>
> Aside from that, I'm planning to run the CTS without enhancement on at
> some point in the next day or so, and want to get these changes
> committed soon, so that we can start figuring out whether or not it
> works.
>
> -Patrick
>
> On 7/22/07, Craig L Russell <Cr...@sun.com> wrote:
>> Hi Patrick,
>>
>> On Jul 22, 2007, at 12:51 PM, Patrick Linskey wrote:
>>
>> >> I think that no other implementation will have much of a better
>> >> solution. So I don't see that we should try to exclude user  
>> options
>> >> or a possible solution just because it's a poor performer.
>> >
>> > What about eviction? My feeling is that wherever OpenJPA would
>> > normally clear state (eviction, certain state transitions), we  
>> should
>> > keep the state available instead when we can't intercept and  
>> reload on
>> > demand.
>>
>> Yes.
>>
>> Craig
>> >
>> > -Patrick
>> >
>> > On 7/21/07, Craig L Russell <Cr...@sun.com> wrote:
>> >>
>> >> On Jul 20, 2007, at 11:42 AM, Patrick Linskey wrote:
>> >>
>> >> > So, I'm looking for answers to the following questions in
>> >> particular:
>> >> >
>> >> > 1. what should we do about { Java 5, no javaagent, field  
>> access }?
>> >> > Should we support this configuration, including the  
>> corresponding
>> >> > extra overhead, or should we require either property access or a
>> >> > javaagent specified in these configurations?
>> >>
>> >> I think we should do EAGER fetching of fields just like the other
>> >> implementations have to do.
>> >> >
>> >> > 2. what should we do about { Java 5, no javaagent, property  
>> access,
>> >> > flushed | cleared instances }? There is a much lower impact to
>> >> doing
>> >> > the dirty tracking in these configurations, since the scope is
>> >> > narrower. However, we might also be able to just not allow  
>> flush or
>> >> > clear or multiple sequential transactions if the persistence
>> >> context
>> >> > has references to unenhanced, unredefined user-created  
>> instances.
>> >>
>> >> I think that no other implementation will have much of a better
>> >> solution. So I don't see that we should try to exclude user  
>> options
>> >> or a possible solution just because it's a poor performer.
>> >>
>> >> Craig
>> >>
>> >> Craig Russell
>> >> Architect, Sun Java Enterprise System http://java.sun.com/ 
>> products/
>> >> jdo
>> >> 408 276-5638 mailto:Craig.Russell@sun.com
>> >> P.S. A good JDO? O, Gasp!
>> >>
>> >>
>> >>
>> >
>> >
>> > --
>> > Patrick Linskey
>> > 202 669 5907
>>
>> Craig Russell
>> Architect, Sun Java Enterprise System http://java.sun.com/products/ 
>> jdo
>> 408 276-5638 mailto:Craig.Russell@sun.com
>> P.S. A good JDO? O, Gasp!
>>
>>
>>
>
>
> -- 
> Patrick Linskey
> 202 669 5907


Re: unenhanced class support

Posted by Patrick Linskey <pl...@gmail.com>.
Hi,

I attached my current patch to OPENJPA-293. There are a number of open
issues in the patch marked by '#####' marks -- I'd appreciate another
set of eyes on those items in particular, and on the patch in general.

Aside from that, I'm planning to run the CTS without enhancement on at
some point in the next day or so, and want to get these changes
committed soon, so that we can start figuring out whether or not it
works.

-Patrick

On 7/22/07, Craig L Russell <Cr...@sun.com> wrote:
> Hi Patrick,
>
> On Jul 22, 2007, at 12:51 PM, Patrick Linskey wrote:
>
> >> I think that no other implementation will have much of a better
> >> solution. So I don't see that we should try to exclude user options
> >> or a possible solution just because it's a poor performer.
> >
> > What about eviction? My feeling is that wherever OpenJPA would
> > normally clear state (eviction, certain state transitions), we should
> > keep the state available instead when we can't intercept and reload on
> > demand.
>
> Yes.
>
> Craig
> >
> > -Patrick
> >
> > On 7/21/07, Craig L Russell <Cr...@sun.com> wrote:
> >>
> >> On Jul 20, 2007, at 11:42 AM, Patrick Linskey wrote:
> >>
> >> > So, I'm looking for answers to the following questions in
> >> particular:
> >> >
> >> > 1. what should we do about { Java 5, no javaagent, field access }?
> >> > Should we support this configuration, including the corresponding
> >> > extra overhead, or should we require either property access or a
> >> > javaagent specified in these configurations?
> >>
> >> I think we should do EAGER fetching of fields just like the other
> >> implementations have to do.
> >> >
> >> > 2. what should we do about { Java 5, no javaagent, property access,
> >> > flushed | cleared instances }? There is a much lower impact to
> >> doing
> >> > the dirty tracking in these configurations, since the scope is
> >> > narrower. However, we might also be able to just not allow flush or
> >> > clear or multiple sequential transactions if the persistence
> >> context
> >> > has references to unenhanced, unredefined user-created instances.
> >>
> >> I think that no other implementation will have much of a better
> >> solution. So I don't see that we should try to exclude user options
> >> or a possible solution just because it's a poor performer.
> >>
> >> Craig
> >>
> >> Craig Russell
> >> Architect, Sun Java Enterprise System http://java.sun.com/products/
> >> jdo
> >> 408 276-5638 mailto:Craig.Russell@sun.com
> >> P.S. A good JDO? O, Gasp!
> >>
> >>
> >>
> >
> >
> > --
> > Patrick Linskey
> > 202 669 5907
>
> Craig Russell
> Architect, Sun Java Enterprise System http://java.sun.com/products/jdo
> 408 276-5638 mailto:Craig.Russell@sun.com
> P.S. A good JDO? O, Gasp!
>
>
>


-- 
Patrick Linskey
202 669 5907

Re: unenhanced class support

Posted by Craig L Russell <Cr...@Sun.COM>.
Hi Patrick,

On Jul 22, 2007, at 12:51 PM, Patrick Linskey wrote:

>> I think that no other implementation will have much of a better
>> solution. So I don't see that we should try to exclude user options
>> or a possible solution just because it's a poor performer.
>
> What about eviction? My feeling is that wherever OpenJPA would
> normally clear state (eviction, certain state transitions), we should
> keep the state available instead when we can't intercept and reload on
> demand.

Yes.

Craig
>
> -Patrick
>
> On 7/21/07, Craig L Russell <Cr...@sun.com> wrote:
>>
>> On Jul 20, 2007, at 11:42 AM, Patrick Linskey wrote:
>>
>> > So, I'm looking for answers to the following questions in  
>> particular:
>> >
>> > 1. what should we do about { Java 5, no javaagent, field access }?
>> > Should we support this configuration, including the corresponding
>> > extra overhead, or should we require either property access or a
>> > javaagent specified in these configurations?
>>
>> I think we should do EAGER fetching of fields just like the other
>> implementations have to do.
>> >
>> > 2. what should we do about { Java 5, no javaagent, property access,
>> > flushed | cleared instances }? There is a much lower impact to  
>> doing
>> > the dirty tracking in these configurations, since the scope is
>> > narrower. However, we might also be able to just not allow flush or
>> > clear or multiple sequential transactions if the persistence  
>> context
>> > has references to unenhanced, unredefined user-created instances.
>>
>> I think that no other implementation will have much of a better
>> solution. So I don't see that we should try to exclude user options
>> or a possible solution just because it's a poor performer.
>>
>> Craig
>>
>> Craig Russell
>> Architect, Sun Java Enterprise System http://java.sun.com/products/ 
>> jdo
>> 408 276-5638 mailto:Craig.Russell@sun.com
>> P.S. A good JDO? O, Gasp!
>>
>>
>>
>
>
> -- 
> Patrick Linskey
> 202 669 5907

Craig Russell
Architect, Sun Java Enterprise System http://java.sun.com/products/jdo
408 276-5638 mailto:Craig.Russell@sun.com
P.S. A good JDO? O, Gasp!


Re: unenhanced class support

Posted by Patrick Linskey <pl...@gmail.com>.
> I think that no other implementation will have much of a better
> solution. So I don't see that we should try to exclude user options
> or a possible solution just because it's a poor performer.

What about eviction? My feeling is that wherever OpenJPA would
normally clear state (eviction, certain state transitions), we should
keep the state available instead when we can't intercept and reload on
demand.

-Patrick

On 7/21/07, Craig L Russell <Cr...@sun.com> wrote:
>
> On Jul 20, 2007, at 11:42 AM, Patrick Linskey wrote:
>
> > So, I'm looking for answers to the following questions in particular:
> >
> > 1. what should we do about { Java 5, no javaagent, field access }?
> > Should we support this configuration, including the corresponding
> > extra overhead, or should we require either property access or a
> > javaagent specified in these configurations?
>
> I think we should do EAGER fetching of fields just like the other
> implementations have to do.
> >
> > 2. what should we do about { Java 5, no javaagent, property access,
> > flushed | cleared instances }? There is a much lower impact to doing
> > the dirty tracking in these configurations, since the scope is
> > narrower. However, we might also be able to just not allow flush or
> > clear or multiple sequential transactions if the persistence context
> > has references to unenhanced, unredefined user-created instances.
>
> I think that no other implementation will have much of a better
> solution. So I don't see that we should try to exclude user options
> or a possible solution just because it's a poor performer.
>
> Craig
>
> Craig Russell
> Architect, Sun Java Enterprise System http://java.sun.com/products/jdo
> 408 276-5638 mailto:Craig.Russell@sun.com
> P.S. A good JDO? O, Gasp!
>
>
>


-- 
Patrick Linskey
202 669 5907

Re: unenhanced class support

Posted by Craig L Russell <Cr...@Sun.COM>.
On Jul 20, 2007, at 11:42 AM, Patrick Linskey wrote:

> So, I'm looking for answers to the following questions in particular:
>
> 1. what should we do about { Java 5, no javaagent, field access }?
> Should we support this configuration, including the corresponding
> extra overhead, or should we require either property access or a
> javaagent specified in these configurations?

I think we should do EAGER fetching of fields just like the other  
implementations have to do.
>
> 2. what should we do about { Java 5, no javaagent, property access,
> flushed | cleared instances }? There is a much lower impact to doing
> the dirty tracking in these configurations, since the scope is
> narrower. However, we might also be able to just not allow flush or
> clear or multiple sequential transactions if the persistence context
> has references to unenhanced, unredefined user-created instances.

I think that no other implementation will have much of a better  
solution. So I don't see that we should try to exclude user options  
or a possible solution just because it's a poor performer.

Craig

Craig Russell
Architect, Sun Java Enterprise System http://java.sun.com/products/jdo
408 276-5638 mailto:Craig.Russell@sun.com
P.S. A good JDO? O, Gasp!


Re: unenhanced class support

Posted by Patrick Linskey <pl...@gmail.com>.
> that big of a deal.  We can explain that better performance and other QOS
> can be had with pre-enhancing the classes.

Also, note that when running in an appserver (or other container that
obeys the JPA persistence container contract), the
auto-enhancement-on-deploy will still kick in, and pre-empt the class
redefinition.

-Patrick

On 7/20/07, Kevin Sutter <kw...@gmail.com> wrote:
> Patrick,
> First off, this sounds great!  From various discussions I've had with
> customers moving off of Hibernate to OpenJPA, our enhancement process is
> always a topic.  I can usually convince them that the enhancement process is
> not that bad, but for the initial out-of-the-box experience, it would be
> great to skip this step.
>
> Given that, if we are going to allow OpenJPA to work with unenhanced
> classes, we probably need to decide what the goal is.  If we are doing it
> just for the initial out-of-the-box experimentation, then I don't see where
> performance should be a concern.  It sounds like we can provide similar
> levels of functionality, but with a performance cost.  That should not be
> that big of a deal.  We can explain that better performance and other QOS
> can be had with pre-enhancing the classes.
>
> But, if our goal is to compete with Hibernate with unenhanced classes, then
> it sounds like we have lots more work to do.  But, is it worth the cost and
> effort?  No "real" bake-off happens with out-of-the-box products.  There are
> always configuration tweaks that help the product's performance.
>
> I would think that getting the unenhanced classes to work with OpenJPA
> should be our priority, with the performance concerns being secondary.  From
> your notes, it sounds like you have a good idea on how to accomplish the
> "missing functions", but they may introduce some performance overhead.  From
> my perspective, that should be okay.
>
> Thanks for experimenting with this!
> Kevin
>
> On 7/20/07, Patrick Linskey <pl...@gmail.com> wrote:
> >
> > Note that there actually is another option for handling instances for
> > which we can't track field mutation: we can just always do an UPDATE
> > with all the fields in the instance, regardless of whether there was a
> > change.
> >
> > Also, I actually have some of the code necessary to do field
> > comparison already written, so we should try to figure out what the
> > right decision is, regardless of implementation cost, at least
> > initially.
> >
> > -Patrick
> >
> > On 7/20/07, Patrick Linskey <pl...@gmail.com> wrote:
> > > Hi,
> > >
> > > Over the last week, I've been working on making OpenJPA work with
> > > unenhanced classes in a variety of different environments. My goal has
> > > been to make the enhancement step (either at build time or via the
> > > javaagent) optional in order to remove a barrier to entry for
> > > newcomers to OpenJPA. There are still reasons why enhancement is a
> > > good thing to do in a running OpenJPA system, but my hope is that we
> > > can make these be mostly quality-of-service issues, and not
> > > functionality issues.
> > >
> > > I've been pretty successful so far, but there are a few limitations in
> > > some environments. I'd like to get some feedback about what to do
> > > about these limitations. First, I'll describe my approach:
> > >
> > > I started looking into this when Marc told me about the new
> > > retransformation capabilities in Java 6. (Briefly, in Sun's Java 6
> > > impl, it is no longer necessary to specify a javaagent flag in order
> > > to retransform classes.) This led me to investigate retransforming
> > > entity types to add field-tracking code to method blocks without
> > > violating any of the retransformation rules. I combined this with a
> > > subclassing approach so that instances created by OpenJPA (vs. created
> > > by the user with 'new') can be treated as proper persistence-capable
> > > instances.
> > >
> > > Since retransformation does not allow fields, methods, or interfaces
> > > to be added to a class, I added a helper class that maintains a static
> > > map of *all* the instances created with 'new' that have been
> > > persisted; this map associates a managed instance with a
> > > ReflectingPersistenceCapable instance, which implements
> > > PersistenceCapable via reflection, as the name implies, and can wrap
> > > unenhanced managed instances for OpenJPA's needs.
> > >
> > > So, the only functionality cost in a Java 6 environment is the extra
> > > overhead of occasionally looking up the PersistenceCapable for a given
> > > unenhanced managed instance. There is still a bit of a performance
> > > cost, since I needed to do reflection in some places instead of direct
> > > field access because of details of the class hierarchy and field
> > > access restrictions.
> > >
> > > In a Java 5 environment, if you specify a new javaagent class, then
> > > the situation is exactly the same. IMO this improves on our current
> > > javaagent in that it releases the restriction that persistent types
> > > must be listed in the first persistence unit in persistence.xml, and
> > > because the redefinition logic is never invoked against any
> > > non-managed types.
> > >
> > > If you do not specify a javaagent class in Java 5, then OpenJPA cannot
> > > do redefinition. However, it can still create subclasses with logic in
> > > overridden setters and getters. So, in Java 5 without a javaagent but
> > > with property access, we only lose dirty tracking and lazy loading for
> > > instances created by  the user code. This only matters for cases where
> > > an instance so created is flushed (for dirty tracking) or cleared (for
> > > lazy loading).
> > >
> > > In Java 5 without a javaagent and with field access, things are a bit
> > > worse -- OpenJPA can't do any dirty tracking or lazy loading, either
> > > for instances created by the user code or for instances created by
> > > OpenJPA.
> > >
> > > We could improve the situation a bit for field access and post-flush
> > > property access in Java 5 with no javaagent. This would require state
> > > comparison, though, which is not the fastest thing in the world, and
> > > requires that we hold hard references to all applicable instances for
> > > the duration of the EM's life (all user-created property access
> > > instances, and all field access instances regardless of how they were
> > > created).
> > >
> > >
> > > So, I'm looking for answers to the following questions in particular:
> > >
> > > 1. what should we do about { Java 5, no javaagent, field access }?
> > > Should we support this configuration, including the corresponding
> > > extra overhead, or should we require either property access or a
> > > javaagent specified in these configurations?
> > >
> > > 2. what should we do about { Java 5, no javaagent, property access,
> > > flushed | cleared instances }? There is a much lower impact to doing
> > > the dirty tracking in these configurations, since the scope is
> > > narrower. However, we might also be able to just not allow flush or
> > > clear or multiple sequential transactions if the persistence context
> > > has references to unenhanced, unredefined user-created instances.
> > >
> > > Thoughts?
> > >
> > > (Oh, and the code is still a work in progress -- everything I
> > > discussed above is working, but serialization and cloning and
> > > detachment are untested, and will probably require some tweaks to get
> > > working correctly.)
> > >
> > > -Patrick
> > >
> > > --
> > > Patrick Linskey
> > > 202 669 5907
> > >
> >
> >
> > --
> > Patrick Linskey
> > 202 669 5907
> >
>


-- 
Patrick Linskey
202 669 5907

Re: unenhanced class support

Posted by Craig L Russell <Cr...@Sun.COM>.
Hey,

On Jul 20, 2007, at 12:07 PM, Kevin Sutter wrote:

> Patrick,
> First off, this sounds great!  From various discussions I've had with
> customers moving off of Hibernate to OpenJPA, our enhancement  
> process is
> always a topic.  I can usually convince them that the enhancement  
> process is
> not that bad, but for the initial out-of-the-box experience, it  
> would be
> great to skip this step.

I agree. I've suffered through people always asking why enhancement  
is required, and even though there is a significant performance  
advantage to be had, they grumble.
>
> Given that, if we are going to allow OpenJPA to work with unenhanced
> classes, we probably need to decide what the goal is.  If we are  
> doing it
> just for the initial out-of-the-box experimentation, then I don't  
> see where
> performance should be a concern.  It sounds like we can provide  
> similar
> levels of functionality, but with a performance cost.  That should  
> not be
> that big of a deal.  We can explain that better performance and  
> other QOS
> can be had with pre-enhancing the classes.

I think this should be the #1 goal.
>
> But, if our goal is to compete with Hibernate with unenhanced  
> classes, then
> it sounds like we have lots more work to do.  But, is it worth the  
> cost and
> effort?  No "real" bake-off happens with out-of-the-box products.   
> There are
> always configuration tweaks that help the product's performance.

We should be prepared to match Hibernate's performance features,  
feature-for-feature. As a longer term objective, it will be nice to  
compete head-on with Hibernate, but by the time we get there,  
Hibernate will be competing with OpenJPA with an enhanced version. So  
if customers compare unenhanced Hibernate vs. OpenJPA and then  
enhanced Hibernate vs. OpenJPA, I'd rather we spent our energy on  
enhanced performance.

Craig
>
> I would think that getting the unenhanced classes to work with OpenJPA
> should be our priority, with the performance concerns being  
> secondary.  From
> your notes, it sounds like you have a good idea on how to  
> accomplish the
> "missing functions", but they may introduce some performance  
> overhead.  From
> my perspective, that should be okay.
>
> Thanks for experimenting with this!
> Kevin
>
> On 7/20/07, Patrick Linskey <pl...@gmail.com> wrote:
>>
>> Note that there actually is another option for handling instances for
>> which we can't track field mutation: we can just always do an UPDATE
>> with all the fields in the instance, regardless of whether there  
>> was a
>> change.
>>
>> Also, I actually have some of the code necessary to do field
>> comparison already written, so we should try to figure out what the
>> right decision is, regardless of implementation cost, at least
>> initially.
>>
>> -Patrick
>>
>> On 7/20/07, Patrick Linskey <pl...@gmail.com> wrote:
>> > Hi,
>> >
>> > Over the last week, I've been working on making OpenJPA work with
>> > unenhanced classes in a variety of different environments. My  
>> goal has
>> > been to make the enhancement step (either at build time or via the
>> > javaagent) optional in order to remove a barrier to entry for
>> > newcomers to OpenJPA. There are still reasons why enhancement is a
>> > good thing to do in a running OpenJPA system, but my hope is  
>> that we
>> > can make these be mostly quality-of-service issues, and not
>> > functionality issues.
>> >
>> > I've been pretty successful so far, but there are a few  
>> limitations in
>> > some environments. I'd like to get some feedback about what to do
>> > about these limitations. First, I'll describe my approach:
>> >
>> > I started looking into this when Marc told me about the new
>> > retransformation capabilities in Java 6. (Briefly, in Sun's Java 6
>> > impl, it is no longer necessary to specify a javaagent flag in  
>> order
>> > to retransform classes.) This led me to investigate retransforming
>> > entity types to add field-tracking code to method blocks without
>> > violating any of the retransformation rules. I combined this with a
>> > subclassing approach so that instances created by OpenJPA (vs.  
>> created
>> > by the user with 'new') can be treated as proper persistence- 
>> capable
>> > instances.
>> >
>> > Since retransformation does not allow fields, methods, or  
>> interfaces
>> > to be added to a class, I added a helper class that maintains a  
>> static
>> > map of *all* the instances created with 'new' that have been
>> > persisted; this map associates a managed instance with a
>> > ReflectingPersistenceCapable instance, which implements
>> > PersistenceCapable via reflection, as the name implies, and can  
>> wrap
>> > unenhanced managed instances for OpenJPA's needs.
>> >
>> > So, the only functionality cost in a Java 6 environment is the  
>> extra
>> > overhead of occasionally looking up the PersistenceCapable for a  
>> given
>> > unenhanced managed instance. There is still a bit of a performance
>> > cost, since I needed to do reflection in some places instead of  
>> direct
>> > field access because of details of the class hierarchy and field
>> > access restrictions.
>> >
>> > In a Java 5 environment, if you specify a new javaagent class, then
>> > the situation is exactly the same. IMO this improves on our current
>> > javaagent in that it releases the restriction that persistent types
>> > must be listed in the first persistence unit in persistence.xml,  
>> and
>> > because the redefinition logic is never invoked against any
>> > non-managed types.
>> >
>> > If you do not specify a javaagent class in Java 5, then OpenJPA  
>> cannot
>> > do redefinition. However, it can still create subclasses with  
>> logic in
>> > overridden setters and getters. So, in Java 5 without a  
>> javaagent but
>> > with property access, we only lose dirty tracking and lazy  
>> loading for
>> > instances created by  the user code. This only matters for cases  
>> where
>> > an instance so created is flushed (for dirty tracking) or  
>> cleared (for
>> > lazy loading).
>> >
>> > In Java 5 without a javaagent and with field access, things are  
>> a bit
>> > worse -- OpenJPA can't do any dirty tracking or lazy loading,  
>> either
>> > for instances created by the user code or for instances created by
>> > OpenJPA.
>> >
>> > We could improve the situation a bit for field access and post- 
>> flush
>> > property access in Java 5 with no javaagent. This would require  
>> state
>> > comparison, though, which is not the fastest thing in the world,  
>> and
>> > requires that we hold hard references to all applicable  
>> instances for
>> > the duration of the EM's life (all user-created property access
>> > instances, and all field access instances regardless of how they  
>> were
>> > created).
>> >
>> >
>> > So, I'm looking for answers to the following questions in  
>> particular:
>> >
>> > 1. what should we do about { Java 5, no javaagent, field access }?
>> > Should we support this configuration, including the corresponding
>> > extra overhead, or should we require either property access or a
>> > javaagent specified in these configurations?
>> >
>> > 2. what should we do about { Java 5, no javaagent, property access,
>> > flushed | cleared instances }? There is a much lower impact to  
>> doing
>> > the dirty tracking in these configurations, since the scope is
>> > narrower. However, we might also be able to just not allow flush or
>> > clear or multiple sequential transactions if the persistence  
>> context
>> > has references to unenhanced, unredefined user-created instances.
>> >
>> > Thoughts?
>> >
>> > (Oh, and the code is still a work in progress -- everything I
>> > discussed above is working, but serialization and cloning and
>> > detachment are untested, and will probably require some tweaks  
>> to get
>> > working correctly.)
>> >
>> > -Patrick
>> >
>> > --
>> > Patrick Linskey
>> > 202 669 5907
>> >
>>
>>
>> --
>> Patrick Linskey
>> 202 669 5907
>>

Craig Russell
Architect, Sun Java Enterprise System http://java.sun.com/products/jdo
408 276-5638 mailto:Craig.Russell@sun.com
P.S. A good JDO? O, Gasp!


Re: unenhanced class support

Posted by Kevin Sutter <kw...@gmail.com>.
Patrick,
First off, this sounds great!  From various discussions I've had with
customers moving off of Hibernate to OpenJPA, our enhancement process is
always a topic.  I can usually convince them that the enhancement process is
not that bad, but for the initial out-of-the-box experience, it would be
great to skip this step.

Given that, if we are going to allow OpenJPA to work with unenhanced
classes, we probably need to decide what the goal is.  If we are doing it
just for the initial out-of-the-box experimentation, then I don't see where
performance should be a concern.  It sounds like we can provide similar
levels of functionality, but with a performance cost.  That should not be
that big of a deal.  We can explain that better performance and other QOS
can be had with pre-enhancing the classes.

But, if our goal is to compete with Hibernate with unenhanced classes, then
it sounds like we have lots more work to do.  But, is it worth the cost and
effort?  No "real" bake-off happens with out-of-the-box products.  There are
always configuration tweaks that help the product's performance.

I would think that getting the unenhanced classes to work with OpenJPA
should be our priority, with the performance concerns being secondary.  From
your notes, it sounds like you have a good idea on how to accomplish the
"missing functions", but they may introduce some performance overhead.  From
my perspective, that should be okay.

Thanks for experimenting with this!
Kevin

On 7/20/07, Patrick Linskey <pl...@gmail.com> wrote:
>
> Note that there actually is another option for handling instances for
> which we can't track field mutation: we can just always do an UPDATE
> with all the fields in the instance, regardless of whether there was a
> change.
>
> Also, I actually have some of the code necessary to do field
> comparison already written, so we should try to figure out what the
> right decision is, regardless of implementation cost, at least
> initially.
>
> -Patrick
>
> On 7/20/07, Patrick Linskey <pl...@gmail.com> wrote:
> > Hi,
> >
> > Over the last week, I've been working on making OpenJPA work with
> > unenhanced classes in a variety of different environments. My goal has
> > been to make the enhancement step (either at build time or via the
> > javaagent) optional in order to remove a barrier to entry for
> > newcomers to OpenJPA. There are still reasons why enhancement is a
> > good thing to do in a running OpenJPA system, but my hope is that we
> > can make these be mostly quality-of-service issues, and not
> > functionality issues.
> >
> > I've been pretty successful so far, but there are a few limitations in
> > some environments. I'd like to get some feedback about what to do
> > about these limitations. First, I'll describe my approach:
> >
> > I started looking into this when Marc told me about the new
> > retransformation capabilities in Java 6. (Briefly, in Sun's Java 6
> > impl, it is no longer necessary to specify a javaagent flag in order
> > to retransform classes.) This led me to investigate retransforming
> > entity types to add field-tracking code to method blocks without
> > violating any of the retransformation rules. I combined this with a
> > subclassing approach so that instances created by OpenJPA (vs. created
> > by the user with 'new') can be treated as proper persistence-capable
> > instances.
> >
> > Since retransformation does not allow fields, methods, or interfaces
> > to be added to a class, I added a helper class that maintains a static
> > map of *all* the instances created with 'new' that have been
> > persisted; this map associates a managed instance with a
> > ReflectingPersistenceCapable instance, which implements
> > PersistenceCapable via reflection, as the name implies, and can wrap
> > unenhanced managed instances for OpenJPA's needs.
> >
> > So, the only functionality cost in a Java 6 environment is the extra
> > overhead of occasionally looking up the PersistenceCapable for a given
> > unenhanced managed instance. There is still a bit of a performance
> > cost, since I needed to do reflection in some places instead of direct
> > field access because of details of the class hierarchy and field
> > access restrictions.
> >
> > In a Java 5 environment, if you specify a new javaagent class, then
> > the situation is exactly the same. IMO this improves on our current
> > javaagent in that it releases the restriction that persistent types
> > must be listed in the first persistence unit in persistence.xml, and
> > because the redefinition logic is never invoked against any
> > non-managed types.
> >
> > If you do not specify a javaagent class in Java 5, then OpenJPA cannot
> > do redefinition. However, it can still create subclasses with logic in
> > overridden setters and getters. So, in Java 5 without a javaagent but
> > with property access, we only lose dirty tracking and lazy loading for
> > instances created by  the user code. This only matters for cases where
> > an instance so created is flushed (for dirty tracking) or cleared (for
> > lazy loading).
> >
> > In Java 5 without a javaagent and with field access, things are a bit
> > worse -- OpenJPA can't do any dirty tracking or lazy loading, either
> > for instances created by the user code or for instances created by
> > OpenJPA.
> >
> > We could improve the situation a bit for field access and post-flush
> > property access in Java 5 with no javaagent. This would require state
> > comparison, though, which is not the fastest thing in the world, and
> > requires that we hold hard references to all applicable instances for
> > the duration of the EM's life (all user-created property access
> > instances, and all field access instances regardless of how they were
> > created).
> >
> >
> > So, I'm looking for answers to the following questions in particular:
> >
> > 1. what should we do about { Java 5, no javaagent, field access }?
> > Should we support this configuration, including the corresponding
> > extra overhead, or should we require either property access or a
> > javaagent specified in these configurations?
> >
> > 2. what should we do about { Java 5, no javaagent, property access,
> > flushed | cleared instances }? There is a much lower impact to doing
> > the dirty tracking in these configurations, since the scope is
> > narrower. However, we might also be able to just not allow flush or
> > clear or multiple sequential transactions if the persistence context
> > has references to unenhanced, unredefined user-created instances.
> >
> > Thoughts?
> >
> > (Oh, and the code is still a work in progress -- everything I
> > discussed above is working, but serialization and cloning and
> > detachment are untested, and will probably require some tweaks to get
> > working correctly.)
> >
> > -Patrick
> >
> > --
> > Patrick Linskey
> > 202 669 5907
> >
>
>
> --
> Patrick Linskey
> 202 669 5907
>

Re: unenhanced class support

Posted by Patrick Linskey <pl...@gmail.com>.
Note that there actually is another option for handling instances for
which we can't track field mutation: we can just always do an UPDATE
with all the fields in the instance, regardless of whether there was a
change.

Also, I actually have some of the code necessary to do field
comparison already written, so we should try to figure out what the
right decision is, regardless of implementation cost, at least
initially.

-Patrick

On 7/20/07, Patrick Linskey <pl...@gmail.com> wrote:
> Hi,
>
> Over the last week, I've been working on making OpenJPA work with
> unenhanced classes in a variety of different environments. My goal has
> been to make the enhancement step (either at build time or via the
> javaagent) optional in order to remove a barrier to entry for
> newcomers to OpenJPA. There are still reasons why enhancement is a
> good thing to do in a running OpenJPA system, but my hope is that we
> can make these be mostly quality-of-service issues, and not
> functionality issues.
>
> I've been pretty successful so far, but there are a few limitations in
> some environments. I'd like to get some feedback about what to do
> about these limitations. First, I'll describe my approach:
>
> I started looking into this when Marc told me about the new
> retransformation capabilities in Java 6. (Briefly, in Sun's Java 6
> impl, it is no longer necessary to specify a javaagent flag in order
> to retransform classes.) This led me to investigate retransforming
> entity types to add field-tracking code to method blocks without
> violating any of the retransformation rules. I combined this with a
> subclassing approach so that instances created by OpenJPA (vs. created
> by the user with 'new') can be treated as proper persistence-capable
> instances.
>
> Since retransformation does not allow fields, methods, or interfaces
> to be added to a class, I added a helper class that maintains a static
> map of *all* the instances created with 'new' that have been
> persisted; this map associates a managed instance with a
> ReflectingPersistenceCapable instance, which implements
> PersistenceCapable via reflection, as the name implies, and can wrap
> unenhanced managed instances for OpenJPA's needs.
>
> So, the only functionality cost in a Java 6 environment is the extra
> overhead of occasionally looking up the PersistenceCapable for a given
> unenhanced managed instance. There is still a bit of a performance
> cost, since I needed to do reflection in some places instead of direct
> field access because of details of the class hierarchy and field
> access restrictions.
>
> In a Java 5 environment, if you specify a new javaagent class, then
> the situation is exactly the same. IMO this improves on our current
> javaagent in that it releases the restriction that persistent types
> must be listed in the first persistence unit in persistence.xml, and
> because the redefinition logic is never invoked against any
> non-managed types.
>
> If you do not specify a javaagent class in Java 5, then OpenJPA cannot
> do redefinition. However, it can still create subclasses with logic in
> overridden setters and getters. So, in Java 5 without a javaagent but
> with property access, we only lose dirty tracking and lazy loading for
> instances created by  the user code. This only matters for cases where
> an instance so created is flushed (for dirty tracking) or cleared (for
> lazy loading).
>
> In Java 5 without a javaagent and with field access, things are a bit
> worse -- OpenJPA can't do any dirty tracking or lazy loading, either
> for instances created by the user code or for instances created by
> OpenJPA.
>
> We could improve the situation a bit for field access and post-flush
> property access in Java 5 with no javaagent. This would require state
> comparison, though, which is not the fastest thing in the world, and
> requires that we hold hard references to all applicable instances for
> the duration of the EM's life (all user-created property access
> instances, and all field access instances regardless of how they were
> created).
>
>
> So, I'm looking for answers to the following questions in particular:
>
> 1. what should we do about { Java 5, no javaagent, field access }?
> Should we support this configuration, including the corresponding
> extra overhead, or should we require either property access or a
> javaagent specified in these configurations?
>
> 2. what should we do about { Java 5, no javaagent, property access,
> flushed | cleared instances }? There is a much lower impact to doing
> the dirty tracking in these configurations, since the scope is
> narrower. However, we might also be able to just not allow flush or
> clear or multiple sequential transactions if the persistence context
> has references to unenhanced, unredefined user-created instances.
>
> Thoughts?
>
> (Oh, and the code is still a work in progress -- everything I
> discussed above is working, but serialization and cloning and
> detachment are untested, and will probably require some tweaks to get
> working correctly.)
>
> -Patrick
>
> --
> Patrick Linskey
> 202 669 5907
>


-- 
Patrick Linskey
202 669 5907