Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[perf] batch-destroy #1529

Closed
wants to merge 1 commit into from
Closed

[perf] batch-destroy #1529

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 33 additions & 0 deletions packages/@glimmer-workspace/integration-tests/lib/suites/initial-render.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1033,6 +1033,39 @@ export class InitialRenderSuite extends RenderTest {
);
}

@test
'Lists could be updated'() {
const people = [
{ handle: 'tomdale', name: 'Tom Dale' },
{ handle: 'chancancode', name: 'Godfrey Chan' },
{ handle: 'wycats', name: 'Yehuda Katz' },
];
this.render(
'<div>{{#each this.people key="handle" as |p|}}<span>{{p.handle}}</span> - {{p.name}}{{/each}}</div>',
{
people,
}
);

this.assertHTML(
'<div><span>tomdale</span> - Tom Dale<span>chancancode</span> - Godfrey Chan<span>wycats</span> - Yehuda Katz</div>'
);

this.rerender({
people: [people[2], people[0]],
});

this.assertHTML('<div><span>wycats</span> - Yehuda Katz<span>tomdale</span> - Tom Dale</div>');

this.rerender({
people,
});

this.assertHTML(
'<div><span>tomdale</span> - Tom Dale<span>chancancode</span> - Godfrey Chan<span>wycats</span> - Yehuda Katz</div>'
);
}

@test
'Simple helpers'() {
this.registerHelper('testing', ([id]) => id);
Expand Down
117 changes: 47 additions & 70 deletions packages/@glimmer/runtime/lib/vm/update.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import type { OpaqueIterationItem, OpaqueIterator, Reference } from '@glimmer/re
import { associateDestroyableChild, destroy, destroyChildren } from '@glimmer/destroyable';
import { LOCAL_DEBUG } from '@glimmer/local-debug-flags';
import { updateRef, valueForRef } from '@glimmer/reference';
import { expect, logStep, Stack, unwrap } from '@glimmer/util';
import { expect, logStep, Stack } from '@glimmer/util';
import { debug, resetTracking } from '@glimmer/validator';

import type { InternalVM, VmInitCallback } from './append';
Expand Down Expand Up @@ -183,7 +183,6 @@ export class TryOpcode extends BlockOpcode implements ExceptionHandler {
}

export class ListItemOpcode extends TryOpcode {
public retained = false;
public index = -1;

constructor(
Expand All @@ -198,18 +197,9 @@ export class ListItemOpcode extends TryOpcode {
}

updateReferences(item: OpaqueIterationItem) {
this.retained = true;
updateRef(this.value, item.value);
updateRef(this.memo, item.memo);
}

shouldRemove(): boolean {
return !this.retained;
}

reset() {
this.retained = false;
}
}

export class ListBlockOpcode extends BlockOpcode {
Expand Down Expand Up @@ -266,76 +256,54 @@ export class ListBlockOpcode extends BlockOpcode {
private sync(iterator: OpaqueIterator) {
let { opcodeMap: itemMap, children } = this;

let currentOpcodeIndex = 0;
let seenIndex = 0;

this.children = this.bounds.boundList = [];

let items: OpaqueIterationItem[] = [];

// eslint-disable-next-line no-constant-condition
while (true) {
let item = iterator.next();

if (item === null) break;
items.push(item);
}

let opcode = children[currentOpcodeIndex];
let { key } = item;
const existingKeys = new Set(itemMap.keys());
const updatingKeys = new Set(items.map((item) => item.key));
const keysToRemove = [...existingKeys].filter((key) => !updatingKeys.has(key));
const removedIndexes = [];

// Items that have already been found and moved will already be retained,
// we can continue until we find the next unretained item
while (opcode !== undefined && opcode.retained === true) {
opcode = children[++currentOpcodeIndex];
if (keysToRemove.length === existingKeys.size) {
this.deleteAllItems(children);
children.splice(0, children.length);
} else {
for (const key of keysToRemove) {
const opcode = itemMap.get(key)!;
removedIndexes.push(opcode.index);
this.deleteItem(opcode);
children.splice(children.indexOf(opcode), 1);
}
}

if (opcode !== undefined && opcode.key === key) {
this.retainItem(opcode, item);
currentOpcodeIndex++;
} else if (itemMap.has(key)) {
let itemOpcode = itemMap.get(key)!;

// The item opcode was seen already, so we should move it.
if (itemOpcode.index < seenIndex) {
this.moveItem(itemOpcode, item, opcode);
} else {
// Update the seen index, we are going to be moving this item around
// so any other items that come before it will likely need to move as
// well.
seenIndex = itemOpcode.index;

let seenUnretained = false;

// iterate through all of the opcodes between the current position and
// the position of the item's opcode, and determine if they are all
// retained.
for (let i = currentOpcodeIndex + 1; i < seenIndex; i++) {
if (unwrap(children[i]).retained === false) {
seenUnretained = true;
break;
}
}

// If we have seen only retained opcodes between this and the matching
// opcode, it means that all the opcodes in between have been moved
// already, and we can safely retain this item's opcode.
if (seenUnretained === false) {
this.retainItem(itemOpcode, item);
currentOpcodeIndex = seenIndex + 1;
} else {
this.moveItem(itemOpcode, item, opcode);
currentOpcodeIndex++;
}
for (const value of children) {
removedIndexes.forEach((index) => {
if (value.index > index) {
value.index--;
}
} else {
this.insertItem(item, opcode);
}
});
}

for (const opcode of children) {
if (opcode.retained === false) {
this.deleteItem(opcode);
items.forEach((item, index) => {
const opcode = itemMap.get(item.key);
if (opcode !== undefined) {
if (opcode.index !== index) {
this.moveItem(opcode, item, index === 0 ? children[index] : undefined);
} else {
this.retainItem(opcode, item);
}
} else {
opcode.reset();
this.insertItem(item, undefined);
}
}
});
}

private retainItem(opcode: ListItemOpcode, item: OpaqueIterationItem) {
Expand All @@ -347,9 +315,6 @@ export class ListBlockOpcode extends BlockOpcode {

updateRef(opcode.memo, item.memo);
updateRef(opcode.value, item.value);
opcode.retained = true;

opcode.index = children.length;
children.push(opcode);
}

Expand Down Expand Up @@ -389,7 +354,6 @@ export class ListBlockOpcode extends BlockOpcode {

updateRef(opcode.memo, item.memo);
updateRef(opcode.value, item.value);
opcode.retained = true;

let currentSibling, nextSibling;

Expand Down Expand Up @@ -417,6 +381,19 @@ export class ListBlockOpcode extends BlockOpcode {
}
}

private deleteAllItems(opcodes: ListItemOpcode[]) {
if (LOCAL_DEBUG) {
for (const opcode of opcodes) {
logStep!('list-updates', ['delete', opcode.key]);
}
}
for (const opcode of opcodes) {
destroy(opcode);
clear(opcode);
}
this.opcodeMap.clear();
}

private deleteItem(opcode: ListItemOpcode) {
if (LOCAL_DEBUG) {
logStep!('list-updates', ['delete', opcode.key]);
Expand Down
Loading