You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by GitBox <gi...@apache.org> on 2022/02/01 14:24:14 UTC
[GitHub] [ignite-3] rpuch opened a new pull request #616: IGNITE-16260 User object serialization performance optimization
rpuch opened a new pull request #616:
URL: https://github.com/apache/ignite-3/pull/616
Co-authored-by: Semyon Danilov <sa...@yandex.ru>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] rpuch commented on a change in pull request #616: IGNITE-16260 User object serialization performance optimization
Posted by GitBox <gi...@apache.org>.
rpuch commented on a change in pull request #616:
URL: https://github.com/apache/ignite-3/pull/616#discussion_r798502727
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/util/io/GridDataOutput.java
##########
@@ -0,0 +1,138 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.util.io;
+
+import java.io.DataOutput;
+import java.io.IOException;
+import java.io.OutputStream;
+
+/**
+ * Extended data output.
+ */
+public interface GridDataOutput extends DataOutput {
Review comment:
Renamed Grid* to Ignite* (this concerns the classes I brought in this PR). For GridUnsafe, we agreed to do it in a separate PR later.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] rpuch commented on a change in pull request #616: IGNITE-16260 User object serialization performance optimization
Posted by GitBox <gi...@apache.org>.
rpuch commented on a change in pull request #616:
URL: https://github.com/apache/ignite-3/pull/616#discussion_r798498907
##########
File path: modules/network/pom.xml
##########
@@ -70,6 +70,22 @@
</dependency>
<!-- Test dependencies -->
+ <dependency>
+ <groupId>org.openjdk.jmh</groupId>
+ <artifactId>jmh-core</artifactId>
+ </dependency>
+
+ <dependency>
+ <groupId>org.openjdk.jmh</groupId>
+ <artifactId>jmh-generator-annprocess</artifactId>
Review comment:
Fixed
##########
File path: parent/pom.xml
##########
@@ -593,6 +594,12 @@
<version>${jmh.framework.version}</version>
</dependency>
+ <dependency>
Review comment:
Sure, added the comment
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] rpuch commented on a change in pull request #616: IGNITE-16260 User object serialization performance optimization
Posted by GitBox <gi...@apache.org>.
rpuch commented on a change in pull request #616:
URL: https://github.com/apache/ignite-3/pull/616#discussion_r798501688
##########
File path: modules/network/src/main/java/org/apache/ignite/internal/network/serialization/marshal/MarshallingContext.java
##########
@@ -17,25 +17,24 @@
package org.apache.ignite.internal.network.serialization.marshal;
-import static java.util.Collections.unmodifiableSet;
-
-import java.io.DataOutputStream;
+import it.unimi.dsi.fastutil.Hash;
+import it.unimi.dsi.fastutil.ints.IntOpenHashSet;
+import it.unimi.dsi.fastutil.ints.IntSet;
+import it.unimi.dsi.fastutil.objects.Object2IntMap;
+import it.unimi.dsi.fastutil.objects.Object2IntOpenCustomHashMap;
import java.io.IOException;
import java.io.NotActiveException;
-import java.util.HashSet;
-import java.util.IdentityHashMap;
-import java.util.Map;
import java.util.Objects;
-import java.util.Set;
import org.apache.ignite.internal.network.serialization.ClassDescriptor;
+import org.apache.ignite.internal.util.io.GridDataOutput;
/**
* Context using during marshalling of an object graph accessible from a root object.
*/
class MarshallingContext {
- private final Set<ClassDescriptor> usedDescriptors = new HashSet<>();
+ private final IntSet usedDescriptorIds = new IntOpenHashSet();
- private final Map<Object, Integer> objectsToIds = new IdentityHashMap<>();
+ private final Object2IntMap<Object> objectsToIds = new Object2IntOpenCustomHashMap<>(new IdentityHashStrategy());
Review comment:
First I also was not seeing any difference, but after some optimizations, on a medium object (`AllTypesMessage` without arrays) it suddenly started to be a little faster. Also, it requires less allocations.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] rpuch commented on a change in pull request #616: IGNITE-16260 User object serialization performance optimization
Posted by GitBox <gi...@apache.org>.
rpuch commented on a change in pull request #616:
URL: https://github.com/apache/ignite-3/pull/616#discussion_r798501688
##########
File path: modules/network/src/main/java/org/apache/ignite/internal/network/serialization/marshal/MarshallingContext.java
##########
@@ -17,25 +17,24 @@
package org.apache.ignite.internal.network.serialization.marshal;
-import static java.util.Collections.unmodifiableSet;
-
-import java.io.DataOutputStream;
+import it.unimi.dsi.fastutil.Hash;
+import it.unimi.dsi.fastutil.ints.IntOpenHashSet;
+import it.unimi.dsi.fastutil.ints.IntSet;
+import it.unimi.dsi.fastutil.objects.Object2IntMap;
+import it.unimi.dsi.fastutil.objects.Object2IntOpenCustomHashMap;
import java.io.IOException;
import java.io.NotActiveException;
-import java.util.HashSet;
-import java.util.IdentityHashMap;
-import java.util.Map;
import java.util.Objects;
-import java.util.Set;
import org.apache.ignite.internal.network.serialization.ClassDescriptor;
+import org.apache.ignite.internal.util.io.GridDataOutput;
/**
* Context using during marshalling of an object graph accessible from a root object.
*/
class MarshallingContext {
- private final Set<ClassDescriptor> usedDescriptors = new HashSet<>();
+ private final IntSet usedDescriptorIds = new IntOpenHashSet();
- private final Map<Object, Integer> objectsToIds = new IdentityHashMap<>();
+ private final Object2IntMap<Object> objectsToIds = new Object2IntOpenCustomHashMap<>(new IdentityHashStrategy());
Review comment:
First I also was not seeing any difference, but after some optimizations, on a medium object (`AllTypesMessage` without arrays) it suddenly started to be a little faster.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] SammyVimes commented on a change in pull request #616: IGNITE-16260 User object serialization performance optimization
Posted by GitBox <gi...@apache.org>.
SammyVimes commented on a change in pull request #616:
URL: https://github.com/apache/ignite-3/pull/616#discussion_r796745076
##########
File path: modules/network/pom.xml
##########
@@ -70,6 +70,22 @@
</dependency>
<!-- Test dependencies -->
+ <dependency>
+ <groupId>org.openjdk.jmh</groupId>
+ <artifactId>jmh-core</artifactId>
Review comment:
Should it be <scope>test</scope>?
##########
File path: modules/network/src/main/java/org/apache/ignite/internal/network/serialization/marshal/MarshallingValidations.java
##########
@@ -19,14 +19,25 @@
import java.lang.reflect.Field;
import java.lang.reflect.Modifier;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.function.Function;
import org.apache.ignite.internal.network.serialization.Classes;
import org.jetbrains.annotations.Nullable;
/**
* Validations that are run before marshalling objects.
*/
class MarshallingValidations {
- static void throwIfMarshallingNotSupported(@Nullable Object object) {
+ private final Map<Class<?>, YesNo> whetherInnerClasses = new HashMap<>();
+ private final Map<Class<?>, YesNo> whetherCapturingClosures = new HashMap<>();
+ private final Map<Class<?>, YesNo> whetherNonSerializableLambdas = new HashMap<>();
+
+ private final Function<Class<?>, YesNo> isInnerClassFunction = MarshallingValidations.this::isInnerClass;
+ private final Function<Class<?>, YesNo> isCapturingClosureFunction = this::isCapturingClosure;
+ private final Function<Class<?>, YesNo> isNonSerializableLambdaFunction = this::isNonSerializableLambda;
+
+ void throwIfMarshallingNotSupported(@Nullable Object object) {
Review comment:
I think we should move these checks to the descriptor generation phase and generate "invalid" descriptors if we can't marshall the class
##########
File path: parent/pom.xml
##########
@@ -593,6 +594,12 @@
<version>${jmh.framework.version}</version>
</dependency>
+ <dependency>
Review comment:
I think there should be a comment that we use kryo for benchmarks only, because otherwise it looks suspicious
##########
File path: modules/network/pom.xml
##########
@@ -70,6 +70,22 @@
</dependency>
<!-- Test dependencies -->
+ <dependency>
+ <groupId>org.openjdk.jmh</groupId>
+ <artifactId>jmh-core</artifactId>
+ </dependency>
+
+ <dependency>
+ <groupId>org.openjdk.jmh</groupId>
+ <artifactId>jmh-generator-annprocess</artifactId>
Review comment:
Same as above
##########
File path: modules/network/src/test/java/org/apache/ignite/internal/network/SerializationMicroBenchmark.java
##########
@@ -0,0 +1,308 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.network;
+
+import com.esotericsoftware.kryo.Kryo;
+import com.esotericsoftware.kryo.io.Input;
+import com.esotericsoftware.kryo.io.Output;
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.ObjectInputStream;
+import java.io.ObjectOutputStream;
+import java.io.Serializable;
+import java.util.concurrent.TimeUnit;
+import org.apache.ignite.internal.network.serialization.ClassDescriptorFactory;
+import org.apache.ignite.internal.network.serialization.ClassDescriptorRegistry;
+import org.apache.ignite.internal.network.serialization.marshal.DefaultUserObjectMarshaller;
+import org.apache.ignite.internal.network.serialization.marshal.MarshalException;
+import org.apache.ignite.internal.network.serialization.marshal.MarshalledObject;
+import org.apache.ignite.internal.network.serialization.marshal.UnmarshalException;
+import org.apache.ignite.internal.network.serialization.marshal.UserObjectMarshaller;
+import org.jetbrains.annotations.Nullable;
+import org.objenesis.strategy.StdInstantiatorStrategy;
+import org.openjdk.jmh.annotations.Benchmark;
+import org.openjdk.jmh.annotations.BenchmarkMode;
+import org.openjdk.jmh.annotations.Fork;
+import org.openjdk.jmh.annotations.Measurement;
+import org.openjdk.jmh.annotations.Mode;
+import org.openjdk.jmh.annotations.OutputTimeUnit;
+import org.openjdk.jmh.annotations.Scope;
+import org.openjdk.jmh.annotations.State;
+import org.openjdk.jmh.annotations.Warmup;
+import org.openjdk.jmh.infra.Blackhole;
+import org.openjdk.jmh.runner.Runner;
+import org.openjdk.jmh.runner.options.Options;
+import org.openjdk.jmh.runner.options.OptionsBuilder;
+
+/**
+ * A micro-benchmark of {@link DefaultUserObjectMarshaller}.
+ */
+@Warmup(iterations = 5, time = 1, timeUnit = TimeUnit.SECONDS)
+@Measurement(iterations = 5, time = 1, timeUnit = TimeUnit.SECONDS)
+@Fork(3)
+@BenchmarkMode(Mode.AverageTime)
+@OutputTimeUnit(TimeUnit.NANOSECONDS)
+@State(Scope.Thread)
+public class SerializationMicroBenchmark {
+
+ static TestClass smallObject;
+ static AllTypesMessage mediumObject;
+ static AllTypesMessage largeObject;
+
+ static ClassDescriptorRegistry registry;
+
+ static UserObjectMarshaller userObjectMarshaller;
+
+ static Kryo kryo;
+
+ static byte[] smallSerializedWithUos;
+ static byte[] smallSerializedWithJava;
+ static byte[] smallSerializedWithKryo;
+
+ static byte[] mediumSerializedWithUos;
+ static byte[] mediumSerializedWithJava;
+ static byte[] mediumSerializedWithKryo;
+
+ static byte[] largeSerializedWithUos;
+ static byte[] largeSerializedWithJava;
+ static byte[] largeSerializedWithKryo;
+
+ static {
+ registry = new ClassDescriptorRegistry();
+ var factory = new ClassDescriptorFactory(registry);
+ userObjectMarshaller = new DefaultUserObjectMarshaller(registry, factory);
+
+ smallObject = new TestClass(1000, false, 3000);
+ mediumObject = AllTypesMessageGenerator.generate(10, true, false);
+ largeObject = AllTypesMessageGenerator.generate(10, true, true);
+
+ factory.create(smallObject.getClass());
+ factory.create(largeObject.getClass());
+
+ kryo = new Kryo();
+ kryo.setInstantiatorStrategy(new StdInstantiatorStrategy());
+
+ try {
+ smallSerializedWithUos = userObjectMarshaller.marshal(smallObject).bytes();
+ mediumSerializedWithUos = userObjectMarshaller.marshal(mediumObject).bytes();
+ largeSerializedWithUos = userObjectMarshaller.marshal(largeObject).bytes();
+ } catch (MarshalException e) {
+ throw new RuntimeException(e);
+ }
+
+ smallSerializedWithJava = serializeWithJdk(smallObject);
+ mediumSerializedWithJava = serializeWithJdk(mediumObject);
+ largeSerializedWithJava = serializeWithJdk(largeObject);
+
+ smallSerializedWithKryo = serializeWithKryo(smallObject);
+ mediumSerializedWithKryo = serializeWithKryo(mediumObject);
+ largeSerializedWithKryo = serializeWithKryo(largeObject);
+
+ System.out.println("Small Java: " + smallSerializedWithJava.length);
+ System.out.println("Small UOS : " + smallSerializedWithUos.length);
+ System.out.println("Small Kryo: " + smallSerializedWithKryo.length);
+
+ System.out.println("Medium Java: " + mediumSerializedWithJava.length);
+ System.out.println("Medium UOS : " + mediumSerializedWithUos.length);
+ System.out.println("Medium Kryo: " + mediumSerializedWithKryo.length);
+
+ System.out.println("Large Java: " + largeSerializedWithJava.length);
+ System.out.println("Large UOS : " + largeSerializedWithUos.length);
+ System.out.println("Large Kryo: " + largeSerializedWithKryo.length);
+ }
+
+ /**
+ * Runs the benchmark.
+ *
+ * @param args args
+ * @throws Exception if something goes wrong
+ */
+ public static void main(String[] args) throws Exception {
+ Options build = new OptionsBuilder()
+ //.addProfiler("stack")
Review comment:
I think we can remove this
##########
File path: modules/network/src/main/java/org/apache/ignite/internal/network/serialization/marshal/MarshallingContext.java
##########
@@ -17,25 +17,24 @@
package org.apache.ignite.internal.network.serialization.marshal;
-import static java.util.Collections.unmodifiableSet;
-
-import java.io.DataOutputStream;
+import it.unimi.dsi.fastutil.Hash;
+import it.unimi.dsi.fastutil.ints.IntOpenHashSet;
+import it.unimi.dsi.fastutil.ints.IntSet;
+import it.unimi.dsi.fastutil.objects.Object2IntMap;
+import it.unimi.dsi.fastutil.objects.Object2IntOpenCustomHashMap;
import java.io.IOException;
import java.io.NotActiveException;
-import java.util.HashSet;
-import java.util.IdentityHashMap;
-import java.util.Map;
import java.util.Objects;
-import java.util.Set;
import org.apache.ignite.internal.network.serialization.ClassDescriptor;
+import org.apache.ignite.internal.util.io.GridDataOutput;
/**
* Context using during marshalling of an object graph accessible from a root object.
*/
class MarshallingContext {
- private final Set<ClassDescriptor> usedDescriptors = new HashSet<>();
+ private final IntSet usedDescriptorIds = new IntOpenHashSet();
- private final Map<Object, Integer> objectsToIds = new IdentityHashMap<>();
+ private final Object2IntMap<Object> objectsToIds = new Object2IntOpenCustomHashMap<>(new IdentityHashStrategy());
Review comment:
Is it any good, btw? During the time when I was running benchmarks, I haven't seen performance improvements while using this map
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/util/io/GridDataOutput.java
##########
@@ -0,0 +1,138 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.util.io;
+
+import java.io.DataOutput;
+import java.io.IOException;
+import java.io.OutputStream;
+
+/**
+ * Extended data output.
+ */
+public interface GridDataOutput extends DataOutput {
Review comment:
Maybe we should call it IgniteDataOutput (and IgniteDataInput for Input)? And also GridUnsafe, probably, should be IgniteUnsafe
##########
File path: modules/network/src/test/java/org/apache/ignite/internal/network/UosProfilerTarget.java
##########
@@ -0,0 +1,104 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.network;
+
+import java.util.Arrays;
+import java.util.Objects;
+import org.apache.ignite.internal.network.SerializationMicroBenchmark.TestClass;
+import org.apache.ignite.internal.network.serialization.ClassDescriptorFactory;
+import org.apache.ignite.internal.network.serialization.ClassDescriptorRegistry;
+import org.apache.ignite.internal.network.serialization.marshal.DefaultUserObjectMarshaller;
+import org.apache.ignite.internal.network.serialization.marshal.MarshalException;
+import org.apache.ignite.internal.network.serialization.marshal.MarshalledObject;
+import org.apache.ignite.internal.network.serialization.marshal.UserObjectMarshaller;
+
+/**
+ * For running with a profiler.
+ */
+public class UosProfilerTarget {
Review comment:
I think this class should be refactored, there are system outs and commented code
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] rpuch commented on a change in pull request #616: IGNITE-16260 User object serialization performance optimization
Posted by GitBox <gi...@apache.org>.
rpuch commented on a change in pull request #616:
URL: https://github.com/apache/ignite-3/pull/616#discussion_r798504957
##########
File path: modules/network/src/test/java/org/apache/ignite/internal/network/UosProfilerTarget.java
##########
@@ -0,0 +1,104 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.network;
+
+import java.util.Arrays;
+import java.util.Objects;
+import org.apache.ignite.internal.network.SerializationMicroBenchmark.TestClass;
+import org.apache.ignite.internal.network.serialization.ClassDescriptorFactory;
+import org.apache.ignite.internal.network.serialization.ClassDescriptorRegistry;
+import org.apache.ignite.internal.network.serialization.marshal.DefaultUserObjectMarshaller;
+import org.apache.ignite.internal.network.serialization.marshal.MarshalException;
+import org.apache.ignite.internal.network.serialization.marshal.MarshalledObject;
+import org.apache.ignite.internal.network.serialization.marshal.UserObjectMarshaller;
+
+/**
+ * For running with a profiler.
+ */
+public class UosProfilerTarget {
Review comment:
I removed one `System.out.println()` but left 2 others. I believe they are needed to make sure that the compiler not optimize the code away when it sees that the code is pure (has no side-effects) and the resulting value is not used.
Also, I removed some commented-out code, but I left some code that creates alternatives for the original object to work with and the line that does deserialization instead of serialization. It's a benchmarking code, after all, it's not going to go to production :)
If you feel this is not enough, please let me know.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] SammyVimes closed pull request #616: IGNITE-16260 User object serialization performance optimization
Posted by GitBox <gi...@apache.org>.
SammyVimes closed pull request #616:
URL: https://github.com/apache/ignite-3/pull/616
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] rpuch commented on a change in pull request #616: IGNITE-16260 User object serialization performance optimization
Posted by GitBox <gi...@apache.org>.
rpuch commented on a change in pull request #616:
URL: https://github.com/apache/ignite-3/pull/616#discussion_r798500715
##########
File path: modules/network/src/main/java/org/apache/ignite/internal/network/serialization/marshal/MarshallingValidations.java
##########
@@ -19,14 +19,25 @@
import java.lang.reflect.Field;
import java.lang.reflect.Modifier;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.function.Function;
import org.apache.ignite.internal.network.serialization.Classes;
import org.jetbrains.annotations.Nullable;
/**
* Validations that are run before marshalling objects.
*/
class MarshallingValidations {
- static void throwIfMarshallingNotSupported(@Nullable Object object) {
+ private final Map<Class<?>, YesNo> whetherInnerClasses = new HashMap<>();
+ private final Map<Class<?>, YesNo> whetherCapturingClosures = new HashMap<>();
+ private final Map<Class<?>, YesNo> whetherNonSerializableLambdas = new HashMap<>();
+
+ private final Function<Class<?>, YesNo> isInnerClassFunction = MarshallingValidations.this::isInnerClass;
+ private final Function<Class<?>, YesNo> isCapturingClosureFunction = this::isCapturingClosure;
+ private final Function<Class<?>, YesNo> isNonSerializableLambdaFunction = this::isNonSerializableLambda;
+
+ void throwIfMarshallingNotSupported(@Nullable Object object) {
Review comment:
As agreed on a call, left the caching approach, but squashed the maps. The idea is that, if we use a descriptor as a medium of the knowledge about 'marshallability' of a class, we still need to look it up in a map (a different map), but this requires to make quite some changes, and it does not seem to be worth it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] rpuch commented on a change in pull request #616: IGNITE-16260 User object serialization performance optimization
Posted by GitBox <gi...@apache.org>.
rpuch commented on a change in pull request #616:
URL: https://github.com/apache/ignite-3/pull/616#discussion_r798498835
##########
File path: modules/network/pom.xml
##########
@@ -70,6 +70,22 @@
</dependency>
<!-- Test dependencies -->
+ <dependency>
+ <groupId>org.openjdk.jmh</groupId>
+ <artifactId>jmh-core</artifactId>
Review comment:
Good catch, fixed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] rpuch commented on a change in pull request #616: IGNITE-16260 User object serialization performance optimization
Posted by GitBox <gi...@apache.org>.
rpuch commented on a change in pull request #616:
URL: https://github.com/apache/ignite-3/pull/616#discussion_r798505612
##########
File path: modules/network/src/test/java/org/apache/ignite/internal/network/SerializationMicroBenchmark.java
##########
@@ -0,0 +1,308 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.network;
+
+import com.esotericsoftware.kryo.Kryo;
+import com.esotericsoftware.kryo.io.Input;
+import com.esotericsoftware.kryo.io.Output;
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.ObjectInputStream;
+import java.io.ObjectOutputStream;
+import java.io.Serializable;
+import java.util.concurrent.TimeUnit;
+import org.apache.ignite.internal.network.serialization.ClassDescriptorFactory;
+import org.apache.ignite.internal.network.serialization.ClassDescriptorRegistry;
+import org.apache.ignite.internal.network.serialization.marshal.DefaultUserObjectMarshaller;
+import org.apache.ignite.internal.network.serialization.marshal.MarshalException;
+import org.apache.ignite.internal.network.serialization.marshal.MarshalledObject;
+import org.apache.ignite.internal.network.serialization.marshal.UnmarshalException;
+import org.apache.ignite.internal.network.serialization.marshal.UserObjectMarshaller;
+import org.jetbrains.annotations.Nullable;
+import org.objenesis.strategy.StdInstantiatorStrategy;
+import org.openjdk.jmh.annotations.Benchmark;
+import org.openjdk.jmh.annotations.BenchmarkMode;
+import org.openjdk.jmh.annotations.Fork;
+import org.openjdk.jmh.annotations.Measurement;
+import org.openjdk.jmh.annotations.Mode;
+import org.openjdk.jmh.annotations.OutputTimeUnit;
+import org.openjdk.jmh.annotations.Scope;
+import org.openjdk.jmh.annotations.State;
+import org.openjdk.jmh.annotations.Warmup;
+import org.openjdk.jmh.infra.Blackhole;
+import org.openjdk.jmh.runner.Runner;
+import org.openjdk.jmh.runner.options.Options;
+import org.openjdk.jmh.runner.options.OptionsBuilder;
+
+/**
+ * A micro-benchmark of {@link DefaultUserObjectMarshaller}.
+ */
+@Warmup(iterations = 5, time = 1, timeUnit = TimeUnit.SECONDS)
+@Measurement(iterations = 5, time = 1, timeUnit = TimeUnit.SECONDS)
+@Fork(3)
+@BenchmarkMode(Mode.AverageTime)
+@OutputTimeUnit(TimeUnit.NANOSECONDS)
+@State(Scope.Thread)
+public class SerializationMicroBenchmark {
+
+ static TestClass smallObject;
+ static AllTypesMessage mediumObject;
+ static AllTypesMessage largeObject;
+
+ static ClassDescriptorRegistry registry;
+
+ static UserObjectMarshaller userObjectMarshaller;
+
+ static Kryo kryo;
+
+ static byte[] smallSerializedWithUos;
+ static byte[] smallSerializedWithJava;
+ static byte[] smallSerializedWithKryo;
+
+ static byte[] mediumSerializedWithUos;
+ static byte[] mediumSerializedWithJava;
+ static byte[] mediumSerializedWithKryo;
+
+ static byte[] largeSerializedWithUos;
+ static byte[] largeSerializedWithJava;
+ static byte[] largeSerializedWithKryo;
+
+ static {
+ registry = new ClassDescriptorRegistry();
+ var factory = new ClassDescriptorFactory(registry);
+ userObjectMarshaller = new DefaultUserObjectMarshaller(registry, factory);
+
+ smallObject = new TestClass(1000, false, 3000);
+ mediumObject = AllTypesMessageGenerator.generate(10, true, false);
+ largeObject = AllTypesMessageGenerator.generate(10, true, true);
+
+ factory.create(smallObject.getClass());
+ factory.create(largeObject.getClass());
+
+ kryo = new Kryo();
+ kryo.setInstantiatorStrategy(new StdInstantiatorStrategy());
+
+ try {
+ smallSerializedWithUos = userObjectMarshaller.marshal(smallObject).bytes();
+ mediumSerializedWithUos = userObjectMarshaller.marshal(mediumObject).bytes();
+ largeSerializedWithUos = userObjectMarshaller.marshal(largeObject).bytes();
+ } catch (MarshalException e) {
+ throw new RuntimeException(e);
+ }
+
+ smallSerializedWithJava = serializeWithJdk(smallObject);
+ mediumSerializedWithJava = serializeWithJdk(mediumObject);
+ largeSerializedWithJava = serializeWithJdk(largeObject);
+
+ smallSerializedWithKryo = serializeWithKryo(smallObject);
+ mediumSerializedWithKryo = serializeWithKryo(mediumObject);
+ largeSerializedWithKryo = serializeWithKryo(largeObject);
+
+ System.out.println("Small Java: " + smallSerializedWithJava.length);
+ System.out.println("Small UOS : " + smallSerializedWithUos.length);
+ System.out.println("Small Kryo: " + smallSerializedWithKryo.length);
+
+ System.out.println("Medium Java: " + mediumSerializedWithJava.length);
+ System.out.println("Medium UOS : " + mediumSerializedWithUos.length);
+ System.out.println("Medium Kryo: " + mediumSerializedWithKryo.length);
+
+ System.out.println("Large Java: " + largeSerializedWithJava.length);
+ System.out.println("Large UOS : " + largeSerializedWithUos.length);
+ System.out.println("Large Kryo: " + largeSerializedWithKryo.length);
+ }
+
+ /**
+ * Runs the benchmark.
+ *
+ * @param args args
+ * @throws Exception if something goes wrong
+ */
+ public static void main(String[] args) throws Exception {
+ Options build = new OptionsBuilder()
+ //.addProfiler("stack")
Review comment:
Removed the line about "stack", but I think it makes sense to leave the line about "gc" as it is useful in some situations and it's a lot easier to uncomment a line than try to recall the correct syntax :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org