-
Notifications
You must be signed in to change notification settings - Fork 322
expose method to create parser from direct memory#435
expose method to create parser from direct memory #435arnaudroger wants to merge 1 commit intomsgpack:main from
Conversation
xerial
commented
Oct 21, 2017
@komamitsu Could you review this change? This seems a good improvement.
@komamitsu
komamitsu
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@arnaudroger Thanks for the contribution! It seems a cool feature.
But I have some questions. Can I ask you to take a look at them?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This static method returns ByteBuffer not ByteBufferInput
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It doesn't seem to me that this method is related to ByteBufferInput.
@xerial Do you think this is the right place to put the method in?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can make DirectBufferAccess a public class instead of adding this method.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually this location is a bit weird.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As you might notice since this method doesn't have @Override annotation, Jackson doesn't support this kind of API, I think.
How do you use this method via Jackson's API? It would be great, if you add some unit tests to make it clear 😺
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I use the MessagePackFactory directly, currently using reflection.
Useful to parse message coming in from netty for example without creating a lot of garbage.
Also expose the method to create a parser from a sub array.