Skip to content

Add ByteBuffer input support again#378

Merged
frsyuki merged 16 commits intodevelopfrom
directbuf
Aug 12, 2016
Merged

Add ByteBuffer input support again#378
frsyuki merged 16 commits intodevelopfrom
directbuf

Conversation

@frsyuki
Copy link
Copy Markdown
Member

@frsyuki frsyuki commented Jul 27, 2016

original code: #377

{
if (isUniversalBuffer || buffer.base instanceof byte[]) {
if (isUniversalBuffer || buffer.base != null) {
// We have nothing to do. Wait until the garbage-collector collects this array object
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 think we should use hasArray instead of buffer.base != null here for future changes

Copy link
Copy Markdown
Member Author

@frsyuki frsyuki Jul 29, 2016

Choose a reason for hiding this comment

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

👍 4cb9324 has this improvement.

/**
* Wraps a ByteBuffer into a MessageBuffer.
*
* The new MessageBuffer will be backed by the given byte array. Modifications to the new MessageBuffer will cause the byte array to be modified and vice versa.
Copy link
Copy Markdown
Member

@xerial xerial Aug 3, 2016

Choose a reason for hiding this comment

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

The new MessageBuffer will be backed by the given byte array, ...

Created MessageBuffer will be a reference to the given byte array, so modifications to ...

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.

well, I copied mof of the text from JavaDoc of ByteBuffer: https://docs.oracle.com/javase/7/docs/api/java/nio/ByteBuffer.html#wrap(byte[])

@xerial
Copy link
Copy Markdown
Member

xerial commented Aug 9, 2016

@miniway Could you check the build failure of Travis?
https://travis-ci.org/msgpack/msgpack-java/builds/151059553

@miniway
Copy link
Copy Markdown
Contributor

miniway commented Aug 10, 2016

I think we need ByteBuffer constructor at the subclasses #381

Add ByteBuffer constructor at MessageBufferU and MessageBufferBE
@miniway
Copy link
Copy Markdown
Contributor

miniway commented Aug 12, 2016

@miniway
Copy link
Copy Markdown
Contributor

miniway commented Aug 12, 2016

Another PR to fix the test failures at the Universal MessageBuffer #383

Fix test failures at universal MessageBuffer
@frsyuki
Copy link
Copy Markdown
Member Author

frsyuki commented Aug 12, 2016

Looks ready to merge 👍

@frsyuki frsyuki changed the title Add ByteBuffer input support again - reviewed Add ByteBuffer input support again Aug 12, 2016
@miniway
Copy link
Copy Markdown
Contributor

miniway commented Aug 12, 2016

I have a small fix to get correct benchmark number, #384

Remove side effect of unpacker benchmark
@xerial
Copy link
Copy Markdown
Member

xerial commented Aug 12, 2016

👍

@frsyuki frsyuki merged commit f119edd into develop Aug 12, 2016
@frsyuki
Copy link
Copy Markdown
Member Author

frsyuki commented Aug 12, 2016

Merged. Great improvement.

@frsyuki frsyuki deleted the directbuf branch August 12, 2016 17:40
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

Successfully merging this pull request may close these issues.

3 participants