On 26/03/15 16:22, Steve Ebersole wrote:
The contract I suggested btw is missing one use case I know we have
discussed in regards to OGM a few times... the ability to store
references to datastore specific locators (I think the use case was to
efficiently load collections through that datastore reference). But
that (obviously) requires passing extra, specific state into
the createEntityEntry method. Not really sure the best way to handle
such things tbh.
If we did have a "creation Context" parameter passed to the
createEntityEntry, we would be instantiating more objects, however if we
went with Sanne' suggestion if caching the EnityEntryFactory with the
EntityPersister, how many objects would be actually created? I am not
sure about the relationship between Entity, EntityEntry and Session but
could this a possibility if there is a 1-1 relationship between
EntityPersister and no. of different Entities definitions?
On Thu, Mar 26, 2015 at 11:16 AM, Steve Ebersole
<steve(a)hibernate.org
<mailto:steve@hibernate.org>> wrote:
Apparently I hit some key combo that means send :) To finish up...
On Thu, Mar 26, 2015 at 10:58 AM, Steve Ebersole
<steve(a)hibernate.org <mailto:steve@hibernate.org>> wrote:
On Thu, Mar 26, 2015 at 10:20 AM, Sanne Grinovero
<sanne(a)hibernate.org <mailto:sanne@hibernate.org>> wrote:
Concurrency
Since the goal is to share the ImmutableEntityEntry
instance among
multiple threads reading it, I'd rather make sure that it
is really
immutable. For example it now holds onto potentially lazy
initialized
fields, such as getEntityKey().
If it's not possible to make it really immutable (all
final fields),
we'll need to make it threadsafe and question the name I'm
proposing.
The specific use-case John is interested in does indeed need
to be completely thread-safe and fully concurrent.
LockMode
From a logical perspective of users, one might think
that an entity
being "immutable" doesn't necessarily imply I can't lock
on it..
right? I'm not sure how far it could be a valid use case,
but these
patches are implying that one can't lock an immutable
entity anymore,
as the lock state would be as immutable as anything else
on the
EntityEntry.
Are we good with that? Alternatively one might need to
think to
separate the lock state handling from the EntityEntry.
Conceptually there is nothing wrong with requesting a READ
lock on an immutable entity. But like you said, what is the
logical expectation there? IMO there should be none. But if
we decide that it is OK to req
But if we decide that it is OK to request a lock on an immutable
entity, that is problematic for the idea of a "SharedEntityEntry"
or an "ImmutableEntityEntry" because a lock is associated with a
Session which is what the EntityEntry is meant to model... an
entity's information in relation to a Session. Aka, it now needs
to hold state.
Notice I said immutable here and not READ_ONLY which is a
specific distinction which is important to other parts of your
email that I will address in a second email.
On smaller details:
# org.hibernate.engine.internal.SharedEntityEntry is
hosted in an
.internal package, I don't think it's right to refactor
all the public
API javadoc which was referring to EntityEntry to now
refer to the
internal implementation.
# things like EntityEntryExtraState should probably get
moved to
.internal packages as well now - we couldn't do that
before without
breaking either encapsulation or APIs.
+1
In terms of git patches, the complexity of the changeset
risks to get
a bit our of hand. What about we focus on creating a clean
pull
request which focuses exclusively on making EntityEntry an
interface,
and move things to the right packages and javadoc?
Agree 100%
You'd have a trivial EntitEntryFactory, and we can then
build the
evolution on top of that, not least maybe helping out by
challenging
some points in parallel work.
These are the things I'd leave for a second iteration:
- add various implementations of EntityEntry iteratively,
as needed
- the strategy such a Factory would use the pick an
implementation
- ultimately, make it possible for an integrator to
override such a Factory
This all seems reasonable. For 5.0 I think the most important
thing is to nail down the idea of EntityEntry as an interface,
introduce a Factory for building them and agree on the signature
for building them. Granted we may need iteration to finalize the
actual Factory signature, especially as OGM finally starts to use
it, but I think that in general we can get it pretty close. Worst
case we just pass high-level constructs like the EntityPersister
(which OGM supplies custom impls) and the Session and all the
"EntityEntry state". For the purpose of starting a discussion:
public interface EntityEntryFactory {
public EntityEntry createEntityEntry(
PersistenceContext persistenceContext,
EntityPersister persister,
Status status,
Serializable id,
Object version,
Object[] loadedState,
Object rowId,
LockMode lockMode,
boolean existsInDatabase,
boolean disableVersionIncrement,
boolean lazyPropertiesAreUnfetched);
}
I would suggest a "creation context" method param object to pass
in here, but seeing as how we are trying to stop instantiations...
I would prefer the definition of the EntityEntryFactory to use be
defined via SessionFactoryBuilder interface. I already have plans
to have a auto-loaded hook for integrators to be able to adjust
the SessionFactoryBuilder. But as a Service is fine too.
For example with Hibernate OGM we might want to override / re
configure the factories to use custom EntityEntry
implementations -
requirements are not fully clear at this point but it
seems likely.
The priority being to define the API as that would be a
blocker for
5.0, we have then better choices to leave more smarter and
advanced
EntityEntry implementations for the future; we'd still need to
implement at least the essential ones to make sure the API
of the
EntityEntryFactory has all the context it needs.
Thanks,
Sanne
On 24 March 2015 at 09:27, John O'Hara <johara(a)redhat.com
<mailto:johara@redhat.com>> wrote:
> Steve,
>
> Have you had chance to look at this? Do you have any
comments/observations?
>
> Thanks
>
> John
>
>
> On 17/03/15 09:24, John O'Hara wrote:
>
> Steve,
>
> I have been having a think about the EntityEntry
interface, and have forked
> a branch here:
>
>
https://github.com/johnaoahra80/hibernate-orm/tree/EntityEntryInterface
>
> I know it is nowhere near complete, but was this the
sort of idea you had in
> mind?
>
> Thanks
>
> John
>
>
> On 13/03/15 09:44, John O'Hara wrote:
>
> EntityEntry retains a reference to a persistenceContext
internally that
> org.hibernate.engine.spi.EntityEntry#setReadOnly makes
calls to, is this
> where the session reference is kept? As
> org.hibernate.engine.spi.PersistenceConext is an
interface could we have a
> different implementation for this use case? e.g. an
> ImmutablePersistenceContext that could be shared across
sessions?
>
> For the bytecode enhancement, could we change the
enhancer so that it adds
> an EntityEntry interface with javassist.
> ClassPool.javassist.ClassPool.makeInterface()() as
opposed to adding a class
> javassist.ClassPool.makeClass()? I need to have a look
at javassit to
> confirm what javassist.ClassPool.makeInterface() does.
>
> Thanks
>
> John
>
> On 12/03/15 18:52, Steve Ebersole wrote:
>
> It is possible. Although some of the changes are
particularly painful.
> Most of EntityEntry, if it is an interface, can be made
to work with your
> use case.
org.hibernate.engine.spi.EntityEntry#setReadOnly I think
is the
> one exception, because:
> 1) your use case needs it
> 2) it expects the Session to be available internally
(its not passed)
>
> The bigger thing I am worried about for you is the
bytecode stuff, as that
> ties very tightly with EntityEntry.
>
_______________________________________________
hibernate-dev mailing list
hibernate-dev(a)lists.jboss.org
<mailto:hibernate-dev@lists.jboss.org>
https://lists.jboss.org/mailman/listinfo/hibernate-dev
--
John O'Hara
johara(a)redhat.com
JBoss, by Red Hat
Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod Street, Windsor,
Berkshire, SI4 1TE, United Kingdom.
Registered in UK and Wales under Company Registration No. 3798903 Directors: Michael
Cunningham (USA), Charlie Peters (USA), Matt Parsons (USA) and Michael O'Neill
(Ireland).