Skip to content

Commit dd6d193

Browse files
committed
Memory store putSync performance improvement
The existing logic splices out a modified element and splices it back in any time an update is made with putSync. With large arrays the performance of Array#splice is bad. Additionally, the store's index is rebuilt to account for moved items. This PR adds a check to see if the item being put has moved. If not, the array is modified in-place with no splicing, and the store's index is not rebuilt. Some boundary tests for putSync are also added to the test suite.
1 parent 227661c commit dd6d193

File tree

2 files changed

+42
-11
lines changed

2 files changed

+42
-11
lines changed

Memory.js

+23-8
Original file line numberDiff line numberDiff line change
@@ -78,24 +78,29 @@ define([
7878
var eventType = id in index ? 'update' : 'add',
7979
event = { target: object },
8080
previousIndex,
81-
defaultDestination;
81+
defaultDestination,
82+
destination;
83+
8284
if (eventType === 'update') {
8385
if (options.overwrite === false) {
8486
throw new Error('Object already exists');
8587
} else {
86-
data.splice(previousIndex = index[id], 1);
88+
previousIndex = index[id];
8789
defaultDestination = previousIndex;
8890
}
8991
} else {
9092
defaultDestination = this.defaultNewToStart ? 0 : data.length;
9193
}
9294

93-
var destination;
9495
if ('beforeId' in options) {
9596
var beforeId = options.beforeId;
9697

9798
if (beforeId === null) {
9899
destination = data.length;
100+
101+
if (eventType === 'update') {
102+
--destination;
103+
}
99104
} else {
100105
destination = index[beforeId];
101106

@@ -114,12 +119,22 @@ define([
114119
} else {
115120
destination = defaultDestination;
116121
}
117-
data.splice(destination, 0, object);
118122

119-
// the fullData has been changed, so the index needs updated
120-
var i = isFinite(previousIndex) ? Math.min(previousIndex, destination) : destination;
121-
for (var l = data.length; i < l; ++i) {
122-
index[this.getIdentity(data[i])] = i;
123+
if (destination === previousIndex) {
124+
data[destination] = object;
125+
}
126+
else {
127+
if (previousIndex !== undefined) {
128+
data.splice(previousIndex, 1);
129+
}
130+
131+
data.splice(destination, 0, object);
132+
133+
// Items have been reordered so the index needs to be updated
134+
var i = previousIndex === undefined ? destination : Math.min(previousIndex, destination);
135+
for (var l = data.length; i < l; ++i) {
136+
index[this.getIdentity(data[i])] = i;
137+
}
123138
}
124139

125140
this.emit(eventType, event);

tests/Memory.js

+19-3
Original file line numberDiff line numberDiff line change
@@ -260,15 +260,31 @@ define([
260260
// Make default put index 0 so it is clear beforeId:null is working
261261
store.defaultNewToStart = true;
262262

263-
store.put({ id: 2 }, { beforeId: 4 });
264-
store.put({ id: 0 }, { beforeId: null });
265-
var results = store.fetchSync();
266263
// Move from a lower source index to a higher destination index because Memory previously had an
267264
// off-by-one bug where it removing an updated item from a lower index and inserted it one past
268265
// the correct destination index
266+
store.put({ id: 2 }, { beforeId: 4 });
267+
268+
// Add a new item and test beforeId:null
269+
store.put({ id: 0 }, { beforeId: null });
270+
271+
var results = store.fetchSync();
269272
assert.strictEqual(results[2].id, 2);
270273
assert.strictEqual(results[3].id, 4);
271274
assert.strictEqual(results[results.length - 1].id, 0);
275+
276+
// Test beforeId puts at the end of the data
277+
var item3 = store.getSync(3);
278+
store.put(item3, { beforeId: 0 });
279+
var item0 = store.getSync(0);
280+
store.put(item0, { beforeId: 3 });
281+
var item5 = store.getSync(5);
282+
store.put(item5, { beforeId: null });
283+
store.put(item5, { beforeId: item5.id });
284+
results = store.fetchSync();
285+
assert.strictEqual(results[3].id, 0);
286+
assert.strictEqual(results[4].id, 3);
287+
assert.strictEqual(results[5].id, 5);
272288
},
273289

274290
'add with options.beforeId': function () {

0 commit comments

Comments
 (0)