Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

Commit

Permalink
batch synced site creation
Browse files Browse the repository at this point in the history
fix #7308

Auditors: @ayumi @alexwykoff

Test Plan:
1. in js/constants/appConfig, change 'isProduction' to true
2. in about:preferences#sync, choose the 'i have an existing sync code' option. enter in the code words that Alex posted in #testers (starts with 'forsaken')
3. open about:bookmarks and wait for 6000+ bookmarks to be synced. the browser should not freeze or use excessive memory during the sync.
  • Loading branch information
diracdeltas committed Feb 23, 2017
1 parent 83180ce commit 9a76006
Showing 1 changed file with 98 additions and 48 deletions.
146 changes: 98 additions & 48 deletions js/state/syncUtil.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,57 +52,95 @@ const pickFields = (object, fields) => {
}

/**
* Apply a bookmark or historySite SyncRecord to the browser data store.
* @param {Object} record
* Apply bookmark or historySite SyncRecords to the browser data store.
* @param {Array.Object} records
* @param {Immutable.Map} siteDetail
*/
const applySiteRecord = (record) => {
const applySiteRecords = (records) => {
const appActions = require('../actions/appActions')
const siteTags = require('../constants/siteTags')
const objectId = new Immutable.List(record.objectId)
const category = CATEGORY_MAP[record.objectData].categoryName
const existingObject = module.exports.getObjectById(objectId, category)
const existingObjectData = existingObject && existingObject[1]
let tag

let siteProps = Object.assign(
{},
existingObjectData && existingObjectData.toJS(),
record.historySite,
record.bookmark,
record.bookmark && record.bookmark.site,
{objectId}
)
if (record.objectData === 'bookmark') {
const existingFolderId = existingObjectData && existingObjectData.get('folderId')
if (existingFolderId) {
siteProps.folderId = existingFolderId

const batchedCreates = {} // map of tag to data
const updates = []
const deletes = []

records.forEach((record) => {
const objectId = new Immutable.List(record.objectId)
const category = CATEGORY_MAP[record.objectData].categoryName
const existingObject = module.exports.getObjectById(objectId, category)
const existingObjectData = existingObject && existingObject[1]
let tag

let siteProps = Object.assign(
{},
existingObjectData && existingObjectData.toJS(),
record.historySite,
record.bookmark,
record.bookmark && record.bookmark.site,
{objectId}
)
if (record.objectData === 'bookmark') {
const existingFolderId = existingObjectData && existingObjectData.get('folderId')
if (existingFolderId) {
siteProps.folderId = existingFolderId
}
const isFolder = (typeof siteProps.isFolder === 'boolean')
? siteProps.isFolder
: !!existingFolderId
tag = isFolder
? siteTags.BOOKMARK_FOLDER
: siteTags.BOOKMARK
const parentFolderObjectId = siteProps.parentFolderObjectId
if (parentFolderObjectId && parentFolderObjectId.length > 0) {
siteProps.parentFolderId =
getFolderIdByObjectId(new Immutable.List(parentFolderObjectId))
}
}
const isFolder = (typeof siteProps.isFolder === 'boolean')
? siteProps.isFolder
: !!existingFolderId
tag = isFolder
? siteTags.BOOKMARK_FOLDER
: siteTags.BOOKMARK
const parentFolderObjectId = siteProps.parentFolderObjectId
if (parentFolderObjectId && parentFolderObjectId.length > 0) {
siteProps.parentFolderId =
getFolderIdByObjectId(new Immutable.List(parentFolderObjectId))
const siteDetail = new Immutable.Map(pickFields(siteProps, SITE_FIELDS))

switch (record.action) {
case writeActions.CREATE:
let key = tag || 'history'
if (batchedCreates[key]) {
batchedCreates[key].push(siteDetail)
} else {
batchedCreates[key] = [siteDetail]
}
break
case writeActions.UPDATE:
if (!existingObjectData) {
// Treat this as a create
let key = tag || 'history'
if (batchedCreates[key]) {
batchedCreates[key].push(siteDetail)
} else {
batchedCreates[key] = [siteDetail]
}
} else {
updates.push({siteDetail, existingObjectData, tag})
}
break
case writeActions.DELETE:
deletes.push({siteDetail, tag})
break
}
}
const siteDetail = new Immutable.Map(pickFields(siteProps, SITE_FIELDS))
})

switch (record.action) {
case writeActions.CREATE:
appActions.addSite(siteDetail, tag, undefined, undefined, true)
break
case writeActions.UPDATE:
appActions.addSite(siteDetail, tag, existingObjectData, null, true)
break
case writeActions.DELETE:
appActions.removeSite(siteDetail, tag, true)
break
// Create all bookmarks in a batch
for (var tag in batchedCreates) {
appActions.addSite(new Immutable.List(batchedCreates[tag]), tag === 'history' ? undefined : tag,
undefined, undefined, true)
}

// Apply updates
updates.forEach((update) => {
appActions.addSite(update.siteDetail, update.tag, update.existingObjectData, null, true)
})

// Apply deletes
deletes.forEach((deletion) => {
appActions.removeSite(deletion.siteDetail, deletion.tag, true)
})
}

const applySiteSettingRecord = (record) => {
Expand Down Expand Up @@ -167,15 +205,15 @@ const applySiteSettingRecord = (record) => {
* Given a SyncRecord, apply it to the browser data store.
* @param {Object} record
*/
module.exports.applySyncRecord = (record) => {
const applySyncRecord = (record) => {
if (!record || !record.objectData) {
console.log(`Warning: Can't apply empty record: ${record}`)
return
}
switch (record.objectData) {
case 'bookmark':
case 'historySite':
applySiteRecord(record)
// these are handled in batches now

This comment has been minimized.

Copy link
@ayumi

ayumi Feb 23, 2017

Contributor

should we emit a warning here?

break
case 'siteSetting':
applySiteSettingRecord(record)
Expand All @@ -194,10 +232,22 @@ module.exports.applySyncRecord = (record) => {
*/
module.exports.applySyncRecords = (records) => {
if (!records || records.length === 0) { return }
const siteRecords = []
const otherRecords = []
records.forEach((record) => {
if (record && ['bookmark', 'historySite'].includes(record.objectData)) {
siteRecords.push(record)
} else {
otherRecords.push(record)
}
})
setImmediate(() => {
const record = otherRecords.shift()
applySyncRecord(record)
module.exports.applySyncRecords(otherRecords)
})
setImmediate(() => {
const record = records.shift()
module.exports.applySyncRecord(record)
module.exports.applySyncRecords(records)
applySiteRecords(siteRecords)
})
}

Expand Down

0 comments on commit 9a76006

Please sign in to comment.