LRU for JDBCHashMap loadOnInit=false, with tests.

node/src/integration-test/kotlin/net/corda/node/utilities/JDBCHashMapTestSuite.kt

@@ -25,8 +25,10 @@ class JDBCHashMapTestSuite {
         lateinit var transaction: Transaction
         lateinit var database: Database
         lateinit var loadOnInitFalseMap: JDBCHashMap<String, String>
+        lateinit var memoryConstrainedMap: JDBCHashMap<String, String>
         lateinit var loadOnInitTrueMap: JDBCHashMap<String, String>
         lateinit var loadOnInitFalseSet: JDBCHashSet<String>
+        lateinit var memoryConstrainedSet: JDBCHashSet<String>
         lateinit var loadOnInitTrueSet: JDBCHashSet<String>
 
         @JvmStatic
@@ -37,8 +39,10 @@ class JDBCHashMapTestSuite {
             database = dataSourceAndDatabase.second
             setUpDatabaseTx()
             loadOnInitFalseMap = JDBCHashMap<String, String>("test_map_false", loadOnInit = false)
+            memoryConstrainedMap = JDBCHashMap<String, String>("test_map_constrained", loadOnInit = false, maxBuckets = 1)
             loadOnInitTrueMap = JDBCHashMap<String, String>("test_map_true", loadOnInit = true)
             loadOnInitFalseSet = JDBCHashSet<String>("test_set_false", loadOnInit = false)
+            memoryConstrainedSet = JDBCHashSet<String>("test_set_constrained", loadOnInit = false, maxBuckets = 1)
             loadOnInitTrueSet = JDBCHashSet<String>("test_set_true", loadOnInit = true)
         }
 
@@ -50,8 +54,8 @@ class JDBCHashMapTestSuite {
         }
 
         @JvmStatic
-        fun createMapTestSuite(loadOnInit: Boolean): TestSuite = com.google.common.collect.testing.MapTestSuiteBuilder
+        fun createMapTestSuite(loadOnInit: Boolean, constrained: Boolean): TestSuite = com.google.common.collect.testing.MapTestSuiteBuilder
-                .using(JDBCHashMapTestGenerator(loadOnInit = loadOnInit))
+                .using(JDBCHashMapTestGenerator(loadOnInit = loadOnInit, constrained = constrained))
                 .named("test JDBCHashMap with loadOnInit=$loadOnInit")
                 .withFeatures(
                         com.google.common.collect.testing.features.CollectionSize.ANY,
@@ -65,8 +69,8 @@ class JDBCHashMapTestSuite {
                 .createTestSuite()
 
         @JvmStatic
-        fun createSetTestSuite(loadOnInit: Boolean): TestSuite = com.google.common.collect.testing.SetTestSuiteBuilder
+        fun createSetTestSuite(loadOnInit: Boolean, constrained: Boolean): TestSuite = com.google.common.collect.testing.SetTestSuiteBuilder
-                .using(JDBCHashSetTestGenerator(loadOnInit = loadOnInit))
+                .using(JDBCHashSetTestGenerator(loadOnInit = loadOnInit, constrained = constrained))
                 .named("test JDBCHashSet with loadOnInit=$loadOnInit")
                 .withFeatures(
                         com.google.common.collect.testing.features.CollectionSize.ANY,
@@ -96,31 +100,41 @@ class JDBCHashMapTestSuite {
     }
 
     /**
-     * Guava test suite generator for JDBCHashMap(loadOnInit=false).
+     * Guava test suite generator for JDBCHashMap(loadOnInit=false, constrained = false).
      */
     class MapLoadOnInitFalse {
         companion object {
             @JvmStatic
-            fun suite(): TestSuite = createMapTestSuite(false)
+            fun suite(): TestSuite = createMapTestSuite(false, false)
         }
     }
 
     /**
-     * Guava test suite generator for JDBCHashMap(loadOnInit=true).
+     * Guava test suite generator for JDBCHashMap(loadOnInit=false, constrained = true).
+     */
+    class MapConstained {
+        companion object {
+            @JvmStatic
+            fun suite(): TestSuite = createMapTestSuite(false, true)
+        }
+    }
+
+    /**
+     * Guava test suite generator for JDBCHashMap(loadOnInit=true, constrained = false).
      */
     class MapLoadOnInitTrue {
         companion object {
             @JvmStatic
-            fun suite(): TestSuite = createMapTestSuite(true)
+            fun suite(): TestSuite = createMapTestSuite(true, false)
         }
     }
 
     /**
      * Generator of map instances needed for testing.
      */
-    class JDBCHashMapTestGenerator(val loadOnInit: Boolean) : com.google.common.collect.testing.TestStringMapGenerator() {
+    class JDBCHashMapTestGenerator(val loadOnInit: Boolean, val constrained: Boolean) : com.google.common.collect.testing.TestStringMapGenerator() {
         override fun create(elements: Array<Map.Entry<String, String>>): Map<String, String> {
-            val map = if (loadOnInit) loadOnInitTrueMap else loadOnInitFalseMap
+            val map = if (loadOnInit) loadOnInitTrueMap else if(constrained) memoryConstrainedMap else loadOnInitFalseMap
             map.clear()
             map.putAll(elements.associate { Pair(it.key, it.value) })
             return map
@@ -128,31 +142,41 @@ class JDBCHashMapTestSuite {
     }
 
     /**
-     * Guava test suite generator for JDBCHashSet(loadOnInit=false).
+     * Guava test suite generator for JDBCHashSet(loadOnInit=false, constrained = false).
      */
     class SetLoadOnInitFalse {
         companion object {
             @JvmStatic
-            fun suite(): TestSuite = createSetTestSuite(false)
+            fun suite(): TestSuite = createSetTestSuite(false, false)
         }
     }
 
     /**
-     * Guava test suite generator for JDBCHashSet(loadOnInit=true).
+     * Guava test suite generator for JDBCHashSet(loadOnInit=false, constrained = true).
+     */
+    class SetConstrained {
+        companion object {
+            @JvmStatic
+            fun suite(): TestSuite = createSetTestSuite(false, true)
+        }
+    }
+
+    /**
+     * Guava test suite generator for JDBCHashSet(loadOnInit=true, constrained = false).
      */
     class SetLoadOnInitTrue {
         companion object {
             @JvmStatic
-            fun suite(): TestSuite = createSetTestSuite(true)
+            fun suite(): TestSuite = createSetTestSuite(true, false)
         }
     }
 
     /**
      * Generator of set instances needed for testing.
      */
-    class JDBCHashSetTestGenerator(val loadOnInit: Boolean) : com.google.common.collect.testing.TestStringSetGenerator() {
+    class JDBCHashSetTestGenerator(val loadOnInit: Boolean, val constrained: Boolean) : com.google.common.collect.testing.TestStringSetGenerator() {
         override fun create(elements: Array<String>): Set<String> {
-            val set = if (loadOnInit) loadOnInitTrueSet else loadOnInitFalseSet
+            val set = if (loadOnInit) loadOnInitTrueSet else if(constrained) memoryConstrainedSet else loadOnInitFalseSet
             set.clear()
             set.addAll(elements)
             return set

node/src/main/kotlin/net/corda/node/utilities/JDBCHashMap.kt

@@ -26,7 +26,10 @@ import kotlin.system.measureTimeMillis
  * If you can extend [AbstractJDBCHashMap] and implement less Kryo dependent key and/or value mappings then that is
  * likely preferrable.
  */
-class JDBCHashMap<K : Any, V : Any>(tableName: String, loadOnInit: Boolean = false) : AbstractJDBCHashMap<K, V, JDBCHashMap.BlobMapTable>(BlobMapTable(tableName), loadOnInit) {
+class JDBCHashMap<K : Any, V : Any>(tableName: String,
+                                    loadOnInit: Boolean = false,
+                                    maxBuckets: Int = 256)
+    : AbstractJDBCHashMap<K, V, JDBCHashMap.BlobMapTable>(BlobMapTable(tableName), loadOnInit, maxBuckets) {
 
     class BlobMapTable(tableName: String) : JDBCHashedTable(tableName) {
         val key = blob("key")
@@ -73,7 +76,10 @@ fun <T : Any> deserializeFromBlob(blob: Blob): T = bytesFromBlob<T>(blob).deseri
  * If you can extend [AbstractJDBCHashSet] and implement less Kryo dependent element mappings then that is
  * likely preferrable.
  */
-class JDBCHashSet<K : Any>(tableName: String, loadOnInit: Boolean = false) : AbstractJDBCHashSet<K, JDBCHashSet.BlobSetTable>(BlobSetTable(tableName), loadOnInit) {
+class JDBCHashSet<K : Any>(tableName: String,
+                           loadOnInit: Boolean = false,
+                           maxBuckets: Int = 256)
+    : AbstractJDBCHashSet<K, JDBCHashSet.BlobSetTable>(BlobSetTable(tableName), loadOnInit, maxBuckets) {
 
     class BlobSetTable(tableName: String) : JDBCHashedTable(tableName) {
         val key = blob("key")
@@ -92,8 +98,10 @@ class JDBCHashSet<K : Any>(tableName: String, loadOnInit: Boolean = false) : Abs
  *
  * See [AbstractJDBCHashMap] for implementation details.
  */
-abstract class AbstractJDBCHashSet<K : Any, out T : JDBCHashedTable>(protected val table: T, loadOnInit: Boolean = false) : MutableSet<K>, AbstractSet<K>() {
+abstract class AbstractJDBCHashSet<K : Any, out T : JDBCHashedTable>(protected val table: T,
-    protected val innerMap = object : AbstractJDBCHashMap<K, Unit, T>(table, loadOnInit) {
+                                                                     loadOnInit: Boolean = false,
+                                                                     maxBuckets: Int = 256) : MutableSet<K>, AbstractSet<K>() {
+    protected val innerMap = object : AbstractJDBCHashMap<K, Unit, T>(table, loadOnInit, maxBuckets) {
         override fun keyFromRow(row: ResultRow): K = this@AbstractJDBCHashSet.elementFromRow(row)
 
         // Return constant.
@@ -163,9 +171,13 @@ abstract class AbstractJDBCHashSet<K : Any, out T : JDBCHashedTable>(protected v
  * inherited columns that all tables must provide to support iteration order and hashing.
  *
  * The map operates in one of two modes.
- * 1. loadOnInit=true where the entire table is materialised in the JVM and only writes need to perform database access.
+ * 1. loadOnInit=true where the entire table is loaded into memory in the constructor and all entries remain in memory,
- * 2. loadOnInit=false where all entries with the same key hash code are materialised in the JVM on demand when accessed
+ * with only writes needing to perform database access.
- * via any method other than via keys/values/entries properties, and thus the whole map is not materialised.
+ * 2. loadOnInit=false where all entries with the same key hash code are loaded from the database on demand when accessed
+ * via any method other than via keys/values/entries properties, and thus the whole map is not loaded into memory.  The number
+ * of entries retained in memory is controlled indirectly by an LRU algorithm (courtesy of [LinkedHashMap]) and a maximum
+ * number of hash "buckets", where one bucket represents all entries with the same hash code.  There is a default value
+ * for maximum buckets.
  *
  * All operations require a [databaseTransaction] to be started.
  *
@@ -175,22 +187,32 @@ abstract class AbstractJDBCHashSet<K : Any, out T : JDBCHashedTable>(protected v
  *
  * This class is *not* thread safe.
  *
- * TODO: buckets grows forever.  Support some form of LRU cache option (e.g. use [LinkedHashMap.removeEldestEntry] feature).
  * TODO: consider caching size once calculated for the first time.
  * TODO: buckets just use a list and so are vulnerable to poor hash code implementations with collisions.
  * TODO: if iterators are used extensively when loadOnInit=true, consider maintaining a collection of keys in iteration order to avoid sorting each time.
  * TODO: revisit whether we need the loadOnInit==true functionality and remove if not.
  */
-abstract class AbstractJDBCHashMap<K : Any, V : Any, out T : JDBCHashedTable>(val table: T, val loadOnInit: Boolean = false) : MutableMap<K, V>, AbstractMap<K,V>() {
+abstract class AbstractJDBCHashMap<K : Any, V : Any, out T : JDBCHashedTable>(val table: T,
+                                                                              val loadOnInit: Boolean = false,
+                                                                              val maxBuckets: Int = 256) : MutableMap<K, V>, AbstractMap<K,V>() {
 
     companion object {
         protected val log = loggerFor<AbstractJDBCHashMap<*,*,*>>()
+
+        private const val INITIAL_CAPACITY: Int = 16
+        private const val LOAD_FACTOR: Float = 0.75f
     }
 
-    // Hash code -> entries mapping.  Lazy when loadOnInit=false.
+    // Hash code -> entries mapping.
-    private val buckets = HashMap<Int, MutableList<NotReallyMutableEntry<K, V>>>()
+    // When loadOnInit = false, size will be limited to maxBuckets entries (which are hash buckets) and map maintains access order rather than insertion order.
+    private val buckets = object : LinkedHashMap<Int, MutableList<NotReallyMutableEntry<K, V>>>(INITIAL_CAPACITY, LOAD_FACTOR, !loadOnInit) {
+        override fun removeEldestEntry(eldest: MutableMap.MutableEntry<Int, MutableList<NotReallyMutableEntry<K, V>>>?): Boolean {
+            return !loadOnInit && size > maxBuckets
+        }
+    }
 
     init {
+        check(maxBuckets > 0) { "The maximum number of buckets to retain in memory must be a positive integer." }
         // TODO: Move this to schema version managment tool.
         createTablesIfNecessary()
         if (loadOnInit) {