fix(java): support TreeSet/TreeMap subclasses without Comparator constructor in SortedSet/SortedMapSerializer#3344
Merged
chaokunyang merged 2 commits intoapache:mainfrom Feb 17, 2026
Conversation
mandrean
force-pushed
the
treeset-subclass-comparator
branch
from
db9b3e4 to
95a5eed
Compare
chaokunyang
reviewed
Feb 17, 2026
java/fory-core/src/main/java/org/apache/fory/serializer/collection/CollectionSerializers.java
Outdated
Show resolved
Hide resolved
Collaborator
|
It seems that the comparator is null if the treeset subclass don't pass comparator to parent class. In such cases, treeset Or treemap will just use key compareTo api for sort. So it's not a bug, could you remove the log and keep constructor branch. And could you ask check TreeMap? I think it has similar issue |
… TreeSet subclasses
… TreeMap subclasses
mandrean
force-pushed
the
treeset-subclass-comparator
branch
from
95a5eed to
ecf8f6d
Compare
mandrean
changed the title
Contributor
Author
|
Thanks, pushed some changes, have a look again @chaokunyang! |
This was referenced Mar 26, 2026
[Java] ObjectStreamSerializer async meta-shared layer JIT fails for TreeSet/TreeMap subclasses
#3515
Open
mandrean
added a commit
to mandrean/fory
that referenced
this pull request
Mar 26, 2026
…classes Adds optimized child-serializers for subclasses of TreeSet, ConcurrentSkipListSet, PriorityQueue, TreeMap, and ConcurrentSkipListMap. Previously these fell back to JDK-compatible serialization because the hierarchy walk hit writeObject/readObject on the parent JDK type. The new serializers handle comparator preservation, slot field serialization, and ref-tracking (including the deferred-ref pattern for concurrent variants). Constructor discovery is centralized in ContainerConstructors, which probes for all standard constructor shapes and gracefully falls through to natural ordering when no comparator-preserving constructor is available (per apache#3344 (comment)). Existing SortedSetSerializer, SortedMapSerializer, and PriorityQueueSerializer are refactored to reuse the same factory logic.
mandrean
added a commit
to mandrean/fory
that referenced
this pull request
Mar 26, 2026
Covers all constructor variants for TreeSet, ConcurrentSkipListSet, PriorityQueue, TreeMap, and ConcurrentSkipListMap child classes: - (Comparator), (SortedSet)/(SortedMap) preserve comparator - (), (Collection)/(Map), (int) use natural ordering - Unsupported constructors and custom-serialized children fall back - Async compilation path verified - InternalComparatorTreeSet verifies the graceful fall-through from apache#3344 (comment) Also adds (Collection) and (SortedSet)/(SortedMap) constructor tests for the existing SortedSetSerializer and SortedMapSerializer.
mandrean
added a commit
to mandrean/fory
that referenced
this pull request
Mar 26, 2026
…classes Adds optimized child-serializers for subclasses of TreeSet, ConcurrentSkipListSet, PriorityQueue, TreeMap, and ConcurrentSkipListMap. Previously these fell back to JDK-compatible serialization because the hierarchy walk hit writeObject/readObject on the parent JDK type. The new serializers handle comparator preservation, slot field serialization, and ref-tracking (including the deferred-ref pattern for concurrent variants). Constructor discovery is centralized in ContainerConstructors, which probes for all standard constructor shapes and gracefully falls through to natural ordering when no comparator-preserving constructor is available (per apache#3344 (comment)). Existing SortedSetSerializer, SortedMapSerializer, and PriorityQueueSerializer are refactored to reuse the same factory logic.
mandrean
added a commit
to mandrean/fory
that referenced
this pull request
Mar 26, 2026
Covers all constructor variants for TreeSet, ConcurrentSkipListSet, PriorityQueue, TreeMap, and ConcurrentSkipListMap child classes: - (Comparator), (SortedSet)/(SortedMap) preserve comparator - (), (Collection)/(Map), (int) use natural ordering - Unsupported constructors and custom-serialized children fall back - Async compilation path verified - InternalComparatorTreeSet verifies the graceful fall-through from apache#3344 (comment) Also adds (Collection) and (SortedSet)/(SortedMap) constructor tests for the existing SortedSetSerializer and SortedMapSerializer.
mandrean
added a commit
to mandrean/fory
that referenced
this pull request
Mar 26, 2026
Covers all constructor variants for TreeSet, ConcurrentSkipListSet, PriorityQueue, TreeMap, and ConcurrentSkipListMap child classes: - (Comparator), (SortedSet)/(SortedMap) preserve comparator - (), (Collection)/(Map), (int) use natural ordering - Unsupported constructors and custom-serialized children fall back - Async compilation path verified - InternalComparatorTreeSet verifies the graceful fall-through from apache#3344 (comment) Also adds (Collection) and (SortedSet)/(SortedMap) constructor tests for the existing SortedSetSerializer and SortedMapSerializer.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why?
SortedSetSerializerandSortedMapSerializerunconditionally require a(Comparator)constructor for allTreeSet/TreeMapsubclasses. Java constructors are not inherited, so aclass ChildTreeSet extends TreeSet<String>(orclass ChildTreeMap extends TreeMap<String, String>) with only a no-arg constructor throwsUnsupportedOperationExceptionwhen registered with the corresponding serializer.What does this PR do?
Changes both
SortedSetSerializerandSortedMapSerializerto try the(Comparator)constructor first, and fall back to the no-arg constructor if not found. At deserialization time, uses whichever constructor is available. The no-arg fallback uses natural ordering, which is the common case forTreeSet/TreeMapsubclasses.Only throws
UnsupportedOperationExceptionif neither constructor is found.Related issues
Does this PR introduce any user-facing change?
Does this PR introduce any public API change?
No. Existing behavior is preserved for classes that have a
(Comparator)constructor. Classes that previously threwUnsupportedOperationExceptionnow work.Does this PR introduce any binary protocol compatibility change?
No. The write path is unchanged (size + comparator ref), and the read path still reads the same bytes in the same order. Only the post-read instantiation strategy differs (which constructor is used). Wire format is identical in both directions.
Benchmark
Not applicable; the constructor lookup happens once at serializer construction time, not per serialization. The runtime paths (
newCollection/newMap) add a single null check (if (comparatorConstructor != null)) which is negligible.