Utilize CanBuildFrom implicits instead of relying on concrete builders

[ryanlecompte]

This pull request addresses one of the TODOs in the code base around utilizing CanBuildFrom to convert to the appropriate collection types. This basically makes the previously required introspection/cloning around mutable collections obsolete and has been removed in this pull request in favor of CanBuildFrom.

One thing to note in this pull request is that we're employing the known 2.9.x workaround c: C with Traversable[T] to get around a compiler inference bug. This issue has been fixed in 2.10 (the compiler lets you just say c: C as you would expect to work). I tested this workaround for things that get converted to Traversable, e.g. strings and arrays as well as BitSet. I couldn't find a collection where the workaround would fail, although there might be a case that I'm unaware of (and not already covered in the unit tests). All the unit tests pass, including the mutable ones!

[ryanlecompte]

FYI - I just also checked in a change to improve the performance of TraversableSerializer's read method. It was building up an intermediate array of items and then handing them off to the builder. Now that we're using CanBuildFrom and the builder is no longer shared, we can get rid of that and just use the CanBuildFrom instance directly.

[johnynek]

Can't we do CanBuildFrom[Nothing, T, C] and avoid taking c?

[ryanlecompte]

Well, the CanBuildFrom implicit needs to capture the actual type of the originating collection in its first type parameter, so using Nothing would not do what we want here. Using CanBuildFrom[C, T, C] lets us get a builder that knows how to take elements from C (of type T), and rebuild another collection of the same type C. If we use Nothing then it loses that concrete collection type for the From type parameter.

[johnynek]

But we never use the From parameter. It seems strange that we actually throw c away. I think this code would work just as well with:

def forTraversableSubclass[T, C](isImmutable: Boolean = true)(implicit mf: ClassManifest[C], cbf: CanBuildFrom[Nothing, T, C]): Kryo = {

Would that cause any issue since we only use the def apply(): Builder[T, C] method?

[ryanlecompte]

If we get rid of the c param, won't we have to register the traversables
like this?

  .forTraversableSubclass[Any, Queue[Any]]()
  .forTraversableSubclass[Any, List[Any]]()

I was keeping the c param to help the type inferencing out and to
properly bind C and T. I futzed with it for a good bit and ended up
with what's in the pull request. My scala-fu may be missing something,
though. Maybe you could try out the code from my fork some time and try to
get it to work with CanBuildFrom[Nothing, T, C]?

[johnynek]

Fair enough. Let's merge what we have and if we can simplify later we will.

[johnynek]

Thanks for taking this on!

[ryanlecompte]

Thanks! I had a lot of fun doing it.