You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@fury.apache.org by "david1437 (via GitHub)" <gi...@apache.org> on 2025/05/11 19:57:50 UTC

[PR] feat: Align object array to collection serialization protocol v2 [fury]

david1437 opened a new pull request, #2218:
URL: https://github.com/apache/fury/pull/2218

   <!--
   **Thanks for contributing to Fury.**
   
   **If this is your first time opening a PR on fury, you can refer to [CONTRIBUTING.md](https://github.com/apache/fury/blob/main/CONTRIBUTING.md).**
   
   Contribution Checklist
   
       - The **Apache Fury (incubating)** community has restrictions on the naming of pr titles. You can also find instructions in [CONTRIBUTING.md](https://github.com/apache/fury/blob/main/CONTRIBUTING.md).
   
       - Fury has a strong focus on performance. If the PR you submit will have an impact on performance, please benchmark it first and provide the benchmark result here.
   -->
   
   ## What does this PR do?
   
   <!-- Describe the purpose of this PR. -->
   
   ## Related issues
   
   - #1229
   
   ## Does this PR introduce any user-facing change?
   
   <!--
   If any user-facing interface changes, please [open an issue](https://github.com/apache/fury/issues/new/choose) describing the need to do so and update the document if necessary.
   -->
   
   - [ ] Does this PR introduce any public API change?
   - [ ] Does this PR introduce any binary protocol compatibility change?
   
   ## Benchmark
   
   <!--
   When the PR has an impact on performance (if you don't know whether the PR will have an impact on performance, you can submit the PR first, and if it will have impact on performance, the code reviewer will explain it), be sure to attach a benchmark data here.
   -->
   


-- 
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: commits-unsubscribe@fury.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org
For additional commands, e-mail: commits-help@fury.apache.org


Re: [PR] feat: Align object array to collection serialization protocol v2 [fury]

Posted by "david1437 (via GitHub)" <gi...@apache.org>.
david1437 commented on code in PR #2218:
URL: https://github.com/apache/fury/pull/2218#discussion_r2083607277


##########
java/fury-core/src/main/java/org/apache/fury/serializer/ArraySerializers.java:
##########
@@ -208,6 +251,30 @@ private Object[] newArray(int numElements) {
       }
       return value;
     }
+
+    private GenericType buildCollectionGenericType(int dimensions) {

Review Comment:
   I am hoping there is a better way to resolve this dyamically rather then using switch? Any suggestions?



-- 
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: commits-unsubscribe@fury.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org
For additional commands, e-mail: commits-help@fury.apache.org


Re: [PR] feat: Align object array to collection serialization protocol v2 [fury]

Posted by "chaokunyang (via GitHub)" <gi...@apache.org>.
chaokunyang commented on code in PR #2218:
URL: https://github.com/apache/fury/pull/2218#discussion_r2084456086


##########
java/fury-core/src/main/java/org/apache/fury/serializer/ArraySerializers.java:
##########
@@ -69,12 +76,23 @@ public ObjectArraySerializer(Fury fury, Class<T[]> cls) {
       if (fury.getClassResolver().isMonomorphic(componentType)) {
         if (fury.isCrossLanguage()) {
           this.componentTypeSerializer = null;
+          this.collectionSerializer = null;
+          this.collectionGenericType = null;
         } else {
           this.componentTypeSerializer = fury.getClassResolver().getSerializer(componentType);
+          if (dimension > 1) {
+            this.collectionSerializer = new FuryArrayAsListSerializer(fury);
+            this.collectionGenericType = buildCollectionGenericType(dimension);
+          } else {
+            this.collectionSerializer = null;

Review Comment:
   That is only for primitive array. Other array still use Collection serialization protocol. `List<String>` and `String[]` should have same layout



-- 
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: commits-unsubscribe@fury.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org
For additional commands, e-mail: commits-help@fury.apache.org


Re: [PR] feat: Align object array to collection serialization protocol v2 [fury]

Posted by "chaokunyang (via GitHub)" <gi...@apache.org>.
chaokunyang commented on code in PR #2218:
URL: https://github.com/apache/fury/pull/2218#discussion_r2084042048


##########
java/fury-core/src/main/java/org/apache/fury/serializer/ArraySerializers.java:
##########
@@ -208,6 +251,30 @@ private Object[] newArray(int numElements) {
       }
       return value;
     }
+
+    private GenericType buildCollectionGenericType(int dimensions) {

Review Comment:
   It's like:
   ```java
   
    Tuple2<Class<?>, Integer> info = TypeUtils.getArrayComponentInfo(declaredClass);
         List<TypeRef<?>> typeRefs = new ArrayList<>(info.f1 + 1);
         typeRefs.add(collectionTypeRef);
         for (int i = 0; i < info.f1; i++) {
           typeRefs.add(TypeUtils.getElementType(typeRefs.get(i)));
         }
         Collections.reverse(typeRefs);
         for (int i = 1; i < typeRefs.size(); i++) {
           TypeRef<?> arrayType = typeRefs.get(i - 1);
           TypeRef<?> typeRef =
               TypeRef.of(
                   Array.newInstance(arrayType.getRawType(), 1).getClass(),
                   typeRefs.get(i).getExtInfo());
           typeRefs.set(i, typeRef);
         }
   ```
   
   See more details in `ClassDef#buildFieldType`



-- 
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: commits-unsubscribe@fury.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org
For additional commands, e-mail: commits-help@fury.apache.org


Re: [PR] feat: Align object array to collection serialization protocol v2 [fury]

Posted by "david1437 (via GitHub)" <gi...@apache.org>.
david1437 commented on code in PR #2218:
URL: https://github.com/apache/fury/pull/2218#discussion_r2084451177


##########
java/fury-core/src/main/java/org/apache/fury/serializer/ArraySerializers.java:
##########
@@ -69,12 +76,23 @@ public ObjectArraySerializer(Fury fury, Class<T[]> cls) {
       if (fury.getClassResolver().isMonomorphic(componentType)) {
         if (fury.isCrossLanguage()) {
           this.componentTypeSerializer = null;
+          this.collectionSerializer = null;
+          this.collectionGenericType = null;
         } else {
           this.componentTypeSerializer = fury.getClassResolver().getSerializer(componentType);
+          if (dimension > 1) {
+            this.collectionSerializer = new FuryArrayAsListSerializer(fury);
+            this.collectionGenericType = buildCollectionGenericType(dimension);
+          } else {
+            this.collectionSerializer = null;

Review Comment:
   I thought for 1D / Non nested generics that we intended to use the normal default serialization path that was in place before?



-- 
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: commits-unsubscribe@fury.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org
For additional commands, e-mail: commits-help@fury.apache.org


Re: [PR] feat: Align object array to collection serialization protocol v2 [fury]

Posted by "david1437 (via GitHub)" <gi...@apache.org>.
david1437 commented on code in PR #2218:
URL: https://github.com/apache/fury/pull/2218#discussion_r2084486090


##########
java/fury-core/src/main/java/org/apache/fury/serializer/ArraySerializers.java:
##########
@@ -69,12 +76,23 @@ public ObjectArraySerializer(Fury fury, Class<T[]> cls) {
       if (fury.getClassResolver().isMonomorphic(componentType)) {
         if (fury.isCrossLanguage()) {
           this.componentTypeSerializer = null;
+          this.collectionSerializer = null;
+          this.collectionGenericType = null;
         } else {
           this.componentTypeSerializer = fury.getClassResolver().getSerializer(componentType);
+          if (dimension > 1) {
+            this.collectionSerializer = new FuryArrayAsListSerializer(fury);
+            this.collectionGenericType = buildCollectionGenericType(dimension);
+          } else {
+            this.collectionSerializer = null;

Review Comment:
   Ok I  am under impression T[] can never have primitives only wrapper primitive classes should those use predvious method as well ie. Double[] ?



##########
java/fury-core/src/main/java/org/apache/fury/serializer/ArraySerializers.java:
##########
@@ -69,12 +76,23 @@ public ObjectArraySerializer(Fury fury, Class<T[]> cls) {
       if (fury.getClassResolver().isMonomorphic(componentType)) {
         if (fury.isCrossLanguage()) {
           this.componentTypeSerializer = null;
+          this.collectionSerializer = null;
+          this.collectionGenericType = null;
         } else {
           this.componentTypeSerializer = fury.getClassResolver().getSerializer(componentType);
+          if (dimension > 1) {
+            this.collectionSerializer = new FuryArrayAsListSerializer(fury);
+            this.collectionGenericType = buildCollectionGenericType(dimension);
+          } else {
+            this.collectionSerializer = null;

Review Comment:
   Ok I  am under impression T[] can never have primitives only wrapper primitive classes should those use previous method as well ie. Double[] ?



-- 
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: commits-unsubscribe@fury.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org
For additional commands, e-mail: commits-help@fury.apache.org


Re: [PR] feat: Align object array to collection serialization protocol v2 [fury]

Posted by "chaokunyang (via GitHub)" <gi...@apache.org>.
chaokunyang commented on code in PR #2218:
URL: https://github.com/apache/fury/pull/2218#discussion_r2083711220


##########
java/fury-core/src/main/java/org/apache/fury/serializer/ArraySerializers.java:
##########
@@ -208,6 +251,30 @@ private Object[] newArray(int numElements) {
       }
       return value;
     }
+
+    private GenericType buildCollectionGenericType(int dimensions) {

Review Comment:
   I think we can write using a loop, and using `TypeRef.of` to build generic type



-- 
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: commits-unsubscribe@fury.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org
For additional commands, e-mail: commits-help@fury.apache.org


Re: [PR] feat: Align object array to collection serialization protocol v2 [fury]

Posted by "david1437 (via GitHub)" <gi...@apache.org>.
david1437 commented on PR #2218:
URL: https://github.com/apache/fury/pull/2218#issuecomment-2887689751

   After resolving the issues around primitives and depth increment it seems only a couple tests are failing, Would appreciate some help on those specific failing cases: JdkProxySerializerTest and NonexistentClassSerializersTest


-- 
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: commits-unsubscribe@fury.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org
For additional commands, e-mail: commits-help@fury.apache.org


Re: [PR] feat: Align object array to collection serialization protocol v2 [fury]

Posted by "david1437 (via GitHub)" <gi...@apache.org>.
david1437 commented on code in PR #2218:
URL: https://github.com/apache/fury/pull/2218#discussion_r2084242117


##########
java/fury-core/src/main/java/org/apache/fury/serializer/ArraySerializers.java:
##########
@@ -208,6 +251,30 @@ private Object[] newArray(int numElements) {
       }
       return value;
     }
+
+    private GenericType buildCollectionGenericType(int dimensions) {

Review Comment:
   Apologies if it is a dumb question, but where is collectionTypeRef coming from in this case? I was thinking it might be TypeUtils.COLLECTION_TYPE but even in this case its adding one element for looping over multiple dimensions so would still fail



-- 
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: commits-unsubscribe@fury.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org
For additional commands, e-mail: commits-help@fury.apache.org


Re: [PR] feat: Align object array to collection serialization protocol v2 [fury]

Posted by "chaokunyang (via GitHub)" <gi...@apache.org>.
chaokunyang commented on code in PR #2218:
URL: https://github.com/apache/fury/pull/2218#discussion_r2084413190


##########
java/fury-core/src/main/java/org/apache/fury/serializer/ArraySerializers.java:
##########
@@ -85,52 +103,69 @@ public void write(MemoryBuffer buffer, T[] arr) {
       int len = arr.length;
       RefResolver refResolver = fury.getRefResolver();
       Serializer componentSerializer = this.componentTypeSerializer;
-      int header = componentSerializer != null ? 0b1 : 0b0;
-      buffer.writeVarUint32Small7(len << 1 | header);
-      if (componentSerializer != null) {
-        for (T t : arr) {
-          if (!refResolver.writeRefOrNull(buffer, t)) {
-            componentSerializer.write(buffer, t);
-          }
-        }
+      if (this.collectionSerializer != null) {
+        fury.getGenerics().pushGenericType(this.collectionGenericType);
+        ArrayAsList list = new ArrayAsList(0);

Review Comment:
   Could we cache this object as a field.  Nested serialization is a rare case, if we cache it as a field, we could save object creation cost. 
   
   We could write like :
   ```java
   ArrayAsList list = this.list;
   if (list == null) {
     list = new ArrayAsList(0);
   } else {
     this.list = null
   }
   
   // before serialiazation
   
   ...
   
   
   // after serialization
   
   this.list = list
   
   ```



-- 
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: commits-unsubscribe@fury.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org
For additional commands, e-mail: commits-help@fury.apache.org


Re: [PR] feat: Align object array to collection serialization protocol v2 [fury]

Posted by "chaokunyang (via GitHub)" <gi...@apache.org>.
chaokunyang commented on code in PR #2218:
URL: https://github.com/apache/fury/pull/2218#discussion_r2084409816


##########
java/fury-core/src/main/java/org/apache/fury/serializer/ArraySerializers.java:
##########
@@ -85,52 +103,69 @@ public void write(MemoryBuffer buffer, T[] arr) {
       int len = arr.length;
       RefResolver refResolver = fury.getRefResolver();
       Serializer componentSerializer = this.componentTypeSerializer;
-      int header = componentSerializer != null ? 0b1 : 0b0;
-      buffer.writeVarUint32Small7(len << 1 | header);
-      if (componentSerializer != null) {
-        for (T t : arr) {
-          if (!refResolver.writeRefOrNull(buffer, t)) {
-            componentSerializer.write(buffer, t);
-          }
-        }
+      if (this.collectionSerializer != null) {
+        fury.getGenerics().pushGenericType(this.collectionGenericType);
+        ArrayAsList list = new ArrayAsList(0);
+        list.setArray(arr);
+        this.collectionSerializer.write(buffer, list);

Review Comment:
   We need to invoke `fury.incDepth` before  and after invoking `collectionSerializer.write`



-- 
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: commits-unsubscribe@fury.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org
For additional commands, e-mail: commits-help@fury.apache.org


Re: [PR] feat: Align object array to collection serialization protocol v2 [fury]

Posted by "chaokunyang (via GitHub)" <gi...@apache.org>.
chaokunyang commented on code in PR #2218:
URL: https://github.com/apache/fury/pull/2218#discussion_r2084294197


##########
java/fury-core/src/main/java/org/apache/fury/serializer/ArraySerializers.java:
##########
@@ -208,6 +251,30 @@ private Object[] newArray(int numElements) {
       }
       return value;
     }
+
+    private GenericType buildCollectionGenericType(int dimensions) {

Review Comment:
   My bad, in your case, I think you can write like:
   ```java
   TypeRef arrayTypeRef = TypeRef.of(componentClass);
   for (int i = 0; i < dims -1; i++ ) {
     arrayTypeRef = collectionOf(arrayTypeRef);
   }
   ```



-- 
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: commits-unsubscribe@fury.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org
For additional commands, e-mail: commits-help@fury.apache.org


Re: [PR] feat: Align object array to collection serialization protocol v2 [fory]

Posted by "chaokunyang (via GitHub)" <gi...@apache.org>.
chaokunyang closed pull request #2218: feat:  Align object array to collection serialization protocol v2
URL: https://github.com/apache/fory/pull/2218


-- 
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: commits-unsubscribe@fory.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fory.apache.org
For additional commands, e-mail: commits-help@fory.apache.org


Re: [PR] feat: Align object array to collection serialization protocol v2 [fury]

Posted by "chaokunyang (via GitHub)" <gi...@apache.org>.
chaokunyang commented on code in PR #2218:
URL: https://github.com/apache/fury/pull/2218#discussion_r2084517993


##########
java/fury-core/src/main/java/org/apache/fury/serializer/ArraySerializers.java:
##########
@@ -85,52 +103,69 @@ public void write(MemoryBuffer buffer, T[] arr) {
       int len = arr.length;
       RefResolver refResolver = fury.getRefResolver();
       Serializer componentSerializer = this.componentTypeSerializer;
-      int header = componentSerializer != null ? 0b1 : 0b0;
-      buffer.writeVarUint32Small7(len << 1 | header);
-      if (componentSerializer != null) {
-        for (T t : arr) {
-          if (!refResolver.writeRefOrNull(buffer, t)) {
-            componentSerializer.write(buffer, t);
-          }
-        }
+      if (this.collectionSerializer != null) {
+        fury.getGenerics().pushGenericType(this.collectionGenericType);
+        ArrayAsList list = new ArrayAsList(0);
+        list.setArray(arr);
+        this.collectionSerializer.write(buffer, list);

Review Comment:
   incDepth(-1) should be invoked after serialization
   
   



-- 
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: commits-unsubscribe@fury.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org
For additional commands, e-mail: commits-help@fury.apache.org


Re: [PR] feat: Align object array to collection serialization protocol v2 [fury]

Posted by "chaokunyang (via GitHub)" <gi...@apache.org>.
chaokunyang commented on code in PR #2218:
URL: https://github.com/apache/fury/pull/2218#discussion_r2084416748


##########
java/fury-core/src/main/java/org/apache/fury/serializer/ArraySerializers.java:
##########
@@ -69,12 +76,23 @@ public ObjectArraySerializer(Fury fury, Class<T[]> cls) {
       if (fury.getClassResolver().isMonomorphic(componentType)) {
         if (fury.isCrossLanguage()) {
           this.componentTypeSerializer = null;
+          this.collectionSerializer = null;
+          this.collectionGenericType = null;
         } else {
           this.componentTypeSerializer = fury.getClassResolver().getSerializer(componentType);
+          if (dimension > 1) {
+            this.collectionSerializer = new FuryArrayAsListSerializer(fury);
+            this.collectionGenericType = buildCollectionGenericType(dimension);
+          } else {
+            this.collectionSerializer = null;

Review Comment:
   Why it's null for 1d object array?



-- 
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: commits-unsubscribe@fury.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org
For additional commands, e-mail: commits-help@fury.apache.org


Re: [PR] feat: Align object array to collection serialization protocol v2 [fury]

Posted by "david1437 (via GitHub)" <gi...@apache.org>.
david1437 commented on code in PR #2218:
URL: https://github.com/apache/fury/pull/2218#discussion_r2093682673


##########
java/fury-core/src/main/java/org/apache/fury/serializer/ArraySerializers.java:
##########
@@ -147,39 +191,49 @@ public void xwrite(MemoryBuffer buffer, T[] arr) {
 
     @Override
     public T[] read(MemoryBuffer buffer) {
+      Object[] value;
       int numElements = buffer.readVarUint32Small7();
-      boolean isFinal = (numElements & 0b1) != 0;
-      numElements >>>= 1;
-      Object[] value = newArray(numElements);
-      RefResolver refResolver = fury.getRefResolver();
-      refResolver.reference(value);
-      if (isFinal) {
-        final Serializer componentTypeSerializer = this.componentTypeSerializer;
-        for (int i = 0; i < numElements; i++) {
-          Object elem;
-          int nextReadRefId = refResolver.tryPreserveRefId(buffer);
-          if (nextReadRefId >= Fury.NOT_NULL_VALUE_FLAG) {
-            elem = componentTypeSerializer.read(buffer);
-            refResolver.setReadObject(nextReadRefId, elem);
-          } else {
-            elem = refResolver.getReadObject();
-          }
-          value[i] = elem;
+      if (this.collectionSerializer != null) {
+        fury.getGenerics().pushGenericType(this.collectionGenericType);
+        value = this.collectionSerializer.read(buffer).toArray();
+        fury.getGenerics().popGenericType();
+        if (this.innerType.isPrimitive() || TypeUtils.isBoxed(this.innerType)) {
+          return Arrays.copyOf(value, value.length, this.type);

Review Comment:
   Considering this does a copy it does not feel good is there a better approach for handling this? without this i get a class cast exception?



-- 
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: commits-unsubscribe@fury.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org
For additional commands, e-mail: commits-help@fury.apache.org


Re: [PR] feat: Align object array to collection serialization protocol v2 [fury]

Posted by "david1437 (via GitHub)" <gi...@apache.org>.
david1437 commented on code in PR #2218:
URL: https://github.com/apache/fury/pull/2218#discussion_r2084490997


##########
java/fury-core/src/main/java/org/apache/fury/serializer/ArraySerializers.java:
##########
@@ -85,52 +103,69 @@ public void write(MemoryBuffer buffer, T[] arr) {
       int len = arr.length;
       RefResolver refResolver = fury.getRefResolver();
       Serializer componentSerializer = this.componentTypeSerializer;
-      int header = componentSerializer != null ? 0b1 : 0b0;
-      buffer.writeVarUint32Small7(len << 1 | header);
-      if (componentSerializer != null) {
-        for (T t : arr) {
-          if (!refResolver.writeRefOrNull(buffer, t)) {
-            componentSerializer.write(buffer, t);
-          }
-        }
+      if (this.collectionSerializer != null) {
+        fury.getGenerics().pushGenericType(this.collectionGenericType);
+        ArrayAsList list = new ArrayAsList(0);
+        list.setArray(arr);
+        this.collectionSerializer.write(buffer, list);

Review Comment:
   I tried with 
   
   incDepth(1)
   ...
   incDepth(1)
   
   Should I be using -1 after serialization to reset the depth?



-- 
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: commits-unsubscribe@fury.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org
For additional commands, e-mail: commits-help@fury.apache.org


Re: [PR] feat: Align object array to collection serialization protocol v2 [fury]

Posted by "david1437 (via GitHub)" <gi...@apache.org>.
david1437 commented on PR #2218:
URL: https://github.com/apache/fury/pull/2218#issuecomment-2886119931

   So I have been running the ArraySerializerTests and they have all been passing so a bit confused on why the CICD tests are failing


-- 
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: commits-unsubscribe@fury.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org
For additional commands, e-mail: commits-help@fury.apache.org


Re: [PR] feat: Align object array to collection serialization protocol v2 [fury]

Posted by "david1437 (via GitHub)" <gi...@apache.org>.
david1437 commented on PR #2218:
URL: https://github.com/apache/fury/pull/2218#issuecomment-2871952378

   Some CI tests are failing I am assuming its related to my implementation of the write / copy / read methods? Possibly not using try finally for push / pop generic types?


-- 
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: commits-unsubscribe@fury.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org
For additional commands, e-mail: commits-help@fury.apache.org