On Jan 13, 2009, at 18:35, Sanne Grinovero wrote:
thank you, you helped me a lot; still I would like to propose an
evolution to cover all use cases correctly.
I've already implemented and tested this, in roughly three steps:
1)
when moving the cursor outside of the loaded window boundaries I
extract the new EntityInfos from the index, then verify if any
matching entity is already contained in the persistence context: if it
is, I discard the EntityInfo and I "batch load" the entities
identified with the remaining EntityInfos
( I am doing this by building the EntityKey to look at
session.getPersistenceContext().containsEntity( key ) ).
2)
Immediately after batch loading, I evict them all right away and
discard the references:
this may look useless but the goal was to put data in the second level
cache, useful for later and possibly living in a more efficient data
structure than a simple soft map.
In a scrollable usage, the 2nd level cache is typically a bad idea (ie
we want to load more that what's available in memory). Plus you don't
know if the entity will be marked for caching.
Please note that I'm evicting only those objects which were not
attached before the loading, so nothing changed in the persistence
context from a client-code-point-of-view.
The original implementation also did a one-by-one load on each entity
as a last initialization step; this is
not needed here as I use load on each entity later on, when I'm sure
the clients actually needs that data.
3)
After that when a get() is called I use load( entityInfo ) to get a
managed instance to return to the user.
The end result is:
* I don't put stuff in the persistence context the user doesn't expect
well you do if the EVICT cascade policy is different that the EAGER
fetching policy
* I don't evict stuff that was attached before
well you do if the EVICT cascade policy is different that the EAGER
feching policy
* no memory leaks
it works correctly even without a second level cache, but of course
you only get good performance when using one.
I really don't like this idea. I would rather get the 80% scenario
work good (as described in the previous email) and not rely on the 2nd
level cache.
You can attach the patch in JIRA but I think we should go for the
previous version.
Do you think this looks reasonable? Besides the EntityKey creation the
code is much simplified, and the lines of code are reduced.
Would you prefer me to keep a soft reference to the loaded entities to
re-attach to the session somehow?
I'll prefer to avoid the soft map for the entities: IMHO having an
additional (inefficient) caching layer on top of object loading, which
has probably lots of other caching layers, is just making the code
more complex.
Sanne
2009/1/13 Emmanuel Bernard <emman...@hibernate.org>:
Clearly we cannot solve all use cases, so let's focus on the most
useful
one.
People use scrollable resultset to walk through all objects in a
window one
after the other wo skipping them.
while( rs.hasNext() ) {
Address a = (Address) rs.get(0)[0];
doSomething(a);
if (i++ % 100 == 0) s.clear();
}
So we should consider that:
- eviction is done by the user
- eviction can happen behind the impl's back
=>
place objects and EntityInfo in a Soft reference (if we need to keep
EntityInfo, don't remember).
when we give an entity, make sure session.contains(entity) == true.
Otherwise, discard the detached one and reload it.
That should help for most cases.
On Jan 11, 2009, at 18:21, Sanne Grinovero wrote:
Hello,
I'm in need of some help about how to fix this memory leak at
HSEARCH-310
in ScrollableResultsImpl of Search, there currently is:
* an internal cache of all loaded EntityInfo
* an internal cache of all loaded managed Entities
Both caches are never emptied during the scrolling; the
implementation
is loading a batch-sized list each time an entity is requested which
was not loaded before and adds new objects to the caches.
We had said to use some Soft ReferenceMap to fix this, but
actually we
should evict the loaded entities which where not requested from the
client code, or I'll still be leaking memory in Hibernate's
persistence context.
I expect the most frequent use case for a ScrollableResults should
be
to iterate at will and periodically evict objects in the client
code;
Still the client could skip some entities we loaded by batching.
At first I thought to keep track of which references were used and
evict the other ones, but this isn't a good solution as the client
could be loading some entities by other means (i.e. another query)
and
won't be happy if I'm evicting some random entities: I don't have
enough information to know for sure what to evict and what is
going to
be used later.
Also a bug, consequence of current implementation, is that if you
get
an object, then you evict it, and then try scrollable.get() you
don't
get an attached object but again the evicted instance; as a client
of
the Hibernate library I expect all objects I ask for to be
attached by
default.
My idea to fix this should be something like:
1) load the batch
2) evict them all
3) and then load one by one as requested, relying on the second
level
cache (I suppose point 1 should have warmed it)
But this has other flaws.. like I should not evict (2) those objects
already existing in the persistence context.
Ideally I should avoid to preload objects already present in the
persistence context; I am considering to look in the
session.persistenceContext.entitiesByKey map to only load+evict by
batch those entities not already managed.
Is there some way to populate the second level cache skipping the
actual load in the persistence context?
Should I keep my own "soft map cache" in the scroll implementation
anyway, and somehow detach and reattach them as needed?
Am I missing some trivial better solution, or is my solution not
going
to work at all?
thanks,
Sanne
_______________________________________________
hibernate-dev mailing list
hibernate-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/hibernate-dev
