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.)

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)

…e BeanCache)

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

rPraml force-pushed the improve-caches branch 3 times, most recently 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 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)

rPraml force-pushed the improve-caches branch 2 times, most recently from 791e85a to d79867f Compare October 7, 2024 12:33

Optimized cache handling (QueryCache for findId, use QueryCache befor…

5b3bfcf

…e BeanCache)

rPraml force-pushed the improve-caches branch from d79867f to 5b3bfcf Compare October 7, 2024 14:53

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

Loading

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 Loading

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

Loading