You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@fory.apache.org by "LiangliangSui (via GitHub)" <gi...@apache.org> on 2025/12/05 02:41:03 UTC
[PR] fix(java): support serialization of CopyOnWriteArraySet [fory]
LiangliangSui opened a new pull request, #2999:
URL: https://github.com/apache/fory/pull/2999
<!--
**Thanks for contributing to Apache Fory™.**
**If this is your first time opening a PR on fory, you can refer to [CONTRIBUTING.md](https://github.com/apache/fory/blob/main/CONTRIBUTING.md).**
Contribution Checklist
- The **Apache Fory™** community has requirements on the naming of pr titles. You can also find instructions in [CONTRIBUTING.md](https://github.com/apache/fory/blob/main/CONTRIBUTING.md).
- Apache Fory™ 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.
-->
## Why?
<!-- Describe the purpose of this PR. -->
## What does this PR do?
<!-- Describe the details of this PR. -->
Support serialization of CopyOnWriteArraySet
## Related issues
<!--
Is there any related issue? If this PR closes them you say say fix/closes:
- #xxxx0
- #xxxx1
- Fixes #xxxx2
-->
https://github.com/apache/fory/issues/2918
## Does this PR introduce any user-facing change?
<!--
If any user-facing interface changes, please [open an issue](https://github.com/apache/fory/issues/new/choose) describing the need to do so and update the document if necessary.
Delete section if not applicable.
-->
- [ ] 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.
Delete section if not applicable.
-->
--
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): support serialization of CopyOnWriteArraySet [fory]
Posted by "LiangliangSui (via GitHub)" <gi...@apache.org>.
LiangliangSui commented on code in PR #2999:
URL: https://github.com/apache/fory/pull/2999#discussion_r2596934373
##########
java/fory-core/src/main/java/org/apache/fory/serializer/collection/CollectionLikeSerializer.java:
##########
@@ -550,6 +552,21 @@ public void copyElements(Collection originCollection, Object[] elements) {
}
}
+ public void copyElementsAddAll(Collection originCollection, Collection newCollection) {
Review Comment:
OK, let's move it to the CollectionSerializers.
--
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): support serialization of CopyOnWriteArraySet [fory]
Posted by "chaokunyang (via GitHub)" <gi...@apache.org>.
chaokunyang commented on code in PR #2999:
URL: https://github.com/apache/fory/pull/2999#discussion_r2596894250
##########
java/fory-core/src/main/java/org/apache/fory/serializer/collection/CollectionSerializer.java:
##########
@@ -61,7 +61,11 @@ public T copy(T originCollection) {
if (needToCopyRef) {
fory.reference(originCollection, newCollection);
}
- copyElements(originCollection, newCollection);
+ onCollectionCopy(originCollection, newCollection);
return (T) newCollection;
}
+
+ protected void onCollectionCopy(Collection originCollection, Collection newCollection) {
Review Comment:
And I don't think we need this API. why not just overrite `copy` API, it's basically same.
--
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): support serialization of CopyOnWriteArraySet [fory]
Posted by "chaokunyang (via GitHub)" <gi...@apache.org>.
chaokunyang commented on code in PR #2999:
URL: https://github.com/apache/fory/pull/2999#discussion_r2591309393
##########
java/fory-core/src/main/java/org/apache/fory/serializer/collection/CollectionSerializers.java:
##########
@@ -294,6 +295,27 @@ public CopyOnWriteArrayList onCollectionRead(Collection collection) {
}
}
+ public static class CopyOnWriteArraySetSerializer
+ extends ConcurrentCollectionSerializer<CopyOnWriteArraySet> {
+
+ public CopyOnWriteArraySetSerializer(Fory fory, Class<CopyOnWriteArraySet> type) {
+ super(fory, type, true);
+ }
+
+ @Override
+ public Collection newCollection(MemoryBuffer buffer) {
+ int numElements = buffer.readVarUint32Small7();
+ setNumElements(numElements);
+ return new CollectionContainer<>(numElements);
+ }
+
+ @Override
+ public CopyOnWriteArraySet onCollectionRead(Collection collection) {
Review Comment:
This is the behaviour of `CopyOnWriteArraySet`, and what it means `CopyOnWrite`
--
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): support serialization of CopyOnWriteArraySet [fory]
Posted by "chaokunyang (via GitHub)" <gi...@apache.org>.
chaokunyang commented on code in PR #2999:
URL: https://github.com/apache/fory/pull/2999#discussion_r2596923882
##########
java/fory-core/src/main/java/org/apache/fory/serializer/collection/CollectionSerializer.java:
##########
@@ -61,7 +61,11 @@ public T copy(T originCollection) {
if (needToCopyRef) {
fory.reference(originCollection, newCollection);
}
- copyElements(originCollection, newCollection);
+ onCollectionCopy(originCollection, newCollection);
return (T) newCollection;
}
+
+ protected void onCollectionCopy(Collection originCollection, Collection newCollection) {
Review Comment:
Abstraction is not always good, overwrite is more simple here, and your implement can't setup correct reference graph, which will an issue too if copy ref is enabled
--
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): support serialization of CopyOnWriteArraySet [fory]
Posted by "chaokunyang (via GitHub)" <gi...@apache.org>.
chaokunyang commented on code in PR #2999:
URL: https://github.com/apache/fory/pull/2999#discussion_r2591274707
##########
java/fory-core/src/main/java/org/apache/fory/serializer/collection/CollectionSerializers.java:
##########
@@ -294,6 +295,27 @@ public CopyOnWriteArrayList onCollectionRead(Collection collection) {
}
}
+ public static class CopyOnWriteArraySetSerializer
+ extends ConcurrentCollectionSerializer<CopyOnWriteArraySet> {
+
+ public CopyOnWriteArraySetSerializer(Fory fory, Class<CopyOnWriteArraySet> type) {
+ super(fory, type, true);
+ }
+
+ @Override
+ public Collection newCollection(MemoryBuffer buffer) {
+ int numElements = buffer.readVarUint32Small7();
+ setNumElements(numElements);
+ return new CollectionContainer<>(numElements);
+ }
+
+ @Override
+ public CopyOnWriteArraySet onCollectionRead(Collection collection) {
Review Comment:
This is what I am worried, you will copy `CopyOnWriteArraySet` every time you put an element into the Set when copying.
You should overide copy method to avoid such cost. It seems copy for other collections can be optimized too.
--
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): support serialization of CopyOnWriteArraySet [fory]
Posted by "LiangliangSui (via GitHub)" <gi...@apache.org>.
LiangliangSui commented on code in PR #2999:
URL: https://github.com/apache/fory/pull/2999#discussion_r2597207083
##########
java/fory-core/src/main/java/org/apache/fory/serializer/collection/CollectionSerializer.java:
##########
@@ -61,7 +61,11 @@ public T copy(T originCollection) {
if (needToCopyRef) {
fory.reference(originCollection, newCollection);
}
- copyElements(originCollection, newCollection);
+ onCollectionCopy(originCollection, newCollection);
return (T) newCollection;
}
+
+ protected void onCollectionCopy(Collection originCollection, Collection newCollection) {
Review Comment:
Good suggestion, done.
--
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): support serialization of CopyOnWriteArraySet [fory]
Posted by "LiangliangSui (via GitHub)" <gi...@apache.org>.
LiangliangSui commented on code in PR #2999:
URL: https://github.com/apache/fory/pull/2999#discussion_r2596906546
##########
java/fory-core/src/main/java/org/apache/fory/serializer/collection/CollectionLikeSerializer.java:
##########
@@ -550,6 +552,21 @@ public void copyElements(Collection originCollection, Object[] elements) {
}
}
+ public void copyElementsAddAll(Collection originCollection, Collection newCollection) {
Review Comment:
I think the naming needs to be revised.
--
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): support serialization of CopyOnWriteArraySet [fory]
Posted by "LiangliangSui (via GitHub)" <gi...@apache.org>.
LiangliangSui commented on code in PR #2999:
URL: https://github.com/apache/fory/pull/2999#discussion_r2596905205
##########
java/fory-core/src/main/java/org/apache/fory/serializer/collection/CollectionSerializer.java:
##########
@@ -61,7 +61,11 @@ public T copy(T originCollection) {
if (needToCopyRef) {
fory.reference(originCollection, newCollection);
}
- copyElements(originCollection, newCollection);
+ onCollectionCopy(originCollection, newCollection);
return (T) newCollection;
}
+
+ protected void onCollectionCopy(Collection originCollection, Collection newCollection) {
Review Comment:
In the `CollectionSerializer#copy` method, it is a framework. The specific logic for the copy operation is different. I don't think it's necessary to rewrite the entire logic all over again.
```java
public T copy(T originCollection) {
if (isImmutable()) {
return originCollection;
}
Preconditions.checkArgument(supportCodegenHook);
Collection newCollection = newCollection(originCollection);
if (needToCopyRef) {
fory.reference(originCollection, newCollection);
}
copyElements(originCollection, newCollection);
return (T) newCollection;
}
```
--
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): support serialization of CopyOnWriteArraySet [fory]
Posted by "chaokunyang (via GitHub)" <gi...@apache.org>.
chaokunyang commented on code in PR #2999:
URL: https://github.com/apache/fory/pull/2999#discussion_r2591250677
##########
java/fory-core/src/main/java/org/apache/fory/serializer/collection/CollectionSerializers.java:
##########
@@ -294,6 +295,27 @@ public CopyOnWriteArrayList onCollectionRead(Collection collection) {
}
}
+ public static class CopyOnWriteArraySetSerializer
+ extends ConcurrentCollectionSerializer<CopyOnWriteArraySet> {
+
+ public CopyOnWriteArraySetSerializer(Fory fory, Class<CopyOnWriteArraySet> type) {
+ super(fory, type, true);
+ }
+
+ @Override
+ public Collection newCollection(MemoryBuffer buffer) {
+ int numElements = buffer.readVarUint32Small7();
+ setNumElements(numElements);
+ return new CollectionContainer<>(numElements);
+ }
+
+ @Override
+ public CopyOnWriteArraySet onCollectionRead(Collection collection) {
Review Comment:
do you need to overwride `public List<?> copy(List<?> originCollection) {`?
##########
java/fory-core/src/main/java/org/apache/fory/serializer/collection/CollectionSerializers.java:
##########
@@ -294,6 +295,27 @@ public CopyOnWriteArrayList onCollectionRead(Collection collection) {
}
}
+ public static class CopyOnWriteArraySetSerializer
+ extends ConcurrentCollectionSerializer<CopyOnWriteArraySet> {
+
+ public CopyOnWriteArraySetSerializer(Fory fory, Class<CopyOnWriteArraySet> type) {
+ super(fory, type, true);
+ }
+
+ @Override
+ public Collection newCollection(MemoryBuffer buffer) {
+ int numElements = buffer.readVarUint32Small7();
+ setNumElements(numElements);
+ return new CollectionContainer<>(numElements);
+ }
+
+ @Override
+ public CopyOnWriteArraySet onCollectionRead(Collection collection) {
Review Comment:
do you need to overwride `public List<?> copy(List<?> originCollection)` too?
--
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): support serialization of CopyOnWriteArraySet [fory]
Posted by "LiangliangSui (via GitHub)" <gi...@apache.org>.
LiangliangSui commented on code in PR #2999:
URL: https://github.com/apache/fory/pull/2999#discussion_r2596906546
##########
java/fory-core/src/main/java/org/apache/fory/serializer/collection/CollectionLikeSerializer.java:
##########
@@ -550,6 +552,21 @@ public void copyElements(Collection originCollection, Object[] elements) {
}
}
+ public void copyElementsAddAll(Collection originCollection, Collection newCollection) {
Review Comment:
I think the naming needs to be revised. Do you have any good suggestions for modifying this name?
--
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): support serialization of CopyOnWriteArraySet [fory]
Posted by "chaokunyang (via GitHub)" <gi...@apache.org>.
chaokunyang merged PR #2999:
URL: https://github.com/apache/fory/pull/2999
--
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): support serialization of CopyOnWriteArraySet [fory]
Posted by "LiangliangSui (via GitHub)" <gi...@apache.org>.
LiangliangSui commented on code in PR #2999:
URL: https://github.com/apache/fory/pull/2999#discussion_r2591264926
##########
java/fory-core/src/main/java/org/apache/fory/serializer/collection/CollectionSerializers.java:
##########
@@ -294,6 +295,27 @@ public CopyOnWriteArrayList onCollectionRead(Collection collection) {
}
}
+ public static class CopyOnWriteArraySetSerializer
+ extends ConcurrentCollectionSerializer<CopyOnWriteArraySet> {
+
+ public CopyOnWriteArraySetSerializer(Fory fory, Class<CopyOnWriteArraySet> type) {
+ super(fory, type, true);
+ }
+
+ @Override
+ public Collection newCollection(MemoryBuffer buffer) {
+ int numElements = buffer.readVarUint32Small7();
+ setNumElements(numElements);
+ return new CollectionContainer<>(numElements);
+ }
+
+ @Override
+ public CopyOnWriteArraySet onCollectionRead(Collection collection) {
Review Comment:
Not necessary.
```java
// src/main/java/org/apache/fory/serializer/collection/CollectionSerializer.java
public T copy(T originCollection) {
if (isImmutable()) {
return originCollection;
}
Preconditions.checkArgument(supportCodegenHook);
Collection newCollection = newCollection(originCollection);
if (needToCopyRef) {
fory.reference(originCollection, newCollection);
}
copyElements(originCollection, newCollection);
return (T) newCollection;
}
```
--
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): support serialization of CopyOnWriteArraySet [fory]
Posted by "LiangliangSui (via GitHub)" <gi...@apache.org>.
LiangliangSui commented on code in PR #2999:
URL: https://github.com/apache/fory/pull/2999#discussion_r2596813047
##########
java/fory-core/src/main/java/org/apache/fory/serializer/collection/CollectionSerializers.java:
##########
@@ -294,6 +295,27 @@ public CopyOnWriteArrayList onCollectionRead(Collection collection) {
}
}
+ public static class CopyOnWriteArraySetSerializer
+ extends ConcurrentCollectionSerializer<CopyOnWriteArraySet> {
+
+ public CopyOnWriteArraySetSerializer(Fory fory, Class<CopyOnWriteArraySet> type) {
+ super(fory, type, true);
+ }
+
+ @Override
+ public Collection newCollection(MemoryBuffer buffer) {
+ int numElements = buffer.readVarUint32Small7();
+ setNumElements(numElements);
+ return new CollectionContainer<>(numElements);
+ }
+
+ @Override
+ public CopyOnWriteArraySet onCollectionRead(Collection collection) {
Review Comment:
@chaokunyang
--
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): support serialization of CopyOnWriteArraySet [fory]
Posted by "chaokunyang (via GitHub)" <gi...@apache.org>.
chaokunyang commented on code in PR #2999:
URL: https://github.com/apache/fory/pull/2999#discussion_r2596921985
##########
java/fory-core/src/main/java/org/apache/fory/serializer/collection/CollectionLikeSerializer.java:
##########
@@ -550,6 +552,21 @@ public void copyElements(Collection originCollection, Object[] elements) {
}
}
+ public void copyElementsAddAll(Collection originCollection, Collection newCollection) {
Review Comment:
I think this method should be removed, it's only related to COpyOnWriteSet/LIst, why you add it to base class
--
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): support serialization of CopyOnWriteArraySet [fory]
Posted by "LiangliangSui (via GitHub)" <gi...@apache.org>.
LiangliangSui commented on code in PR #2999:
URL: https://github.com/apache/fory/pull/2999#discussion_r2596967789
##########
java/fory-core/src/main/java/org/apache/fory/serializer/collection/CollectionLikeSerializer.java:
##########
@@ -550,6 +552,21 @@ public void copyElements(Collection originCollection, Object[] elements) {
}
}
+ public void copyElementsAddAll(Collection originCollection, Collection newCollection) {
Review Comment:
The `copyElementsAddAll` method requires a `TypeResolver` and cannot be made static. It also cannot be placed in the `CollectionSerializers`.
If it needs to be further downgraded, it can only be placed in the `ConcurrentCollectionSerializer`.
--
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): support serialization of CopyOnWriteArraySet [fory]
Posted by "chaokunyang (via GitHub)" <gi...@apache.org>.
chaokunyang commented on code in PR #2999:
URL: https://github.com/apache/fory/pull/2999#discussion_r2597353994
##########
java/fory-core/src/main/java/org/apache/fory/serializer/collection/CollectionSerializers.java:
##########
@@ -292,6 +293,53 @@ public CopyOnWriteArrayList onCollectionRead(Collection collection) {
Object[] elements = ((CollectionContainer) collection).elements;
return new CopyOnWriteArrayList(elements);
}
+
+ @Override
+ public CopyOnWriteArrayList copy(CopyOnWriteArrayList originCollection) {
+ Preconditions.checkArgument(supportCodegenHook);
+ CopyOnWriteArrayList newCollection = new CopyOnWriteArrayList();
+ if (needToCopyRef) {
+ fory.reference(originCollection, newCollection);
+ }
+ List copyList = new ArrayList(originCollection.size());
+ copyElements(originCollection, copyList);
+ newCollection.addAll(copyList);
+ return newCollection;
+ }
+ }
+
+ public static class CopyOnWriteArraySetSerializer
+ extends ConcurrentCollectionSerializer<CopyOnWriteArraySet> {
+
+ public CopyOnWriteArraySetSerializer(Fory fory, Class<CopyOnWriteArraySet> type) {
+ super(fory, type, true);
+ }
+
+ @Override
+ public Collection newCollection(MemoryBuffer buffer) {
+ int numElements = buffer.readVarUint32Small7();
+ setNumElements(numElements);
+ return new CollectionContainer<>(numElements);
+ }
+
+ @Override
+ public CopyOnWriteArraySet onCollectionRead(Collection collection) {
+ Object[] elements = ((CollectionContainer) collection).elements;
+ return new CopyOnWriteArraySet(Arrays.asList(elements));
+ }
+
+ @Override
+ public CopyOnWriteArraySet copy(CopyOnWriteArraySet originCollection) {
+ Preconditions.checkArgument(supportCodegenHook);
Review Comment:
```suggestion
```
--
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): support serialization of CopyOnWriteArraySet [fory]
Posted by "LiangliangSui (via GitHub)" <gi...@apache.org>.
LiangliangSui commented on code in PR #2999:
URL: https://github.com/apache/fory/pull/2999#discussion_r2591688153
##########
java/fory-core/src/main/java/org/apache/fory/serializer/collection/CollectionSerializers.java:
##########
@@ -294,6 +295,27 @@ public CopyOnWriteArrayList onCollectionRead(Collection collection) {
}
}
+ public static class CopyOnWriteArraySetSerializer
+ extends ConcurrentCollectionSerializer<CopyOnWriteArraySet> {
+
+ public CopyOnWriteArraySetSerializer(Fory fory, Class<CopyOnWriteArraySet> type) {
+ super(fory, type, true);
+ }
+
+ @Override
+ public Collection newCollection(MemoryBuffer buffer) {
+ int numElements = buffer.readVarUint32Small7();
+ setNumElements(numElements);
+ return new CollectionContainer<>(numElements);
+ }
+
+ @Override
+ public CopyOnWriteArraySet onCollectionRead(Collection collection) {
Review Comment:
Done
--
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): support serialization of CopyOnWriteArraySet [fory]
Posted by "chaokunyang (via GitHub)" <gi...@apache.org>.
chaokunyang commented on code in PR #2999:
URL: https://github.com/apache/fory/pull/2999#discussion_r2597351547
##########
java/fory-core/src/main/java/org/apache/fory/serializer/collection/CollectionSerializers.java:
##########
@@ -292,6 +293,53 @@ public CopyOnWriteArrayList onCollectionRead(Collection collection) {
Object[] elements = ((CollectionContainer) collection).elements;
return new CopyOnWriteArrayList(elements);
}
+
+ @Override
+ public CopyOnWriteArrayList copy(CopyOnWriteArrayList originCollection) {
+ Preconditions.checkArgument(supportCodegenHook);
Review Comment:
```suggestion
```
--
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): support serialization of CopyOnWriteArraySet [fory]
Posted by "LiangliangSui (via GitHub)" <gi...@apache.org>.
LiangliangSui commented on code in PR #2999:
URL: https://github.com/apache/fory/pull/2999#discussion_r2596901566
##########
java/fory-core/src/main/java/org/apache/fory/serializer/collection/CollectionLikeSerializer.java:
##########
@@ -550,6 +552,21 @@ public void copyElements(Collection originCollection, Object[] elements) {
}
}
+ public void copyElementsAddAll(Collection originCollection, Collection newCollection) {
Review Comment:
I have adopted the same definition as that of `public void copyElements(Collection originCollection, Collection newCollection)`, and`public void copyElements(Collection originCollection, Collection newCollection)` is only used in the CollectionSerializers.
--
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): support serialization of CopyOnWriteArraySet [fory]
Posted by "LiangliangSui (via GitHub)" <gi...@apache.org>.
LiangliangSui commented on code in PR #2999:
URL: https://github.com/apache/fory/pull/2999#discussion_r2596901566
##########
java/fory-core/src/main/java/org/apache/fory/serializer/collection/CollectionLikeSerializer.java:
##########
@@ -550,6 +552,21 @@ public void copyElements(Collection originCollection, Object[] elements) {
}
}
+ public void copyElementsAddAll(Collection originCollection, Collection newCollection) {
Review Comment:
I have adopted the same definition as that of `public void copyElements(Collection originCollection, Collection newCollection)`, and`public void copyElements(Collection originCollection, Collection newCollection)` is only used in the CollectionSerializer.
--
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): support serialization of CopyOnWriteArraySet [fory]
Posted by "chaokunyang (via GitHub)" <gi...@apache.org>.
chaokunyang commented on code in PR #2999:
URL: https://github.com/apache/fory/pull/2999#discussion_r2596923882
##########
java/fory-core/src/main/java/org/apache/fory/serializer/collection/CollectionSerializer.java:
##########
@@ -61,7 +61,11 @@ public T copy(T originCollection) {
if (needToCopyRef) {
fory.reference(originCollection, newCollection);
}
- copyElements(originCollection, newCollection);
+ onCollectionCopy(originCollection, newCollection);
return (T) newCollection;
}
+
+ protected void onCollectionCopy(Collection originCollection, Collection newCollection) {
Review Comment:
Abstraction is not always good, overwrite is more simple here, and your implement can't setup correct reference graph, which will an issue too if copy ref is enabled
--
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): support serialization of CopyOnWriteArraySet [fory]
Posted by "LiangliangSui (via GitHub)" <gi...@apache.org>.
LiangliangSui commented on code in PR #2999:
URL: https://github.com/apache/fory/pull/2999#discussion_r2591299282
##########
java/fory-core/src/main/java/org/apache/fory/serializer/collection/CollectionSerializers.java:
##########
@@ -294,6 +295,27 @@ public CopyOnWriteArrayList onCollectionRead(Collection collection) {
}
}
+ public static class CopyOnWriteArraySetSerializer
+ extends ConcurrentCollectionSerializer<CopyOnWriteArraySet> {
+
+ public CopyOnWriteArraySetSerializer(Fory fory, Class<CopyOnWriteArraySet> type) {
+ super(fory, type, true);
+ }
+
+ @Override
+ public Collection newCollection(MemoryBuffer buffer) {
+ int numElements = buffer.readVarUint32Small7();
+ setNumElements(numElements);
+ return new CollectionContainer<>(numElements);
+ }
+
+ @Override
+ public CopyOnWriteArraySet onCollectionRead(Collection collection) {
Review Comment:
> This is what I am worried, you will copy CopyOnWriteArraySet every time you put an element into the Set when copying.
Could you provide some details?
--
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): support serialization of CopyOnWriteArraySet [fory]
Posted by "chaokunyang (via GitHub)" <gi...@apache.org>.
chaokunyang commented on code in PR #2999:
URL: https://github.com/apache/fory/pull/2999#discussion_r2596893255
##########
java/fory-core/src/main/java/org/apache/fory/serializer/collection/CollectionLikeSerializer.java:
##########
@@ -550,6 +552,21 @@ public void copyElements(Collection originCollection, Object[] elements) {
}
}
+ public void copyElementsAddAll(Collection originCollection, Collection newCollection) {
Review Comment:
This API is vague to me, and it should not be added as a public API of CollectionLikeSerializer. And most serializer would never use it, why not just create a private static method in CollectionSerializers with clear doc which indicate why it's needed, and clarify it's only used by CopyOnWriteList/Set?
--
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