Replace mutable list usages with TreeList in stack operations#1033
Conversation
9ae3fb8 to
2ddfb0a
Compare
24b4424 to
b7864a8
Compare
Robotgiggle
left a comment
Robotgiggle
left a comment
There was a problem hiding this comment.
I haven't tested exhaustively but nothing seems to be broken. Simulate still works fine, as does the Shepherd Directrix.
|
|
||
| - Removed APIs deprecated in `0.11.4` ([#1126](https://github.com/FallingColors/HexMod/pull/1126), [#1127](https://github.com/FallingColors/HexMod/pull/1127) [#1129](https://github.com/FallingColors/HexMod/pull/1129), [#1137](https://github.com/FallingColors/HexMod/pull/1137), [#1142](https://github.com/FallingColors/HexMod/pull/1142)) @s5bug | ||
| - ListIota now uses the asymptotically-efficient TreeList internally ([#1032](https://github.com/FallingColors/HexMod/pull/1032)) @s5bug | ||
| - SpellList has been removed, the Casting Image and Casting Frames now store iotas in a TreeList ([#1033](https://github.com/FallingColors/HexMod/pull/1033)) @s5bug |
|
|
||
| val image2 = image.withUsedOp().copy(stack = stack) | ||
| // FIXME this is expensive if the stack is incredibly large | ||
| // someone should rewrite this op to cut and replace a `slice` of the `TreeList` stack |
There was a problem hiding this comment.
I think I have an implementation for this - can I push it to this PR or do you want it in a separate PR after this goes in?
There was a problem hiding this comment.
Ok, it's been pushed. Let me know if you think I did anything wrong.
There was a problem hiding this comment.
Is there any advantage to having newOrder be a TreeList? Or would it be better to have it also be a standard MutableList?
There was a problem hiding this comment.
At such a small size I think it makes almost no difference. If we know the "eventual" size of newOrder then I'd say make it an ArrayList pre-allocated to that size.
There was a problem hiding this comment.
There's an asymptotic fast-path for if the thing being appended is already a treelist:
- With an arbitrary non-TreeList collection,
appendedAlllooks likenewBuilder.append(this).append(that).build, O(m+n) - If either
thisorthatare relatively small enough, the fast path is O(min(m, n))
So, I'd say maybe just leave it a TreeList if we're optimizing for the "people are lagging the server" case, but I don't know 😄 not experienced in that area
|
New Swindler's looks good, I just replaced the |
2 participants
Stacked on #1032
TODO is make a pass through all the list ops and make sure they're not doing O(n) copies but I'm not going to waste my time with that in case I have to go back and modify other stuff