You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@fory.apache.org by "gudzpoz (via GitHub)" <gi...@apache.org> on 2025/09/28 09:52:15 UTC

[PR] fix(java): retain serializers in graalvm native images [fory]

gudzpoz opened a new pull request, #2680:
URL: https://github.com/apache/fory/pull/2680

   ## Why?
   
   Custom serializers are not retained during GraalVM native image builds.
   
   ## What does this PR do?
   
   The original code checks against `initialClassInfos` and removes serializers outside the set during native image builds. This PR removes that as well as `initialClassInfos`.
   
   I don't know what `initialClassInfos` is for, because it is only involved in clearing serializers, which this PR is to remove.
   
   Also, since the tests before weren't retaining some serializers, it turns out the original `native-image.properties` might be incomplete. And I added more classes to `--initialize-at-build-time` as is hinted by the `native-image` command.
   
   ## Related issues
   
   - Fixes #2669 
   


-- 
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] fix(java): retain serializers in graalvm native images [fory]

Posted by "chaokunyang (via GitHub)" <gi...@apache.org>.
chaokunyang merged PR #2680:
URL: https://github.com/apache/fory/pull/2680


-- 
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] fix(java): retain serializers in graalvm native images [fory]

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


##########
java/fory-core/src/main/java/org/apache/fory/resolver/ClassResolver.java:
##########
@@ -1802,15 +1794,6 @@ public void ensureSerializersCompiled() {
               }
             }
           });
-      if (GraalvmSupport.isGraalBuildtime()) {
-        classInfoMap.forEach(
-            (cls, classInfo) -> {
-              if (classInfo.serializer != null
-                  && !extRegistry.initialClassInfos.contains(classInfo)) {
-                classInfo.serializer = null;

Review Comment:
   Actually we do need the clear, otherwise current Fory instance may hold some unexpected serializer. For example, `ObjectStreamSerializer` hold CompatibleSerializer serializer instead of generated by Fory.
   
   ```java
         if (GraalvmSupport.isGraalBuildtime()) {
           // trigger serializer constructor method handle generate.
           Serializers.newSerializer(fory, type, sc);
         }
         if (sc == CompatibleSerializer.class || GraalvmSupport.isGraalBuildtime()) {
           // skip init generated serializer at graalvm build time
           this.slotsSerializer = new CompatibleSerializer(fory, type, fieldResolver);
         } else {
           this.slotsSerializer = (CompatibleSerializerBase) Serializers.newSerializer(fory, type, sc);
         }
   ```



-- 
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] fix(java): retain serializers in graalvm native images [fory]

Posted by "chaokunyang (via GitHub)" <gi...@apache.org>.
chaokunyang commented on PR #2680:
URL: https://github.com/apache/fory/pull/2680#issuecomment-3343082085

   The `initialClassInfos` was used to record fory registered serializer for JDK types to avoid serializers for thoese types are cleared. But since we use GraalvmSerializerHolder to aviod instance of generated serializer, there won't be serializer instance at build time. So we can remove the clear logic.
   
   Nice Fix, thanks for this PR


-- 
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