Skip to content

Commit

Permalink
fix(java): fix getClassDef thead safety (#1597)
Browse files Browse the repository at this point in the history
## What does this PR do?

Fix ci failuure introduced in #1556 :

![image](https://github.com/apache/incubator-fury/assets/12445254/5977b4ca-07b9-456b-82d8-a2779a08d01f)


## Related issues

<!--
Is there any related issue? Please attach here.

- #xxxx0
- #xxxx1
- #xxxx2
-->


## Does this PR introduce any user-facing change?

<!--
If any user-facing interface changes, please [open an
issue](https://github.com/apache/incubator-fury/issues/new/choose)
describing the need to do so and update the document if necessary.
-->

- [ ] 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.
-->
  • Loading branch information
chaokunyang committed May 3, 2024
1 parent 63bbe45 commit 48361cb
Show file tree
Hide file tree
Showing 6 changed files with 21 additions and 27 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ public String codecQualifiedClassName(Class<?> beanClass) {

protected abstract String codecSuffix();

<T> T visitFury(Function<Fury, T> function) {
protected <T> T visitFury(Function<Fury, T> function) {
return fury.getJITContext().asyncVisitFury(function);
}

Expand Down Expand Up @@ -441,8 +441,7 @@ protected Expression writeForNotNullNonFinalObject(
classInfo,
inlineInvoke(classResolverRef, "getClassInfo", classInfoTypeRef, clsExpr))));
}
writeClassAndObject.add(
fury.getClassResolver().writeClassExpr(classResolverRef, buffer, classInfo));
writeClassAndObject.add(classResolver.writeClassExpr(classResolverRef, buffer, classInfo));
writeClassAndObject.add(
new Invoke(
inlineInvoke(classInfo, "getSerializer", SERIALIZER_TYPE),
Expand Down Expand Up @@ -659,8 +658,7 @@ protected Expression serializeForCollection(
new Assign(
classInfo,
inlineInvoke(classResolverRef, "getClassInfo", classInfoTypeRef, clsExpr))));
writeClassAction.add(
fury.getClassResolver().writeClassExpr(classResolverRef, buffer, classInfo));
writeClassAction.add(classResolver.writeClassExpr(classResolverRef, buffer, classInfo));
serializer = new Invoke(classInfo, "getSerializer", "serializer", SERIALIZER_TYPE, false);
serializer = new Cast(serializer, TypeRef.of(AbstractCollectionSerializer.class));
writeClassAction.add(serializer, new Return(serializer));
Expand Down Expand Up @@ -965,8 +963,7 @@ protected Expression serializeForMap(
classInfo,
inlineInvoke(classResolverRef, "getClassInfo", classInfoTypeRef, clsExpr))));
// Note: writeClassExpr is thread safe.
writeClassAction.add(
fury.getClassResolver().writeClassExpr(classResolverRef, buffer, classInfo));
writeClassAction.add(classResolver.writeClassExpr(classResolverRef, buffer, classInfo));
serializer = new Invoke(classInfo, "getSerializer", "serializer", SERIALIZER_TYPE, false);
serializer = new Cast(serializer, TypeRef.of(AbstractMapSerializer.class));
writeClassAction.add(serializer, new Return(serializer));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
import java.util.stream.Collectors;
import org.apache.fury.Fury;
import org.apache.fury.builder.Generated.GeneratedCompatibleSerializer;
import org.apache.fury.codegen.CodegenContext;
Expand Down Expand Up @@ -97,10 +96,6 @@ public CompatibleCodecBuilder(
super(beanType, fury, superSerializerClass);
this.fieldResolver = fieldResolver;
if (isRecord) {
List<String> fieldNames =
fieldResolver.getAllFieldsList().stream()
.map(FieldResolver.FieldInfo::getName)
.collect(Collectors.toList());
recordReversedMapping = RecordUtils.buildFieldToComponentMapping(beanClass);
}
ctx.reserveName(FIELD_RESOLVER_NAME);
Expand Down Expand Up @@ -879,7 +874,7 @@ protected Expression writeFinalClassInfo(Expression buffer, Class<?> cls) {
Preconditions.checkArgument(ReflectionUtils.isMonomorphic(cls));
ClassInfo classInfo = visitFury(f -> f.getClassResolver().getClassInfo(cls, false));
if (classInfo != null && classInfo.getClassId() != ClassResolver.NO_CLASS_ID) {
return fury.getClassResolver().writeClassExpr(buffer, classInfo.getClassId());
return classResolver.writeClassExpr(buffer, classInfo.getClassId());
}
Expression classInfoExpr = getFinalClassInfo(cls);
return new Invoke(classResolverRef, "writeClass", buffer, classInfoExpr);
Expand All @@ -889,7 +884,7 @@ protected Expression skipFinalClassInfo(Class<?> cls, Expression buffer) {
Preconditions.checkArgument(ReflectionUtils.isMonomorphic(cls));
ClassInfo classInfo = visitFury(f -> f.getClassResolver().getClassInfo(cls, false));
if (classInfo != null && classInfo.getClassId() != ClassResolver.NO_CLASS_ID) {
return fury.getClassResolver().skipRegisteredClassExpr(buffer);
return classResolver.skipRegisteredClassExpr(buffer);
}
// read `ClassInfo` is not used, set `inlineReadClassInfo` false,
// to avoid read doesn't happen in generated code.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@
import org.apache.fury.codegen.Expression.StaticInvoke;
import org.apache.fury.codegen.ExpressionVisitor;
import org.apache.fury.memory.Platform;
import org.apache.fury.meta.ClassDef;
import org.apache.fury.reflect.TypeRef;
import org.apache.fury.serializer.ObjectSerializer;
import org.apache.fury.serializer.PrimitiveSerializers.LongSerializer;
Expand Down Expand Up @@ -91,8 +90,12 @@ public ObjectCodecBuilder(Class<?> beanClass, Fury fury) {
Collection<Descriptor> descriptors;
boolean shareMeta = fury.getConfig().shareMetaContext();
if (shareMeta) {
ClassDef classDef = classResolver.getClassDef(beanClass, true);
descriptors = classDef.getDescriptors(classResolver, beanClass);
descriptors =
visitFury(
f ->
f.getClassResolver()
.getClassDef(beanClass, true)
.getDescriptors(classResolver, beanClass));
} else {
descriptors = fury.getClassResolver().getAllDescriptorsMap(beanClass, true).values();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,14 @@ public static ClassDef readClassDef(
return ClassDefDecoder.decodeClassDef(classResolver, buffer, header);
}

/**
* Consolidate fields of <code>classDef</code> with <code>cls</code>. If some field exists in
* <code>cls</code> but not in <code>classDef</code>, it won't be returned in final collection. If
* some field exists in <code>classDef</code> but not in <code> cls</code>, it will be added to
* final collection.
*
* @param cls class load in current process.
*/
public List<Descriptor> getDescriptors(ClassResolver resolver, Class<?> cls) {
if (descriptors == null) {
SortedMap<Field, Descriptor> allDescriptorsMap = resolver.getAllDescriptorsMap(cls, true);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -510,7 +510,7 @@ public List<Class<?>> getRegisteredClasses() {
/**
* Mark non-inner registered final types as non-final to write class def for those types. Note if
* a class is registered but not an inner class with inner serializer, it will still be taken as
* non-final to write class def, so that it can be deserialized by the peer still..
* non-final to write class def, so that it can be deserialized by the peer still.
*/
public boolean isMonomorphic(Class<?> clz) {
if (fury.getConfig().shareMetaContext()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -287,15 +287,6 @@ static boolean skipPrimitiveFieldValueFailed(Fury fury, short classId, MemoryBuf
}
}

/**
* Consolidate fields of <code>classDef</code> with <code>cls</code>. If some field exists in
* <code>cls</code> but not in <code>classDef</code>, it won't be returned in final collection. If
* some field exists in <code>classDef</code> but not in <code> cls</code>, it will be added to
* final collection.
*
* @param cls class load in current process.
* @param classDef class definition sent from peer.
*/
public static Collection<Descriptor> consolidateFields(
ClassResolver classResolver, Class<?> cls, ClassDef classDef) {
return classDef.getDescriptors(classResolver, cls);
Expand Down

0 comments on commit 48361cb

Please sign in to comment.