Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FOLLOWUP][PR514] Update to use the newer Kryo 5.5.0 #747

Open
wants to merge 20 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions build.sbt
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import com.typesafe.tools.mima.plugin.MimaPlugin.mimaDefaultSettings
val akkaVersion = "2.6.20"
val algebirdVersion = "0.13.9"
val bijectionVersion = "0.9.7"
val kryoVersion = "4.0.2"
val kryoVersion = "5.5.0"
val scroogeVersion = "21.2.0"
val asmVersion = "4.16"
val protobufVersion = "3.22.2"
Expand Down Expand Up @@ -53,7 +53,7 @@ val sharedSettings = Seq(
"org.scalacheck" %% "scalacheck" % "1.15.2" % "test",
"org.scalatest" %% "scalatest" % "3.2.15" % "test",
"org.scalatestplus" %% "scalatestplus-scalacheck" % "3.1.0.0-RC2" % "test",
"com.esotericsoftware" % "kryo-shaded" % kryoVersion
"com.esotericsoftware" % "kryo" % kryoVersion

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to the description in the Kryo repository, it seems that com.esotericsoftware.kryo:kryo5:5.5.0 should be used?

https://github.com/EsotericSoftware/kryo

image

Copy link
Author

@roczei roczei Dec 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @LuciferYang,

I agree with you! Yes, it would be better to use com.esotericsoftware.kryo:kryo5:5.5.0. I have switched to versionless kryo5 package to fix all mimaReport issues. We can switch back to this artifact any time before it would be merged. (I have mentioned this in the PR's description as well). Related commit: 0a6dda1

Currently the main issue is that we cannot go further with this PR because seems like there is no active maintainer of this twitter/chill repository. :-( I am just a simple contributor who opened this PR.

),
Test / parallelExecution := true,
pomExtra := <url>https://github.com/twitter/chill</url>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ class ActorRefSerializer(system: ExtendedActorSystem) extends Serializer[ActorRe
kryo.forSubclass[ActorRef](this)
}

override def read(kryo: Kryo, input: Input, typ: Class[ActorRef]): ActorRef = {
override def read(kryo: Kryo, input: Input, typ: Class[_ <: ActorRef]): ActorRef = {
val path = ActorPath.fromString(input.readString())
system.provider.resolveActorRef(path)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ class AveragedValueSerializer extends KSerializer[AveragedValue] {
out.writeLong(s.count, true)
out.writeDouble(s.value)
}
def read(kser: Kryo, in: Input, cls: Class[AveragedValue]): AveragedValue =
def read(kser: Kryo, in: Input, cls: Class[_ <: AveragedValue]): AveragedValue =
AveragedValue(in.readLong(true), in.readDouble)
}

Expand All @@ -42,7 +42,7 @@ class MomentsSerializer extends KSerializer[Moments] {
out.writeDouble(s.m3)
out.writeDouble(s.m4)
}
def read(kser: Kryo, in: Input, cls: Class[Moments]): Moments =
def read(kser: Kryo, in: Input, cls: Class[_ <: Moments]): Moments =
Moments(in.readLong(true), in.readDouble, in.readDouble, in.readDouble, in.readDouble)
}

Expand All @@ -52,7 +52,7 @@ class DecayedValueSerializer extends KSerializer[DecayedValue] {
out.writeDouble(s.value)
out.writeDouble(s.scaledTime)
}
def read(kser: Kryo, in: Input, cls: Class[DecayedValue]): DecayedValue =
def read(kser: Kryo, in: Input, cls: Class[_ <: DecayedValue]): DecayedValue =
DecayedValue(in.readDouble, in.readDouble)
}

Expand All @@ -63,7 +63,7 @@ class HLLSerializer extends KSerializer[HLL] {
out.writeInt(bytes.size, true)
out.writeBytes(bytes)
}
def read(kser: Kryo, in: Input, cls: Class[HLL]): HLL =
def read(kser: Kryo, in: Input, cls: Class[_ <: HLL]): HLL =
HyperLogLog.fromBytes(in.readBytes(in.readInt(true)))
}

Expand All @@ -72,15 +72,15 @@ class HLLMonoidSerializer extends KSerializer[HyperLogLogMonoid] {
val hllMonoids: MMap[Int, HyperLogLogMonoid] = MMap[Int, HyperLogLogMonoid]()
def write(kser: Kryo, out: Output, mon: HyperLogLogMonoid): Unit =
out.writeInt(mon.bits, true)
def read(kser: Kryo, in: Input, cls: Class[HyperLogLogMonoid]): HyperLogLogMonoid = {
def read(kser: Kryo, in: Input, cls: Class[_ <: HyperLogLogMonoid]): HyperLogLogMonoid = {
val bits = in.readInt(true)
hllMonoids.getOrElseUpdate(bits, new HyperLogLogMonoid(bits))
}
}

class QTreeSerializer extends KSerializer[QTree[Any]] {
setImmutable(true)
override def read(kryo: Kryo, input: Input, cls: Class[QTree[Any]]): QTree[Any] = {
override def read(kryo: Kryo, input: Input, cls: Class[_ <: QTree[Any]]): QTree[Any] = {
val (v1, v2, v3) = (input.readLong(), input.readInt(), input.readLong())
val v4 = kryo.readClassAndObject(input)
val v5 = kryo.readClassAndObject(input).asInstanceOf[Option[QTree[Any]]]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import avro.FiscalRecord;
import com.esotericsoftware.kryo.Kryo;
import com.esotericsoftware.kryo.Serializer;
import org.objenesis.strategy.StdInstantiatorStrategy;
import com.twitter.chill.KryoInstantiator;
import com.twitter.chill.KryoPool;
import org.apache.avro.Schema;
Expand All @@ -11,7 +12,6 @@
import org.apache.avro.generic.GenericRecordBuilder;
import org.junit.Before;
import org.junit.Test;
import org.objenesis.strategy.StdInstantiatorStrategy;
import scala.reflect.ClassTag;

import static org.junit.Assert.assertEquals;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ object BijectionEnrichedKryo {
)(implicit bij: ImplicitBijection[A, B], cmf: ClassTag[B]): KSerializer[A] =
new KSerializer[A] {
def write(k: Kryo, out: Output, obj: A): Unit = kser.write(k, out, bij(obj))
def read(k: Kryo, in: Input, cls: Class[A]): A =
def read(k: Kryo, in: Input, cls: Class[_ <: A]): A =
bij.invert(kser.read(k, in, cmf.runtimeClass.asInstanceOf[Class[B]]))
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ class InjectiveSerializer[T] private (injection: Injection[T, Array[Byte]])
out.writeBytes(bytes)
}

def read(kser: Kryo, in: Input, cls: Class[T]): T = {
def read(kser: Kryo, in: Input, cls: Class[_ <: T]): T = {
val bytes = new Array[Byte](in.readInt(true))
in.readBytes(bytes)
injection.invert(bytes).get
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,13 @@

import com.esotericsoftware.kryo.Kryo;
import com.esotericsoftware.kryo.io.Output;
import org.objenesis.strategy.StdInstantiatorStrategy;

import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.conf.Configured;
import org.apache.hadoop.io.serializer.Deserializer;
import org.apache.hadoop.io.serializer.Serialization;
import org.apache.hadoop.io.serializer.Serializer;
import org.objenesis.strategy.StdInstantiatorStrategy;

import com.twitter.chill.KryoPool;
import com.twitter.chill.KryoInstantiator;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,7 @@ limitations under the License.
package com.twitter.chill.hadoop

import com.esotericsoftware.kryo.Kryo

import org.objenesis.strategy.StdInstantiatorStrategy
import org.objenesis.strategy.StdInstantiatorStrategy;

import java.io.{ByteArrayInputStream => BAIn, ByteArrayOutputStream => BAOut}
import org.apache.hadoop.conf.Configuration
Expand All @@ -28,9 +27,18 @@ import com.twitter.chill.KryoInstantiator
import org.scalatest.matchers.should.Matchers
import org.scalatest.wordspec.AnyWordSpec

class AnyKryoInstantiator extends KryoInstantiator {
override def newKryo: Kryo = {
val k = new Kryo
k.register(Class.forName("com.twitter.chill.hadoop.HadoopTests"))
k.register(Class.forName("scala.collection.immutable.List"))
k
}
}
class StdKryoInstantiator extends KryoInstantiator {
override def newKryo: Kryo = {
val k = new Kryo
k.register(Class.forName("scala.Tuple2$mcII$sp"))
k.setInstantiatorStrategy(new StdInstantiatorStrategy)
k
}
Expand All @@ -56,7 +64,7 @@ class HadoopTests extends AnyWordSpec with Matchers {
"accept anything" in {
val conf = new Configuration
val hc = new HadoopConfig(conf)
ConfiguredInstantiator.setReflect(hc, classOf[KryoInstantiator])
ConfiguredInstantiator.setReflect(hc, classOf[AnyKryoInstantiator])

val ks = new KryoSerialization(conf)
Seq(classOf[List[_]], classOf[Int], this.getClass).forall { cls =>
Expand Down
2 changes: 1 addition & 1 deletion chill-java/src/main/java/com/twitter/chill/SerDeState.java
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ protected SerDeState(Kryo k, Input in, Output out) {
/** Call this when to reset the state to the initial state */
public void clear() {
input.setBuffer(EMPTY_BUFFER);
output.clear();
output.reset();
}

public void setInput(byte[] in) { input.setBuffer(in); }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ public ArraysAsListSerializer() {
}

@Override
public List<?> read(final Kryo kryo, final Input input, final Class<List<?>> type) {
public List<?> read(final Kryo kryo, final Input input, final Class<? extends List<?>> type) {
final int length = input.readInt(true);
Class<?> componentType = kryo.readClass(input).getType();
try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ public void write(Kryo kryo, Output output, BitSet bitSet) {
}

@Override
public BitSet read(Kryo kryo, Input input, Class<BitSet> bitSetClass) {
public BitSet read(Kryo kryo, Input input, Class<? extends BitSet> bitSetClass) {
int len = input.readInt(true);
long[] target = new long[len];

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ public void write(Kryo kryo, Output output, InetSocketAddress obj) {
}

@Override
public InetSocketAddress read(Kryo kryo, Input input, Class<InetSocketAddress> klass) {
public InetSocketAddress read(Kryo kryo, Input input, Class<? extends InetSocketAddress> klass) {
String host = input.readString();
int port = input.readInt(true);
return new InetSocketAddress(host, port);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ public void write(Kryo kryo, Output output, IterableRegistrar obj) {
}
kryo.writeClassAndObject(output, null);
}
public IterableRegistrar read(Kryo kryo, Input input, Class<IterableRegistrar> type) {
public IterableRegistrar read(Kryo kryo, Input input, Class<? extends IterableRegistrar> type) {
ArrayList<IKryoRegistrar> krs = new ArrayList<IKryoRegistrar>();
IKryoRegistrar thisKr = (IKryoRegistrar)kryo.readClassAndObject(input);
while(thisKr != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ public void write(Kryo k, Output o, PriorityQueue<?> q) {
o.flush();
}
}
public PriorityQueue<?> read(Kryo k, Input i, Class<PriorityQueue<?>> c) {
public PriorityQueue<?> read(Kryo k, Input i, Class<? extends PriorityQueue<?>> c) {
Comparator<Object> comp = (Comparator<Object>)k.readClassAndObject(i);
int sz = i.readInt(true);
// can't create with size 0:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ public void write(Kryo kryo, Output output, Pattern pattern) {
}

@Override
public Pattern read(Kryo kryo, Input input, Class<Pattern> patternClass) {
public Pattern read(Kryo kryo, Input input, Class<? extends Pattern> patternClass) {
return Pattern.compile(input.readString());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ public void write(Kryo kryo, Output output, Date date) {
}

@Override
public Date read(Kryo kryo, Input input, Class<Date> dateClass) {
public Date read(Kryo kryo, Input input, Class<? extends Date> dateClass) {
return new Date(input.readLong(true));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ public void write(Kryo kryo, Output output, Time time) {
}

@Override
public Time read(Kryo kryo, Input input, Class<Time> timeClass) {
public Time read(Kryo kryo, Input input, Class<? extends Time> timeClass) {
return new Time(input.readLong(true));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ public void write(Kryo kryo, Output output, Timestamp timestamp) {
}

@Override
public Timestamp read(Kryo kryo, Input input, Class<Timestamp> timestampClass) {
public Timestamp read(Kryo kryo, Input input, Class<? extends Timestamp> timestampClass) {
Timestamp ts = new Timestamp(input.readLong(true));
ts.setNanos(input.readInt(true));
return ts;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ public void write(Kryo kryo, Output output, URI uri) {
}

@Override
public URI read(Kryo kryo, Input input, Class<URI> uriClass) {
public URI read(Kryo kryo, Input input, Class<? extends URI> uriClass) {
return URI.create(input.readString());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ public void write(Kryo kryo, Output output, UUID uuid) {
output.writeLong(uuid.getLeastSignificantBits(), false);
}

@Override public UUID read(Kryo kryo, Input input, Class<UUID> uuidClass) {
@Override public UUID read(Kryo kryo, Input input, Class<? extends UUID> uuidClass) {
return new UUID(input.readLong(false), input.readLong(false));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ abstract class UnmodifiableJavaCollectionSerializer<T> extends Serializer<T> {

@Override
@SuppressWarnings("unchecked")
public T read(Kryo kryo, Input input, Class<T> type) {
public T read(Kryo kryo, Input input, Class<? extends T> type) {
try {
T u = (T) kryo.readClassAndObject(input);
return newInstance(u);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,14 @@ public class TestCollections {
test_map.put(4, "four");
}

public static <T> T serializeAndDeserialize(T t) {
public static <T> T serializeAndDeserialize(T t) throws ClassNotFoundException {
Output output = new Output(1000, -1);
kryo.register(Class.forName("java.util.ArrayList"));
kryo.register(Class.forName("java.util.LinkedList"));
kryo.register(Class.forName("java.util.HashMap"));
kryo.register(Class.forName("java.util.TreeMap"));
kryo.register(Class.forName("java.util.HashSet"));
kryo.register(Class.forName("java.util.TreeSet"));
kryo.writeClassAndObject(output, t);
Input input = new Input(output.toBytes());
return (T) kryo.readClassAndObject(input);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,9 @@ public class TestLists {
));
}

public static <T> T serializeAndDeserialize(T t) {
public static <T> T serializeAndDeserialize(T t) throws ClassNotFoundException {
Output output = new Output(1000, -1);
kryo.register(Class.forName("java.util.List"));
kryo.writeClassAndObject(output, t);
Input input = new Input(output.toBytes());
return (T) kryo.readClassAndObject(input);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,14 @@ import org.scalatest.wordspec.AnyWordSpec
class TestInst extends KryoInstantiator { override def newKryo = new Kryo }
class TestInstTwo extends KryoInstantiator { override def newKryo = new Kryo }

class DefaultKryoInstantiator extends KryoInstantiator {
override def newKryo: Kryo = {
val k = new Kryo
k.setRegistrationRequired(false)
k
}
}

class ReflectingInstantiatorTest extends AnyWordSpec with Matchers {
"A ConfiguredInstantiator" should {
"work with a reflected instantiator" in {
Expand All @@ -38,15 +46,15 @@ class ReflectingInstantiatorTest extends AnyWordSpec with Matchers {
}
"work with a serialized instantiator" in {
val conf = new JavaMapConfig
ConfiguredInstantiator.setSerialized(conf, new TestInst)
ConfiguredInstantiator.setSerialized(conf, classOf[DefaultKryoInstantiator], new TestInst)
val cci = new ConfiguredInstantiator(conf)
// Here is the only assert:
cci.getDelegate.getClass should equal(classOf[TestInst])
// Verify that caching is working:
val cci2 = new ConfiguredInstantiator(conf)
cci.getDelegate should equal(cci2.getDelegate)
// Set a new serialized and verify caching is still correct:
ConfiguredInstantiator.setSerialized(conf, new TestInstTwo)
ConfiguredInstantiator.setSerialized(conf, classOf[DefaultKryoInstantiator], new TestInstTwo)
val cci3 = new ConfiguredInstantiator(conf)
cci3.getDelegate.getClass should equal(classOf[TestInstTwo])
(cci3.getDelegate should not).equal(cci2.getDelegate)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ package com.twitter.chill.java
import com.esotericsoftware.kryo.Kryo
import com.esotericsoftware.kryo.io.Input
import com.esotericsoftware.kryo.io.Output

import org.objenesis.strategy.StdInstantiatorStrategy

import _root_.java.util.Locale
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ public void write(Kryo kryo, Output output, BitSet bitSet) {
}

@Override
public BitSet read(Kryo kryo, Input input, Class<BitSet> bitSetClass) {
public BitSet read(Kryo kryo, Input input, Class<? extends BitSet> bitSetClass) {
int len = input.readInt(true);
BitSet ret = new BitSet(len);

Expand Down
Loading