mirrors
/
CC-Tweaked
mirror of https://github.com/SquidDev-CC/CC-Tweaked
1
0
Fork 0
Code Issues Releases Wiki Activity

More efficient removal of wired nodes from networks 

When we remove a wired node from a network, we need to find connected
components in the rest of the graph. Typically, this requires a
traversal of the whole graph, taking O(|V| + |E|) time.

If we remove a lot of nodes at once (such as when unloading chunks),
this ends up being quadratic in the number of nodes. In some test
networks, this can take anywhere from a few seconds, to hanging the game
indefinitely.

This attempts to reduce the cases where this can happen, with a couple
of optimisations:

 - Instead of constructing a new hash set of reachable nodes (requiring
   multiple allocations and hash lookups), we store reachability as a
   temporary field on the WiredNode.

 - We abort our traversal of the graph if we can prove the graph remains
   connected after removing the node.

There's definitely future work to be done here in optimising large wired
networks, but this is a good first step.
This commit is contained in:
Jonathan Coates 2024-02-24 14:56:53 +00:00
parent 31aaf46d09
commit 84b6edab82
No known key found for this signature in database
GPG Key ID: B9E431FF07C98D06
4 changed files with 239 additions and 58 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

55
projects/common/src/main/java/dan200/computercraft/impl/network/wired/InvariantChecker.java
View File

@ -4,45 +4,66 @@
package dan200.computercraft.impl.network.wired;
import org.jetbrains.annotations.Contract;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import javax.annotation.Nullable;
/**
* Verifies certain elements of a network are "well formed".
* Verifies certain elements of a network are well-formed.
* <p>
* This adds substantial overhead to network modification, and so should only be enabled
* in a development environment.
* This adds substantial overhead to network modification, and so is only enabled when assertions are enabled.
*/
public final class InvariantChecker {
final class InvariantChecker {
private static final Logger LOG = LoggerFactory.getLogger(InvariantChecker.class);
private static final boolean ENABLED = false;
private InvariantChecker() {
}
public static void checkNode(WiredNodeImpl node) {
if (!ENABLED) return;
static void checkNode(WiredNodeImpl node) {
assert checkNodeImpl(node) : "Node invariants failed. See logs.";
}
var network = node.network;
if (network == null) {
LOG.error("Node's network is null", new Exception());
return;
private static boolean checkNodeImpl(WiredNodeImpl node) {
var okay = true;
if (node.currentSet != null) {
okay = false;
LOG.error("{}: currentSet was not cleared.", node);
}
if (network.nodes == null || !network.nodes.contains(node)) {
LOG.error("Node's network does not contain node", new Exception());
var network = makeNullable(node.network);
if (network == null) {
okay = false;
LOG.error("{}: Node's network is null.", node);
} else if (makeNullable(network.nodes) == null || !network.nodes.contains(node)) {
okay = false;
LOG.error("{}: Node's network does not contain node.", node);
}
for (var neighbour : node.neighbours) {
if (!neighbour.neighbours.contains(node)) {
LOG.error("Neighbour is missing node", new Exception());
okay = false;
LOG.error("{}: Neighbour {}'s neighbour set does not contain origianl node.", node, neighbour);
}
}
return okay;
}
public static void checkNetwork(WiredNetworkImpl network) {
if (!ENABLED) return;
static void checkNetwork(WiredNetworkImpl network) {
assert checkNetworkImpl(network) : "Network invariants failed. See logs.";
}
for (var node : network.nodes) checkNode(node);
private static boolean checkNetworkImpl(WiredNetworkImpl network) {
var okay = true;
for (var node : network.nodes) okay &= checkNodeImpl(node);
return okay;
}
@Contract("")
private static <T> @Nullable T makeNullable(T object) {
return object;
}
}

100
projects/common/src/main/java/dan200/computercraft/impl/network/wired/NodeSet.java Normal file
View File

@ -0,0 +1,100 @@
// SPDX-FileCopyrightText: 2024 The CC: Tweaked Developers
//
// SPDX-License-Identifier: MPL-2.0
package dan200.computercraft.impl.network.wired;
import dan200.computercraft.api.network.wired.WiredNode;
import javax.annotation.Nullable;
import java.util.Objects;
/**
* A disjoint-set/union-find of {@link WiredNodeImpl}s.
* <p>
* Rather than actually maintaining a list of included nodes, wired nodes store {@linkplain WiredNodeImpl#currentSet the
* set they're part of}. This means that we can only have one disjoint-set at once, but that is not a problem in
* practice.
*
* @see WiredNodeImpl#currentSet
* @see WiredNetworkImpl#remove(WiredNode)
* @see <a href="https://en.wikipedia.org/wiki/Disjoint-set_data_structure">Disjoint-set data structure</a>
*/
class NodeSet {
private NodeSet parent = this;
private int size = 1;
private @Nullable WiredNetworkImpl network;
private boolean isRoot() {
return parent == this;
}
/**
* Resolve this union, finding the root {@link NodeSet}.
*
* @return The root union.
*/
NodeSet find() {
var self = this;
while (!self.isRoot()) self = self.parent = self.parent.parent;
return self;
}
/**
* Get the size of this node set.
*
* @return The size of the set.
*/
int size() {
return find().size;
}
/**
* Add a node to this {@link NodeSet}.
*
* @param node The node to add to the set.
*/
void addNode(WiredNodeImpl node) {
if (!isRoot()) throw new IllegalStateException("Cannot grow a non-root set.");
if (node.currentSet != null) throw new IllegalArgumentException("Node is already in a set.");
node.currentSet = this;
size++;
}
/**
* Merge two nodes sets together.
*
* @param left The first union.
* @param right The second union.
* @return The union which was subsumed.
*/
public static NodeSet merge(NodeSet left, NodeSet right) {
if (!left.isRoot() || !right.isRoot()) throw new IllegalArgumentException("Cannot union a non-root set.");
if (left == right) throw new IllegalArgumentException("Cannot merge a node into itself.");
return left.size >= right.size ? mergeInto(left, right) : mergeInto(right, left);
}
private static NodeSet mergeInto(NodeSet root, NodeSet child) {
assert root.size > child.size;
child.parent = root;
root.size += child.size;
return child;
}
void setNetwork(WiredNetworkImpl network) {
if (!isRoot()) throw new IllegalStateException("Set is not the root.");
if (this.network != null) throw new IllegalStateException("Set already has a network.");
this.network = network;
}
/**
* Get the associated network.
*
* @return The associated network.
*/
WiredNetworkImpl network() {
return Objects.requireNonNull(find().network);
}
}

134
projects/common/src/main/java/dan200/computercraft/impl/network/wired/WiredNetworkImpl.java
View File

@ -8,6 +8,7 @@
import dan200.computercraft.api.network.wired.WiredNetwork;
import dan200.computercraft.api.network.wired.WiredNode;
import dan200.computercraft.api.peripheral.IPeripheral;
import dan200.computercraft.core.util.Nullability;
import java.util.*;
import java.util.concurrent.locks.ReadWriteLock;
@ -187,10 +188,76 @@ public boolean remove(WiredNode node) {
return true;
}
var reachable = reachableNodes(neighbours.iterator().next());
assert neighbours.size() >= 2 : "Must have more than one neighbour.";
/*
Otherwise we need to find all sets of connected nodes within the graph, and split them off into their own
networks.
With our current graph representation[^1], this requires a traversal of the graph, taking O(|V| + |E))
time, which can get quite expensive for large graphs. We try to avoid this traversal where possible, by
optimising for the case where the graph remains fully connected after removing this node, for instance,
removing "A" here:
A---B B
| | => |
C---D C---D
We observe that these sorts of loops tend to be local, and so try to identify them as quickly as possible.
To do this, we do a standard breadth-first traversal of the graph starting at the neighbours of the
removed node, building sets of connected nodes.
If, at any point, all nodes visited so far are connected to each other, then we know all remaining nodes
will also be connected. This allows us to abort our traversal of the graph, and just remove the node (much
like we do in the single neighbour case above).
Otherwise, we then just create a new network for each disjoint set of connected nodes.
{^1]:
There are efficient (near-logarithmic) algorithms for this (e.g. https://arxiv.org/pdf/1609.05867.pdf),
but they are significantly more complex to implement.
*/
// Create a new set of nodes for each neighbour, and add them to our queue of nodes to visit.
List<WiredNodeImpl> queue = new ArrayList<>();
Set<NodeSet> nodeSets = new HashSet<>(neighbours.size());
for (var neighbour : neighbours) {
nodeSets.add(neighbour.currentSet = new NodeSet());
queue.add(neighbour);
}
// Perform a breadth-first search of the graph, starting from the neighbours.
graphSearch:
for (var i = 0; i < queue.size(); i++) {
var enqueuedNode = queue.get(i);
for (var neighbour : enqueuedNode.neighbours) {
var nodeSet = Nullability.assertNonNull(enqueuedNode.currentSet).find();
// The neighbour has no set and so has not been visited yet. Add it to the current set and enqueue
// it to be visited.
if (neighbour.currentSet == null) {
nodeSet.addNode(neighbour);
queue.add(neighbour);
continue;
}
// Otherwise, take the union of the two nodes' sets if needed. If we've only got a single node set
// left, then we know the whole graph is network is connected (even if not all nodes have been
// visited) and so can abort early.
var neighbourSet = neighbour.currentSet.find();
if (nodeSet != neighbourSet) {
var removed = nodeSets.remove(NodeSet.merge(nodeSet, neighbourSet));
assert removed : "Merged set should have been ";
if (nodeSets.size() == 1) break graphSearch;
}
}
}
// If we have a single subset, then all nodes are reachable - just clear the set and exit.
if (nodeSets.size() == 1) {
assert nodeSets.iterator().next().size() == queue.size();
for (var neighbour : queue) neighbour.currentSet = null;
// If all nodes are reachable then exit.
if (reachable.size() == nodes.size()) {
// Broadcast our simple peripheral changes
removeSingleNode(wired, wiredNetwork);
InvariantChecker.checkNode(wired);
@ -198,43 +265,46 @@ public boolean remove(WiredNode node) {
return true;
}
// A split may cause 2..neighbours.size() separate networks, so we
// iterate through our neighbour list, generating child networks.
neighbours.removeAll(reachable);
var maximals = new ArrayList<WiredNetworkImpl>(neighbours.size() + 1);
maximals.add(wiredNetwork);
maximals.add(new WiredNetworkImpl(reachable));
assert queue.size() == nodes.size() : "Expected queue to contain all nodes.";
while (!neighbours.isEmpty()) {
reachable = reachableNodes(neighbours.iterator().next());
neighbours.removeAll(reachable);
maximals.add(new WiredNetworkImpl(reachable));
// Otherwise we need to create our new networks.
var networks = new ArrayList<WiredNetworkImpl>(1 + nodeSets.size());
// Add the network we've created for the removed node.
networks.add(wiredNetwork);
// And then create a new network for each disjoint subset.
for (var set : nodeSets) {
var network = new WiredNetworkImpl(new HashSet<>(set.size()));
set.setNetwork(network);
networks.add(network);
}
for (var network : maximals) network.lock.writeLock().lock();
for (var network : networks) network.lock.writeLock().lock();
try {
// We special case the original node: detaching all peripherals when needed.
wired.network = wiredNetwork;
wired.peripherals = Map.of();
wired.neighbours.clear();
// Ensure every network is finalised
for (var network : maximals) {
for (var child : network.nodes) {
child.network = network;
network.peripherals.putAll(child.peripherals);
}
// Add all nodes to their appropriate network.
for (var child : queue) {
var network = Nullability.assertNonNull(child.currentSet).network();
child.currentSet = null;
child.network = network;
network.nodes.add(child);
network.peripherals.putAll(child.peripherals);
}
for (var network : maximals) InvariantChecker.checkNetwork(network);
for (var network : networks) InvariantChecker.checkNetwork(network);
InvariantChecker.checkNode(wired);
// Then broadcast network changes once all nodes are finalised
for (var network : maximals) {
for (var network : networks) {
WiredNetworkChangeImpl.changeOf(peripherals, network.peripherals).broadcast(network.nodes);
}
} finally {
for (var network : maximals) network.lock.writeLock().unlock();
for (var network : networks) network.lock.writeLock().unlock();
}
nodes.clear();
@ -373,22 +443,4 @@ private static WiredNodeImpl checkNode(WiredNode node) {
throw new IllegalArgumentException("Unknown implementation of IWiredNode: " + node);
}
}
private static Set<WiredNodeImpl> reachableNodes(WiredNodeImpl start) {
Queue<WiredNodeImpl> enqueued = new ArrayDeque<>();
var reachable = new HashSet<WiredNodeImpl>();
reachable.add(start);
enqueued.add(start);
WiredNodeImpl node;
while ((node = enqueued.poll()) != null) {
for (var neighbour : node.neighbours) {
// Otherwise attempt to enqueue this neighbour as well.
if (reachable.add(neighbour)) enqueued.add(neighbour);
}
}
return reachable;
}
}

8
projects/common/src/main/java/dan200/computercraft/impl/network/wired/WiredNodeImpl.java
View File

@ -27,6 +27,14 @@ public final class WiredNodeImpl implements WiredNode {
final HashSet<WiredNodeImpl> neighbours = new HashSet<>();
volatile WiredNetworkImpl network;
/**
* A temporary field used when checking network connectivity.
*
* @see WiredNetworkImpl#remove(WiredNode)
*/
@Nullable
NodeSet currentSet;
public WiredNodeImpl(WiredElement element) {
this.element = element;
network = new WiredNetworkImpl(this);