Restructure ListBuffer to build and insert chains #9257
Conversation
Restructure `ListBuffer` to build fresh `ListBuffer`s and then insert them into the existing chain. Fixes calling `ListBuffer#patchInPlace` with itself ([bug#12121]).
NthPortal
added
the
library:collections
|
@joshlemer / @Ichoran would either of you like to review this? |
| val it = xs.iterator | ||
| if (it.hasNext) { | ||
| ensureUnaliased() | ||
| // MUST only be called on fresh instances |
There was a problem hiding this comment.
If the only possible use is to call it on a fresh instance, why not create the instance too? Otherwise, doesn't it just expose a risk of mis-use while requiring more boilerplate at the call site?
| @@ -407,7 +395,7 @@ class ListBuffer[A] | |||
| @SerialVersionUID(3L) | |||
| object ListBuffer extends StrictOptimizedSeqFactory[ListBuffer] { | |||
|
|
|||
| def from[A](coll: collection.IterableOnce[A]): ListBuffer[A] = new ListBuffer[A] ++= coll | |||
| def from[A](coll: collection.IterableOnce[A]): ListBuffer[A] = new ListBuffer[A].freshFrom(coll) | |||
There was a problem hiding this comment.
Since you never use freshFrom to do anything but this, why not just use from? Is this to avoid possible overhead from checking the initialization of the companion object?
There was a problem hiding this comment.
I don't know what the overhead is from, but back when I was benchmarking #9174, I discovered that from had significantly worse performance than new (with ++= I think). if someone has a more reliable benchmarking setup and wants to run it again, that'd be great. that was my primary motivation for using new and a private method
Restructure
ListBufferto build freshListBuffers andthen insert them into the existing chain.
Fixes calling
ListBuffer#patchInPlacewith itself(scala/bug#12121).
Follow-up to #9174