-
Notifications
You must be signed in to change notification settings - Fork 72
optimise arrayOps in strawman collection #317
Conversation
Hey, thanks for taking this initiative but I’m afraid this is going to overlap with #278. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good in general but should be rebased on top of #278 once that has been merged. This will remove ArrayView
and leave us with ArrayOps
, WrappedArray
and ImmutableArray
as possible candidates for this optimization.
@@ -12,7 +12,20 @@ package object collection extends LowPriority { | |||
implicit def stringToStringOps(s: String): immutable.StringOps = new immutable.StringOps(s) | |||
|
|||
/** Decorator to add collection operations to arrays. */ | |||
implicit def arrayToArrayOps[A](as: Array[A]): ArrayOps[A] = new ArrayOps[A](as) | |||
object ArrayOpsDecorators { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Needs a fallback for Array[Any]
that dispatches dynamically
Hey @ackratos, now that #278 has been merged you can continue this work. I think this optimization is useful. I’m wondering what’s the cost of its implementation in terms of bytecode size. |
collection-strawman's Array slice still has space to enhance, in following result, above is collection-strawman's result, below is Mike's optimization:
|
Close this as I have a new one contains less changes and easier to review: #354 |
Benchmarked array slice performance, @mkeskells optimization have performance gain to scala.ArrayOps implementation and a big win of strawman collection. I am trying to optimize strawman implementation (but not sure whether its a good idea considered current implementation has a good, unified abstraction on View)