Skip to content
This repository was archived by the owner on Jun 7, 2021. It is now read-only.

APEXCORE-502 Unnecessary byte array copy in DefaultKryoStreamCodec.toByteArray#370

Merged
asfgit merged 1 commit into
apache:masterfrom
vrozov:APEXCORE-502
Aug 18, 2016
Merged

APEXCORE-502 Unnecessary byte array copy in DefaultKryoStreamCodec.toByteArray#370
asfgit merged 1 commit into
apache:masterfrom
vrozov:APEXCORE-502

Conversation

@vrozov
Copy link
Copy Markdown
Member

@vrozov vrozov commented Aug 17, 2016

@tweise Please review

@asfgit asfgit merged commit c210dc2 into apache:master Aug 18, 2016
final Output output = new Output(32, -1);
try {
ByteArrayOutputStream os = new ByteArrayOutputStream();
Output output = new Output(os);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like it is a pass through to the output stream? Granted the extra wrapping is not needed.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, Output already has byte array for storing serialized object, flushing internal byte array to optional OutputStream when OutputStream is ByteArrayOutputStream() is unnecessary and must be avoided.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see so it is not a pass through when the stream is supplied

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is not a direct pass through. Output creates an internal byte array buffer and flushes it to the OutputStream (if one is provided) when the internal buffer becomes full. As in this case OutputStream is ByteArrayOutputStream anyway, this leads to 3 unnecessary byte arrays copy operations.

@vrozov vrozov deleted the APEXCORE-502 branch September 8, 2016 20:46
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants