V07 unpacker #100
Merged
V07 unpacker #100
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…dd property based tests
…npackNil return null for convenience
Member
There was a problem hiding this comment.
Can you explain why you use the value bb.position() - bb.remaining()?
Member
Author
There was a problem hiding this comment.
It's my mistake. This should be bb.limit() - bb.remaining(). Thanks.
Member
|
👍 |
Member
There was a problem hiding this comment.
Should we check whether obj is null or not?
if (other == null) {
return false;
} else if(obj instanceof ExtendedTypeHeader) {
Member
Author
There was a problem hiding this comment.
null instanceof xxx returns false, so we do not need null check here:
http://stackoverflow.com/questions/2950319/is-null-check-needed-before-calling-instanceof
xerial
added a commit
to xerial/msgpack-java
that referenced
this pull request
Jun 3, 2014
xerial
added a commit
to xerial/msgpack-java
that referenced
this pull request
Jun 3, 2014
xerial
added a commit
to xerial/msgpack-java
that referenced
this pull request
Jun 3, 2014
xerial
added a commit
to xerial/msgpack-java
that referenced
this pull request
Jun 3, 2014
xerial
added a commit
to xerial/msgpack-java
that referenced
this pull request
Jun 3, 2014
xerial
added a commit
to xerial/msgpack-java
that referenced
this pull request
Jun 3, 2014
…fferBE.slice() to return big-endian buffer
Optimize unpackBinary
Member
Author
|
Now closes this ticket. Thanks for the review! |
Member
|
Epic 100th issue! 👍 |
Member
|
Thanks for your great work, @xerial! |
Member
|
Good job 👍 @xerial |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Implemented MessageUnpacker whose performance is faster than v06. You can run benchmark tests that compare the performance of v06 and v07:
benchmark code: https://github.com/msgpack/msgpack-java/blob/v07-unpacker/msgpack-core/src/test/scala/org/msgpack/core/MessageUnpackerTest.scala
When reading 10,000 message-packed records (including arrays, maps, strings, etc.), skipping records is 4 times faster than v6, and reading values is about 2 times faster:
Summary of the changes:
MessageUnpacker.unpackXXXmethods block until the next input buffer is available. Supporting event-driven I/O Supporting event-driven I/O in unpackXXX #92 is not included in this change.MessageBuffers, the unpacker copies the contents of the current buffer and the next buffers into an extra buffer so that readXXX can read its required length. (seeensure(n)inMessageUnpacker)skipValue()uses switch-case statements instead of polymorphic function calls inMessageFormatbecause using switch statement is slightly faster. The skip length table Using skip length table in MessageUnpacker #94 is not yet implemented because this part is already fast enough.packStringandunpackStringthat use pre-allocated buffers to encode/decode UTF-8 strings.MessagePackTest.scala)./sbt jacoco:coverhttp://www.eclemma.org/jacoco/./sbt findbugshttp://findbugs.sourceforge.net/TODO