Skip to content
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

Unnecessary allocation of a ByteBuffer in AbstractAvroMessageConverter. #1292

Closed
jmax01 opened this issue Mar 9, 2018 · 2 comments
Closed
Assignees
Milestone

Comments

@jmax01
Copy link
Contributor

jmax01 commented Mar 9, 2018

Unnecessary allocation of a ByteBuffer in AbstractAvroMessageConverter.

PR forthcoming.

    @Override
    protected Object convertFromInternal(Message<?> message, Class<?> targetClass, Object conversionHint) {
        Object result = null;
        try {
            byte[] payload = (byte[]) message.getPayload();

            //What purpose does this serve?
            ByteBuffer buf = ByteBuffer.wrap(payload);

            MimeType mimeType = getContentTypeResolver().resolve(message.getHeaders());
            if (mimeType == null) {
                if (conversionHint instanceof MimeType) {
                    mimeType = (MimeType) conversionHint;
                }
                else {
                    return null;
                }
            }

            //What purpose does this serve?
            buf.get(payload);

            Schema writerSchema = resolveWriterSchemaForDeserialization(mimeType);
            Schema readerSchema = resolveReaderSchemaForDeserialization(targetClass);
            DatumReader<Object> reader = getDatumReader((Class<Object>) targetClass, readerSchema, writerSchema);
            Decoder decoder = DecoderFactory.get().binaryDecoder(payload, null);
            result = reader.read(null, decoder);
        }
        catch (IOException e) {
            throw new MessageConversionException(message, "Failed to read payload", e);
        }
        return result;
@jmax01 jmax01 changed the title Unnecessary allocation of a ByteBuffer in AbstractAvroMessageConverter. Unnecessary allocation of a ByteBuffer in AbstractAvroMessageConverter. Mar 9, 2018
@jmax01 jmax01 changed the title Unnecessary allocation of a ByteBuffer in AbstractAvroMessageConverter. Unnecessary allocation of a ByteBuffer in AbstractAvroMessageConverter. Mar 9, 2018
@garyrussell
Copy link
Contributor

garyrussell commented Mar 9, 2018

Unnecessary allocation of a ByteBuffer

It's actually (much) worse than that; the get() causes an unnecessary copy of each byte in the buffer to itself.

ByteBuffer (at least the Sun/Oracle implementation) doesn't have the smarts to short-circuit the copy if the wrapped and target buffers are identical.

The fix should be back-ported to 1.3.x.

@jmax01
Copy link
Contributor Author

jmax01 commented Mar 9, 2018

Yes please backport.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants