On Thu, Jul 4, 2019 at 2:45 AM Andrus Adamchik <and...@objectstyle.org>
wrote:
> > My intention was to separate the local and shared query caches while not
> > eliminating either, so that is the reason both are used. So if you have
> > explicitly set a custom localQueryCache then calling getQueryCache() will
> > allow you to still access the *shared* query cache as well.
>
> But this is called by the framework, not the user. And from what I see,
> some of Cayenne code calls the former, and other - the later.
>
> I general I am thinking we should make the type of per-context QueryCache
> a property of the entire runtime (it is either a segment of the main cache,
> or a separate per-context object ... either one or the other for all
> contexts). Is this too limiting?
>
I think that would be too limiting. I would prefer to be able to use caches
with both behavoirs/lifecycles in the same context. The choice of the scope
or lifetime of the cache should be at the query level rather than the
context level.
I realize now this commit didn't have unit tests to target the new
behaviors when the localQueryCache is set. I have some tests for it in my
application, but those didn't make it into Cayenne. And I need to write
some more to check the behavior of the shared query cache when the
localQueryCache is set.
I'm out of the office until Monday, so I can't work on it until then, but
we can keep discussing beforehand. Thanks.
> On Jul 3, 2019, at 5:34 PM, John Huss <johnth...@gmail.com> wrote:
> >
> > On Wed, Jul 3, 2019 at 5:33 AM Andrus Adamchik <and...@objectstyle.org>
> > wrote:
> >
> >> Hi John,
> >>
> >> I think I understand where you are going with this feature. As we
> >> discussed before, sometimes keeping a context cache as a transparent
> region
> >> of the main cache is undesirable, e.g. for memory management reasons.
> But I
> >> have some questions about the implementation committed to "master":
> >>
> >> 1. The commit seems incomplete - sometimes we use the new
> >> "getLocalQueryCache()", sometimes - the old "getQueryCache()".
> >>
> >
> > My intention was to separate the local and shared query caches while not
> > eliminating either, so that is the reason both are used. So if you have
> > explicitly set a custom localQueryCache then calling getQueryCache() will
> > allow you to still access the *shared* query cache as well.
> >
> >
> >
> >> 2. Having both "queryCache" and "localQueryCache" as ivars of
> BaseContext
> >> may be confusing. What do you think of moving cache type selection
> logic in
> >> ObjectContextFactory?
> >>
> >
> > Do you mean exposing this in the API of the interface? Currently I am
> > configuring this with a DataContextFactory subclass like this:
> >
> > *public* *static* *class* Factory *extends* DataContextFactory {
> >
> > @Override
> >
> > *protected* DataContext newInstance(DataChannel parent, ObjectStore
> > objectStore) {
> >
> > *return* *new* IcsDataContext(parent, objectStore);
> >
> > }
> >
> > @Override
> >
> > *protected* ObjectContext createdFromDataDomain(DataDomain parent) {
> >
> > DataContext result = (DataContext) *super*.createdFromDataDomain(parent);
> >
> > result.setLocalQueryCache(*new* MapQueryCache(Integer.*MAX_VALUE*)); //
> use
> > separate unbounded cache whose lifetime is bound to the ObjectContext
> itself
> >
> > *return* result;
> >
> > }
> >
> > @Override
> >
> > *protected* ObjectContext createFromDataContext(DataContext parent) {
> >
> > DataContext result = (DataContext) *super*.createFromDataContext(parent);
> >
> > result.setLocalQueryCache(*new* MapQueryCache(Integer.*MAX_VALUE*));
> >
> > *return* result;
> >
> > }
> >
> > @Override
> >
> > *protected* ObjectContext createFromGenericChannel(DataChannel parent) {
> >
> > DataContext result = (DataContext)
> *super*.createFromGenericChannel(parent);
> >
> > result.setLocalQueryCache(*new* MapQueryCache(Integer.*MAX_VALUE*));
> >
> > *return* result;
> >
> > }
> >
> > }
> >
> > I'm open to other ways of configuring this if you have ideas, perhaps
> like
> > using DI. Thanks for your feedback.
> >
> >
> >
> >>
> >> Thanks,
> >> Andrus
> >>
> >>
> >>> On Jul 1, 2019, at 5:53 PM, johnth...@apache.org wrote:
> >>>
> >>> This is an automated email from the ASF dual-hosted git repository.
> >>>
> >>> johnthuss pushed a commit to branch master
> >>> in repository https://gitbox.apache.org/repos/asf/cayenne.git
> >>>
> >>>
> >>> The following commit(s) were added to refs/heads/master by this push:
> >>> new 597376a CAY-2589 Allow optionally using a local query cache
> >> that is separate from the shared query cache.
> >>> 597376a is described below
> >>>
> >>> commit 597376ae558b9a0c5ddc4390da188b9530204e3d
> >>> Author: John Huss <johnth...@apache.org>
> >>> AuthorDate: Mon Jun 3 16:57:20 2019 -0500
> >>>
> >>> CAY-2589 Allow optionally using a local query cache that is separate
> >> from the shared query cache.
> >>>
> >>> The local query cache can be customized by creating a custom
> >> DataContextFactory.
> >>>
> >>> A separate cache can prevent memory leaks from occurring if you used
> >> non-expiring cache
> >>> groups (like the default cache group perhaps) along with the local
> >> cache, which wasn't
> >>> intuitive if you expected the lifetime of the cache to match the
> >> lifetime of the ObjectContext.
> >>> ---
> >>> RELEASE-NOTES.txt | 1 +
> >>> .../main/java/org/apache/cayenne/BaseContext.java | 27
> >> +++++++++++++++++++++-
> >>> .../cayenne/util/ObjectContextQueryAction.java | 9 +++++++-
> >>> 3 files changed, 35 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/RELEASE-NOTES.txt b/RELEASE-NOTES.txt
> >>> index 6f45986..3aa2fa8 100644
> >>> --- a/RELEASE-NOTES.txt
> >>> +++ b/RELEASE-NOTES.txt
> >>> @@ -36,6 +36,7 @@ CAY-2569 Custom 'Naming Strategy' in Cayenne Modeler
> >>> CAY-2570 Use MySQL adapter for latest versions of MariaDB
> >>> CAY-2579 Review and possibly relax usage of readonly flag of
> >> ObjRelationship
> >>> CAY-2585 Rename scalarQuery and params methods in SQLSelect
> >>> +CAY-2589 - Allow optionally using a local query cache that is separate
> >> from the shared query cache.
> >>>
> >>> Bug Fixes:
> >>>
> >>> diff --git
> >> a/cayenne-server/src/main/java/org/apache/cayenne/BaseContext.java
> >> b/cayenne-server/src/main/java/org/apache/cayenne/BaseContext.java
> >>> index 981db73..ffae953 100644
> >>> --- a/cayenne-server/src/main/java/org/apache/cayenne/BaseContext.java
> >>> +++ b/cayenne-server/src/main/java/org/apache/cayenne/BaseContext.java
> >>> @@ -98,6 +98,7 @@ public abstract class BaseContext implements
> >> ObjectContext {
> >>> // registry
> >>> protected transient DataChannel channel;
> >>> protected transient QueryCache queryCache;
> >>> + protected transient QueryCache localQueryCache;
> >>> protected transient EntityResolver entityResolver;
> >>>
> >>> protected boolean validatingObjectsOnCommit = true;
> >>> @@ -469,17 +470,41 @@ public abstract class BaseContext implements
> >> ObjectContext {
> >>> @Override
> >>> public abstract Collection<?> uncommittedObjects();
> >>>
> >>> + /**
> >>> + * Used for storing cached query results available to all
> >> ObjectContexts.
> >>> + */
> >>> public QueryCache getQueryCache() {
> >>> attachToRuntimeIfNeeded();
> >>> return queryCache;
> >>> }
> >>>
> >>> /**
> >>> - * Sets a QueryCache to be used for storing cached query results.
> >>> + * Sets a QueryCache to be used for storing cached query results
> >> available to all ObjectContexts.
> >>> */
> >>> public void setQueryCache(QueryCache queryCache) {
> >>> this.queryCache = queryCache;
> >>> }
> >>> +
> >>> + /**
> >>> + * Used for storing cached query results available only to this
> >> ObjectContext.
> >>> + * By default the local query cache and the shared query cache
> >> will use the same underlying storage.
> >>> + *
> >>> + * @since 4.2
> >>> + */
> >>> + public QueryCache getLocalQueryCache() {
> >>> + attachToRuntimeIfNeeded();
> >>> + return localQueryCache != null ? localQueryCache :
> >> getQueryCache();
> >>> + }
> >>> +
> >>> + /**
> >>> + * Sets a QueryCache to be used for storing cached query results
> >> available only to this ObjectContext.
> >>> + * By default the local query cache and the shared query cache
> >> will use the same underlying storage.
> >>> + *
> >>> + * @since 4.2
> >>> + */
> >>> + public void setLocalQueryCache(QueryCache queryCache) {
> >>> + this.localQueryCache = queryCache;
> >>> + }
> >>>
> >>> /**
> >>> * Returns EventManager associated with the ObjectStore.
> >>> diff --git
> >>
> a/cayenne-server/src/main/java/org/apache/cayenne/util/ObjectContextQueryAction.java
> >>
> b/cayenne-server/src/main/java/org/apache/cayenne/util/ObjectContextQueryAction.java
> >>> index 75dbdfa..acef54e 100644
> >>> ---
> >>
> a/cayenne-server/src/main/java/org/apache/cayenne/util/ObjectContextQueryAction.java
> >>> +++
> >>
> b/cayenne-server/src/main/java/org/apache/cayenne/util/ObjectContextQueryAction.java
> >>> @@ -360,7 +360,7 @@ public abstract class ObjectContextQueryAction {
> >>> return !DONE;
> >>> }
> >>>
> >>> - QueryCache queryCache = getQueryCache();
> >>> + QueryCache queryCache = getLocalQueryCache();
> >>> QueryCacheEntryFactory factory = getCacheObjectFactory();
> >>>
> >>> if (cache) {
> >>> @@ -385,6 +385,13 @@ public abstract class ObjectContextQueryAction {
> >>> protected QueryCache getQueryCache() {
> >>> return ((BaseContext) actingContext).getQueryCache();
> >>> }
> >>> +
> >>> + /**
> >>> + * @since 4.2
> >>> + */
> >>> + protected QueryCache getLocalQueryCache() {
> >>> + return ((BaseContext) actingContext).getLocalQueryCache();
> >>> + }
> >>>
> >>> /**
> >>> * @since 3.0
> >>>
> >>
> >>
>
>
