Skip to content

chore: improve performance of DOM traversal operations #12863

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

Merged
merged 3 commits into from
Aug 15, 2024
Merged

chore: improve performance of DOM traversal operations #12863

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a98c895
chore: improve performance of DOM traversal operations
trueadm Aug 15, 2024
d859f0a
feedback
trueadm Aug 15, 2024
39acd8d
merge main
Rich-Harris Aug 15, 2024
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
5 changes: 5 additions & 0 deletions .changeset/odd-toys-glow.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'svelte': patch
---

chore: improve performance of DOM traversal operations
3 changes: 2 additions & 1 deletion packages/svelte/src/internal/client/dom/blocks/css-props.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/** @import { TemplateNode } from '#client' */
import { render_effect, teardown } from '../../reactivity/effects.js';
import { hydrate_node, hydrating, set_hydrate_node } from '../hydration.js';
import { get_first_child } from '../operations.js';

/**
* @param {HTMLDivElement | SVGGElement} element
Expand All @@ -9,7 +10,7 @@ import { hydrate_node, hydrating, set_hydrate_node } from '../hydration.js';
*/
export function css_props(element, get_styles) {
if (hydrating) {
set_hydrate_node(/** @type {TemplateNode} */ (element.firstChild));
set_hydrate_node(/** @type {TemplateNode} */ (get_first_child(element)));
}

render_effect(() => {
Expand Down
11 changes: 8 additions & 3 deletions packages/svelte/src/internal/client/dom/blocks/each.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,12 @@ import {
set_hydrate_node,
set_hydrating
} from '../hydration.js';
import { clear_text_content, create_text } from '../operations.js';
import {
clear_text_content,
create_text,
get_first_child,
get_next_sibling
} from '../operations.js';
import {
block,
branch,
Expand Down Expand Up @@ -116,7 +121,7 @@ export function each(node, flags, get_collection, get_key, render_fn, fallback_f
var parent_node = /** @type {Element} */ (node);

anchor = hydrating
? set_hydrate_node(/** @type {Comment | Text} */ (parent_node.firstChild))
? set_hydrate_node(/** @type {Comment | Text} */ (get_first_child(parent_node)))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In #12863 (comment) I was referring to these calls too. Isn't parent_node.firstChild preferable? Surely get_first_child is polymorphic (megamorphic?), given we call it with different node types

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Screenshot 2024-08-15 at 18 36 43

All the IC call-sites that use these functions come back as inlined so they're not really in the JIT code.

: parent_node.appendChild(create_text());
}

Expand Down Expand Up @@ -510,7 +515,7 @@ function move(item, next, anchor) {
var node = /** @type {EffectNodes} */ (item.e.nodes).start;

while (node !== end) {
var next_node = /** @type {TemplateNode} */ (node.nextSibling);
var next_node = /** @type {TemplateNode} */ (get_next_sibling(node));
dest.before(node);
node = next_node;
}
Expand Down
11 changes: 6 additions & 5 deletions packages/svelte/src/internal/client/dom/blocks/html.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import * as w from '../../warnings.js';
import { hash } from '../../../../utils.js';
import { DEV } from 'esm-env';
import { dev_current_component_function } from '../../runtime.js';
import { get_first_child, get_next_sibling } from '../operations.js';

/**
* @param {Element} element
Expand Down Expand Up @@ -71,7 +72,7 @@ export function html(node, get_value, svg, mathml, skip_warning) {
(next.nodeType !== 8 || /** @type {Comment} */ (next).data !== '')
) {
last = next;
next = /** @type {TemplateNode} */ (next.nextSibling);
next = /** @type {TemplateNode} */ (get_next_sibling(next));
}

if (next === null) {
Expand All @@ -98,17 +99,17 @@ export function html(node, get_value, svg, mathml, skip_warning) {
var node = create_fragment_from_html(html);

if (svg || mathml) {
node = /** @type {Element} */ (node.firstChild);
node = /** @type {Element} */ (get_first_child(node));
}

assign_nodes(
/** @type {TemplateNode} */ (node.firstChild),
/** @type {TemplateNode} */ (get_first_child(node)),
/** @type {TemplateNode} */ (node.lastChild)
);

if (svg || mathml) {
while (node.firstChild) {
anchor.before(node.firstChild);
while (get_first_child(node)) {
anchor.before(/** @type {Node} */ (get_first_child(node)));
}
} else {
anchor.before(node);
Expand Down
5 changes: 3 additions & 2 deletions packages/svelte/src/internal/client/dom/blocks/snippet.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { create_fragment_from_html } from '../reconciler.js';
import { assign_nodes } from '../template.js';
import * as w from '../../warnings.js';
import { DEV } from 'esm-env';
import { get_first_child, get_next_sibling } from '../operations.js';

/**
* @template {(node: TemplateNode, ...args: any[]) => void} SnippetFn
Expand Down Expand Up @@ -89,9 +90,9 @@ export function createRawSnippet(fn) {
} else {
var html = snippet.render().trim();
var fragment = create_fragment_from_html(html);
element = /** @type {Element} */ (fragment.firstChild);
element = /** @type {Element} */ (get_first_child(fragment));

if (DEV && (element.nextSibling !== null || element.nodeType !== 3)) {
if (DEV && (get_next_sibling(element) !== null || element.nodeType !== 3)) {
w.invalid_raw_snippet_render();
}

Expand Down
4 changes: 2 additions & 2 deletions packages/svelte/src/internal/client/dom/blocks/svelte-element.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import {
set_hydrate_node,
set_hydrating
} from '../hydration.js';
import { create_text } from '../operations.js';
import { create_text, get_first_child } from '../operations.js';
import {
block,
branch,
Expand Down Expand Up @@ -119,7 +119,7 @@ export function element(node, get_tag, is_svg, render_fn, get_namespace, locatio
// If hydrating, use the existing ssr comment as the anchor so that the
// inner open and close methods can pick up the existing nodes correctly
var child_anchor = /** @type {TemplateNode} */ (
hydrating ? element.firstChild : element.appendChild(create_text())
hydrating ? get_first_child(element) : element.appendChild(create_text())
);

if (hydrating) {
Expand Down
8 changes: 4 additions & 4 deletions packages/svelte/src/internal/client/dom/blocks/svelte-head.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/** @import { TemplateNode } from '#client' */
import { hydrate_node, hydrating, set_hydrate_node, set_hydrating } from '../hydration.js';
import { create_text } from '../operations.js';
import { create_text, get_first_child, get_next_sibling } from '../operations.js';
import { block } from '../../reactivity/effects.js';
import { HEAD_EFFECT } from '../../constants.js';
import { HYDRATION_START } from '../../../../constants.js';
Expand Down Expand Up @@ -32,22 +32,22 @@ export function head(render_fn) {

// There might be multiple head blocks in our app, so we need to account for each one needing independent hydration.
if (head_anchor === undefined) {
head_anchor = /** @type {TemplateNode} */ (document.head.firstChild);
head_anchor = /** @type {TemplateNode} */ (get_first_child(document.head));
}

while (
head_anchor !== null &&
(head_anchor.nodeType !== 8 || /** @type {Comment} */ (head_anchor).data !== HYDRATION_START)
) {
head_anchor = /** @type {TemplateNode} */ (head_anchor.nextSibling);
head_anchor = /** @type {TemplateNode} */ (get_next_sibling(head_anchor));
}

// If we can't find an opening hydration marker, skip hydration (this can happen
// if a framework rendered body but not head content)
if (head_anchor === null) {
set_hydrating(false);
} else {
head_anchor = set_hydrate_node(/** @type {TemplateNode} */ (head_anchor.nextSibling));
head_anchor = set_hydrate_node(/** @type {TemplateNode} */ (get_next_sibling(head_anchor)));
}
}

Expand Down
4 changes: 2 additions & 2 deletions packages/svelte/src/internal/client/dom/elements/misc.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { hydrating } from '../hydration.js';
import { clear_text_content } from '../operations.js';
import { clear_text_content, get_first_child } from '../operations.js';
import { queue_micro_task } from '../task.js';

/**
Expand Down Expand Up @@ -27,7 +27,7 @@ export function autofocus(dom, value) {
* @returns {void}
*/
export function remove_textarea_child(dom) {
if (hydrating && dom.firstChild !== null) {
if (hydrating && get_first_child(dom) !== null) {
clear_text_content(dom);
}
}
Expand Down
7 changes: 4 additions & 3 deletions packages/svelte/src/internal/client/dom/hydration.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
HYDRATION_START_ELSE
} from '../../../constants.js';
import * as w from '../warnings.js';
import { get_next_sibling } from './operations.js';

/**
* Use this variable to guard everything related to hydration code so it can be treeshaken out
Expand Down Expand Up @@ -39,15 +40,15 @@ export function set_hydrate_node(node) {
}

export function hydrate_next() {
return set_hydrate_node(/** @type {TemplateNode} */ (hydrate_node.nextSibling));
return set_hydrate_node(/** @type {TemplateNode} */ (get_next_sibling(hydrate_node)));
}

/** @param {TemplateNode} node */
export function reset(node) {
if (!hydrating) return;

// If the node has remaining siblings, something has gone wrong
if (hydrate_node.nextSibling !== null) {
if (get_next_sibling(hydrate_node) !== null) {
w.hydration_mismatch();
throw HYDRATION_ERROR;
}
Expand Down Expand Up @@ -90,7 +91,7 @@ export function remove_nodes() {
}
}

var next = /** @type {TemplateNode} */ (node.nextSibling);
var next = /** @type {TemplateNode} */ (get_next_sibling(node));
node.remove();
node = next;
}
Expand Down
42 changes: 36 additions & 6 deletions packages/svelte/src/internal/client/dom/operations.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import { hydrate_node, hydrating, set_hydrate_node } from './hydration.js';
import { DEV } from 'esm-env';
import { init_array_prototype_warnings } from '../dev/equality.js';
import { get_descriptor } from '../../shared/utils.js';

// export these for reference in the compiled code, making global name deduplication unnecessary
/** @type {Window} */
Expand All @@ -10,6 +11,11 @@ export var $window;
/** @type {Document} */
export var $document;

/** @type {() => Node | null} */
var first_child_getter;
/** @type {() => Node | null} */
var next_sibling_getter;

/**
* Initialize these lazily to avoid issues when using the runtime in a server context
* where these globals are not available while avoiding a separate server entry point
Expand All @@ -23,6 +29,12 @@ export function init_operations() {
$document = document;

var element_prototype = Element.prototype;
var node_prototype = Node.prototype;

// @ts-ignore
first_child_getter = get_descriptor(node_prototype, 'firstChild').get;
// @ts-ignore
next_sibling_getter = get_descriptor(node_prototype, 'nextSibling').get;

// the following assignments improve perf of lookups on DOM nodes
// @ts-expect-error
Expand Down Expand Up @@ -53,6 +65,24 @@ export function create_text(value = '') {
return document.createTextNode(value);
}

/**
* @template {Node} N
* @param {N} node
* @returns {Node | null}
*/
export function get_first_child(node) {
return first_child_getter.call(node);
}

/**
* @template {Node} N
* @param {N} node
* @returns {Node | null}
*/
export function get_next_sibling(node) {
return next_sibling_getter.call(node);
}

/**
* Don't mark this as side-effect-free, hydration needs to walk all nodes
* @template {Node} N
Expand All @@ -61,10 +91,10 @@ export function create_text(value = '') {
*/
export function child(node) {
if (!hydrating) {
return node.firstChild;
return get_first_child(node);
}

var child = /** @type {TemplateNode} */ (hydrate_node.firstChild);
var child = /** @type {TemplateNode} */ (get_first_child(hydrate_node));

// Child can be null if we have an element with a single child, like `<p>{text}</p>`, where `text` is empty
if (child === null) {
Expand All @@ -84,10 +114,10 @@ export function child(node) {
export function first_child(fragment, is_text) {
if (!hydrating) {
// when not hydrating, `fragment` is a `DocumentFragment` (the result of calling `open_frag`)
var first = /** @type {DocumentFragment} */ (fragment).firstChild;
var first = /** @type {DocumentFragment} */ (get_first_child(/** @type {Node} */ (fragment)));

// TODO prevent user comments with the empty string when preserveComments is true
if (first instanceof Comment && first.data === '') return first.nextSibling;
if (first instanceof Comment && first.data === '') return get_next_sibling(first);

return first;
}
Expand All @@ -114,10 +144,10 @@ export function first_child(fragment, is_text) {
*/
export function sibling(node, is_text = false) {
if (!hydrating) {
return /** @type {TemplateNode} */ (node.nextSibling);
return /** @type {TemplateNode} */ (get_next_sibling(node));
}

var next_sibling = /** @type {TemplateNode} */ (hydrate_node.nextSibling);
var next_sibling = /** @type {TemplateNode} */ (get_next_sibling(hydrate_node));

var type = next_sibling.nodeType;

Expand Down
16 changes: 8 additions & 8 deletions packages/svelte/src/internal/client/dom/template.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/** @import { Effect, EffectNodes, TemplateNode } from '#client' */
import { hydrate_next, hydrate_node, hydrating, set_hydrate_node } from './hydration.js';
import { create_text } from './operations.js';
import { create_text, get_first_child } from './operations.js';
import { create_fragment_from_html } from './reconciler.js';
import { current_effect } from '../runtime.js';
import { TEMPLATE_FRAGMENT, TEMPLATE_USE_IMPORT_NODE } from '../../../constants.js';
Expand Down Expand Up @@ -41,15 +41,15 @@ export function template(content, flags) {

if (!node) {
node = create_fragment_from_html(has_start ? content : '<!>' + content);
if (!is_fragment) node = /** @type {Node} */ (node.firstChild);
if (!is_fragment) node = /** @type {Node} */ (get_first_child(node));
}

var clone = /** @type {TemplateNode} */ (
use_import_node ? document.importNode(node, true) : node.cloneNode(true)
);

if (is_fragment) {
var start = /** @type {TemplateNode} */ (clone.firstChild);
var start = /** @type {TemplateNode} */ (get_first_child(clone));
var end = /** @type {TemplateNode} */ (clone.lastChild);

assign_nodes(start, end);
Expand Down Expand Up @@ -113,22 +113,22 @@ export function ns_template(content, flags, ns = 'svg') {

if (!node) {
var fragment = /** @type {DocumentFragment} */ (create_fragment_from_html(wrapped));
var root = /** @type {Element} */ (fragment.firstChild);
var root = /** @type {Element} */ (get_first_child(fragment));

if (is_fragment) {
node = document.createDocumentFragment();
while (root.firstChild) {
node.appendChild(root.firstChild);
while (get_first_child(root)) {
node.appendChild(/** @type {Node} */ (get_first_child(root)));
}
} else {
node = /** @type {Element} */ (root.firstChild);
node = /** @type {Element} */ (get_first_child(root));
}
}

var clone = /** @type {TemplateNode} */ (node.cloneNode(true));

if (is_fragment) {
var start = /** @type {TemplateNode} */ (clone.firstChild);
var start = /** @type {TemplateNode} */ (get_first_child(clone));
var end = /** @type {TemplateNode} */ (clone.lastChild);

assign_nodes(start, end);
Expand Down
3 changes: 2 additions & 1 deletion packages/svelte/src/internal/client/reactivity/effects.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import { set } from './sources.js';
import * as e from '../errors.js';
import { DEV } from 'esm-env';
import { define_property } from '../../shared/utils.js';
import { get_next_sibling } from '../dom/operations.js';

/**
* @param {'$effect' | '$effect.pre' | '$inspect'} rune
Expand Down Expand Up @@ -361,7 +362,7 @@ export function destroy_effect(effect, remove_dom = true) {

while (node !== null) {
/** @type {TemplateNode | null} */
var next = node === end ? null : /** @type {TemplateNode} */ (node.nextSibling);
var next = node === end ? null : /** @type {TemplateNode} */ (get_next_sibling(node));

node.remove();
node = next;
Expand Down
Loading
Loading