This is an automated email from the ASF dual-hosted git repository.
chaokunyang pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/incubator-fury.git
The following commit(s) were added to refs/heads/main by this push:
new cf13c997 fix(java): fix abstract collection elems same type
serialization (#1641)
cf13c997 is described below
commit cf13c99710b5e6360f8dac9a6d88f954119d599d
Author: Shawn Yang <[email protected]>
AuthorDate: Mon May 20 14:49:29 2024 +0800
fix(java): fix abstract collection elems same type serialization (#1641)
## What does this PR do?
This PR fix inlineable expression codegen and reduce class load cost in
generated code.
With those changes, it fixed abstract collection elems same type
serialization in #1640
## Related issues
Closes #1640
## 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.
-->
---
.../fury/builder/BaseObjectCodecBuilder.java | 8 +------
.../java/org/apache/fury/builder/CodecBuilder.java | 16 +++++++++----
.../java/org/apache/fury/codegen/Expression.java | 28 ++++++++++++++++++----
.../collection/CollectionSerializersTest.java | 27 +++++++++++++++++++++
4 files changed, 62 insertions(+), 17 deletions(-)
diff --git
a/java/fury-core/src/main/java/org/apache/fury/builder/BaseObjectCodecBuilder.java
b/java/fury-core/src/main/java/org/apache/fury/builder/BaseObjectCodecBuilder.java
index 34bb51ba..cc825a69 100644
---
a/java/fury-core/src/main/java/org/apache/fury/builder/BaseObjectCodecBuilder.java
+++
b/java/fury-core/src/main/java/org/apache/fury/builder/BaseObjectCodecBuilder.java
@@ -73,7 +73,6 @@ import org.apache.fury.codegen.Expression.ListExpression;
import org.apache.fury.codegen.Expression.Literal;
import org.apache.fury.codegen.Expression.Reference;
import org.apache.fury.codegen.Expression.Return;
-import org.apache.fury.codegen.Expression.StaticInvoke;
import org.apache.fury.codegen.ExpressionUtils;
import org.apache.fury.codegen.ExpressionVisitor.ExprHolder;
import org.apache.fury.collection.Tuple2;
@@ -521,12 +520,7 @@ public abstract class BaseObjectCodecBuilder extends
CodecBuilder {
if (sourcePublicAccessible(cls)) {
return Literal.ofClass(cls);
} else {
- return new StaticInvoke(
- ReflectionUtils.class,
- "loadClass",
- CLASS_TYPE,
- beanClassExpr(),
- Literal.ofString(cls.getName()));
+ return staticClassFieldExpr(cls, "__class__" +
cls.getName().replace(".", "_"));
}
}
diff --git
a/java/fury-core/src/main/java/org/apache/fury/builder/CodecBuilder.java
b/java/fury-core/src/main/java/org/apache/fury/builder/CodecBuilder.java
index b9c54218..d0f88062 100644
--- a/java/fury-core/src/main/java/org/apache/fury/builder/CodecBuilder.java
+++ b/java/fury-core/src/main/java/org/apache/fury/builder/CodecBuilder.java
@@ -519,16 +519,22 @@ public abstract class CodecBuilder {
}
protected Expression staticBeanClassExpr() {
+ if (sourcePublicAccessible(beanClass)) {
+ return Literal.ofClass(beanClass);
+ }
+ return staticClassFieldExpr(beanClass, "__class__");
+ }
+
+ protected Expression staticClassFieldExpr(Class<?> cls, String fieldName) {
+ Preconditions.checkArgument(
+ !sourcePublicAccessible(cls), "Public class %s should use class
literal instead", cls);
return getOrCreateField(
true,
Class.class,
- "__class__",
+ fieldName,
() ->
new StaticInvoke(
- ReflectionUtils.class,
- "loadClass",
- CLASS_TYPE,
- Literal.ofString(beanClass.getName()))
+ ReflectionUtils.class, "loadClass", CLASS_TYPE,
Literal.ofString(cls.getName()))
.inline());
}
diff --git
a/java/fury-core/src/main/java/org/apache/fury/codegen/Expression.java
b/java/fury-core/src/main/java/org/apache/fury/codegen/Expression.java
index 71c0f29f..d2572f77 100644
--- a/java/fury-core/src/main/java/org/apache/fury/codegen/Expression.java
+++ b/java/fury-core/src/main/java/org/apache/fury/codegen/Expression.java
@@ -102,6 +102,15 @@ public interface Expression {
} else {
ExprCode genCode = doGenCode(ctx);
ctx.exprState.put(this, new ExprState(new ExprCode(genCode.isNull(),
genCode.value())));
+ if (this instanceof Inlineable) {
+ if (!((Inlineable) this).inlineCall) {
+ Preconditions.checkArgument(
+ StringUtils.isNotBlank(genCode.code()),
+ "Expression %s has empty code %s",
+ this,
+ genCode);
+ }
+ }
return genCode;
}
}
@@ -136,7 +145,7 @@ public interface Expression {
}
/** An expression that have a value as the result of the evaluation. */
- abstract class ValueExpression implements Expression {
+ abstract class ValueExpression extends Inlineable {
// set to others to get a more context-dependent variable name.
public String valuePrefix = "value";
@@ -781,6 +790,16 @@ public interface Expression {
}
}
+ @Override
+ public Inlineable inline(boolean inlineCall) {
+ if (!inlineCall) {
+ if (targetObject instanceof Inlineable) {
+ ((Inlineable) targetObject).inlineCall = false;
+ }
+ }
+ return super.inline(inlineCall);
+ }
+
@Override
public boolean nullable() {
return targetObject.nullable();
@@ -1805,7 +1824,6 @@ public interface Expression {
}
class BinaryOperator extends ValueExpression {
- private final boolean inline;
private final String operator;
private final TypeRef<?> type;
private Expression left;
@@ -1821,7 +1839,7 @@ public interface Expression {
protected BinaryOperator(
boolean inline, String operator, Expression left, Expression right,
TypeRef<?> t) {
- this.inline = inline;
+ this.inlineCall = inline;
this.operator = operator;
this.left = left;
this.right = right;
@@ -1871,7 +1889,7 @@ public interface Expression {
}
}
- if (inline) {
+ if (inlineCall) {
String value = String.format("(%s)", arith);
String code = StringUtils.isBlank(codeBuilder) ? null :
codeBuilder.toString();
return new ExprCode(code, FalseLiteral,
Code.variable(getRawType(type), value));
@@ -2025,7 +2043,7 @@ public interface Expression {
this.predicate = predicate;
this.action = action;
this.cutPoints = cutPoints;
- Preconditions.checkArgument(predicate.inline, predicate);
+ Preconditions.checkArgument(predicate.inlineCall, predicate);
}
@Override
diff --git
a/java/fury-core/src/test/java/org/apache/fury/serializer/collection/CollectionSerializersTest.java
b/java/fury-core/src/test/java/org/apache/fury/serializer/collection/CollectionSerializersTest.java
index f8f79b4d..72b0850f 100644
---
a/java/fury-core/src/test/java/org/apache/fury/serializer/collection/CollectionSerializersTest.java
+++
b/java/fury-core/src/test/java/org/apache/fury/serializer/collection/CollectionSerializersTest.java
@@ -617,4 +617,31 @@ public class CollectionSerializersTest extends
FuryTestBase {
Fury f =
Fury.builder().withLanguage(Language.JAVA).withRefTracking(refTracking).build();
serDeCheck(f, data);
}
+
+ @Data
+ abstract static class Foo {
+ private int f1;
+ }
+
+ static class Foo1 extends Foo {}
+
+ @Data
+ static class CollectionAbstractTest {
+ private List<Foo> fooList;
+ }
+
+ @Test(dataProvider = "enableCodegen")
+ public void testAbstractCollectionElementsSerialization(boolean
enableCodegen) {
+ Fury fury =
Fury.builder().withCodegen(enableCodegen).requireClassRegistration(false).build();
+ {
+ CollectionAbstractTest test = new CollectionAbstractTest();
+ test.fooList = new ArrayList<>(ImmutableList.of(new Foo1(), new Foo1()));
+ serDeCheck(fury, test);
+ }
+ {
+ CollectionAbstractTest test = new CollectionAbstractTest();
+ test.fooList = new ArrayList<>(ofArrayList(new Foo1(), new Foo1()));
+ serDeCheck(fury, test);
+ }
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
