Change type (int->long) of field total in class Output return int

[ghost]

From [email protected] on October 18, 2013 11:33:21

What steps will reproduce the problem?
int output.total()

What is the expected output? What do you see instead?
long output.total()

What version of the Kryo are you using?
2.22

Original issue: http://code.google.com/p/kryo/issues/detail?id=142

[ghost]

From romixlev on October 19, 2013 00:18:07

Can you elaborate a bit? Why do you want it to be changed to long? Is is the only method whose signature should be changed or do you see more of those?

[ghost]

From [email protected] on October 22, 2013 03:49:12

I implemented the save data from the database to a file. When saving, I show the number of recorded objects and the size of the serialized data.
Now to calculate the size I have to consider Integer.MAX_VALUE, because more than 2GB in size, the value is negative.

[ghost]

From romixlev on October 22, 2013 05:03:30

OK. I understand your point now.

But if you look at different JDK stream classes (DataOutputStream, InputStream, etc), they all use "int" for returning the number of available bytes and so on. And they all wrap around Integer.MAX_VALUE. So, we are simply following the JDK way of doing things at the moment. And I'm not sure that it makes a lot of sense to make Kryo IO streams different from other streams...

[ghost]

From [email protected] on October 22, 2013 05:22:11

In InputStream int is used for quantities that don't exceed the 2GB limit.

DataOutputStream.size() returning int is a VERY bad design decision in my opinion.
I'd bet that most of the people using that API don't even realize that it can overflow.

I'd suggest not copying bad design decisions from the JDK.

[ghost]

From [email protected] on October 22, 2013 06:05:51

Changing Input/Output#total to long is probably fine for Kryo.

[ghost]

From romixlev on October 22, 2013 13:25:56

This issue was closed by revision r423.

Status: Fixed

[ghost]

From romixlev on October 22, 2013 13:27:26

Fixed in trunk (r423).