Optimize the storage backend used in sketch map #301
Conversation
|
|
||
| // The adaptive monoid to swap between sparse modes. | ||
| implicit def monoid[V:Monoid]: Monoid[AdaptiveMatrix[V]] = new Monoid[AdaptiveMatrix[V]] { | ||
| private[this] final val innerZero = implicitly[Monoid[V]].zero |
There was a problem hiding this comment.
is there a reason you generally prefer the implicitly[Monoid... approach vs. Monoid.zero[V], and so on?
There was a problem hiding this comment.
Mostly not really, this ensures its just a field access for the zero constant in here if we hit it often rather than potentially being a def/virtual call
There was a problem hiding this comment.
How smart is implicitly? I always thought it was just something like
def implicitly[T](implicit t: T) = t
Which would mean that there is still a virtual call. Is the scala compiler actually doing something smarter here, like making a direct reference to the implicit type that is in scope?
There was a problem hiding this comment.
Right, but i assign it to a final val, that is then used in any loops/accesses within the monoid. So for those it should be a constant. I could have used Monoid.zero[T] in the constant here too. Sorry I think I might have answered a different question than asked. both put into this line I presume both are just equiv
There was a problem hiding this comment.
Ok, yeah, I think we diverged, but I am with you.
| case d@DenseMatrix(_, _, _) => return denseUpdate(d, iter) | ||
| case s@SparseColumnMatrix(_) => | ||
| sparseUpdate(sparseStorage, s) | ||
| if(sparseStorage(0).size > current.cols/4) { |
There was a problem hiding this comment.
perhaps this logic should be in a method/constant whatever?
There was a problem hiding this comment.
Well this is the only place it exists in the code I modified, now if we should have some notion for all our sparse/adaptive systems for when to go dense, maybe. Also maybe should be a parameter. Not sure really.
Optimize the storage backend used in sketch map
Seeing somewhere between 1.7x and 5x perf improvements depending on loaded ness of the sketchmap.