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

apply #2220 to 2.6.x branch, issue #2178 #3519

Merged
merged 7 commits into from Feb 21, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -16,21 +16,33 @@
*/
package com.alibaba.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<Class, Object>();

/**
* 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;
}
}
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,13 +29,13 @@ 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 {
}

private class B {
}
}
}
Expand Up @@ -37,7 +37,7 @@ public static FstFactory getDefaultFactory() {
}

public FstFactory() {
for (Class clazz : SerializableClassRegistry.getRegisteredClasses()) {
for (Class clazz : SerializableClassRegistry.getRegisteredClasses().keySet()) {
conf.registerClass(clazz);
}
}
Expand Down
Expand Up @@ -34,12 +34,22 @@ 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");
}
return new JavaSerializer();
}

return super.getDefaultSerializer(type);
}
}
Expand Up @@ -20,6 +20,7 @@
import com.alibaba.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 Down Expand Up @@ -48,6 +49,7 @@
import java.util.Hashtable;
import java.util.LinkedHashSet;
import java.util.LinkedList;
import java.util.Map;
import java.util.Set;
import java.util.TreeSet;
import java.util.UUID;
Expand Down Expand Up @@ -134,8 +136,12 @@ public Kryo create() {
kryo.register(clazz);
}

for (Class clazz : SerializableClassRegistry.getRegisteredClasses()) {
kryo.register(clazz);
for (Map.Entry<Class, Object> entry : SerializableClassRegistry.getRegisteredClasses().entrySet()) {
if (entry.getValue() == null) {
kryo.register(entry.getKey());
} else {
kryo.register(entry.getKey(), (Serializer) entry.getValue());
}
}

return kryo;
Expand Down
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.");
}
}
Expand Up @@ -67,7 +67,7 @@ public abstract class AbstractSerializationTest {
URL url = new URL("protocol", "1.1.1.1", 1234);
ByteArrayOutputStream byteArrayOutputStream = new ByteArrayOutputStream();

// ================ Primitive Type ================
// ================ Primitive Type ================
BigPerson bigPerson;
MediaContent mediaContent;

Expand Down Expand Up @@ -383,7 +383,7 @@ public void test_BytesRange() throws Exception {
}
}

// ================ Array Type ================
// ================ Array Type ================

<T> void assertObjectArray(T[] data, Class<T[]> clazz) throws Exception {
ObjectOutput objectOutput = serialization.serialize(url, byteArrayOutputStream);
Expand Down Expand Up @@ -762,7 +762,7 @@ public void test_StringArray_withType() throws Exception {
assertObjectArrayWithType(new String[]{"1", "b"}, String[].class);
}

// ================ Simple Type ================
// ================ Simple Type ================

@Test
public void test_IntegerArray() throws Exception {
Expand Down Expand Up @@ -979,7 +979,7 @@ public void test_LinkedHashMap() throws Exception {
}
}

// ================ Complex Collection Type ================
// ================ Complex Collection Type ================

@Test
public void test_SPersonList() throws Exception {
Expand Down Expand Up @@ -1111,7 +1111,7 @@ public void test_MultiObject_WithType() throws Exception {
}


// abnormal case
// abnormal case

@Test
public void test_MediaContent_badStream() throws Exception {
Expand Down Expand Up @@ -1207,4 +1207,4 @@ public void test_URL_mutable_withType() throws Exception {
} catch (IOException expected) {
}
}
}
}
Expand Up @@ -21,7 +21,7 @@
xsi:schemaLocation="http://www.springframework.org/schema/beans http://www.springframework.org/schema/beans/spring-beans-4.3.xsd
http://dubbo.apache.org/schema/dubbo http://dubbo.apache.org/schema/dubbo/dubbo.xsd">

<dubbo:application name="memcached-consumer"/>
<dubbo:application name="redis-consumer"/>

<dubbo:reference id="cache" interface="java.util.Map" url="redis://10.20.153.10:6379"/>

Expand Down