LiangliangSui commented on code in PR #1406:
URL: https://github.com/apache/incubator-fury/pull/1406#discussion_r1527107427
##########
java/fury-core/src/main/java/org/apache/fury/resolver/ClassResolver.java:
##########
@@ -391,12 +391,7 @@ private void registerDefaultClasses() {
/** register class. */
public void register(Class<?> cls) {
- if (!extRegistry.registeredClassIdMap.containsKey(cls)) {
- while
(extRegistry.registeredId2Classes.containsKey(extRegistry.registeredClassIdCounter))
{
- extRegistry.registeredClassIdCounter++;
- }
- register(cls, extRegistry.registeredClassIdCounter);
- }
+ register(cls, extRegistry.registeredClassIdCounter);
Review Comment:
> > What will happen if there are some hold in `registeredId2ClassInfo`? For
example:
> > ```java
> > // registeredClassIdCounter = 99
> > fury.register(class1); // registeredClassIdCounter = 100 now
> > fury.register(class2, 101); // registeredClassIdCounter = 101 now
> > fury.register(class3); // throw exception since 101 has been used
> > ```
>
> I think for
`org.apache.fury.resolver.ClassResolver#register(java.lang.Class<?>)` , it
should always succeed. The implemention shoukd handle class id conflict inside
Therefore, there is no need to deal with Classd conflicts in
`org.apache.fury.resolver.ClassResolver#register(java.lang.Class<?>)`, because
there will be no conflicts in ClassId allocated using `ClassIdAllocator`
--
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]
