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:18:04 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

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 <plinskey@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 <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
>


-- 
Patrick Linskey
202 669 5907

Mime
View raw message