chaokunyang commented on code in PR #1616:
URL: https://github.com/apache/incubator-fury/pull/1616#discussion_r1596955939
##########
java/fury-core/src/main/java/org/apache/fury/serializer/collection/CollectionSerializers.java:
##########
@@ -433,28 +436,18 @@ public SetFromMapSerializer(Fury fury, Class<Set<?>>
type) {
}
@Override
- public Collection newCollection(MemoryBuffer buffer) {
- int numElements = buffer.readVarUint32Small7();
- setNumElements(numElements);
- final ClassInfo mapClassInfo =
fury.getClassResolver().readClassInfo(buffer);
- final MethodHandle methodHandle =
ReflectionUtils.getCtrHandle(mapClassInfo.getCls(), true);
- Map map;
- try {
- map = (Map) methodHandle.invoke();
- } catch (Throwable e) {
- throw new RuntimeException(e);
- }
- final Set set = Collections.newSetFromMap(map);
- fury.getRefResolver().reference(set);
- return set;
+ public void write(MemoryBuffer buffer, Set<?> value) {
+ Object fieldValue = Platform.getObject(value, MAP_FIELD_OFFSET);
+ fury.writeRef(buffer, fieldValue);
}
@Override
- public Collection onCollectionWrite(MemoryBuffer buffer, Set<?> value) {
- buffer.writeVarUint32Small7(value.size());
- final Map<?, Boolean> map = (Map<?, Boolean>) Platform.getObject(value,
MAP_FIELD_OFFSET);
- fury.getClassResolver().writeClassAndUpdateCache(buffer, map.getClass());
- return value;
+ public Set<?> read(MemoryBuffer buffer) {
+ final Map map = (Map) fury.readRef(buffer);
+ final Set set = Collections.newSetFromMap(Collections.emptyMap());
Review Comment:
I see, thanks. If only allow users to create set first, then add values.
If so, the serialization should be simplified.
Here is the serialization code:
```java
public boolean add(E e) { return m.put(e, Boolean.TRUE) == null; }
```
All map values should not be serialized. And map will be empty, so there is
not shared reference for this map.
Your previous implementation seems better, only need to set codegen to true.
So fury will generate serialization code for this set. In `onCollectonWrite`,
you write map class, and `newCollection`, you read class and create map from
it. You can get serializer for that map, and it must be a
AbstractMapSerializer, then you can use ` AbstractMapSerializer#newMap` to
create the empty map, and build the set from it.
Current implementation is OK for me too. The default implementation in Fury
will serializer whole map too.
We can merge this PR first, and create another PR to optimize it if you like
.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
