JetBrains / Exposed

Kotlin SQL Framework
http://jetbrains.github.io/Exposed/
Apache License 2.0
8.25k stars 689 forks source link

Removing from entity cache is extremely expensive #1121

Open jnfeinstein opened 3 years ago

jnfeinstein commented 3 years ago

Issue

This line results in the complete eviction of any record that refers to the table of the record being deleted. This means that completely unrelated references are evicted, which is not expected.

Expected Behavior

Based on the current code, it would be preferable to evict only the references that point to the table of the record being deleted. Even better would be to track objects with references to the record being deleted, and evict only those entries.

jnfeinstein commented 3 years ago

Here is my workaround, which runs into other Exposed issues (see #1123). This adds bulk delete to SizedIterables, which can be used to invalidate entity cache entries containing said SizedIterable. This preserves the majority of the entity cache and drastically increases performance (like, a lot).

@Suppress("unchecked_cast")
fun <ID: Comparable<ID>> SizedIterable<Entity<ID>>.delete(entities: Iterable<Entity<ID>>) {
    if (entities.count() == 0) return
    // Check that all entities come from the same table
    val table = entities.mapToSet { it.klass.table }.takeIf { it.size == 1 }?.first()
        ?: throw IllegalArgumentException("Can't delete from multiple tables")
    // Check that all entities are part of this SizedIterable
    val presentEntityIds = mapToSet { it.id }
    entities.filter { it.id !in presentEntityIds }.takeIf { it.isNotEmpty() }?.let {
        throw IllegalArgumentException("Can't delete entities")
    }
    val transaction = TransactionManager.current()
    val cache = transaction.entityCache
    // Invalidate entity cache entries, mimicking `EntityClass#removeFromCache`
    entities.forEach {
        cache.remove(table, it)
        cache.referrers.remove(it.id)
    }
    // Invalidate references to this SizedIterable, replacing `EntityClass#removeTablesReferrers`
    cache.referrers.mapNotNull { (entityId, colCache) ->
        colCache.entries.firstOrNull { it.value === this }?.let { colCache.remove(it.key) }
        entityId.takeIf { colCache.isEmpty() }
    }.forEach {
        cache.referrers.remove(it)
    }
    // Perform deletes mimicking `Entity#delete` - `EntityLifecycleInterceptor` will flush necessary caches
    // Cannot use `DeleteStatement` because that will also obliterate the entity cache
    val where = SqlExpressionBuilder.run { table.id inList entities.map { it.id }}
    val whereClause = QueryBuilder(true).append(where)
    transaction.exec(
        transaction.db.dialect.functionProvider.delete(false, table, whereClause.toString(), null, transaction),
        // See https://github.com/JetBrains/Exposed/pull/1123
        whereClause.args as List<Pair<ColumnType, EntityID<ID>>>
    )
    // Call removed hooks mimicking `Entity#delete`
    entities.forEach {
        transaction.registerChange(it.klass, it.id, EntityChangeType.Removed)
    }
}

This is obviously not an ideal situation because it repeats a number of patterns found throughout Exposed and replaces removeTablesReferrers. It also requires pulling all of the SizedIterable into memory to validate the presence of the entities-to-delete, although I see this as a more minor offense because the entities-to-delete presumably came from this iterable in the first place.