diff --git a/packages/rearch/lib/src/node.dart b/packages/rearch/lib/src/node.dart index 4c8e425..7f269f5 100644 --- a/packages/rearch/lib/src/node.dart +++ b/packages/rearch/lib/src/node.dart @@ -32,31 +32,31 @@ abstract class DataflowGraphNode implements Disposable { final selfChanged = buildSelf(); if (!selfChanged) return; - /* - // TODO only build dependents when buildSelf returns true, - // and only gc nodes when depenency changes - - final disposable = <_DataflowGraphNode>{}; - - buildOrder.reversed.where((node) { - final dependentsAllDisposable = - node.dependents.every(disposable.contains); - return node.isSuperPure && dependentsAllDisposable; - }).forEach(disposable.add); - - return disposable; - */ - - // Garbage collect all possible dependents and then build the rest - final buildOrder = garbageCollectDisposableNodes( - createBuildOrder().skip(1).toList(), - ); + // Build or garbage collect (dispose) all remaining nodes + // (We use skip(1) to avoid building this node twice) + final buildOrder = _createBuildOrder().skip(1).toList(); + final disposableNodes = _getDisposableNodesFromBuildOrder(buildOrder); + final changedNodes = {this}; for (final node in buildOrder) { - node.buildSelf(); + final haveDepsChanged = node._dependencies.any(changedNodes.contains); + if (!haveDepsChanged) continue; + + if (disposableNodes.contains(node)) { + // Note: dependency/dependent relationships will be after this, + // since we are disposing all dependents in the build order, + // because we are adding this node to changedNodes + node.dispose(); + changedNodes.add(node); + } else { + final didNodeChange = node.buildSelf(); + if (didNodeChange) { + changedNodes.add(node); + } + } } } - List createBuildOrder() { + List _createBuildOrder() { // We need some more information alongside of each node // in order to do the topological sort: // - False is for the first visit, which adds all deps to be visited, @@ -85,20 +85,17 @@ abstract class DataflowGraphNode implements Disposable { return buildOrderStack.reversed.toList(); } - static Iterable garbageCollectDisposableNodes( + static Set _getDisposableNodesFromBuildOrder( List buildOrder, ) { - final nonDisposable = []; + final disposable = {}; - for (final node in buildOrder.reversed) { - final isDisposable = node.isSuperPure && node._dependents.isEmpty; - if (isDisposable) { - node.dispose(); - } else { - nonDisposable.add(node); - } - } + buildOrder.reversed.where((node) { + final dependentsAllDisposable = + node._dependents.every(disposable.contains); + return node.isSuperPure && dependentsAllDisposable; + }).forEach(disposable.add); - return nonDisposable.reversed; + return disposable; } } diff --git a/packages/rearch/test/basic_test.dart b/packages/rearch/test/basic_test.dart index 795339e..f32ccd2 100644 --- a/packages/rearch/test/basic_test.dart +++ b/packages/rearch/test/basic_test.dart @@ -115,6 +115,116 @@ void main() { expect(states, equals([1, 2, 3, 5, 6, 7])); }); + test('== check skips unneeded rebuilds', () { + final builds = , int>{}; + + (int, void Function(int)) stateful(CapsuleHandle use) { + builds.update(stateful, (count) => count + 1, ifAbsent: () => 1); + return use.state(0); + } + + int unchangingSuperPureDep(CapsuleHandle use) { + builds.update( + unchangingSuperPureDep, + (count) => count + 1, + ifAbsent: () => 1, + ); + use(stateful); + return 0; + } + + int unchangingWatcher(CapsuleHandle use) { + builds.update( + unchangingWatcher, + (count) => count + 1, + ifAbsent: () => 1, + ); + return use(unchangingSuperPureDep); + } + + int changingSuperPureDep(CapsuleHandle use) { + builds.update( + changingSuperPureDep, + (count) => count + 1, + ifAbsent: () => 1, + ); + return use(stateful).$1; + } + + int changingWatcher(CapsuleHandle use) { + builds.update( + changingWatcher, + (count) => count + 1, + ifAbsent: () => 1, + ); + return use(changingSuperPureDep); + } + + void impureSink(CapsuleHandle use) { + use.register((_) {}); + use(changingWatcher); + use(unchangingWatcher); + } + + final container = useContainer(); + + expect(container.read(unchangingWatcher), equals(0)); + expect(container.read(changingWatcher), equals(0)); + expect(builds[stateful], equals(1)); + expect(builds[unchangingSuperPureDep], equals(1)); + expect(builds[changingSuperPureDep], equals(1)); + expect(builds[unchangingWatcher], equals(1)); + expect(builds[changingWatcher], equals(1)); + + container.read(stateful).$2(0); + expect(builds[stateful], equals(2)); + expect(builds[unchangingSuperPureDep], equals(1)); + expect(builds[changingSuperPureDep], equals(1)); + expect(builds[unchangingWatcher], equals(1)); + expect(builds[changingWatcher], equals(1)); + + expect(container.read(unchangingWatcher), equals(0)); + expect(container.read(changingWatcher), equals(0)); + expect(builds[stateful], equals(2)); + expect(builds[unchangingSuperPureDep], equals(1)); + expect(builds[changingSuperPureDep], equals(1)); + expect(builds[unchangingWatcher], equals(1)); + expect(builds[changingWatcher], equals(1)); + + container.read(stateful).$2(1); + expect(builds[stateful], equals(3)); + expect(builds[unchangingSuperPureDep], equals(1)); + expect(builds[changingSuperPureDep], equals(1)); + expect(builds[unchangingWatcher], equals(1)); + expect(builds[changingWatcher], equals(1)); + + expect(container.read(unchangingWatcher), equals(0)); + expect(container.read(changingWatcher), equals(1)); + expect(builds[stateful], equals(3)); + expect(builds[unchangingSuperPureDep], equals(2)); + expect(builds[changingSuperPureDep], equals(2)); + expect(builds[unchangingWatcher], equals(2)); + expect(builds[changingWatcher], equals(2)); + + // Disable the super pure gc + container.read(impureSink); + + container.read(stateful).$2(2); + expect(builds[stateful], equals(4)); + expect(builds[unchangingSuperPureDep], equals(3)); + expect(builds[changingSuperPureDep], equals(3)); + expect(builds[unchangingWatcher], equals(2)); + expect(builds[changingWatcher], equals(3)); + + expect(container.read(unchangingWatcher), equals(0)); + expect(container.read(changingWatcher), equals(2)); + expect(builds[stateful], equals(4)); + expect(builds[unchangingSuperPureDep], equals(3)); + expect(builds[changingSuperPureDep], equals(3)); + expect(builds[unchangingWatcher], equals(2)); + expect(builds[changingWatcher], equals(3)); + }); + // We use a more sophisticated graph here for a more thorough // test of all functionality //