chill icon indicating copy to clipboard operation
chill copied to clipboard

Improve performance by using the Unsafe-based serialization supportted by trunk version of Kryo

Open romix opened this issue 11 years ago • 8 comments

It could be worth trying to test with the current upcoming trunk version of Kryo. It contains significant perormance improvements including Unsafe-based serialization and ability to serialize directly into off-heap memory with almost a native speed if required. To use these new features, you only need to use new new IO classes (like UnsafeInput and UnsafeOutput) instead of standrad Input/Output classes.

IMHO, these features could be very useful in general and for Big Data apps in particular (Hadoop, Cassandra, Storm, etc).

BTW, after I added Unsafe-serialization to Kryo, some other well-known frameworks included similar features. The list includes: Avro (I added it there myself), Hazelcast (they reused may ideas and adpated it to their APIs). MapDB could be the next one.

Expected performance improvements are rather significant: You often see 3-4X speedups. If your classes contain arrays of native types (ints, floats, doubles) you can expect even 10x performance improvements.

romix avatar Jun 22 '13 06:06 romix

That's awesome! Sounds very promising.

On Jun 21, 2013, at 11:32 PM, romix [email protected] wrote:

It could be worth trying to test with the current upcoming trunk version of Kryo. It contains significant perormance improvements including Unsafe-based serialization and ability to serialize directly into off-heap memory with almost a native speed if required. To use these new features, you only need to use new new IO classes (like UnsafeInput and UnsafeOutput) instead of standrad Input/Output classes.

IMHO, these features could be very useful in general and for Big Data apps in particular (Hadoop, Cassandra, Storm, etc).

BTW, after I added Unsafe-serialization to Kryo, some other well-known frameworks included similar features. The list includes: Avro (I added it there myself), Hazelcast (they reused may ideas and adpated it to their APIs). MapDB could be the next one.

Expected performance improvements are rather significant: You often see 3-4X speedups. If your classes contain arrays of native types (ints, floats, doubles) you can expect even 10x performance improvements.

— Reply to this email directly or view it on GitHub.

ryanlecompte avatar Jun 22 '13 06:06 ryanlecompte

This would be nice. Just strapped for time. I'd love a patch on the chill-hadoop stuff, for instance, that showed an example here.

johnynek avatar Jun 22 '13 22:06 johnynek

If I find time, I'll try to provide an example. But as I said, in principle, the only change is to use UnsafeInput and UnsafeOutput instead of Input/Output classes. No further changes are required.

BTW, Kryo unit tests from the trunk contain examples using those new Unsafe streams. More over, it also contains a benchmark making use of them. The new Unsafe-based implementation beats standard Kryo by a almost an order of magnitude on that use-case (class with a lot of arrays).

romix avatar Jun 22 '13 23:06 romix

What's the plan on this? Is Chill going to stick with Kryo 2.21 indefinitely? 2.24 has been out for 6 months, and now 3.0 is released too. As I understand, 3.0 is incompatible only in the Unsafe format, so it is not an unreasonable progression from 2.21 (which does not have Unsafe anyway).

I don't know much about Chill (just use it through Spark). Is it possible for the user to override the Kryo version? We use Kryo directly as well (not just through Chill), and I'd really like to use Unsafe on that code path at least. Thanks!

darabos avatar Nov 21 '14 10:11 darabos

:+1: Also, "romix opened this Issue on 22 Jun 2013", 1.5 years ago.

ngocdaothanh avatar Nov 24 '14 23:11 ngocdaothanh

I agree we should be able to move forward. There could be a few ways:

  1. we could check and it could be that later versions are not binary incompatible, and if that is so, end users can choose a later version and ivy/sbt should choose the latest version on the classpath.

  2. if the above does not work, we can force an upgrade. One reason we are very slow to do this is that kryo winds up on the classpath of storm with a fixed version. The version here (I believe) is a storm compatible version.

  3. A huge amount of production jobs depend on this code being correct and so we are very circumspect about upgrading as we have had a lot of runtime failures in the past due to Kryo.

Perhaps the best way to help is to check that all our tests pass when we bump the version up. Also, it would be interesting to see if you can run spark with a current version of chill with a later version of Kryo. It would be nice if everyone is not stuck on the same version together.

Lastly, the KryoInstantiator class is used to create Kryo instances. It may be possible to make Unsafe support opt-in using a different instantiator in a way that poses little risk.

The fact is, upgrading a library used by spark, scalding, storm, summingbird and cascalog is probably going to be a slow going operation. We probably need to see strong reasons to upgrade, rather than a presumption that the latest is greatest.

johnynek avatar Nov 26 '14 04:11 johnynek

  1. we could check and it could be that later versions are not binary incompatible

https://github.com/EsotericSoftware/kryo/blob/master/CHANGES.md lists the binary compatibility relationships between versions. Unfortunately it appears that each version is binary incompatible.

Perhaps the best way to help is to check that all our tests pass when we bump the version up.

With a simple bump (to Kryo 2.22 or higher) Chill does not compile. The offending Kryo changes are https://github.com/EsotericSoftware/kryo/commit/1fc2dc8ad484ab0dc0af6ce86a5bef44c699631e, https://github.com/EsotericSoftware/kryo/commit/859de2ea94aa1e1e8a54c0b763f3e9f5315f0438 and https://github.com/EsotericSoftware/kryo/commit/9d71ad17209bbf68134a8dc492922be976fc631f. (Likely not an exhaustive list.) It's probably minor things and I could try to work them out in a fork if you think that'd be useful.

Also, it would be interesting to see if you can run spark with a current version of chill with a later version of Kryo. It would be nice if everyone is not stuck on the same version together.

I'd be happy to experiment! I don't even mind what Spark is using, I just want to use a fresh Kryo from our own code. The most naive approach does not work. I put "com.esotericsoftware" % "kryo" % "3.0.0" in our build.sbt and replaced kryo.Output with kryo.UnsafeOutput in our code. It compiles, but at runtime I get:

java.lang.IllegalAccessError: tried to access method com.esotericsoftware.kryo.io.Output.require(I)Z from class com.esotericsoftware.kryo.io.UnsafeOutput

Which I think means the classloader has built a chimera of the two Kryo versions (2.21 pulled in through Spark and Chill and 3.0.0 pulled in directly) and it does not work. Is there a better way to do this?

We probably need to see strong reasons to upgrade, rather than a presumption that the latest is greatest.

There seem to be a great number of important bugfixes. We ran into a performance degradation issue for example, that was fixed more than a year ago in trunk. Until we can upgrade Kryo we have a hacky workaround for it.

darabos avatar Nov 26 '14 15:11 darabos

What run time are you using? Likely if your running with a spark or storm then kryo is being injected into your classpath at the earlier version. (Exactly the issue we have for storm that makes this painful). If you can enable classloader isolation between you and the runtime you might have better luck.

At some point we may need to just fork off the storm portion of things. We can't follow the spark route of shading troublesome upstream dependencies here since chill often interfaces along boundaries with the frameworks its used in, so need the same classpath's. The source and binary incompatible nature of the kryo versions above this caused us huge headaches, we had a large storm install base that this wound up being a non-starter for.

@johnynek we could force off a release branch for storm launched jobs in ivy and basically pin it to a binary release branch and let develop move forward?

ianoc avatar Nov 26 '14 16:11 ianoc