Hi, On Wed, Jan 29, 2014 at 4:56 AM, Michael Dürig <[email protected]> wrote: > I can't follow here. Apart from not doing beforeNames.set(a, afterName), I > don't see any optimisation.
The code was O(n^2) for the common case when no reorderings (just added/removed nodes) took place. With the added !afterName.equals(beforeName) check that case is now an O(n) operation. > Moreover the initial code was in my eye much clearer and more concise. > Finally we shouldn't bother to much re. optimisation of reorder operation > as they occur rather rarely and when they do we know that there won't be > many child nodes as this is a design limitation anyway. The problem I see here is that the reordering detection gets triggered also in the much more common case of adding or removing a node. In a semi-flat hierarchy of up to 1k nodes (which is the rough limit for orderable nodes we've used so far), the check without this optimization would have ended making hundreds of thousands of String.equals() calls for each commit that adds or removes a node. Multiplied by the number of observation listeners, this seemed like an important enough corner case to optimize for. BR, Jukka Zitting
