Skip to content

Add ByteBuffer input support again - #378

Merged
frsyuki merged 16 commits into
developfrom
directbuf
Aug 12, 2016
Merged

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

Conversation

@frsyuki

@frsyuki frsyuki commented Jul 27, 2016

Copy link
Copy Markdown
Member

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

@frsyuki frsyuki Jul 29, 2016

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.

👍 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.

@xerial xerial Aug 3, 2016

Copy link
Copy Markdown
Member

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

xerial commented Aug 9, 2016

Copy link
Copy Markdown
Member

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

@miniway

miniway commented Aug 10, 2016

Copy link
Copy Markdown
Contributor

I think we need ByteBuffer constructor at the subclasses #381

Add ByteBuffer constructor at MessageBufferU and MessageBufferBE
@miniway

miniway commented Aug 12, 2016

Copy link
Copy Markdown
Contributor

@miniway

miniway commented Aug 12, 2016

Copy link
Copy Markdown
Contributor

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

Fix test failures at universal MessageBuffer
@frsyuki

frsyuki commented Aug 12, 2016

Copy link
Copy Markdown
Member Author

Looks ready to merge 👍

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

miniway commented Aug 12, 2016

Copy link
Copy Markdown
Contributor

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

Remove side effect of unpacker benchmark
@xerial

xerial commented Aug 12, 2016

Copy link
Copy Markdown
Member

👍

@frsyuki
frsyuki merged commit f119edd into develop Aug 12, 2016
@frsyuki

frsyuki commented Aug 12, 2016

Copy link
Copy Markdown
Member Author

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