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/03/05 12:21:06 UTC
[GitHub] [ignite-3] SammyVimes opened a new pull request #705: IGNITE-16393 Move network's user object serialization to the user thread
SammyVimes opened a new pull request #705:
URL: https://github.com/apache/ignite-3/pull/705
https://issues.apache.org/jira/browse/IGNITE-16393
--
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 pull request #705: IGNITE-16393 Move network's user object serialization to the user thread
Posted by GitBox <gi...@apache.org>.
rpuch commented on pull request #705:
URL: https://github.com/apache/ignite-3/pull/705#issuecomment-1063803143
Looks good to me, thanks!
--
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 #705: IGNITE-16393 Move network's user object serialization to the user thread
Posted by GitBox <gi...@apache.org>.
rpuch commented on a change in pull request #705:
URL: https://github.com/apache/ignite-3/pull/705#discussion_r822515724
##########
File path: modules/network-annotation-processor/src/main/java/org/apache/ignite/internal/network/processor/messages/MessageImplGenerator.java
##########
@@ -85,7 +101,7 @@ public TypeSpec generateMessageImpl(MessageClass message, TypeSpec builderInterf
String getterName = getter.getSimpleName().toString();
FieldSpec field = FieldSpec.builder(getterReturnType, getterName)
- .addModifiers(Modifier.PRIVATE, Modifier.FINAL)
Review comment:
Do we need to make all the fields non-final? Maybe it's enough to have only marshallable fields without the `final` modifier? `final` has a lot of advantages...
##########
File path: modules/network-annotation-processor/src/main/java/org/apache/ignite/internal/network/processor/messages/MessageBuilderGenerator.java
##########
@@ -62,30 +64,45 @@ public TypeSpec generateBuilderInterface(MessageClass message) {
processingEnvironment.getMessager()
.printMessage(Diagnostic.Kind.NOTE, "Generating " + builderName, message.element());
- // generate a setter for each getter in the original interface
- List<MethodSpec> setters = message.getters().stream()
- .map(getter -> {
- String getterName = getter.getSimpleName().toString();
-
- return MethodSpec.methodBuilder(getterName)
- .addModifiers(Modifier.PUBLIC, Modifier.ABSTRACT)
- .addParameter(TypeName.get(getter.getReturnType()), getterName)
- .returns(builderName)
- .build();
- })
- .collect(Collectors.toList());
-
- // generate a getter for each getter in the original interface
- List<MethodSpec> getters = message.getters().stream()
- .map(getter -> {
- String getterName = getter.getSimpleName().toString();
-
- return MethodSpec.methodBuilder(getterName)
- .addModifiers(Modifier.PUBLIC, Modifier.ABSTRACT)
- .returns(TypeName.get(getter.getReturnType()))
- .build();
- })
- .collect(Collectors.toList());
+ var setters = new ArrayList<MethodSpec>(message.getters().size());
Review comment:
The original way to generate getters and setters separately seems to be better because there is less coupling, also it is easier to break the code to logical blocks: 'generate getters', 'generate setters', ...
I suggest to separate getters/settes/BAgetters generation again, doing it in 3 steps. Also, I suggest to extract each of the 3 blocks to a method of each own (like 'generateGetters()` and so on), so that `generateBuilderInterface()` become easier to read.
##########
File path: modules/network-annotation-processor/src/main/java/org/apache/ignite/internal/network/processor/messages/MessageImplGenerator.java
##########
@@ -156,13 +189,265 @@ public TypeSpec generateMessageImpl(MessageClass message, TypeSpec builderInterf
messageImpl.addMethod(builderMethod);
+ generatePrepareMarshal(messageImpl, message);
+ generateUnmarshalMethod(messageImpl, message);
+
messageImpl
.addOriginatingElement(message.element())
.addOriginatingElement(messageGroup.element());
return messageImpl.build();
}
+ /**
+ * Resolves type of an object to a type that may hold a message. Returns {@code null} if the type
+ * can't hold a message.
+ *
+ * @param parameterType Type.
+ * @return {@link MaybeMessageType} or {@code null} if the type can't hold a message.
+ */
+ @Nullable
+ private MessageImplGenerator.MaybeMessageType resolveType(TypeMirror parameterType) {
+ if (parameterType.getKind() == TypeKind.ARRAY) {
+ if (!((ArrayType) parameterType).getComponentType().getKind().isPrimitive()) {
+ return MaybeMessageType.OBJECT_ARRAY;
+ }
+ } else if (parameterType.getKind() == TypeKind.DECLARED) {
+ if (typeUtils.isSameType(parameterType, Collection.class)) {
+ return MaybeMessageType.COLLECTION;
+ } else if (typeUtils.isSameType(parameterType, Map.class)) {
+ return MaybeMessageType.MAP;
+ } else if (typeUtils.isSubType(parameterType, NetworkMessage.class)) {
+ return MaybeMessageType.MESSAGE;
+ }
+ }
+ return null;
+ }
+
+ private void generatePrepareMarshal(TypeSpec.Builder messageImplBuild, MessageClass message) {
+ boolean isNeeded = false;
+
+ ParameterizedTypeName setType = ParameterizedTypeName.get(ClassName.get(Set.class), TypeName.INT.box());
+
+ Builder prepareMarshal = MethodSpec.methodBuilder("prepareMarshal")
+ .addAnnotation(Override.class)
+ .addModifiers(Modifier.PUBLIC)
+ .addException(Exception.class)
+ .addParameter(setType, "set")
+ .addParameter(Object.class, "marshallerObj");
+
+ String marshallerPackage = "org.apache.ignite.internal.network.serialization.marshal";
+ ClassName marshallerClass = ClassName.get(marshallerPackage, "UserObjectMarshaller");
+ ClassName marshalledObjectClass = ClassName.get(marshallerPackage, "MarshalledObject");
+
+ prepareMarshal.addStatement("$T marshaller = ($T) marshallerObj", marshallerClass, marshallerClass);
+
+ for (ExecutableElement executableElement : message.getters()) {
+ TypeMirror type = executableElement.getReturnType();
+ String objectName = executableElement.getSimpleName().toString();
+
+ if (executableElement.getAnnotation(Marshallable.class) != null) {
+ isNeeded = true;
+
+ String baName = objectName + "ByteArray";
+ String moName = baName + "mo";
+ prepareMarshal.addStatement("$T $N = marshaller.marshal($N)", marshalledObjectClass, moName, objectName);
+ prepareMarshal.addStatement("set.addAll($N.usedDescriptorIds())", moName);
+ prepareMarshal.addStatement("$N = $N.bytes()", baName, moName).addCode("\n");
+ } else {
+ MaybeMessageType objectType = resolveType(type);
+
+ if (objectType == null) {
+ continue;
+ }
+
+ switch (objectType) {
+ case OBJECT_ARRAY:
+ ArrayType arrayType = (ArrayType) type;
+ if (typeUtils.isSubType(arrayType.getComponentType(), NetworkMessage.class)) {
+ isNeeded = true;
+
+ prepareMarshal.beginControlFlow("if ($N != null)", objectName);
+ prepareMarshal.beginControlFlow("for (int i = 0; i < $N.length; i++)", objectName);
+ prepareMarshal.addStatement("if ($N[i] != null) $N[i].prepareMarshal(set, marshaller)", objectName,
+ objectName);
+ prepareMarshal.endControlFlow();
+ prepareMarshal.endControlFlow().addCode("\n");
+ }
+ break;
+ case COLLECTION:
+ DeclaredType declaredType = (DeclaredType) type;
+ TypeMirror elementType = declaredType.getTypeArguments().get(0);
+ if (typeUtils.isSubType(elementType, NetworkMessage.class)) {
+ isNeeded = true;
+
+ prepareMarshal.beginControlFlow("if ($N != null)", objectName);
+ prepareMarshal.beginControlFlow("for ($T obj : $N)", elementType, objectName);
+ prepareMarshal.addStatement("if (obj != null) obj.prepareMarshal(set, marshaller)", objectName);
+ prepareMarshal.endControlFlow();
+ prepareMarshal.endControlFlow().addCode("\n");
+ }
+ break;
+ case MESSAGE:
+ isNeeded = true;
+
+ prepareMarshal.addStatement("if ($N != null) $N.prepareMarshal(set, marshaller)", objectName, objectName);
+ break;
+ case MAP:
+ DeclaredType mapType = (DeclaredType) type;
+ TypeMirror keyType = mapType.getTypeArguments().get(0);
+ boolean keyIsMessage = typeUtils.isSubType(keyType, NetworkMessage.class);
+ TypeMirror valueType = mapType.getTypeArguments().get(1);
+ boolean valueIsMessage = typeUtils.isSubType(valueType, NetworkMessage.class);
+
+ if (keyIsMessage || valueIsMessage) {
+ isNeeded = true;
+
+ ParameterizedTypeName entryType = ParameterizedTypeName.get(
+ ClassName.get(Entry.class),
+ TypeName.get(keyType),
+ TypeName.get(valueType)
+ );
+ ParameterizedTypeName entrySetType = ParameterizedTypeName.get(ClassName.get(Set.class), entryType);
+ String entrySetName = objectName + "EntrySet";
+
+ prepareMarshal.beginControlFlow("if ($N != null)", objectName);
+ prepareMarshal.addStatement("$T $N = $N.entrySet()", entrySetType, entrySetName, objectName);
+ prepareMarshal.beginControlFlow("for ($T entry : $N)", entryType, entrySetName);
+ prepareMarshal.addStatement("$T key = entry.getKey()", keyType);
+ prepareMarshal.addStatement("$T value = entry.getValue()", valueType);
+ if (keyIsMessage) {
+ prepareMarshal.addStatement("if (key != null) key.prepareMarshal(set, marshaller)");
+ }
+ if (valueIsMessage) {
+ prepareMarshal.addStatement("if (value != null) value.prepareMarshal(set, marshaller)");
+ }
+ prepareMarshal.endControlFlow().addCode("\n");
+ prepareMarshal.endControlFlow();
+ }
+ break;
+ default:
+ break;
+ }
+ }
+ }
+
+ if (isNeeded) {
+ messageImplBuild.addMethod(prepareMarshal.build());
+ }
+ }
+
+ private void generateUnmarshalMethod(TypeSpec.Builder messageImplBuild, MessageClass message) {
Review comment:
Another long method
##########
File path: modules/network-annotation-processor/src/main/java/org/apache/ignite/internal/network/processor/messages/MessageBuilderGenerator.java
##########
@@ -62,30 +64,45 @@ public TypeSpec generateBuilderInterface(MessageClass message) {
processingEnvironment.getMessager()
.printMessage(Diagnostic.Kind.NOTE, "Generating " + builderName, message.element());
- // generate a setter for each getter in the original interface
- List<MethodSpec> setters = message.getters().stream()
- .map(getter -> {
- String getterName = getter.getSimpleName().toString();
-
- return MethodSpec.methodBuilder(getterName)
- .addModifiers(Modifier.PUBLIC, Modifier.ABSTRACT)
- .addParameter(TypeName.get(getter.getReturnType()), getterName)
- .returns(builderName)
- .build();
- })
- .collect(Collectors.toList());
-
- // generate a getter for each getter in the original interface
- List<MethodSpec> getters = message.getters().stream()
- .map(getter -> {
- String getterName = getter.getSimpleName().toString();
-
- return MethodSpec.methodBuilder(getterName)
- .addModifiers(Modifier.PUBLIC, Modifier.ABSTRACT)
- .returns(TypeName.get(getter.getReturnType()))
- .build();
- })
- .collect(Collectors.toList());
+ var setters = new ArrayList<MethodSpec>(message.getters().size());
+ var getters = new ArrayList<MethodSpec>(message.getters().size());
+ message.getters().forEach(getter -> {
+ String getterName = getter.getSimpleName().toString();
+
+ TypeMirror type = getter.getReturnType();
+
+ // generate a setter for each getter in the original interface
+ MethodSpec setterSpec = MethodSpec.methodBuilder(getterName)
+ .addModifiers(Modifier.PUBLIC, Modifier.ABSTRACT)
+ .addParameter(TypeName.get(type), getterName)
+ .returns(builderName)
+ .build();
+
+ // generate a getter for each getter in the original interface
+ MethodSpec getterSpec = MethodSpec.methodBuilder(getterName)
+ .addModifiers(Modifier.PUBLIC, Modifier.ABSTRACT)
+ .returns(TypeName.get(type))
+ .build();
+
+ setters.add(setterSpec);
+ getters.add(getterSpec);
+
+ if (getter.getAnnotation(Marshallable.class) != null) {
+ MethodSpec baSetter = MethodSpec.methodBuilder(getterName + "ByteArray")
Review comment:
What if I declare two methods: `abc()` (a `Marshallable`) and `abcByteArray()`. Would a collision occur? If yes, we probably need some kind of a validation to avoid confusion.
##########
File path: modules/network-annotation-processor/src/main/java/org/apache/ignite/internal/network/processor/messages/MessageImplGenerator.java
##########
@@ -156,13 +189,265 @@ public TypeSpec generateMessageImpl(MessageClass message, TypeSpec builderInterf
messageImpl.addMethod(builderMethod);
+ generatePrepareMarshal(messageImpl, message);
+ generateUnmarshalMethod(messageImpl, message);
+
messageImpl
.addOriginatingElement(message.element())
.addOriginatingElement(messageGroup.element());
return messageImpl.build();
}
+ /**
+ * Resolves type of an object to a type that may hold a message. Returns {@code null} if the type
+ * can't hold a message.
+ *
+ * @param parameterType Type.
+ * @return {@link MaybeMessageType} or {@code null} if the type can't hold a message.
+ */
+ @Nullable
+ private MessageImplGenerator.MaybeMessageType resolveType(TypeMirror parameterType) {
+ if (parameterType.getKind() == TypeKind.ARRAY) {
+ if (!((ArrayType) parameterType).getComponentType().getKind().isPrimitive()) {
+ return MaybeMessageType.OBJECT_ARRAY;
+ }
+ } else if (parameterType.getKind() == TypeKind.DECLARED) {
+ if (typeUtils.isSameType(parameterType, Collection.class)) {
+ return MaybeMessageType.COLLECTION;
+ } else if (typeUtils.isSameType(parameterType, Map.class)) {
+ return MaybeMessageType.MAP;
+ } else if (typeUtils.isSubType(parameterType, NetworkMessage.class)) {
+ return MaybeMessageType.MESSAGE;
+ }
+ }
+ return null;
+ }
+
+ private void generatePrepareMarshal(TypeSpec.Builder messageImplBuild, MessageClass message) {
+ boolean isNeeded = false;
+
+ ParameterizedTypeName setType = ParameterizedTypeName.get(ClassName.get(Set.class), TypeName.INT.box());
+
+ Builder prepareMarshal = MethodSpec.methodBuilder("prepareMarshal")
+ .addAnnotation(Override.class)
+ .addModifiers(Modifier.PUBLIC)
+ .addException(Exception.class)
+ .addParameter(setType, "set")
+ .addParameter(Object.class, "marshallerObj");
+
+ String marshallerPackage = "org.apache.ignite.internal.network.serialization.marshal";
+ ClassName marshallerClass = ClassName.get(marshallerPackage, "UserObjectMarshaller");
+ ClassName marshalledObjectClass = ClassName.get(marshallerPackage, "MarshalledObject");
+
+ prepareMarshal.addStatement("$T marshaller = ($T) marshallerObj", marshallerClass, marshallerClass);
+
+ for (ExecutableElement executableElement : message.getters()) {
+ TypeMirror type = executableElement.getReturnType();
+ String objectName = executableElement.getSimpleName().toString();
+
+ if (executableElement.getAnnotation(Marshallable.class) != null) {
+ isNeeded = true;
+
+ String baName = objectName + "ByteArray";
+ String moName = baName + "mo";
+ prepareMarshal.addStatement("$T $N = marshaller.marshal($N)", marshalledObjectClass, moName, objectName);
+ prepareMarshal.addStatement("set.addAll($N.usedDescriptorIds())", moName);
+ prepareMarshal.addStatement("$N = $N.bytes()", baName, moName).addCode("\n");
+ } else {
+ MaybeMessageType objectType = resolveType(type);
+
+ if (objectType == null) {
+ continue;
+ }
+
+ switch (objectType) {
+ case OBJECT_ARRAY:
+ ArrayType arrayType = (ArrayType) type;
+ if (typeUtils.isSubType(arrayType.getComponentType(), NetworkMessage.class)) {
+ isNeeded = true;
+
+ prepareMarshal.beginControlFlow("if ($N != null)", objectName);
+ prepareMarshal.beginControlFlow("for (int i = 0; i < $N.length; i++)", objectName);
+ prepareMarshal.addStatement("if ($N[i] != null) $N[i].prepareMarshal(set, marshaller)", objectName,
Review comment:
Why classic `for` style and not its enhanced variant (`for T item : items)`?
##########
File path: modules/network-annotation-processor/src/main/java/org/apache/ignite/internal/network/processor/messages/MessageImplGenerator.java
##########
@@ -156,13 +189,265 @@ public TypeSpec generateMessageImpl(MessageClass message, TypeSpec builderInterf
messageImpl.addMethod(builderMethod);
+ generatePrepareMarshal(messageImpl, message);
+ generateUnmarshalMethod(messageImpl, message);
+
messageImpl
.addOriginatingElement(message.element())
.addOriginatingElement(messageGroup.element());
return messageImpl.build();
}
+ /**
+ * Resolves type of an object to a type that may hold a message. Returns {@code null} if the type
+ * can't hold a message.
+ *
+ * @param parameterType Type.
+ * @return {@link MaybeMessageType} or {@code null} if the type can't hold a message.
+ */
+ @Nullable
+ private MessageImplGenerator.MaybeMessageType resolveType(TypeMirror parameterType) {
+ if (parameterType.getKind() == TypeKind.ARRAY) {
+ if (!((ArrayType) parameterType).getComponentType().getKind().isPrimitive()) {
+ return MaybeMessageType.OBJECT_ARRAY;
+ }
+ } else if (parameterType.getKind() == TypeKind.DECLARED) {
+ if (typeUtils.isSameType(parameterType, Collection.class)) {
+ return MaybeMessageType.COLLECTION;
+ } else if (typeUtils.isSameType(parameterType, Map.class)) {
+ return MaybeMessageType.MAP;
+ } else if (typeUtils.isSubType(parameterType, NetworkMessage.class)) {
+ return MaybeMessageType.MESSAGE;
+ }
+ }
+ return null;
+ }
+
+ private void generatePrepareMarshal(TypeSpec.Builder messageImplBuild, MessageClass message) {
Review comment:
This method is very long. Is it possible to extract some parts out of it? For example, switch cases seem to be naturally extractable.
##########
File path: modules/network/src/integrationTest/java/org/apache/ignite/internal/network/processor/ItTransferableObjectProcessorTest.java
##########
@@ -250,9 +250,9 @@ private Compilation compile(String messageSource) {
*/
private static List<JavaFileObject> sources(String... sources) {
return Arrays.stream(sources)
- .map(source -> RESOURCE_PACKAGE_NAME.replace('.', '/') + '/' + source + ".java")
- .map(JavaFileObjects::forResource)
- .collect(Collectors.toList());
+ .map(source -> RESOURCE_PACKAGE_NAME.replace('.', '/') + '/' + source + ".java")
Review comment:
Continuation is 8 characters, IIRC
##########
File path: modules/network-annotation-processor/src/main/java/org/apache/ignite/internal/network/processor/messages/MessageImplGenerator.java
##########
@@ -95,6 +111,23 @@ public TypeSpec generateMessageImpl(MessageClass message, TypeSpec builderInterf
.build();
getterImpls.add(getterImpl);
+
+ if (getter.getAnnotation(Marshallable.class) != null) {
+ ArrayTypeName arrayTypeName = ArrayTypeName.of(TypeName.BYTE);
+ String name = getterName + "ByteArray";
Review comment:
How about extracting this `getterName + "ByteArray"` expression as a static method to some util class? It is encountered a few times in at least 2 classes.
##########
File path: modules/network-annotation-processor/src/main/java/org/apache/ignite/internal/network/processor/messages/MessageImplGenerator.java
##########
@@ -156,13 +189,265 @@ public TypeSpec generateMessageImpl(MessageClass message, TypeSpec builderInterf
messageImpl.addMethod(builderMethod);
+ generatePrepareMarshal(messageImpl, message);
+ generateUnmarshalMethod(messageImpl, message);
+
messageImpl
.addOriginatingElement(message.element())
.addOriginatingElement(messageGroup.element());
return messageImpl.build();
}
+ /**
+ * Resolves type of an object to a type that may hold a message. Returns {@code null} if the type
+ * can't hold a message.
+ *
+ * @param parameterType Type.
+ * @return {@link MaybeMessageType} or {@code null} if the type can't hold a message.
+ */
+ @Nullable
+ private MessageImplGenerator.MaybeMessageType resolveType(TypeMirror parameterType) {
Review comment:
It is compile-time code, so we don't need it to spare each and every instance on heap. So my suggestion is to use `Optional` instead of `Nullable` here.
##########
File path: modules/network-annotation-processor/src/main/java/org/apache/ignite/internal/network/processor/messages/MessageImplGenerator.java
##########
@@ -156,13 +189,265 @@ public TypeSpec generateMessageImpl(MessageClass message, TypeSpec builderInterf
messageImpl.addMethod(builderMethod);
+ generatePrepareMarshal(messageImpl, message);
+ generateUnmarshalMethod(messageImpl, message);
+
messageImpl
.addOriginatingElement(message.element())
.addOriginatingElement(messageGroup.element());
return messageImpl.build();
}
+ /**
+ * Resolves type of an object to a type that may hold a message. Returns {@code null} if the type
+ * can't hold a message.
+ *
+ * @param parameterType Type.
+ * @return {@link MaybeMessageType} or {@code null} if the type can't hold a message.
+ */
+ @Nullable
+ private MessageImplGenerator.MaybeMessageType resolveType(TypeMirror parameterType) {
+ if (parameterType.getKind() == TypeKind.ARRAY) {
+ if (!((ArrayType) parameterType).getComponentType().getKind().isPrimitive()) {
+ return MaybeMessageType.OBJECT_ARRAY;
+ }
+ } else if (parameterType.getKind() == TypeKind.DECLARED) {
+ if (typeUtils.isSameType(parameterType, Collection.class)) {
+ return MaybeMessageType.COLLECTION;
+ } else if (typeUtils.isSameType(parameterType, Map.class)) {
+ return MaybeMessageType.MAP;
+ } else if (typeUtils.isSubType(parameterType, NetworkMessage.class)) {
+ return MaybeMessageType.MESSAGE;
+ }
+ }
+ return null;
+ }
+
+ private void generatePrepareMarshal(TypeSpec.Builder messageImplBuild, MessageClass message) {
+ boolean isNeeded = false;
+
+ ParameterizedTypeName setType = ParameterizedTypeName.get(ClassName.get(Set.class), TypeName.INT.box());
+
+ Builder prepareMarshal = MethodSpec.methodBuilder("prepareMarshal")
+ .addAnnotation(Override.class)
+ .addModifiers(Modifier.PUBLIC)
+ .addException(Exception.class)
+ .addParameter(setType, "set")
Review comment:
Set of what? How about renaming the parameter to make it clear what is contained in this set? Like `usedDescriptorIds`?
##########
File path: modules/network/src/main/java/org/apache/ignite/internal/network/netty/InNetworkObject.java
##########
@@ -0,0 +1,70 @@
+/*
+ * 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.netty;
+
+import org.apache.ignite.internal.network.serialization.CompositeDescriptorRegistry;
+import org.apache.ignite.network.NetworkMessage;
+import org.apache.ignite.network.annotations.Marshallable;
+
+/**
+ * Wrapper for the received network message.
+ */
+public class InNetworkObject {
+ /** Message. */
+ private final NetworkMessage message;
+
+ /** Sender's consistent id. */
+ private final String consistentId;
+
+ /** DescriptorRegistry that will be used for the deserialization of the message's {@link Marshallable} fields. */
+ private final CompositeDescriptorRegistry registry;
Review comment:
Does it need to know the exact type? How about changing it to just `DescriptorRegistry`?
##########
File path: modules/network-annotation-processor/src/main/java/org/apache/ignite/internal/network/processor/messages/MessageImplGenerator.java
##########
@@ -156,13 +189,265 @@ public TypeSpec generateMessageImpl(MessageClass message, TypeSpec builderInterf
messageImpl.addMethod(builderMethod);
+ generatePrepareMarshal(messageImpl, message);
+ generateUnmarshalMethod(messageImpl, message);
+
messageImpl
.addOriginatingElement(message.element())
.addOriginatingElement(messageGroup.element());
return messageImpl.build();
}
+ /**
+ * Resolves type of an object to a type that may hold a message. Returns {@code null} if the type
+ * can't hold a message.
+ *
+ * @param parameterType Type.
+ * @return {@link MaybeMessageType} or {@code null} if the type can't hold a message.
+ */
+ @Nullable
+ private MessageImplGenerator.MaybeMessageType resolveType(TypeMirror parameterType) {
+ if (parameterType.getKind() == TypeKind.ARRAY) {
+ if (!((ArrayType) parameterType).getComponentType().getKind().isPrimitive()) {
+ return MaybeMessageType.OBJECT_ARRAY;
+ }
+ } else if (parameterType.getKind() == TypeKind.DECLARED) {
+ if (typeUtils.isSameType(parameterType, Collection.class)) {
+ return MaybeMessageType.COLLECTION;
+ } else if (typeUtils.isSameType(parameterType, Map.class)) {
+ return MaybeMessageType.MAP;
+ } else if (typeUtils.isSubType(parameterType, NetworkMessage.class)) {
+ return MaybeMessageType.MESSAGE;
+ }
+ }
+ return null;
+ }
+
+ private void generatePrepareMarshal(TypeSpec.Builder messageImplBuild, MessageClass message) {
+ boolean isNeeded = false;
+
+ ParameterizedTypeName setType = ParameterizedTypeName.get(ClassName.get(Set.class), TypeName.INT.box());
Review comment:
It seems that this should be `IntSet`, not `Set<Integer>`, otherwise we lose primitive set advantages.
##########
File path: modules/network/src/main/java/org/apache/ignite/internal/network/netty/OutboundEncoder.java
##########
@@ -74,10 +87,29 @@ protected void encode(ChannelHandlerContext ctx, NetworkMessage msg, List<Object
* @param serializationService Serialization service.
*/
private NetworkMessageChunkedInput(
- NetworkMessage msg,
+ OutNetworkObject object,
Review comment:
I suggest renaming this parameter as `object` most of the time has type `Object`, but here it's not the case. How about `networkObject` or `outObject`?
##########
File path: modules/network/src/main/java/org/apache/ignite/network/NettyBootstrapFactory.java
##########
@@ -139,6 +139,27 @@ public void start() {
clientWorkerGroup = NamedNioEventLoopGroup.create(eventLoopGroupNamePrefix + "-client");
}
+ /**
+ * Returns {@code true} if the current thread is a network thread, {@code false} otherwise.
+ *
+ * @return {@code true} if the current thread is a network thread, {@code false} otherwise.
+ */
+ public static boolean isInNetworkThread() {
+ String name = Thread.currentThread().getName();
+
+ if (name.contains("-srv-worker")) {
Review comment:
It seems a bit scary to detect threads by name. It is possible to attach some thread-local flag to these threads and check using this flag?
--
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 merged pull request #705: IGNITE-16393 Move network's user object serialization to the user thread
Posted by GitBox <gi...@apache.org>.
SammyVimes merged pull request #705:
URL: https://github.com/apache/ignite-3/pull/705
--
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