perf(db): Use subselect to improve thread list query perf (53%!)

Summary:
- Use a sub-select query with much better performance to display the thread list
- Perform analyze on tables after launch

The new query is:

```
SELECT `Thread`.`data` FROM `Thread` WHERE `Thread`.`id` IN (SELECT `id` FROM `ThreadCategory` AS `M26` WHERE `M26`.`value` IN ('9m9ks71k06n5rmx82kgues09p','9s9k25q6j1krjgpkovbcjm7d','13b7ufruoymvih07ki0uahlto','dtmhlzz6phr47zp512knhjgf8','16dvjb84bszfh15kgfrjj37i3','aclwmgncdqjfibp51bvgbeik','17qad7jhbp6tozog3klm5zagt','4x4bkbawiq825u4eu3aus8tll','7axr9f5f1lzpwm2rw2ghkirhq','dsnn660af0pmou2gg3nstga8a','361qr5rva1ieby2r0ec3sn0bm','10fyvba7pjyjgeyr5i65i1zri') AND `M26`.`in_all_mail` = 1  ORDER BY `M26`.`last_message_received_timestamp` DESC LIMIT 200 OFFSET 0) ORDER BY `Thread`.`last_message_received_timestamp` DESC;
`
0|0|0|SEARCH TABLE Thread USING INDEX Thread_id (id=?)
0|0|0|EXECUTE LIST SUBQUERY 1
1|0|0|SCAN TABLE Thread-Category AS M26 USING COVERING INDEX ThreadFancyIndex
1|0|0|EXECUTE LIST SUBQUERY 2
0|0|0|USE TEMP B-TREE FOR (only on 200 result items)
```

Which is twice as performant as:
```
SELECT `Thread`.`data` FROM `Thread` INNER JOIN `ThreadCategory` AS `M26` ON `M26`.`id` = `Thread`.`id` WHERE `M26`.`value` IN ('9m9ks71k06n5rmx82kgues09p','9s9k25q6j1krjgpkovbcjm7d','13b7ufruoymvih07ki0uahlto','dtmhlzz6phr47zp512knhjgf8','16dvjb84bszfh15kgfrjj37i3','aclwmgncdqjfibp51bvgbeik','17qad7jhbp6tozog3klm5zagt','4x4bkbawiq825u4eu3aus8tll','7axr9f5f1lzpwm2rw2ghkirhq','361qr5rva1ieby2r0ec3sn0bm','10fyvba7pjyjgeyr5i65i1zri') AND `M26`.`in_all_mail` = 1  ORDER BY `M26`.`last_message_received_timestamp` DESC LIMIT 200 OFFSET 0;

0|0|1|SCAN TABLE Thread-Category AS M26 USING COVERING INDEX ThreadFancyIndex
0|0|0|EXECUTE LIST SUBQUERY 1
0|1|0|SEARCH TABLE Thread USING INDEX Thread_id (id=?)
```

Test Plan: Broken!

Reviewers: evan, juan

Reviewed By: juan

Differential Revision: https://phab.nylas.com/D2869
This commit is contained in:
Ben Gotow 2016-04-11 13:29:05 -07:00
parent ded1b657d3
commit b75ed6f920
12 changed files with 133 additions and 48 deletions

View file

@ -66,11 +66,15 @@ TestModel.configureWithCollectionAttribute = ->
queryable: true
modelKey: 'serverId'
jsonKey: 'server_id'
'other': Attributes.String
queryable: true,
modelKey: 'other'
'categories': Attributes.Collection
queryable: true
queryable: true,
modelKey: 'categories'
itemClass: Category
itemClass: Category,
joinOnField: 'id',
joinQueryableBy: ['other'],
TestModel.configureWithJoinedDataAttribute = ->
TestModel.additionalSQLiteConfig = undefined

View file

@ -29,9 +29,9 @@ describe "DatabaseSetupQueryBuilder", ->
TestModel.configureWithCollectionAttribute()
queries = @builder.setupQueriesForTable(TestModel)
expected = [
'CREATE TABLE IF NOT EXISTS `TestModel` (id TEXT PRIMARY KEY,data BLOB,client_id TEXT,server_id TEXT)',
'CREATE TABLE IF NOT EXISTS `TestModel` (id TEXT PRIMARY KEY,data BLOB,client_id TEXT,server_id TEXT,other TEXT)',
'CREATE UNIQUE INDEX IF NOT EXISTS `TestModel_id` ON `TestModel` (`id`)',
'CREATE TABLE IF NOT EXISTS `TestModelCategory` (id TEXT KEY, `value` TEXT)'
'CREATE TABLE IF NOT EXISTS `TestModelCategory` (id TEXT KEY,`value` TEXT,other TEXT)'
'CREATE INDEX IF NOT EXISTS `TestModelCategory_id` ON `TestModelCategory` (`id` ASC)'
'CREATE UNIQUE INDEX IF NOT EXISTS `TestModelCategory_val_id` ON `TestModelCategory` (`value` ASC, `id` ASC)',
]

View file

@ -195,7 +195,7 @@ describe "DatabaseTransaction", ->
it "should compose a REPLACE INTO query to save the model", ->
TestModel.configureWithCollectionAttribute()
@transaction._writeModels([testModelInstance])
expect(@performed[0].query).toBe("REPLACE INTO `TestModel` (id,data,client_id,server_id) VALUES (?,?,?,?)")
expect(@performed[0].query).toBe("REPLACE INTO `TestModel` (id,data,client_id,server_id,other) VALUES (?,?,?,?,?)")
it "should save the model JSON into the data column", ->
@transaction._writeModels([testModelInstance])
@ -228,21 +228,21 @@ describe "DatabaseTransaction", ->
describe "when the model has collection attributes", ->
beforeEach ->
TestModel.configureWithCollectionAttribute()
@m = new TestModel(id: 'local-6806434c-b0cd')
@m = new TestModel(id: 'local-6806434c-b0cd', other: 'other')
@m.categories = [new Category(id: 'a'),new Category(id: 'b')]
@transaction._writeModels([@m])
it "should delete all association records for the model from join tables", ->
expect(@performed[1].query).toBe('DELETE FROM `TestModelCategory` WHERE `id` IN (\'local-6806434c-b0cd\')')
it "should insert new association records into join tables in a single query", ->
expect(@performed[2].query).toBe('INSERT OR IGNORE INTO `TestModelCategory` (`id`, `value`) VALUES (?,?),(?,?)')
expect(@performed[2].values).toEqual(['local-6806434c-b0cd', 'a','local-6806434c-b0cd', 'b'])
it "should insert new association records into join tables in a single query, and include queryableBy columns", ->
expect(@performed[2].query).toBe('INSERT OR IGNORE INTO `TestModelCategory` (`id`,`value`,`other`) VALUES (?,?,?),(?,?,?)')
expect(@performed[2].values).toEqual(['local-6806434c-b0cd', 'a', 'other','local-6806434c-b0cd', 'b', 'other'])
describe "model collection attributes query building", ->
beforeEach ->
TestModel.configureWithCollectionAttribute()
@m = new TestModel(id: 'local-6806434c-b0cd')
@m = new TestModel(id: 'local-6806434c-b0cd', other: 'other')
@m.categories = []
it "should page association records into multiple queries correctly", ->
@ -253,7 +253,7 @@ describe "DatabaseTransaction", ->
i.query.indexOf('INSERT OR IGNORE INTO `TestModelCategory`') == 0
expect(collectionAttributeQueries.length).toBe(1)
expect(collectionAttributeQueries[0].values[399]).toEqual('id-199')
expect(collectionAttributeQueries[0].values[200*3-2]).toEqual('id-199')
it "should page association records into multiple queries correctly", ->
@m.categories.push(new Category(id: "id-#{i}")) for i in [0..200]
@ -263,7 +263,7 @@ describe "DatabaseTransaction", ->
i.query.indexOf('INSERT OR IGNORE INTO `TestModelCategory`') == 0
expect(collectionAttributeQueries.length).toBe(2)
expect(collectionAttributeQueries[0].values[399]).toEqual('id-199')
expect(collectionAttributeQueries[0].values[200*3-2]).toEqual('id-199')
expect(collectionAttributeQueries[1].values[1]).toEqual('id-200')
it "should page association records into multiple queries correctly", ->
@ -274,9 +274,9 @@ describe "DatabaseTransaction", ->
i.query.indexOf('INSERT OR IGNORE INTO `TestModelCategory`') == 0
expect(collectionAttributeQueries.length).toBe(2)
expect(collectionAttributeQueries[0].values[399]).toEqual('id-199')
expect(collectionAttributeQueries[0].values[200*3-2]).toEqual('id-199')
expect(collectionAttributeQueries[1].values[1]).toEqual('id-200')
expect(collectionAttributeQueries[1].values[3]).toEqual('id-201')
expect(collectionAttributeQueries[1].values[4]).toEqual('id-201')
describe "when the model has joined data attributes", ->
beforeEach ->

View file

@ -32,10 +32,11 @@ The value of this attribute is always an array of other model objects.
Section: Database
###
class AttributeCollection extends Attribute
constructor: ({modelKey, jsonKey, itemClass, joinOnField}) ->
constructor: ({modelKey, jsonKey, itemClass, joinOnField, joinQueryableBy}) ->
super
@itemClass = itemClass
@joinOnField = joinOnField
@joinQueryableBy = joinQueryableBy || []
@
toJSON: (vals) ->

View file

@ -83,11 +83,15 @@ class Matcher
else
throw new Error("Matcher.evaulate() not sure how to evaluate @{@attr.modelKey} with comparator #{@comparator}")
joinTableRef: ->
"M#{@muid}"
joinSQL: (klass) ->
switch @comparator
when 'contains', 'containsAny'
joinTable = tableNameForJoin(klass, @attr.itemClass)
return "INNER JOIN `#{joinTable}` AS `M#{@muid}` ON `M#{@muid}`.`id` = `#{klass.name}`.`id`"
joinTableRef = @joinTableRef()
return "INNER JOIN `#{joinTable}` AS `#{joinTableRef}` ON `#{joinTableRef}`.`id` = `#{klass.name}`.`id`"
else
return false
@ -118,9 +122,9 @@ class Matcher
when 'startsWith'
return " RAISE `TODO`; "
when 'contains'
return "`M#{@muid}`.`value` = #{escaped}"
return "`#{@joinTableRef()}`.`value` = #{escaped}"
when 'containsAny'
return "`M#{@muid}`.`value` IN #{escaped}"
return "`#{@joinTableRef()}`.`value` IN #{escaped}"
else
return "`#{klass.name}`.`#{@attr.jsonKey}` #{@comparator} #{escaped}"
@ -233,7 +237,8 @@ class SearchMatcher extends Matcher
joinSQL: (klass) =>
searchTable = "#{klass.name}Search"
return "INNER JOIN `#{searchTable}` AS `M#{@muid}` ON `M#{@muid}`.`content_id` = `#{klass.name}`.`id`"
joinTableRef = @joinTableRef()
return "INNER JOIN `#{searchTable}` AS `#{joinTableRef}` ON `#{joinTableRef}`.`content_id` = `#{klass.name}`.`id`"
whereSQL: (klass) =>
searchTable = "#{klass.name}Search"

View file

@ -26,7 +26,11 @@ class MutableQuerySubscription extends QuerySubscription {
if (!this._query) {
return
}
this.replaceQuery(this._query.clone().page(start, end))
const next = this._query.clone().page(start, end);
if (!next.range().isEqual(this._query.range())) {
this.replaceQuery(next);
}
}
}

View file

@ -1,4 +1,4 @@
{Matcher, AttributeJoinedData} = require '../attributes'
{Matcher, AttributeJoinedData, AttributeCollection} = require '../attributes'
QueryRange = require './query-range'
Utils = require './utils'
_ = require 'underscore'
@ -271,7 +271,43 @@ class ModelQuery
limit += " OFFSET #{@_range.offset}"
distinct = if @_distinct then ' DISTINCT' else ''
"SELECT#{distinct} #{result} FROM `#{@_klass.name}` #{@_whereClause()} #{order} #{limit}"
joins = @_matchers.filter (matcher) -> matcher.attr instanceof AttributeCollection
if joins.length is 1 and @_canSubselectForJoin(joins[0])
subSql = @_subselectSQL(joins[0], @_matchers, order, limit)
return "SELECT#{distinct} #{result} FROM `#{@_klass.name}` WHERE `id` IN (#{subSql}) #{order}"
else
return "SELECT#{distinct} #{result} FROM `#{@_klass.name}` #{@_whereClause()} #{order} #{limit}"
# If one of our matchers requires a join, and the attribute configuration lists
# all of the other order and matcher attributes in `joinQueryableBy`, it means
# we can make the entire WHERE and ORDER BY on a sub-query, which improves
# performance considerably vs. finding all results from the join table and then
# doing the ordering after pulling the results in the main table.
#
# Note: This is currently only intended for use in the thread list
#
_canSubselectForJoin: (matcher) ->
joinAttribute = matcher.attribute()
return false unless joinAttribute.joinOnField is 'id'
allMatchersOnJoinTable = _.every @_matchers, (m) ->
m is matcher or joinAttribute.joinQueryableBy.indexOf(m.attr.modelKey) isnt -1
allOrdersOnJoinTable = _.every @_orders, (o) ->
joinAttribute.joinQueryableBy.indexOf(o.attr.modelKey) isnt -1
return allMatchersOnJoinTable and allOrdersOnJoinTable
_subselectSQL: (returningMatcher, subselectMatchers, order, limit) ->
returningAttribute = returningMatcher.attribute()
table = Utils.tableNameForJoin(@_klass, returningAttribute.itemClass)
wheres = _.compact subselectMatchers.map (c) => c.whereSQL(@_klass)
innerSQL = "SELECT `id` FROM `#{table}` WHERE #{wheres.join(' AND ')} #{order} #{limit}"
innerSQL = innerSQL.replace(new RegExp("`#{@_klass.name}`", 'g'), "`#{table}`")
innerSQL = innerSQL.replace(new RegExp("`#{returningMatcher.joinTableRef()}`", 'g'), "`#{table}`")
innerSQL
_whereClause: ->
joins = []

View file

@ -67,6 +67,8 @@ class Thread extends ModelWithMetadata {
'categories': Attributes.Collection({
queryable: true,
modelKey: 'categories',
joinOnField: 'id',
joinQueryableBy: ['inAllMail', 'lastMessageReceivedTimestamp', 'lastMessageSentTimestamp', 'unread'],
itemClass: Category,
}),
@ -112,8 +114,9 @@ class Thread extends ModelWithMetadata {
setup: () => [
'CREATE TABLE IF NOT EXISTS `ThreadCounts` (`category_id` TEXT PRIMARY KEY, `unread` INTEGER, `total` INTEGER)',
'CREATE UNIQUE INDEX IF NOT EXISTS ThreadCountsIndex ON `ThreadCounts` (category_id DESC)',
'CREATE INDEX IF NOT EXISTS ThreadListIndex ON Thread(last_message_received_timestamp DESC, id)',
'CREATE INDEX IF NOT EXISTS ThreadListSentIndex ON Thread(last_message_sent_timestamp DESC, id)',
'CREATE INDEX IF NOT EXISTS ThreadListCategoryIndex ON `ThreadCategory`(last_message_received_timestamp DESC, value, in_all_mail, unread, id)',
'CREATE INDEX IF NOT EXISTS ThreadListCategorySentIndex ON `ThreadCategory`(last_message_sent_timestamp DESC, value, in_all_mail, unread, id)',
'CREATE INDEX IF NOT EXISTS ThreadStarIndex ON Thread(account_id, starred)',
],
}

View file

@ -134,21 +134,14 @@ class CategoryStore extends NylasStore
# account have been loaded into the local cache
#
whenCategoriesReady: (accountOrId) =>
account = null
account = asAccount(accountOrId) if accountOrId
isSyncComplete = =>
if account
categoryCollection = account.categoryCollection()
NylasSyncStatusStore.isSyncCompleteForAccount(account.id, categoryCollection)
else
accounts = AccountStore.accounts()
_.every(accounts, (acc) =>
NylasSyncStatusStore.isSyncCompleteForAccount(acc.id, acc.categoryCollection())
)
accounts = AccountStore.accounts()
accounts = [asAccount(accountOrId)] if accountOrId
categoriesReady = =>
@categories(account).length > 0 and isSyncComplete()
_.every accounts, (acc) =>
populated = @categories(acc).length > 0
fetched = NylasSyncStatusStore.isSyncCompleteForAccount(acc.id, acc.categoryCollection())
return populated and fetched
if not categoriesReady()
return new Promise (resolve) =>
@ -165,7 +158,6 @@ class CategoryStore extends NylasStore
return Promise.resolve()
_onCategoriesChanged: (categories) =>
@_categoryResult = categories
@_categoryCache = {}

View file

@ -16,6 +16,19 @@ class DatabaseSetupQueryBuilder
queries = queries.concat @setupQueriesForTable(klass)
return queries
analyzeQueries: ->
queries = []
for key, klass of DatabaseObjectRegistry.classMap()
attributes = _.values(klass.attributes)
collectionAttributes = _.filter attributes, (attr) ->
attr.queryable && attr instanceof AttributeCollection
queries.push("ANALYZE `#{klass.name}`")
collectionAttributes.forEach (attribute) ->
queries.push("ANALYZE `#{tableNameForJoin(klass, attribute.itemClass)}`")
return queries
setupQueriesForTable: (klass) =>
attributes = _.values(klass.attributes)
queries = []
@ -40,12 +53,17 @@ class DatabaseSetupQueryBuilder
attr.queryable && attr instanceof AttributeCollection
collectionAttributes.forEach (attribute) ->
joinTable = tableNameForJoin(klass, attribute.itemClass)
queries.push("CREATE TABLE IF NOT EXISTS `#{joinTable}` (id TEXT KEY, `value` TEXT)")
joinColumns = attribute.joinQueryableBy.map (name) ->
klass.attributes[name].columnSQL()
joinColumns.unshift('id TEXT KEY', '`value` TEXT')
queries.push("CREATE TABLE IF NOT EXISTS `#{joinTable}` (#{joinColumns.join(',')})")
queries.push("CREATE INDEX IF NOT EXISTS `#{joinTable.replace('-', '_')}_id` ON `#{joinTable}` (`id` ASC)")
queries.push("CREATE UNIQUE INDEX IF NOT EXISTS `#{joinTable.replace('-', '_')}_val_id` ON `#{joinTable}` (`value` ASC, `id` ASC)")
joinedDataAttributes = _.filter attributes, (attr) ->
attr instanceof AttributeJoinedData
joinedDataAttributes.forEach (attribute) ->
queries.push("CREATE TABLE IF NOT EXISTS `#{attribute.modelTable}` (id TEXT PRIMARY KEY, `value` TEXT)")

View file

@ -16,7 +16,7 @@ DatabaseTransaction = require './database-transaction'
{ipcRenderer} = require 'electron'
DatabaseVersion = 22
DatabaseVersion = 23
DatabasePhase =
Setup: 'setup'
Ready: 'ready'
@ -115,6 +115,7 @@ class DatabaseStore extends NylasStore
@_checkDatabaseVersion {allowNotSet: true}, =>
@_runDatabaseSetup =>
app.setDatabasePhase(DatabasePhase.Ready)
setTimeout(@_runDatabaseAnalyze, 60 * 1000)
else if phase is DatabasePhase.Ready
@_openDatabase =>
@ -204,6 +205,13 @@ class DatabaseStore extends NylasStore
console.log("Could not re-import mail rules: #{err}")
ready()
_runDatabaseAnalyze: =>
builder = new DatabaseSetupQueryBuilder()
async.each builder.analyzeQueries(), (query, callback) =>
@_db.run(query, [], callback)
, (err) =>
console.log("Completed ANALYZE of database")
_handleSetupError: (err) =>
NylasEnv.reportError(err)

View file

@ -175,8 +175,10 @@ class DatabaseTransaction
values = []
marks = []
ids = []
modelsJSONs = []
for model in models
json = model.toJSON(joined: false)
modelsJSONs.push(json)
ids.push(model.id)
values.push(model.id, JSON.stringify(json, Utils.registeredObjectReplacer))
columnAttributes.forEach (attr) ->
@ -199,24 +201,36 @@ class DatabaseTransaction
joinMarks = []
joinedValues = []
for model in models
joinMarkUnit = "(" + ["?", "?"].concat(attr.joinQueryableBy.map( -> '?')).join(',') + ")"
joinQueryableByJSONKeys = attr.joinQueryableBy.map (joinedModelKey) ->
klass.attributes[joinedModelKey].jsonKey
joinColumns = ['id', 'value'].concat(joinQueryableByJSONKeys)
# https://www.sqlite.org/limits.html: SQLITE_MAX_VARIABLE_NUMBER
valuesPerRow = joinColumns.length
rowsPerInsert = Math.floor(600 / valuesPerRow)
valuesPerInsert = rowsPerInsert * valuesPerRow
for model, idx in models
joinedModels = model[attr.modelKey]
if joinedModels
for joined in joinedModels
joinMarks.push('(?,?)')
joinValue = joined[attr.joinOnField ? "id"]
joinValue = joined[attr.joinOnField]
joinMarks.push(joinMarkUnit)
joinedValues.push(model.id, joinValue)
for joinedJsonKey in joinQueryableByJSONKeys
joinedValues.push(modelsJSONs[idx][joinedJsonKey])
unless joinedValues.length is 0
# Write no more than 200 items (400 values) at once to avoid sqlite limits
# 399 values: slices:[0..0]
# 400 values: slices:[0..0]
# 401 values: slices:[0..1]
slicePageCount = Math.ceil(joinedValues.length / 400) - 1
slicePageCount = Math.ceil(joinMarks.length / rowsPerInsert) - 1
for slice in [0..slicePageCount] by 1
[ms, me] = [slice*200, slice*200 + 199]
[vs, ve] = [slice*400, slice*400 + 399]
promises.push @_query("INSERT OR IGNORE INTO `#{joinTable}` (`id`, `value`) VALUES #{joinMarks[ms..me].join(',')}", joinedValues[vs..ve])
[ms, me] = [slice * rowsPerInsert, slice * rowsPerInsert + (rowsPerInsert - 1)]
[vs, ve] = [slice * valuesPerInsert, slice * valuesPerInsert + (valuesPerInsert - 1)]
promises.push @_query("INSERT OR IGNORE INTO `#{joinTable}` (`#{joinColumns.join('`,`')}`) VALUES #{joinMarks[ms..me].join(',')}", joinedValues[vs..ve])
# For each joined data property stored in another table...
values = []