Review: std.msgpack

[Robert Jacques]

On Fri, 18 Jun 2010 00:29:50 -0400, Andrei Alexandrescu  
<SeeWebsiteForEmail at erdani.org> wrote:

> There's one larger submission that needs review - Masahiro Nakagawa's  
> std.msgpack. I think it would be great if this community established a  
> review process. To do so, we should abstract away starting from at least  
> one review :o). Here's Masahiro's work:
>
> code: https://dl.dropbox.com/u/374829/std/msgpack.d
> doc:  https://dl.dropbox.com/u/374829/std/msgpack.html
> zip:  https://dl.dropbox.com/u/374829/std_msgpack.zip (archives code and  
> doc)
>
> Feel free to comment here.
>
>
> Andrei

First, +1 to the review process.

Second, a lot of the design and examples are very similar to the MsgPack  
reference implementations, which are all Apache licensed. Since some of  
these design decisions seem to not be 'D'-ish (more on that later) this  
raises questions on just how clean room the implementation was.

Third, there's a lot of things that shouldn't be in the final module.
-Should be removed:
SimpleBuffer, VRefBuffer/vRefBuffer,
-Redundant with other parts of phobos:
BinaryFileWriter => LockingTextWriter, mp_Type/mp_Object/mp_KeyValue =>  
std.variant
-Should be moved elsewhere
DeflateFilter/deflateFilter => zip/zlib

Forth, msgpack was designed to be a simple, wire level encoding of some  
higher structure. As opposed to JSON or XML, you are not going to be  
modifying it dynamically. The very concept that it would use buffers  
internally, particularly the existence of the "zero-copy" buffer, baffles  
me. (Not to mention it leaves one open to aliasing bugs.) MsgPack has to  
decorate every single type it stores, so there is nothing "zero-copy"  
about it. To that end, I think MsgPack/MsgUnPack should wrap an input or  
output range as appropriate and be able to be used by a general  
serialization library such as Orange.

Other little things,
-Why isn't nil mapped to null?
-Why isn't there any big warnings about serializing cycles?