Skip to content

Fix MessageBufferU#copyTo called by sliced buffer#283

Merged
xerial merged 3 commits intov07-developfrom
issue_282
Oct 26, 2015
Merged

Fix MessageBufferU#copyTo called by sliced buffer#283
xerial merged 3 commits intov07-developfrom
issue_282

Conversation

@komamitsu
Copy link
Copy Markdown
Member

Fix #282

@komamitsu
Copy link
Copy Markdown
Member Author

@xerial Can you take a look at this pull request?

@komamitsu komamitsu changed the title Issue 282 Fix MessageBufferU#copyTo called by sliced buffer Oct 24, 2015
@komamitsu
Copy link
Copy Markdown
Member Author

https://travis-ci.org/msgpack/msgpack-java/jobs/87210273 MessageBufferBE can't be used in Travis CI...?

@xerial xerial self-assigned this Oct 24, 2015
@xerial
Copy link
Copy Markdown
Member

xerial commented Oct 24, 2015

right. MessageBufferBE only works in big endian machine. And initializing MessageBufferU and MessageBufferBE in the code slows down the performance of MessageBuffer test code.

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.

MessageBufferU should not be called directly. This changes every MessageBuffer method call to invokeinterface, and deteriorates the performance. TravisCI uses MessageBufferU for jvm6, I think.

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.

Sorry. For some reason, jvm6 test was turned off. Run test with -Dmsgpack.universal-buffer=true

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.

https://github.com/msgpack/msgpack-java/blob/v07-develop/.travis.yml#L14
I found we are testing MessageBufferU here, so having a test code that copies data between buffers is sufficient.

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.

Good to hear. I'll change the test code later. Thanks.

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.

@xerial Fixed the test code. Can you review it again?

xerial added a commit that referenced this pull request Oct 26, 2015
Fix MessageBufferU#copyTo called by sliced buffer
@xerial xerial merged commit 9ad0405 into v07-develop Oct 26, 2015
@xerial
Copy link
Copy Markdown
Member

xerial commented Oct 26, 2015

Merged. Thanks!

@komamitsu komamitsu deleted the issue_282 branch December 25, 2015 06:44
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.

2 participants