Enable getClass equality checking in tests

[koertkuipers]

Not sure if removing the serializer for 5 item map concrete class is safe on all platforms/scala versions!

Perhaps the registrations for "small" maps should be generated automatically by looping over a range [1, 10], creating maps and only registering a serializer if the generated class is not the same as for some map of size > 10 (which will be a HashMap)

[johnynek]

okay, forgot what this does. This is testing that a core type (maps in this case) are registered so that we don't have to write the whole class name into the stream. I think we can't make this change. We need to figure out a way to make this pass (even if we just register and not add a serializer, which let's Kryo write an ID into the stream rather than a string).

[koertkuipers]

this serialization stuff makes my head hurt.

ok so the registration
.forConcreteTraversableClass(Map[Any, Any]('a -> 'a, 'b -> 'b, 'c -> 'c,
'd -> 'd, 'e -> 'e))

was actually meant to register a HashTrieMap. what threw me off was the
comments above it
// specifically register small maps since Scala represents them
differently

so if this is supposed to register HashTrieMaps (to avoid the name being
written upon serialization), then why does it mess up the roundtrip for
HashMap("good" -> 0.5, "bad" -> -1.0)? it starts life as a HashTrieMap but
comes out of the serialization and deserialization as a
scala.collection.immutable.Map$Map2
it must be because we say Map[Any, Any] in the registration, which i am
guessing is exactly what you want to avoid name being stored.

On Mon, Nov 11, 2013 at 1:13 PM, P. Oscar Boykin
notifications@github.comwrote:

In chill-scala/src/test/scala/com/twitter/chill/KryoSpec.scala:

@@ -213,8 +213,7 @@ class KryoSpec extends Specification with BaseProperties {
val m2 = Map('a -> 'a, 'b -> 'b)
val m3 = Map('a -> 'a, 'b -> 'b, 'c -> 'c)
val m4 = Map('a -> 'a, 'b -> 'b, 'c -> 'c, 'd -> 'd)

•  val m5 = Map('a -> 'a, 'b -> 'b, 'c -> 'c, 'd -> 'd, 'e -> 'e)
  

•  Seq(m1, m2, m3, m4, m5).foreach { m =>
  

okay, forgot what this does. This is testing that a core type (maps in
this case) are registered so that we don't have to write the whole class
name into the stream. I think we can't make this change. We need to figure
out a way to make this pass (even if we just register and not add a
serializer, which let's Kryo write an ID into the stream rather than a
string).

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/165/files#r7567422
.

[koertkuipers]

if i change this line:
.forConcreteTraversableClass(Map[Any, Any]('a -> 'a, 'b -> 'b, 'c -> 'c,
'd -> 'd, 'e -> 'e))
to
.forConcreteTraversableClass(HashMap[Any, Any]('a -> 'a, 'b -> 'b, 'c ->
'c, 'd -> 'd, 'e -> 'e))

it works. i also checked the serialization sizes and see no change.

On Mon, Nov 11, 2013 at 1:45 PM, Koert Kuipers koert@tresata.com wrote:

this serialization stuff makes my head hurt.

ok so the registration
.forConcreteTraversableClass(Map[Any, Any]('a -> 'a, 'b -> 'b, 'c -> 'c,
'd -> 'd, 'e -> 'e))

was actually meant to register a HashTrieMap. what threw me off was the
comments above it
// specifically register small maps since Scala represents them
differently

so if this is supposed to register HashTrieMaps (to avoid the name being
written upon serialization), then why does it mess up the roundtrip for
HashMap("good" -> 0.5, "bad" -> -1.0)? it starts life as a HashTrieMap but
comes out of the serialization and deserialization as a
scala.collection.immutable.Map$Map2
it must be because we say Map[Any, Any] in the registration, which i am
guessing is exactly what you want to avoid name being stored.

On Mon, Nov 11, 2013 at 1:13 PM, P. Oscar Boykin <notifications@github.com

wrote:

In chill-scala/src/test/scala/com/twitter/chill/KryoSpec.scala:

@@ -213,8 +213,7 @@ class KryoSpec extends Specification with BaseProperties {
val m2 = Map('a -> 'a, 'b -> 'b)
val m3 = Map('a -> 'a, 'b -> 'b, 'c -> 'c)
val m4 = Map('a -> 'a, 'b -> 'b, 'c -> 'c, 'd -> 'd)

•  val m5 = Map('a -> 'a, 'b -> 'b, 'c -> 'c, 'd -> 'd, 'e -> 'e)
  

•  Seq(m1, m2, m3, m4, m5).foreach { m =>
  

okay, forgot what this does. This is testing that a core type (maps in
this case) are registered so that we don't have to write the whole class
name into the stream. I think we can't make this change. We need to figure
out a way to make this pass (even if we just register and not add a
serializer, which let's Kryo write an ID into the stream rather than a
string).

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/165/files#r7567422
.