Optimized cache handling (QueryCache for findId, use QueryCache before BeanCache) #2937

rPraml · 2023-01-19T07:08:51Z

In this PR I'll try to improve the cache-handling in ebean.

Optimized dependent table handling this is required for further refactoring. It shouldn't break anything. Note "prepareQuery" is now a "one-time" operation. Further invocations will not consume CPU-time, but we may update the query plan, if we have cache hits
Hit QueryCache before BeanCache, if both are enabled The QueryCache is the faster one, so it should be hit first when both QueryCache and BeanCache is enabled.
BUG: DB is hit unnecessarily, if deleted bean is found in PC-cache This is a bug, fixed by FIX: return immediately on PC-cache hit, even if bean is deleted
NEW: Support for queryCache on find-by-id This is an improvement, as you can use QueryCache now also for ID-queries. In our use case, we have a central configuration, that is retrieved very often and we want to keep that object in memory

Some important notes about Caches (maybe worth to add to documentation)

While BeanCaches store objects in serialized form as "Key/value" pairs, QueryCaches are real object-caches, that means, you get references to an existing object. No deserializion is needed
if you modify such a object, you modify the object in the cache and the next query will give you that object
There is some code to do "shallow copies" on read/write queries on lists, but not for beans in that list
it may be a good idea to use "setUseQueryCache" always with "setUseReadOnly"
The "DefaultServerCache" (used for bean & query-cache) implementation has some logic to remove old entries, but it uses internally SoftReferences, so that the GC can remove every entry at any time if it comes to a low memory situation (in other words, there is no guarantee that entries will remain in the cache for the specified TTL times etc.)

This requires inlining findIdCheckPersistenceContextAndCache

rPraml

I'm finished with this change. I tried to separate this PR in easily reviewable commits.
Can you review this as one PR or should I create one PR after the other.

What do you think about the change of first hitting the (faster) QueryCache and then the (slower) BeanCache?

rPraml · 2023-01-19T07:26:33Z

ebean-core/src/main/java/io/ebeaninternal/server/core/OrmQueryRequest.java

@@ -687,7 +722,7 @@ private boolean readAuditQueryType() {
  }

  public void putToQueryCache(Object result) {
-    beanDescriptor.queryCachePut(cacheKey, new QueryCacheEntry(result, dependentTables, transaction.getStartNanoTime()));
+    beanDescriptor.queryCachePut(cacheKey, new QueryCacheEntry(result, queryPlan.dependentTables(), transaction.getStartNanoTime()));


During an ormQueryRequest, the current queryPlan should be always present, so it should be safe, to use it here. I did not see any code, that calls "addDependetTables" multiple times.

rPraml · 2023-01-19T14:39:07Z

ebean-core/src/main/java/io/ebeaninternal/server/core/DefaultServer.java

-        if (o != null) {
-          if (o.isDeleted()) {
-            // Bean was previously deleted in the same transaction / persistence context
-            return null;


returning "null" here, means that we will not find anything, even if we hit the DB again (see failing test)

rPraml · 2023-01-19T14:41:04Z

ebean-core/src/main/java/io/ebeaninternal/server/core/DefaultServer.java

      }
+      BeanDescriptor<T> desc = spiQuery.getBeanDescriptor();


this is mainly the inlined findIdCheckPersistenceContextAndCache.
I did not find an easy way to split it into separate methods. What do you think, can we keep this so or should I try to reduce complexity of that method

rPraml · 2023-01-19T14:41:37Z

ebean-core/src/main/java/io/ebeaninternal/server/core/DefaultServer.java

@@ -1093,15 +1103,18 @@ private <T> T extractUnique(List<T> list) {
  public <T> Set<T> findSet(Query<T> query, Transaction transaction) {
    SpiOrmQueryRequest request = buildQueryRequest(Type.SET, query, transaction);
    request.resetBeanCacheAutoMode(false);
+    if (request.isQueryCacheActive()) {
+      request.prepareQuery();


I think, the queryCache should be checked BEFORE the bean cache.

rPraml · 2023-01-19T14:42:29Z

ebean-core/src/main/java/io/ebeaninternal/server/core/OrmQueryRequest.java

@@ -68,6 +70,7 @@ public boolean isDeleteByStatement() {
    } else {
      // delete by ids due to cascading delete needs
      queryPlanKey = query.setDeleteByIdsPlan();
+      queryPlan = null;


when the queryPlanKey changes, we must throw away also the cached queryPlan

rPraml · 2023-01-19T14:42:58Z

ebean-core/src/main/java/io/ebeaninternal/server/core/OrmQueryRequest.java

-    beanDescriptor.prepareQuery(query);
-    adapterPreQuery();
-    queryPlanKey = query.prepare(this);
+    if (!prepared) {


prepareQuery is only executed once now

rPraml · 2023-01-19T14:43:50Z

ebean-core/src/main/java/io/ebeaninternal/server/core/OrmQueryRequest.java

@@ -469,7 +486,10 @@ public BeanPropertyAssocMany<?> manyPropertyForOrderBy() {
   * query plan for this query exists.
   */
  public CQueryPlan queryPlan() {
-    return beanDescriptor.queryPlan(queryPlanKey);
+    if (queryPlan == null) {
+      queryPlan = beanDescriptor.queryPlan(queryPlanKey);


here we cache the current query plan, that made the "dependentTables" refactoring possible

rPraml · 2023-01-19T14:45:15Z

ebean-test/src/test/java/org/tests/basic/TestDeleteByIdCollection.java

    c0Back = DB.find(Customer.class, c0.getId());
    c1Back = DB.find(Customer.class, "" + c1.getId());
+    assertThat(LoggedSql.stop()).isEmpty();


This should not hit the DB (but it currently does on master branch)

…use QueryCache before BeanCache)

rPraml changed the title ~~[In progress] Optimized dependent table handling (RPr)~~ [In progress] Optimized cache handling (RPr) Jan 19, 2023

Optimized dependent table handling (RPr)

717474a

rPraml force-pushed the improve-caches branch from 149edc3 to 717474a Compare January 19, 2023 07:24

rPraml added 3 commits January 19, 2023 08:37

Hit QueryCache before BeanCache, if both are enabled

1622fbc

BUG: DB is hit unnecessarily, if deleted bean is found in PC-cache

4c708c4

FIX: return immediately on PC-cache hit, even if bean is deleted

b0f11f9

This requires inlining findIdCheckPersistenceContextAndCache

rPraml force-pushed the improve-caches branch from 4fe6e23 to fd67fc5 Compare January 19, 2023 14:04

NEW: Support for queryCache on find-by-id

afc789d

rPraml force-pushed the improve-caches branch from fd67fc5 to afc789d Compare January 19, 2023 14:09

rPraml commented Jan 19, 2023

View reviewed changes

rPraml changed the title ~~[In progress] Optimized cache handling (RPr)~~ Optimized cache handling (QueryCache for findId, use QueryCache before BeanCache) Jan 19, 2023

rPraml added 2 commits August 8, 2023 13:41

Merge branch 'master-rob' into improve-caches

a64f32b

Removed obsolete code

c58189e

rPraml mentioned this pull request Aug 9, 2023

[WIP] Optimized the DependantTable handling #3150

Open

rPraml added a commit to FOCONIS/ebean that referenced this pull request Aug 10, 2023

PR ebean-orm#2937 - Optimized cache handling (QueryCache for findId, …

cd2cff0

…use QueryCache before BeanCache)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Optimized cache handling (QueryCache for findId, use QueryCache before BeanCache) #2937

Optimized cache handling (QueryCache for findId, use QueryCache before BeanCache) #2937

rPraml commented Jan 19, 2023 •

edited

rPraml left a comment

rPraml Jan 19, 2023

rPraml Jan 19, 2023

rPraml Jan 19, 2023

rPraml Jan 19, 2023

rPraml Jan 19, 2023

rPraml Jan 19, 2023

rPraml Jan 19, 2023

rPraml Jan 19, 2023

Optimized cache handling (QueryCache for findId, use QueryCache before BeanCache) #2937

Are you sure you want to change the base?

Optimized cache handling (QueryCache for findId, use QueryCache before BeanCache) #2937

Conversation

rPraml commented Jan 19, 2023 • edited

rPraml left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rPraml commented Jan 19, 2023 •

edited