Slow HashMap update when the new Map is large

[dcheung2]

Fix 1. Batching in JoinMapStore

Given

class Foo{
 HashMap<String, Bar> map;
}
class Bar{
 String value;
}

HashMap<String, Bar> newMap=new HashMap<>();
for(int i=0;i<10000;i++){
  newMap.put(Integer.toString(i), bar);
}

Foo foo = pm.getObjectById(Foo.class, \"1\");
Foo newFoo = new Foo();
pm.makePersistent(newFoo);

Slow performance for
foo.map.clear();
foo.map.putAll(newMap);
or
foo.map=newMap;
or
newFoo.map.clear();
newFoo.map.putAll(newMap);
or
newFoo.map=newMap;

Under the hood JoinMapStore.putAll execute getValue for N entity and each introduce a SQL round-trip.
And the internalPut and internalUpdate methods does not using JDBC batch.

As a result 2N of SQL round trip, or O(N) performance.

This naive patch attempt to optimize the logic in to a O(1) by using ONE SELECT statement (reusing MapEntrySetStore).
And enable using the batch API for INSERT/UPDATE operations.

Also overload the method MapEntrySetStore.iterator and attempt to SELECT only given keys, rather than SELECT the whole map.
This should speed up some another case when the map is large and added few entries to it. Avoiding reading unnecessary rows into L1Cache.

With a limitation of only supporting SingleFieldMapping.
With a limitation up to 1024 keys (or fallback to whole table selection).

Tested with MySQL 8.

It is very likely does work with Oracle due to 1000 element IN limit (if I remember it correctly).

It is possible to expand to MultiMapping with DBMS whom support IN with multi columns
(e.g. PostgreSQL (foo,bar) IN ((1,2),(3,4)))
or long where generation for DBMS does not support IN with multi columns
(i.e. foo in (1,3) AND bar IN (2,4) AND ((foo = 1 AND bar=2 ) or (foo=2 AND bar=4)))

But none of these fancy case are current my interest nor priority.

---

Fix 2.

JoinMapStore incorrectly cached generated getStmt (and few other variables) for getValue with FetchPlan. And possibility use it with a another FetchPlan.
Move the cache to queryManager with a cacheKey contains the table name and FetchPlan.

---

Fix 3.

Simple typo/variable name correction in JoinListStore for better JDBC batch.

[lgtm-com]

This pull request introduces 6 alerts when merging 8c550d3 into c263588 - view on LGTM.com

new alerts:

• 6 for Spurious Javadoc @param tags

[andyjefferson]

While MapStore doesn't do batching, and there is already an issue to sort this out (so if something would resolve that it would be very welcome) ,I can't apply this patch. Reason being : we have a set of tests and this breaks some of them.

Go to this repository, build framework and samples, and then navigate to jdo/identity. Type "mvn clean test". Tests such as
HashtableTest, PropertiesTest, etc fail (on H2) with errors like

Caused by: org.h2.jdbc.JdbcSQLDataException: 
Parameter "#1026" is not set; SQL statement:
SELECT A0."KEY",A0."VALUE" FROM HASHTABLE3_ITEMS A0 WHERE A0.IDENTIFIERA_OID = ? AND A0.IDENTIFIERB_OID = ? AND A0."KEY" IS NOT NULL AND "KEY" IN (?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?)  [90012-200]
        at org.datanucleus.tests.types.HashtableTest.testNormalQueriesPrimitive(HashtableTest.java:233)

Bearing in mind the test has 5 entries in the map, it makes no sense (to me) to have a cached statement of 1024 entries.

[andyjefferson]

See also #395 where I enabled the batching for internalPut/internalUpdate methods, backported from master branch

[dcheung2]

HashtableTest, PropertiesTest, etc fail

It looks a off by one issue when the owner object has more then one OID columns.

A0."KEY" IS NOT NULL AND "KEY"
And I also spotted the 2nd "KEY" doesn't have a table alias

Actually I tried to generate the SQL by SelectStatement but I cannot make it work with " IN " with unknown number of variable runtime.

I am also thinking to move the cached SQL to QueryManager.getDatastoreQueryCompilation
As you see I am hacking the iteratorSelectWithKeysStmtLockedSql and I assume
SQLStatement.EXTENSION_LOCK_FOR_UPDATE only append to the SQL.

Bearing in mind the test has 5 entries in the map, it makes no sense (to me) to have a cached statement of 1024 entries.

It could be using IN-list-padding (https://www.jooq.org/doc/latest/manual/sql-building/dsl-context/custom-settings/settings-in-list-padding/) for higher performance but could cause the SQL caching a bit more complex.

Or doesn't cache it at all and generate a new SQL everytime.

I have to focus back to my main project and will not have time to help clean it up for a week or two.
I will rebase by branch later

[andyjefferson]

Closing this since would need reworking. Raise a new one (preferably using multiple smaller PRs if proposing large changes, makes it more manageable) when you have something that works. Also anything on 5.2 would need to be relatively small, too late in its development for major changes, unlike 6.0 which is open to more substantial changes

The DatastoreQueryCompilation cache is just for JDOQL/JPQL/SQL queries invoked via the API. A JoinMapStore's queries are something else, specific to a field, and so should be cached separately (hence why the SQL has been cached in the JoinMapStore thus far, since that is specific to a field).