-
Notifications
You must be signed in to change notification settings - Fork 322
Move msgpack value classes to another project msgpack-value#571
Move msgpack value classes to another project msgpack-value #571dmikurube wants to merge 5 commits intomsgpack:main from
Conversation
...sageTypeException and MessageTypeCastException to msgpack-value
@dmikurube My preference order is:
First, release Timestamp support without introducing such interface changes like this PR. msgpack-java has been carefully implemented so as not to use any TypeProfile, which will be generated when extending classes (e.g., implementing interfaces, extending abstract classes). In an old benchmark, extending any classes had non-negligible overhead. I'm not sure this result still holds in modern JVM, though. The next msgpack-java version would be 0.9.0.
If we release a next msgpack-java version and the performance difference is negligible, I'd like to simplify all of the interfaces. by changing the value classes to always use immutable values, and discard existing immutable value interfaces, which is just adding complexity to the entire code base. Then, it would be possible to make the value interfaces more stable. For example, creating msgpack-spi module and introducing org.msgpack.spi package. Value classes will be org.msgpack.spi.Value.
If we will introduce such breaking changes, I'm not sure how valuable it is to split the msgpack-value as a separate module at this moment as embulk SPI will be broken again by such a change.
I don't think we can create org.msgpack.spi soon, so a reasonable release plan would be like this:
- 0.9.0 adds TimestampValue
- 0.10.0 split value interface like this PR. Adding JMH based benchmark would be good.
- 0.11.0 (This might be a year later or really quick if I have a chance to work on it) introduce org.msgpack.spi and separate packer/unpacker/value impl.
- 1.0.0 finalize
dmikurube
commented
May 20, 2021
@xerial Thanks. Got it. I understand your concerns and plan.
Then, I think we'll make a choice of removing msgpack-java's classes from Embulk's API/SPI in a (very) long-term. Our plan would be:
- Before Embulk v0.11.0 (expected to be Embulk v0.10.35), we upgrade
msgpack-coreinembulk-apito0.8.24- Upgrade msgpack-core to 0.8.24 embulk/embulk#1459
- Leaving a comment that Embulk is going to remove
msgpack-corefromembulk-api's dependencies. - Q:
msgpack-core:0.8.24is expected to be the last 0.8?
- Then, we'll introduce a workaround class
org.embulk.spi.json.JsonValueto encapsulateorg.msgpack.value.Value.
The removal in Embulk API will be much much later to wait for plugins to catch up with the latest manner. But, I believe that making a clear declaration during v0.10 would be reasonable and making things smoother.
P.S. You may be interested in this: https://gist.github.com/dmikurube/c48ac6432cb4c2151ff9c75cd253bcd5
Q: msgpack-core:0.8.24 is expected to be the last 0.8?
Yes, unless there are no critical bugs
Uh oh!
There was an error while loading. Please reload this page.
Some
org.msgpack.valueclasses (typicallyorg.msgpack.value.Value) are used in method signatures of other software's API/SPI, unfortunately.https://dev.embulk.org/embulk-api/0.10.31/javadoc/org/embulk/spi/PageBuilder.html#setJson-org.embulk.spi.Column-org.msgpack.value.Value-
Against such cases, I was wondering if those value classes are separated in a different Maven artifact so that API/SPI includes only stable
msgpack-value, and implementation is released from dependency hell...Can I hear your thoughts?
I don't stick with the implementation shown in this PR, such as naming, class structures, and else. This PR is just as a basis for discussion. But such a separation is really helpful.