search

a2 / MessagePack.swift

It's like JSON, but fast and small…and Swift! – msgpack.org[Swift]
http://msgpack.org
MIT License
283 stars 60 forks source link

Added MessagePackEncoder/Decoder for Codable support #61

Open andrew-eng opened 6 years ago

andrew-eng commented 6 years ago

Hi,

I am using MessagePack pod in my Swift project and it is great. However, currently it is quite painful to manually encode and decode my models.

I have written an implementation of MessagePackEncoder that heavily references JSONEncoder.swift from the swift foundation library. The structure and logic is almost identical to that of JSONEncoder.swift. However, it turns out that implementing a Encoder/Decoder is non-trivial and the logic is rather involved.

Therefore, I am not sure if incorporating this code into your project is the the right way forward as your library is very light weight.

Nevertheless, I'll submit a PR to further discuss this issue.

andrew-eng commented 6 years ago

Thats a valid point, I think this is definitely considered as a derivative work of JSONEncoder.swift. A rewrite will be necessary to resolve this and it might take awhile. How is your implementation coming along?

mgadda commented 6 years ago

@andrew-eng I'm not a lawyer of course. And perhaps there's case to be made that the protocol-based design of Codable basically requires that the code look pretty close to what JSONEncoder and PlistEncoder. So I'm not sure how much a rewrite will remedy the situation. But I think all you need to do is ensure that you follow the steps outlined in Redistribution of https://swift.org/LICENSE.txt and you should be fine.

As for my implementation, I've gone to a good deal of effort to simplify when possible. For example, because the MessagePack serializer is pure swift, there's no need to convert objects to or from anything deriving NSObject. In some places this reduces the total set of possible types that can be encoded or decoded from Any to just MessgePackValue. I skipped the storage class because it didn't seem to offer much value on top of what's already provided by Array. To be fair, though, it's still quite similar in structure to JSONEncoder, which again seems unavoidable.

<rant> While Codable is a definite improvement over its predecessor, it would have been awesome if the designers had considered just how much duplication is required to actually implement new encoders and decoders. It seems to me like it wouldn't have been that much more work to go one step further and abstract away all that boilerplate from the layer responsible encoding and decoding. As it stands currently, most implementations of Encoder will likely almost exactly the same. This should been obvious when the effort required to implement PlistEncoder involved such a tremendous amount of copy and pasting from JSONEncoder (or vice versa). </rant>

mikecsh commented 6 years ago

Hello, are there still plans to merge this PR? Sound's very interesting!

rustle commented 6 years ago

there are swift.org mailing lists where you could ask for guidance https://lists.swift.org/mailman/listinfo

a2 commented 6 years ago

@rustle @mgadda I've posted this topic on the Swift forums. Let's see what people say.

andrew-eng commented 6 years ago

Hey guys, sorry for the late reply, recently I decided to write my own MessagePack parser because i encounter some performance issue when parsing large data. I also rewrote the Encoder/Decoder from scratch because the code in the above PR is indeed monstrous.

I am planning to port over the new Encoder/Decoder and submit a new PR in a few days time. Hopefully the new PR will be easier to manage and merge into this project.

a2 commented 6 years ago

@andrew-eng Sounds good. Would love to see the Encoder/Decoder stuff you're working on.

cherrywoods commented 6 years ago

I would also love to see your implementation @andrew-eng! How did you manage to shrink the code?

mgadda commented 6 years ago

@andrew-eng I also ended up abandoning this pure swift implementation because it was far too slow for my needs. I ended up using the C-based https://github.com/camgunz/cmp which was many orders faster and integrates easily into an existing swift project.

I did write some wrapping code which may or may not be useful for anyone wishing to use cmp. ~I'll post that in a separate repo this week.~ I just pushed it into a new swift project: https://github.com/mgadda/cmpencoder.

andrew-eng commented 6 years ago

hey guys, I have submitted PR #66 containing a simplified Encoder/Decoder. Still haven't figured out why the tests ran successfully on the Linux machine but not on Mac. Will look at it over the next few days. @cherrywoods, i ended up not implementing several features that are not commonly used, turns out that simplified the code a lot. @mgadda nice find. I'll do a performance test against that. I wrote mine in pure swift while avoiding expensive operations. Curious to see how swift fare against good old C.