Skip to content

Commit

Permalink
Fixes apache#2178, leave jdk standard classes to kryo itself.
Browse files Browse the repository at this point in the history
  • Loading branch information
chickenlj committed Aug 9, 2018
1 parent 895a4dd commit 260ef5d
Show file tree
Hide file tree
Showing 6 changed files with 50 additions and 24 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,21 +16,34 @@
*/
package org.apache.dubbo.common.serialize.support;

import java.util.LinkedHashSet;
import java.util.Set;
import com.esotericsoftware.kryo.Serializer;

import java.util.LinkedHashMap;
import java.util.Map;

public abstract class SerializableClassRegistry {

private static final Set<Class> registrations = new LinkedHashSet<Class>();

private static final Map<Class, Object> registrations = new LinkedHashMap<>();

/**
* only supposed to be called at startup time
*/
public static void registerClass(Class clazz) {
registrations.add(clazz);
registerClass(clazz, null);
}

/**
* only supposed to be called at startup time
*/
public static void registerClass(Class clazz, Serializer serializer) {
if (clazz == null) {
throw new IllegalArgumentException("Class registered to kryo cannot be null!");
}
registrations.put(clazz, serializer);
}

public static Set<Class> getRegisteredClasses() {
public static Map<Class, Object> getRegisteredClasses() {
return registrations;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,9 @@

import org.junit.Test;

import java.util.Set;
import java.util.Map;

import static org.hamcrest.Matchers.hasSize;
import static org.hamcrest.Matchers.equalTo;
import static org.junit.Assert.assertThat;

public class SerializableClassRegistryTest {
Expand All @@ -29,8 +29,8 @@ public void testAddClasses() {
SerializableClassRegistry.registerClass(A.class);
SerializableClassRegistry.registerClass(B.class);

Set<Class> registeredClasses = SerializableClassRegistry.getRegisteredClasses();
assertThat(registeredClasses, hasSize(2));
Map<Class, Object> registeredClasses = SerializableClassRegistry.getRegisteredClasses();
assertThat(registeredClasses.size(), equalTo(2));
}

private class A {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
package org.apache.dubbo.common.serialize.fst;

import org.apache.dubbo.common.serialize.support.SerializableClassRegistry;

import org.nustaq.serialization.FSTConfiguration;
import org.nustaq.serialization.FSTObjectInput;
import org.nustaq.serialization.FSTObjectOutput;
Expand All @@ -37,9 +36,7 @@ public static FstFactory getDefaultFactory() {
}

public FstFactory() {
for (Class clazz : SerializableClassRegistry.getRegisteredClasses()) {
conf.registerClass(clazz);
}
SerializableClassRegistry.getRegisteredClasses().keySet().forEach(conf::registerClass);
}

public FSTObjectOutput getObjectOutput(OutputStream outputStream) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,12 @@
*/
package org.apache.dubbo.common.serialize.kryo;

import org.apache.dubbo.common.logger.Logger;
import org.apache.dubbo.common.logger.LoggerFactory;
import org.apache.dubbo.common.serialize.kryo.utils.ReflectionUtils;

import com.esotericsoftware.kryo.Kryo;
import com.esotericsoftware.kryo.Serializer;
import com.esotericsoftware.kryo.serializers.JavaSerializer;
import org.apache.dubbo.common.logger.Logger;
import org.apache.dubbo.common.logger.LoggerFactory;
import org.apache.dubbo.common.serialize.kryo.utils.ReflectionUtils;

public class CompatibleKryo extends Kryo {

Expand All @@ -34,7 +33,16 @@ public Serializer getDefaultSerializer(Class type) {
throw new IllegalArgumentException("type cannot be null.");
}

if (!type.isArray() && !type.isEnum() && !ReflectionUtils.checkZeroArgConstructor(type)) {
/**
* Kryo requires every class to provide a zero argument constructor. For any class does not match this condition, kryo have two ways:
* 1. Use JavaSerializer,
* 2. Set 'kryo.setInstantiatorStrategy(new DefaultInstantiatorStrategy(new StdInstantiatorStrategy()));', StdInstantiatorStrategy can generate an instance bypassing the constructor.
*
* In practice, it's not possible for Dubbo users to register kryo Serializer for every customized class. So in most cases, customized classes with/without zero argument constructor will
* default to the default serializer.
* It is the responsibility of kryo to handle with every standard jdk classes, so we will just escape these classes.
*/
if (!ReflectionUtils.isJdk(type) && !type.isArray() && !type.isEnum() && !ReflectionUtils.checkZeroArgConstructor(type)) {
if (logger.isWarnEnabled()) {
logger.warn(type + " has no zero-arg constructor and this will affect the serialization performance");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,8 @@
*/
package org.apache.dubbo.common.serialize.kryo.utils;

import org.apache.dubbo.common.serialize.kryo.CompatibleKryo;
import org.apache.dubbo.common.serialize.support.SerializableClassRegistry;

import com.esotericsoftware.kryo.Kryo;
import com.esotericsoftware.kryo.Serializer;
import com.esotericsoftware.kryo.pool.KryoFactory;
import com.esotericsoftware.kryo.serializers.DefaultSerializers;
import de.javakaffee.kryoserializers.ArraysAsListSerializer;
Expand All @@ -31,6 +29,8 @@
import de.javakaffee.kryoserializers.URISerializer;
import de.javakaffee.kryoserializers.UUIDSerializer;
import de.javakaffee.kryoserializers.UnmodifiableCollectionsSerializer;
import org.apache.dubbo.common.serialize.kryo.CompatibleKryo;
import org.apache.dubbo.common.serialize.support.SerializableClassRegistry;

import java.lang.reflect.InvocationHandler;
import java.math.BigDecimal;
Expand Down Expand Up @@ -134,9 +134,13 @@ public Kryo create() {
kryo.register(clazz);
}

for (Class clazz : SerializableClassRegistry.getRegisteredClasses()) {
kryo.register(clazz);
}
SerializableClassRegistry.getRegisteredClasses().forEach((clazz, ser) -> {
if (ser == null) {
kryo.register(clazz);
} else {
kryo.register(clazz, (Serializer) ser);
}
});

return kryo;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,4 +26,8 @@ public static boolean checkZeroArgConstructor(Class clazz) {
return false;
}
}

public static boolean isJdk(Class clazz) {
return clazz.getName().startsWith("java.") || clazz.getName().startsWith("javax.");
}
}

0 comments on commit 260ef5d

Please sign in to comment.