Skip to content

Commit

Permalink
switch to special casing 0-value
Browse files Browse the repository at this point in the history
  • Loading branch information
samouri committed Feb 23, 2022
1 parent 3946843 commit 3ca275e
Show file tree
Hide file tree
Showing 14 changed files with 61 additions and 46 deletions.
10 changes: 5 additions & 5 deletions src/main-thread/commands/storage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { CommandExecutorInterface } from './interface';
import { TransferrableMutationType, StorageMutationIndex } from '../../transfer/TransferrableMutation';
import { StorageLocation } from '../../transfer/TransferrableStorage';
import { TransferrableKeys } from '../../transfer/TransferrableKeys';
import { MessageType, StorageValueToWorker, GetOrSet, DELETION_INDEX } from '../../transfer/Messages';
import { MessageType, StorageValueToWorker, GetOrSet } from '../../transfer/Messages';

export const StorageProcessor: CommandExecutorInterface = (strings, nodeContext, workerContext, objectContext, config) => {
const allowedExecution = config.executorsAllowed.includes(TransferrableMutationType.STORAGE);
Expand Down Expand Up @@ -64,8 +64,8 @@ export const StorageProcessor: CommandExecutorInterface = (strings, nodeContext,

// TODO(choumx): Clean up key/value strings (or don't store them in the first place)
// to avoid leaking memory.
const key = keyIndex != DELETION_INDEX && keyIndex >= 0 ? strings.get(keyIndex) : '';
const value = valueIndex != DELETION_INDEX && valueIndex >= 0 ? strings.get(valueIndex) : null;
const key = keyIndex > 0 ? strings.get(keyIndex) : '';
const value = valueIndex > 0 ? strings.get(valueIndex) : null;

if (operation === GetOrSet.GET) {
get(location, key);
Expand All @@ -82,8 +82,8 @@ export const StorageProcessor: CommandExecutorInterface = (strings, nodeContext,
const keyIndex = mutations[startPosition + StorageMutationIndex.Key];
const valueIndex = mutations[startPosition + StorageMutationIndex.Value];

const key = keyIndex >= 0 ? strings.get(keyIndex) : null;
const value = valueIndex >= 0 ? strings.get(valueIndex) : null;
const key = keyIndex > 0 ? strings.get(keyIndex) : null;
const value = valueIndex > 0 ? strings.get(valueIndex) : null;

return {
type: 'STORAGE',
Expand Down
6 changes: 5 additions & 1 deletion src/main-thread/strings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,11 @@ export class StringContext {
private strings: Array<string>;

constructor() {
this.strings = [];
this.strings = [
// The 0th index is reserved for meaning no value. Or delete.
// For example, the storage.removeItem code path.
null as any,
];
}

/**
Expand Down
5 changes: 4 additions & 1 deletion src/test/Emitter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,10 @@ export interface Emitter {
* mutation transfer state is also stored at the module-level.
* TODO: Remove this and replace with strings.getForTesting() at call sites.
*/
const strings: Array<string> = [];
const strings: Array<string> = [
// First index reserved to signify deletion.
null as unknown as string,
];

/**
* Stubs `document.postMessage` to invoke callbacks passed to `once()`.
Expand Down
6 changes: 3 additions & 3 deletions src/test/main-thread/commands/function.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ test('Returns the value of a resolved function', async (t) => {
const mutations: number[] = [];
mutations[FunctionMutationIndex.Status] = ResolveOrReject.RESOLVE;
mutations[FunctionMutationIndex.Index] = index;
mutations[FunctionMutationIndex.Value] = 0;
mutations[FunctionMutationIndex.Value] = 1;

processor.execute(new Uint16Array(mutations), 0, true);
t.deepEqual(await promise, { val: true });
Expand All @@ -41,7 +41,7 @@ test('Is able to return undefined', async (t) => {
const mutations: number[] = [];
mutations[FunctionMutationIndex.Status] = ResolveOrReject.RESOLVE;
mutations[FunctionMutationIndex.Index] = index;
mutations[FunctionMutationIndex.Value] = 0;
mutations[FunctionMutationIndex.Value] = 1;

processor.execute(new Uint16Array(mutations), 0, true);
t.deepEqual(await promise, undefined);
Expand All @@ -53,7 +53,7 @@ test('Returns the value of a rejected value', (t) => {
const mutations: number[] = [];
mutations[FunctionMutationIndex.Status] = ResolveOrReject.REJECT;
mutations[FunctionMutationIndex.Index] = index;
mutations[FunctionMutationIndex.Value] = 0;
mutations[FunctionMutationIndex.Value] = 1;

processor.execute(new Uint16Array(mutations), 0, true);
return promise.then(
Expand Down
6 changes: 3 additions & 3 deletions src/test/main-thread/commands/storage.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ test('StorageProcessor sends storage value event to worker', async (t) => {
const mutation: number[] = [];
mutation[StorageMutationIndex.Operation] = GetOrSet.GET;
mutation[StorageMutationIndex.Location] = StorageLocation.AmpState;
mutation[StorageMutationIndex.Key] = 0;
mutation[StorageMutationIndex.Key] = 1;
const mutations = new Uint16Array(mutation);

processor.execute(mutations, 0, true);
Expand All @@ -77,8 +77,8 @@ test('StorageProcessor handles deletion event from worker', async (t) => {
const mutation: number[] = [];
mutation[StorageMutationIndex.Operation] = GetOrSet.SET;
mutation[StorageMutationIndex.Location] = StorageLocation.Local;
mutation[StorageMutationIndex.Key] = 1;
mutation[StorageMutationIndex.Value] = -1;
mutation[StorageMutationIndex.Key] = 2;
mutation[StorageMutationIndex.Value] = 0;
const mutations = new Uint16Array(mutation);

processor.execute(mutations, 0, true);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ test('Returns the correct end offset', (t) => {

// main-thread's strings API does not return an ID when storing a string
// so for convenience:
function storeString(stringContext: StringContext, text: string, currentIndex = -1) {
function storeString(stringContext: StringContext, text: string, currentIndex = 0) {
stringContext.store(text);
return ++currentIndex;
}
Expand Down
36 changes: 18 additions & 18 deletions src/test/main-thread/mutator.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,9 +70,9 @@ test.serial('batch mutations', (t) => {
new Uint16Array([
TransferrableMutationType.ATTRIBUTES,
2, // Base Node
1,
0,
0,
0 + 1,
1 + 1,
]),
);
mutator.mutate(
Expand All @@ -82,9 +82,9 @@ test.serial('batch mutations', (t) => {
new Uint16Array([
TransferrableMutationType.ATTRIBUTES,
2, // Base Node
1,
2,
0,
1 + 1,
2 + 1,
]),
);

Expand All @@ -102,9 +102,9 @@ test.serial('batch mutations', (t) => {
new Uint16Array([
TransferrableMutationType.ATTRIBUTES,
2, // Base Node
2,
3,
0,
2 + 1,
3 + 1,
]),
);

Expand Down Expand Up @@ -140,9 +140,9 @@ test.serial('batch mutations with custom pump', (t) => {
new Uint16Array([
TransferrableMutationType.ATTRIBUTES,
2, // Base Node
1,
0,
0,
0 + 1,
1 + 1,
]),
);
mutator.mutate(
Expand All @@ -152,9 +152,9 @@ test.serial('batch mutations with custom pump', (t) => {
new Uint16Array([
TransferrableMutationType.ATTRIBUTES,
2, // Base Node
1,
2,
0,
1 + 1,
2 + 1,
]),
);

Expand All @@ -174,9 +174,9 @@ test.serial('batch mutations with custom pump', (t) => {
new Uint16Array([
TransferrableMutationType.ATTRIBUTES,
2, // Base Node
2,
3,
0,
2 + 1,
3 + 1,
]),
);

Expand Down Expand Up @@ -209,9 +209,9 @@ test.serial('leverage allowlist to exclude mutation type', (t) => {
new Uint16Array([
TransferrableMutationType.ATTRIBUTES,
2, // Base Node
1,
0,
0,
0 + 1,
1 + 1,
]),
);
mutator.mutate(
Expand All @@ -221,9 +221,9 @@ test.serial('leverage allowlist to exclude mutation type', (t) => {
new Uint16Array([
TransferrableMutationType.ATTRIBUTES,
2, // Base Node
1,
2,
0,
1 + 1,
2 + 1,
]),
);

Expand Down Expand Up @@ -257,9 +257,9 @@ test.serial('split strings from mutations', (t) => {
new Uint16Array([
TransferrableMutationType.ATTRIBUTES,
2, // Base Node
1,
0,
0,
0 + 1,
1 + 1,
]),
);

Expand Down
2 changes: 1 addition & 1 deletion src/test/main-thread/object-creation.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ test('Returns correct end offset', (t) => {
t.is(mutationsArray[endOffset], 32);
});

function storeString(stringContext: StringContext, text: string, currentIndex = -1) {
function storeString(stringContext: StringContext, text: string, currentIndex = 0) {
stringContext.store(text);
return ++currentIndex;
}
2 changes: 1 addition & 1 deletion src/test/main-thread/object-mutation.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,7 @@ function executeCall(

// main-thread's strings API does not return an ID when storing a string
// so for convenience:
function storeString(stringContext: StringContext, text: string, currentIndex = -1) {
function storeString(stringContext: StringContext, text: string, currentIndex = 0) {
stringContext.store(text);
return ++currentIndex;
}
2 changes: 1 addition & 1 deletion src/test/main-thread/string-properties.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ test('setting property back to an empty string', (t) => {
t.is(inputElement.value, secondUpdateValue);
});

function storeString(stringContext: StringContext, text: string, currentIndex = -1) {
function storeString(stringContext: StringContext, text: string, currentIndex = 0) {
stringContext.store(text);
return ++currentIndex;
}
6 changes: 3 additions & 3 deletions src/test/mutation-transfer/storage.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import anyTest, { TestInterface } from 'ava';
import { Document } from '../../worker-thread/dom/Document';
import { DELETION_INDEX, GetOrSet } from '../../transfer/Messages';
import { GetOrSet } from '../../transfer/Messages';
import { Storage, createStorage } from '../../worker-thread/Storage';
import { StorageLocation } from '../../transfer/TransferrableStorage';
import { TransferrableMutationType } from '../../transfer/TransferrableMutation';
Expand Down Expand Up @@ -54,7 +54,7 @@ test.serial.cb('Storage.removeItem', (t) => {
const { document, storage } = t.context;

expectMutations(document, (mutations) => {
t.deepEqual(mutations, [TransferrableMutationType.STORAGE, GetOrSet.SET, StorageLocation.Local, getForTesting('foo'), DELETION_INDEX]);
t.deepEqual(mutations, [TransferrableMutationType.STORAGE, GetOrSet.SET, StorageLocation.Local, getForTesting('foo'), 0]);
t.end();
});

Expand All @@ -65,7 +65,7 @@ test.serial.cb('Storage.clear', (t) => {
const { document, storage } = t.context;

expectMutations(document, (mutations) => {
t.deepEqual(mutations, [TransferrableMutationType.STORAGE, GetOrSet.SET, StorageLocation.Local, DELETION_INDEX, DELETION_INDEX]);
t.deepEqual(mutations, [TransferrableMutationType.STORAGE, GetOrSet.SET, StorageLocation.Local, 0, 0]);
t.end();
});

Expand Down
4 changes: 0 additions & 4 deletions src/transfer/Messages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,3 @@ export const enum ResolveOrReject {
RESOLVE = 1,
REJECT = 2,
}

// Mutations are are modeled as Uint16Array.
// Deletion is specified as a -1, which when placed into the array becomes 2^16-1.
export const DELETION_INDEX = 2 ** 16 - 1;
18 changes: 15 additions & 3 deletions src/worker-thread/Storage.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Document } from './dom/Document';
import { DELETION_INDEX, GetOrSet } from '../transfer/Messages';
import { GetOrSet } from '../transfer/Messages';
import { StorageLocation } from '../transfer/TransferrableStorage';
import { TransferrableMutationType } from '../transfer/TransferrableMutation';
import { store } from './strings';
Expand Down Expand Up @@ -58,7 +58,13 @@ export function createStorage(document: Document | DocumentStub, location: Stora
value(key: string): void {
delete this[key];

transfer(document, [TransferrableMutationType.STORAGE, GetOrSet.SET, location, store(key), DELETION_INDEX]);
transfer(document, [
TransferrableMutationType.STORAGE,
GetOrSet.SET,
location,
store(key),
0, // value == 0 represents deletion.
]);
},
});
define(storage, 'clear', {
Expand All @@ -67,7 +73,13 @@ export function createStorage(document: Document | DocumentStub, location: Stora
delete this[key];
});

transfer(document, [TransferrableMutationType.STORAGE, GetOrSet.SET, location, DELETION_INDEX, DELETION_INDEX]);
transfer(document, [
TransferrableMutationType.STORAGE,
GetOrSet.SET,
location,
0, // key == 0 represents all keys.
0, // value == 0 represents deletion.
]);
},
});
return storage as Storage;
Expand Down
2 changes: 1 addition & 1 deletion src/worker-thread/strings.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
let count: number = 0;
let count: number = 1; // count starts from 1 since 0th index is reserved to mean "no value" or "delete".
let transfer: Array<string> = [];
const mapping: Map<string, number> = new Map();

Expand Down

0 comments on commit 3ca275e

Please sign in to comment.