-
Notifications
You must be signed in to change notification settings - Fork 8.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feature: support kryo codec #1437
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #1437 +/- ##
=============================================
+ Coverage 46.38% 46.72% +0.33%
- Complexity 1717 1730 +13
=============================================
Files 350 353 +3
Lines 12855 12949 +94
Branches 1619 1619
=============================================
+ Hits 5963 6050 +87
- Misses 6242 6246 +4
- Partials 650 653 +3
Continue to review full report at Codecov.
|
Duplicate with #1418, can you close one PR? |
@jsbxyyx sorry, I didn't notice that. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add module in maven-shade-plugin
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add kryo-serializers
and kryo
dependency in all.pom , just like protostuff
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
test function pass
KryoSerializer kryoSerializer = KryoSerializerFactory.getInstance().get(); | ||
try { | ||
T t = kryoSerializer.deserialize(bytes); | ||
return t; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return kryoSerializer.serialize(t);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
KryoSerializer kryoSerializer = KryoSerializerFactory.getInstance().get(); | ||
try { | ||
byte[] bytes = kryoSerializer.serialize(t); | ||
return bytes; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return kryoSerializer.serialize(t);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
ByteArrayOutputStream baos = new ByteArrayOutputStream(); | ||
Output output = new Output(baos); | ||
kryo.writeClassAndObject(output, t); | ||
output.close(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need flush?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
close() inner call flush()
public <T> T deserialize(byte[] bytes) { | ||
ByteArrayInputStream bais = new ByteArrayInputStream(bytes); | ||
Input input = new Input(bais); | ||
input.close(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FileInputStream close and ByteArrayInputStream close implements different, here not problem.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
protocol kryo serializer