Skip to content

feat: Add Memo<T> node type for skipping diff on unchanged data#51

Closed
MCGPPeters wants to merge 2 commits intomainfrom
perf/memoize-elements
Closed

feat: Add Memo<T> node type for skipping diff on unchanged data#51
MCGPPeters wants to merge 2 commits intomainfrom
perf/memoize-elements

Conversation

@MCGPPeters
Copy link
Copy Markdown
Contributor

📝 Description

What

Implements Memo<T> node type for skipping diffing when element data hasn't changed.

Why

When re-rendering large lists, most elements often remain unchanged. By tracking the data used to render each element, we can skip the entire diff subtree when the data matches.

How

  • Add IMemoNode interface with HasSameData() for type-safe comparison
  • Add Memo<T> record to wrap nodes with data for equality checking
  • Add memo<T>() helper function for convenient usage
  • Update DiffInternal to check memo data equality and skip diffing when data matches

🔗 Related Issues

Fixes #49

✅ Type of Change

  • ✨ New feature (non-breaking change which adds functionality)
  • ⚡ Performance improvement
  • ✅ Test update

🧪 Testing

Test Coverage

  • Unit tests added/updated
  • Manual testing performed

Testing Details

Added 5 new unit tests:

  • Memo_SkipsDiffing_WhenDataEqual - Verifies no patches when data unchanged
  • Memo_DiffsContent_WhenDataDifferent - Verifies patches when data changes
  • Memo_SkipsDiffing_ForReorderedChildren - Verifies memo works during reorder
  • Memo_WorksWithRenderNode - Verifies render function is called correctly
  • Memo_ShouldRenderInnerNode - Verifies HTML rendering works

All 90 tests pass.

📸 Usage Example

// Without memo - diffs every render
private static Node TableRow(Row row) =>
    tr([...], [...]);

// With memo - skips diff when row data unchanged
private static Node TableRow(Row row) =>
    memo(row, data => tr([...], [...]));

✨ Changes Made

  • Add IMemoNode interface with Data, InnerNode, and HasSameData()
  • Add Memo<T> record implementing IMemoNode
  • Add memo<T>() helper in Elements.cs
  • Add UnwrapMemoNodes() helper in diffing logic
  • Update DiffInternal to handle memo nodes
  • Update Render.Html and GetKey to unwrap memo nodes
  • Add 5 comprehensive unit tests

🔍 Code Review Checklist

  • Code follows the project's style guidelines
  • Self-review of code performed
  • Comments added for complex/non-obvious code
  • No new warnings generated
  • Tests added/updated and passing
  • All commits follow Conventional Commits format
  • Branch is up-to-date with main

🚀 Deployment Notes

None

📋 Additional Context

Current Limitations

The memo optimization provides the most benefit when combined with the LIS optimization (#50). In the current architecture:

  • Reorder operations: The remove-all/add-all path doesn't call DiffInternal, so memo can't help
  • Same-order diffs: Memo helps by skipping unchanged subtrees
  • Membership changes: Memo helps when diffing matched elements

Future Improvements

Once #50 is merged, memo will provide additional benefits:

  • During moves, memo can skip diffing moved elements entirely
  • Combined with LIS, this minimizes both DOM operations AND diff computation

Thank you for contributing to Abies! 🌲

Copilot AI review requested due to automatic review settings February 9, 2026 08:35
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds a new Memo<T> virtual-DOM node wrapper intended to skip diffing subtrees when the associated data hasn’t changed, reducing diff cost for large stable lists.

Changes:

  • Introduces IMemoNode + Memo<T> in DOM layer and hooks memo unwrapping into DiffInternal, HTML rendering, and key extraction.
  • Adds Elements.memo<T>(...) helpers for creating memo-wrapped nodes.
  • Adds unit tests covering memo diffing/rendering scenarios.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 8 comments.

File Description
Abies/Html/Elements.cs Adds memo<T> helper overloads and XML docs for creating memo-wrapped nodes.
Abies/DOM/Operations.cs Introduces IMemoNode/Memo<T> and integrates memo unwrapping into rendering/diffing/keying.
Abies.Tests/DomBehaviorTests.cs Adds memo-focused tests and minor formatting/cleanup.

Comment on lines +286 to +297
public interface IMemoNode
{
/// <summary>
/// The underlying node that will be rendered.
/// </summary>
Node InnerNode { get; }

/// <summary>
/// Checks if this memo node has the same data as another memo node.
/// Returns false if the other node is not a compatible IMemoNode.
/// </summary>
bool HasSameData(IMemoNode other);
Copy link

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

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

Render.Html now unwraps IMemoNode, but other framework traversals still operate only on Element (e.g., handler registration/unregistration and ID preservation). As a result, memo-wrapped subtrees can render handler attributes into HTML but never get their handlers registered, breaking event dispatch. Memo support should include unwrapping in all DOM-walkers that assume Node is an Element tree.

Copilot uses AI. Check for mistakes.
Comment on lines +496 to +510
[Fact]
public void Memo_WithSameData_SkipsDiffing()
{
// Create two memo nodes with the same data
var row = new TestRow(1, "Test");
var element = new Element("row-1", "tr", [], new Text("t1", "Test"));

var oldNode = new Memo<TestRow>(row, element);
var newNode = new Memo<TestRow>(row, element);

var patches = Operations.Diff(oldNode, newNode);

// Should produce no patches because data is equal
Assert.Empty(patches);
}
Copy link

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

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

Memo_WithSameData_SkipsDiffing is not a strong regression test for memo diff-skipping: it diffs two Memo roots and reuses the same Element instance, which could yield no patches even without a correct memo implementation. Consider embedding memo nodes as children of a real root Element (matching production usage) and using distinct but equivalent inner node instances so the test fails if memo unwrapping/skipping is removed or broken.

Copilot uses AI. Check for mistakes.
Comment on lines +565 to +595
[Fact]
public void Memo_ListReorder_SkipsDiffingUnchangedItems()
{
// Simulate a list reorder where items don't change, just move
var rows = new[]
{
new TestRow(1, "First"),
new TestRow(2, "Second"),
new TestRow(3, "Third")
};

// Create memo-wrapped elements for old tree
var oldChildren = rows.Select((row, i) =>
(Node)new Memo<TestRow>(row, new Element($"row-{row.Id}", "tr", [], new Text($"t{row.Id}", row.Label)))
).ToArray();
var oldTree = new Element("tbody", "tbody", [], oldChildren);

// Reorder: swap first and last
var reorderedRows = new[] { rows[2], rows[1], rows[0] };
var newChildren = reorderedRows.Select((row, i) =>
(Node)new Memo<TestRow>(row, new Element($"row-{row.Id}", "tr", [], new Text($"t{row.Id}", row.Label)))
).ToArray();
var newTree = new Element("tbody", "tbody", [], newChildren);

var patches = Operations.Diff(oldTree, newTree);

// The memo optimization means we should NOT see UpdateText patches
// because the data is equal and diffing was skipped for the inner content.
// We may see structural patches (AddChild, RemoveChild) for reordering.
Assert.DoesNotContain(patches, p => p is UpdateText);
}
Copy link

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

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

Memo_ListReorder_SkipsDiffingUnchangedItems currently only asserts the absence of UpdateText patches. With the current diff algorithm, a reorder produces only structural patches (or, if memo children are accidentally ignored, no patches at all), so this test can pass even when memo support is broken. Please assert the expected structural behavior too (e.g., patches contain the expected Add/Remove/Move operations, or apply patches and compare final HTML).

Copilot uses AI. Check for mistakes.
Comment on lines +528 to +533
/// <example>
/// // Without memo: diffs every row on every render
/// model.Rows.Select(row => TableRow(row))
///
/// // With memo: only diffs rows whose data changed
/// model.Rows.Select(row => memo(row, r => TableRow(r)))
Copy link

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

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

The XML doc example is not valid XML documentation markup as-is (plain code/comments inside <example>). Consider wrapping the sample in <code> (and escaping </> if present) so it renders correctly in generated docs/IntelliSense.

Suggested change
/// <example>
/// // Without memo: diffs every row on every render
/// model.Rows.Select(row => TableRow(row))
///
/// // With memo: only diffs rows whose data changed
/// model.Rows.Select(row => memo(row, r => TableRow(r)))
/// <example>
/// <code>
/// // Without memo: diffs every row on every render
/// model.Rows.Select(row => TableRow(row))
///
/// // With memo: only diffs rows whose data changed
/// model.Rows.Select(row => memo(row, r => TableRow(r)))
/// </code>

Copilot uses AI. Check for mistakes.
Comment on lines +302 to +311
/// The node is only re-rendered when the underlying data changes.
/// This is similar to React.memo or Elm's lazy.
/// </summary>
/// <typeparam name="T">The type of the data used to create the node.</typeparam>
/// <param name="Data">The data that was used to create the wrapped node.</param>
/// <param name="InnerNode">The actual DOM node (will be rendered lazily or reused).</param>
/// <remarks>
/// When diffing, if two Memo nodes have equal Data (via Equals),
/// the inner nodes are assumed to be identical and no diff is performed.
/// This dramatically reduces diffing overhead for large lists of stable items.
Copy link

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

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

Memo<T> XML docs claim the node is "only re-rendered when the underlying data changes" and that the inner node "will be rendered lazily or reused", but the current API always constructs InnerNode eagerly (e.g., Elements.memo(data, render) calls render(data) every render). Please adjust the documentation to match actual behavior (skips diffing only), or change the implementation so it actually avoids re-rendering/allocation when data is equal.

Suggested change
/// The node is only re-rendered when the underlying data changes.
/// This is similar to React.memo or Elm's lazy.
/// </summary>
/// <typeparam name="T">The type of the data used to create the node.</typeparam>
/// <param name="Data">The data that was used to create the wrapped node.</param>
/// <param name="InnerNode">The actual DOM node (will be rendered lazily or reused).</param>
/// <remarks>
/// When diffing, if two Memo nodes have equal Data (via Equals),
/// the inner nodes are assumed to be identical and no diff is performed.
/// This dramatically reduces diffing overhead for large lists of stable items.
/// Memo nodes allow the diff algorithm to skip comparing inner nodes when the underlying data is unchanged.
/// This is conceptually similar to React.memo or Elm's lazy diffing semantics.
/// </summary>
/// <typeparam name="T">The type of the data used to create the node.</typeparam>
/// <param name="Data">The data that was used to create the wrapped node.</param>
/// <param name="InnerNode">The actual DOM node associated with this memoized value.</param>
/// <remarks>
/// When diffing, if two Memo nodes have equal <paramref name="Data"/> (via <see cref="object.Equals(object?, object?)"/>),
/// the inner nodes are assumed to be equivalent and no diff is performed between them.
/// This reduces diffing overhead for large lists of stable items, but does not by itself control when or how
/// <paramref name="InnerNode"/> instances are created.

Copilot uses AI. Check for mistakes.
Comment on lines +1626 to +1631
// Unwrap Memo nodes to get the inner node's key
if (node is IMemoNode memo)
{
node = memo.InnerNode;
}

Copy link

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

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

GetKey unwraps a memo node, but DiffChildrenCore still treats Node values as Element/RawHtml/Text when generating Add/Remove patches. If list children are Memo<T> (the expected usage), the reorder/membership paths will generate no structural patches because memo nodes won't match those type checks, leaving the real DOM out of sync. Consider unwrapping memo nodes (recursively) before all child-type switches used for patch creation (remove/add/diff), e.g. via a shared UnwrapMemo(Node) helper.

Copilot uses AI. Check for mistakes.
Comment on lines +1671 to +1701
// Fast path: neither is a Memo node
if (oldNode is not IMemoNode && newNode is not IMemoNode)
{
return (oldNode, newNode, false);
}

// Both are Memo nodes - check if data is equal
if (oldNode is IMemoNode oldMemo && newNode is IMemoNode newMemo)
{
if (oldMemo.HasSameData(newMemo))
{
// Data is equal - skip diffing entirely
return (oldMemo.InnerNode, newMemo.InnerNode, true);
}
// Data changed - need to diff the inner nodes
return (oldMemo.InnerNode, newMemo.InnerNode, false);
}

// Only one is Memo - unwrap it and continue diffing
if (oldNode is IMemoNode oldMemoOnly)
{
return (oldMemoOnly.InnerNode, newNode, false);
}

if (newNode is IMemoNode newMemoOnly)
{
return (oldNode, newMemoOnly.InnerNode, false);
}

// Shouldn't reach here, but return unchanged
return (oldNode, newNode, false);
Copy link

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

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

UnwrapMemoNodes only unwraps a single memo layer. If a memo node wraps another memo node (easy to do via the memo(data, Node node) overload), DiffInternal will continue with oldNode/newNode still being memo nodes and then fall through without producing correct patches. Please unwrap memo nodes recursively/iteratively until the underlying non-memo Node is reached.

Suggested change
// Fast path: neither is a Memo node
if (oldNode is not IMemoNode && newNode is not IMemoNode)
{
return (oldNode, newNode, false);
}
// Both are Memo nodes - check if data is equal
if (oldNode is IMemoNode oldMemo && newNode is IMemoNode newMemo)
{
if (oldMemo.HasSameData(newMemo))
{
// Data is equal - skip diffing entirely
return (oldMemo.InnerNode, newMemo.InnerNode, true);
}
// Data changed - need to diff the inner nodes
return (oldMemo.InnerNode, newMemo.InnerNode, false);
}
// Only one is Memo - unwrap it and continue diffing
if (oldNode is IMemoNode oldMemoOnly)
{
return (oldMemoOnly.InnerNode, newNode, false);
}
if (newNode is IMemoNode newMemoOnly)
{
return (oldNode, newMemoOnly.InnerNode, false);
}
// Shouldn't reach here, but return unchanged
return (oldNode, newNode, false);
// Iteratively unwrap memo nodes on both sides until we reach the underlying non-memo nodes.
// While unwrapping, preserve the optimization of skipping the diff when both memo nodes
// report equal data via HasSameData.
var currentOld = oldNode;
var currentNew = newNode;
while (true)
{
// Both are Memo nodes - check if data is equal
if (currentOld is IMemoNode oldMemo && currentNew is IMemoNode newMemo)
{
if (oldMemo.HasSameData(newMemo))
{
// Data is equal - skip diffing entirely, but still return the inner nodes
return (oldMemo.InnerNode, newMemo.InnerNode, true);
}
// Data changed - unwrap one level on both sides and continue
currentOld = oldMemo.InnerNode;
currentNew = newMemo.InnerNode;
continue;
}
// Only the old node is a Memo - unwrap and continue
if (currentOld is IMemoNode oldMemoOnly)
{
currentOld = oldMemoOnly.InnerNode;
continue;
}
// Only the new node is a Memo - unwrap and continue
if (currentNew is IMemoNode newMemoOnly)
{
currentNew = newMemoOnly.InnerNode;
continue;
}
// Neither side is a Memo node anymore - we're fully unwrapped
return (currentOld, currentNew, false);
}

Copilot uses AI. Check for mistakes.
Comment on lines +1682 to +1684
// Data is equal - skip diffing entirely
return (oldMemo.InnerNode, newMemo.InnerNode, true);
}
Copy link

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

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

Memo diff-skipping currently depends only on HasSameData(). If Data is equal but the InnerNode's type/tag/id differs (or for Text/RawHtml, if the id changes), skipping the diff can leave the real DOM inconsistent because no Replace/Update patches will be emitted. To keep correctness, require at least InnerNode.Id equality and the same node shape/type before returning skipDiff=true (otherwise unwrap and continue diffing).

Suggested change
// Data is equal - skip diffing entirely
return (oldMemo.InnerNode, newMemo.InnerNode, true);
}
// Data is equal - only skip diffing if the inner node identity and shape are also stable.
// This ensures the real DOM remains consistent even when memo data stays the same
// but the underlying node type or Id would otherwise require a Replace/Update patch.
var oldInner = oldMemo.InnerNode;
var newInner = newMemo.InnerNode;
// Id must be equal for safe diff skipping.
if (!string.Equals(oldInner.Id, newInner.Id, StringComparison.Ordinal))
{
return (oldInner, newInner, false);
}
// Node shape/type must be the same to avoid mismatched DOM structures.
if (oldInner.GetType() != newInner.GetType())
{
return (oldInner, newInner, false);
}
// Data, Id, and node shape are all equal - safe to skip diffing entirely.
return (oldInner, newInner, true);
}

Copilot uses AI. Check for mistakes.
Implements memoization to skip diffing when element data hasn't changed:

- Add IMemoNode interface with DataEquals() for type-safe comparison
- Add Memo<T> record to wrap nodes with data for equality comparison
- Add memo<T>() helper function in Elements.cs
- Update DiffInternal to skip diffing when memo data matches
- Add UnwrapMemoNodes helper to extract inner nodes and check equality
- Update Render.Html and GetKey to handle IMemoNode

This optimization allows the framework to skip the entire diff subtree
when the underlying data is the same, reducing patch generation overhead
for unchanged elements during reorder operations.

Closes #49
@MCGPPeters MCGPPeters force-pushed the perf/memoize-elements branch from 1845e1a to 29889bc Compare February 9, 2026 10:11
Copy link
Copy Markdown

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'Rendering Engine Throughput'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.05.

Benchmark suite Current: 8855684 Previous: 7850fba Ratio
Abies.Benchmarks.Diffing/SmallDomDiff 536.6434462229411 ns (± 1.621570972794626) 494.8377917607625 ns (± 1.110715084276751) 1.08
Abies.Benchmarks.Diffing/LargeDomDiff 589.2071933746338 ns (± 1.4754873613824988) 527.1427401029147 ns (± 2.080621126147704) 1.12

This comment was automatically generated by workflow using github-action-benchmark.

CC: @MCGPPeters

@MCGPPeters MCGPPeters closed this Feb 9, 2026
@MCGPPeters MCGPPeters deleted the perf/memoize-elements branch February 12, 2026 15:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

perf: Skip diffing for unchanged elements during reorder

2 participants