creswick / StreamingQR

Apache License 2.0
2 stars 0 forks source link

Measure & reduce serialization overhead. #45

Closed creswick closed 10 years ago

creswick commented 10 years ago

Imported from trac #45.

Sending short notes turned into 70+ QR codes when sent from google keep, and was still significant when sent from extensive notes (but not as bad).

creswick commented 10 years ago

Lots of comments didn't make the port to github on this one; here's the last exchange:

Replying to donpdonp:

Does the amount of overhead stay the same for larger objects?

It actually increases as object size goes up (albiet slowly). I did not try with objects that have more than 2-3 fields, but based on that it seems to need ~2 bytes per field (a /very/ rough approximation... that will depend on the types used). The amount of data also seems to have a direct relationship to the overhead -- I tested with payloads up to 20k bytes, and the overhead gradually increased by 1-2 bytes as data size went up. It's probably worth using protobufs eventually, but I don't want to make the switch right now (maybe in 2-3 weeks when we have a better idea about the limits of our bandwidth).

creswick commented 10 years ago

Here's a summary of what we learned:

For an object like this:

class Data {
 String mimeType;
 byte[] data;
}

We're currently using the last option, and looking into alternatives as we figure out if we should integrate directly with zxing.

creswick commented 10 years ago

Code review comments from @donpdonp:

in StreamUtils.java: the various occurences of: in.read(buf); may need multiple calls to completely consume the stream. In googling around for an IO helper, the Guava library which is already in the project, has: ByteStreams.readFully(in, buf); while looking for an equivalent method for fully writing to an output stream, I came across Guava's DataInputStream and DataOutputStream which have their own readInt/writeInt, etc.. I'm not sure if its any more efficient than the java lib, but if it is, it could replace StreamUtils in Job.read and Job.write. The changes in TransmitFragment to serialize/deserialize the Job look good.

creswick commented 10 years ago

I've made the changes to DataInputStream/OutputStream, but retained the wrapper in StreamUtils because of the exceptions that DataInputStream can throw.

I've also made updates to the tests to also cover the int<-> byte code with the parameterized test suite, and removed the old Utils.shortToBytes (and reciprocal) methods.

See commit 5da149dab0a3cebe0

creswick commented 10 years ago

closing this until we have bandwidth measurements and concrete goals.