Optimize NestedPackages by deferring sorting #513
Conversation
enigma-swing/src/main/java/cuchaz/enigma/gui/node/SortedMutableTreeNode.java
Show resolved
Hide resolved
|
|
||
| @Override | ||
| public int getIndex(TreeNode node) { | ||
| return Iterables.indexOf(this.children, other -> this.comparator.compare(node, other) == 0); |
There was a problem hiding this comment.
Shouldn't this call checkSorted() as well, since the order of the node in the list before and after sorting may be different? (Or, if this is intended, perhaps a comment is in order.)
Also, why use Iterables.indexOf instead of List.indexOf? If the comparator is known to be consistent with equals (and the javadoc for Comparator recommends that comparators used with sorted collections be consistent with equals), then both ways should be equivalent. (Theoretically, List.indexOf would be faster by virtue of the List implementation taking advantage of impl. details.)
There was a problem hiding this comment.
Can't the ClassEntry comparator be inconsistent with equals, though?
In any case, I'm probably just going to turn this into a manual for loop.
There was a problem hiding this comment.
As far as I can tell, the ClassEntry comparators -- defined in ClassSelector#DEOBF_CLASS_COMPARATOR and used in DeobfPanel#<init>, and defined as a lambda in ObfPanel#<init> -- are both consistent with equals.
ClassEntry#equals checks if the parent and class name of the entry are equivalent; the comparators above compare using the full name of the class1, which is derived from the class name of the entry and of its parent. Because of that, if two ClassEntrys are equal in terms of equals, the comparators will also declare them equivalent (returning 0).
Though, that is the present state of the code; the future may possibly bring comparators inconsistent with equals, but I think that's improbable (and would already cause problems anyway, because then you'd have entries that don't equal themselves or are equal to other entries according to the comparator).
Footnotes
-
In the
ObfPanelcomparator's case which compares the lengths of the full names first: if the two full names are equivalent, then necessarily the lengths are also the same. ↩
NestedPackagesuses a linear search to add in new entries right now. Unsurprisingly, this is really bad when there are a lot of entries.This PR changes the tree node type to use a non-synchronized tree node class that defers sorting, which makes adding nodes a lot faster.
This is basically just a port of QuiltMC/enigma#84 (my own PR) to Fabric Enigma.