Switch to JS implementation of indexedDB.cmp(). #1412
+247
−26
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,18 +1,89 @@ | ||
| // Implementation of https://www.w3.org/TR/IndexedDB-3/#compare-two-keys | ||
|
|
||
| import { toStringTag } from './utils'; | ||
|
|
||
| // ... with the adjustment to return NaN instead of throwing. | ||
| export function cmp(a: any, b: any): number { | ||
| try { | ||
| const ta = type(a); | ||
| const tb = type(b); | ||
| if (ta !== tb) { | ||
| if (ta === 'Array') return 1; | ||
| if (tb === 'Array') return -1; | ||
| if (ta === 'binary') return 1; | ||
| if (tb === 'binary') return -1; | ||
| if (ta === 'string') return 1; | ||
| if (tb === 'string') return -1; | ||
| if (ta === 'Date') return 1; | ||
| if (tb !== 'Date') return NaN; | ||
| return -1; | ||
| } | ||
| switch (ta) { | ||
| case 'number': | ||
| case 'Date': | ||
| case 'string': | ||
| return a > b ? 1 : a < b ? -1 : 0; | ||
| case 'binary': { | ||
| return compareUint8Arrays(getUint8Array(a), getUint8Array(b)); | ||
| } | ||
| case 'Array': | ||
| return compareArrays(a, b); | ||
| } | ||
| } catch {} | ||
| return NaN; // Return value if any given args are valid keys. | ||
| } | ||
|
|
||
| export function compareArrays(a: any[], b: any[]): number { | ||
| const al = a.length; | ||
| const bl = b.length; | ||
| const l = al < bl ? al : bl; | ||
| for (let i = 0; i < l; ++i) { | ||
| const res = cmp(a[i], b[i]); | ||
| if (res !== 0) return res; | ||
| } | ||
| return al === bl ? 0 : al < bl ? -1 : 1; | ||
| } | ||
|
|
||
| export function compareUint8Arrays( | ||
| a: Uint8Array, | ||
| b: Uint8Array | ||
| ) { | ||
| const al = a.length; | ||
| const bl = b.length; | ||
| const l = al < bl ? al : bl; | ||
| for (let i = 0; i < l; ++i) { | ||
| if (a[i] !== b[i]) return a[i] < b[i] ? -1 : 1; | ||
|
There was a problem hiding this comment. ...as a follow up to my previous comment, here's an experiment with function compareUint8Arrays(a,b) {
const dv1 = new DataView(a.buffer);
const dv2 = new DataView(b.buffer);
let idx = 0;
let bytes = Math.min(dv1.byteLength, dv2.byteLength);
if (bytes > 4) {
let len = bytes >> 2;
while (idx < len) {
let b1 = dv1.getUint32(idx);
let b2 = dv2.getUint32(idx);
if (b1 !== b2) return b1 < b2 ? -1 : 1;
++idx;
}
bytes -= idx << 2;
}
if (bytes > 2) {
let len = bytes >> 1;
while (idx < len) {
let b1 = dv1.getUint16(idx);
let b2 = dv2.getUint16(idx);
if (b1 !== b2) return b1 < b2 ? -1 : 1;
++idx;
}
bytes -= idx << 1;
}
while (idx < bytes) {
let b1 = dv1.getUint8(idx);
let b2 = dv2.getUint8(idx);
if (b1 !== b2) return b1 < b2 ? -1 : 1;
++idx;
}
return dv1.byteLength === dv2.byteLength ? 0 : dv1.byteLength < dv2.byteLength ? -1 : 1;
}b1=cryptHelper.getRandomValues(4730)
b2=b1
> console.time('dd');for(let i = 1e6; i--; a2());console.timeEnd('dd');
< dd: 1382.230224609375 ms... 83% faster with 4KB of data (?) but still slower than There was a problem hiding this comment. If I get it right, the conclusion of all this is that:
Now, when we are comparing keys, we only do it for indexed keys and properties and these shall not be large anyway (see #1392) and there is almost never any good reason to index large binaries. A typical binary index represent either a primary key of maybe 16 (random) bytes, or digests that may be between 8-32 bytes and have the same randomness. There might be other use cases, like indexing paths of tree structures that may be longer, and having identical start sequences, but probably never that large. Indexing binaries larger than say 1500 bytes would create other performance issues anyway because it bloats the BTree in the database - which is the reason that most databases limit the permitted size of BTree indexes to 1700-3500 bytes depending on DB engine. |
||
| } | ||
| return al === bl ? 0 : al < bl ? -1 : 1; | ||
| } | ||
|
|
||
| // Implementation of https://www.w3.org/TR/IndexedDB-3/#key-type | ||
| function type(x: any) { | ||
| const t = typeof x; | ||
| if (t !== 'object') return t; | ||
| if (ArrayBuffer.isView(x)) return 'binary'; | ||
| const tsTag = toStringTag(x); // Cannot use instanceof in Safari | ||
| return tsTag === 'ArrayBuffer' ? 'binary' : (tsTag as 'Array' | 'Date'); | ||
| } | ||
|
|
||
| type BinaryType = | ||
| | ArrayBuffer | ||
| | DataView | ||
| | Uint8ClampedArray | ||
| | ArrayBufferView | ||
| | Uint8Array | ||
| | Int8Array | ||
| | Uint16Array | ||
| | Int16Array | ||
| | Uint32Array | ||
| | Int32Array | ||
| | Float32Array | ||
| | Float64Array; | ||
|
|
||
| function getUint8Array(a: BinaryType): Uint8Array { | ||
| if (a instanceof Uint8Array) return a; | ||
| if (ArrayBuffer.isView(a)) | ||
| // TypedArray or DataView | ||
| return new Uint8Array(a.buffer, a.byteOffset, a.byteLength); | ||
| return new Uint8Array(a); // ArrayBuffer | ||
| } | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,150 @@ | ||
| import Dexie from 'dexie'; | ||
| import { module, test, equal, ok } from 'QUnit'; | ||
|
|
||
| function fillArrayBuffer(ab, val) { | ||
| const view = new Uint8Array(ab); | ||
| for (let i = 0; i < view.byteLength; ++i) { | ||
| view[i] = val; | ||
| } | ||
| } | ||
|
|
||
| module('cmp'); | ||
|
|
||
| const { cmp } = Dexie; | ||
|
|
||
| test('it should support indexable types', () => { | ||
| // numbers | ||
| ok(cmp(1, 1) === 0, 'Equal numbers should return 0'); | ||
| ok(cmp(1, 2) === -1, 'Less than numbers should return -1'); | ||
| ok(cmp(-1, -2000) === 1, 'Greater than numbers should return 1'); | ||
| // strings | ||
| ok(cmp('A', 'A') === 0, 'Equal strings should return 0'); | ||
| ok(cmp('A', 'B') === -1, 'Less than strings should return -1'); | ||
| ok(cmp('C', 'A') === 1, 'Greater than strings should return 1'); | ||
| // Dates | ||
| ok(cmp(new Date(1), new Date(1)) === 0, 'Equal dates should return 0'); | ||
| ok(cmp(new Date(1), new Date(2)) === -1, 'Less than dates should return -1'); | ||
| ok( | ||
| cmp(new Date(1000), new Date(500)) === 1, | ||
| 'Greater than dates should return 1' | ||
| ); | ||
| // Arrays | ||
| ok(cmp([1, 2, '3'], [1, 2, '3']) === 0, 'Equal arrays should return 0'); | ||
| ok(cmp([-1], [1]) === -1, 'Less than arrays should return -1'); | ||
| ok(cmp([1], [-1]) === 1, 'Greater than arrays should return 1'); | ||
| ok(cmp([1], [1, 0]) === -1, 'If second array is longer with same leading entries, return -1'); | ||
| ok(cmp([1, 0], [1]) === 1, 'If first array is longer with same leading entries, return 1'); | ||
| ok(cmp([1], [0,0]) === 1, 'If first array is shorter but has greater leading entries, return 1'); | ||
| ok(cmp([0,0], [1]) === -1, 'If second array is shorter but has greater leading entries, return -1'); | ||
|
|
||
| /* Binary types | ||
| | DataView | ||
| | Uint8ClampedArray | ||
| | Uint8Array | ||
| | Int8Array | ||
| | Uint16Array | ||
| | Int16Array | ||
| | Uint32Array | ||
| | Int32Array | ||
| | Float32Array | ||
| | Float64Array; | ||
| */ | ||
| const viewTypes = [ | ||
| 'DataView', | ||
| 'Uint8ClampedArray', | ||
| 'Uint8Array', | ||
| 'Int8Array', | ||
| 'Uint16Array', | ||
| 'Uint32Array', | ||
| 'Int32Array', | ||
| 'Float32Array', | ||
| 'Float64Array', | ||
| ] | ||
| .map((typeName) => [typeName, self[typeName]]) | ||
| .filter(([_, ctor]) => !!ctor); // Don't try to test types not supported by the browser | ||
|
|
||
| const zeroes1 = new ArrayBuffer(16); | ||
| const zeroes2 = new ArrayBuffer(16); | ||
| const ones = new ArrayBuffer(16); | ||
| fillArrayBuffer(zeroes1, 0); | ||
| fillArrayBuffer(zeroes2, 0); | ||
| fillArrayBuffer(ones, 1); | ||
|
|
||
| for (const [typeName, ArrayBufferView] of viewTypes) { | ||
| // Equals | ||
| let v1 = new ArrayBufferView(zeroes1); | ||
| let v2 = new ArrayBufferView(zeroes2); | ||
| ok(cmp(v1, v2) === 0, `Equal ${typeName}s should return 0`); | ||
| // Less than | ||
| v1 = new ArrayBufferView(zeroes1); | ||
| v2 = new ArrayBufferView(ones); | ||
| ok(cmp(v1, v2) === -1, `Less than ${typeName}s should return -1`); | ||
| // Less than | ||
| v1 = new ArrayBufferView(ones); | ||
| v2 = new ArrayBufferView(zeroes1); | ||
| ok(cmp(v1, v2) === 1, `Greater than ${typeName}s should return 1`); | ||
| } | ||
| }); | ||
| test("it should respect IndexedDB's type order", () => { | ||
| const zoo = [ | ||
| 'meow', | ||
| 1, | ||
| new Date(), | ||
| Infinity, | ||
| -Infinity, | ||
| new ArrayBuffer(1), | ||
| [[]], | ||
| ]; | ||
| const [minusInfinity, num, infinity, date, string, binary, array] = | ||
| zoo.sort(cmp); | ||
| equal(minusInfinity, -Infinity, 'Minus infinity is sorted first'); | ||
| equal(num, 1, 'Numbers are sorted second'); | ||
| equal(infinity, Infinity, 'Infinity is sorted third'); | ||
| ok(date instanceof Date, 'Date is sorted fourth'); | ||
| ok(typeof string === 'string', 'strings are sorted fifth'); | ||
| ok(binary instanceof ArrayBuffer, 'binaries are sorted sixth'); | ||
| ok(Array.isArray(array), 'Arrays are sorted seventh'); | ||
| }); | ||
|
|
||
| test('it should return NaN on invalid types', () => { | ||
| ok( | ||
| isNaN(cmp(1, { foo: 'bar' })), | ||
| 'Comparing a number against an object returns NaN (would throw in indexedDB)' | ||
| ); | ||
| ok( | ||
| isNaN(cmp({ foo: 'bar' }, 1)), | ||
| 'Comparing an object against a number returns NaN also' | ||
| ); | ||
| }); | ||
|
|
||
| test('it should treat different binary types as if they were equal', () => { | ||
| const viewTypes = [ | ||
| 'DataView', | ||
| 'Uint8ClampedArray', | ||
| 'Uint8Array', | ||
| 'Int8Array', | ||
| 'Uint16Array', | ||
| 'Uint32Array', | ||
| 'Int32Array', | ||
| 'Float32Array', | ||
| 'Float64Array', | ||
| ] | ||
| .map((typeName) => [typeName, self[typeName]]) | ||
| .filter(([_, ctor]) => !!ctor); // Don't try to test types not supported by the browser | ||
|
|
||
| const zeroes1 = new ArrayBuffer(16); | ||
| const zeroes2 = new ArrayBuffer(16); | ||
| fillArrayBuffer(zeroes1, 0); | ||
| fillArrayBuffer(zeroes2, 0); | ||
|
|
||
| for (const [typeName, ArrayBufferView] of viewTypes) { | ||
| let v1 = new ArrayBufferView(zeroes1); | ||
| ok(cmp(v1, zeroes1) === 0, `Comparing ${typeName} with ArrayBuffer should return 0 if they have identical data`); | ||
| } | ||
| }); | ||
|
|
||
| test('it should return NaN if comparing arrays where any item or sub array item includes an invalid key', ()=> { | ||
| ok(cmp([1, [[2, "3"]]], [1,[[2, "3"]]]) === 0, "It can deep compare arrays with valid keys (equals)"); | ||
| ok(cmp([1, [[2, "3"]]], [1,[[2, 3]]]) === 1, "It can deep compare arrays with valid keys (greater than)"); | ||
| ok(isNaN(cmp([1, [[2, 3]]], [1,[[{foo: "bar"}, 3]]])), "It returns NaN when any item in the any of the arrays are invalid keys"); | ||
| }); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello, i was very excited when i first read about those performance improvements, however when taking a look at the code i got some doubts.
Function.bind(?)Check this out:
All good here, with 473bytes of data and not equal.
Still reasonable here when the data is equal, but...
Thoughts? Not sure if i did overlooked something, those tests were made under Chrome 96 on Windows.