openjpa-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Patrick Linskey" <plins...@gmail.com>
Subject Re: unenhanced class support
Date Thu, 26 Jul 2007 02:06:13 GMT
> 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 <mprudhom@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 <Craig.Russell@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 <Craig.Russell@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

Mime
View raw message