[JSON][Review] Another Boost.JSON review

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

[JSON][Review] Another Boost.JSON review

Boost - Dev mailing list
Howdy,

I vote to ACCEPT Boost.JSON into boost. I think it’s a fine implementation for what it’s intended for.

I don’t understand some of the other reviews that reject it for not being something else - they got an apple and are complaining it’s the wrong color for an orange.

It’s fine to prefer oranges over apples, but it’s also ok to offer apples for sale in your store even if you wouldn’t buy it yourself. A lot of people like apples - they’re simple and convenient, without having to peel away the layers of complexity involved in something more pulpy. (Ok, I’ve squeezed as much juice out of that metaphor as possible...)


 - What is your evaluation of the design?

Good. It took me a bit to get used to it, because I was used to something else (Facebook’s folly::dynamic mostly); but that would be true of any new library.


 - What is your evaluation of the implementation?

I didn’t study the internal details. I used it as a consumer of the library. I thought the user-facing API was reasonable. The one change I made to the code was to force it to use std::string_view instead of boost::string_view, even though I used it in non-standalone mode. It would be nice if there were a separate compile flag for that, but it’s not a big deal.

Having yet another custom/purpose-built string implementation was surprising. It’s not an issue, but just surprising that yet another string type is being created.

It was also a bit surprising there wasn’t a function to pretty-print the serialized output (e..g, via an option arg for `serialize()`). It’s fairly common to need such a thing, for example to write to user-viewable files (e.g., configuration or property files), and potentially to debug logs, etc. Obviously the user can write a pretty-printer of their own, but this is one of those things that I think should come out-of-the-box eventually.


 - What is your evaluation of the documentation?

Very good. As a consumer, it had most everything I needed.


 - What is your evaluation of the potential usefulness of the library?

I think it’s very useful.

Whether it will become _popular_ I don’t know. There’s no debate there is high demand for a JSON-based variant-structure library similar to Boost.JSON, but there are several popular choices already. Boost.JSON has better performance and memory usage than some of the ones I know of (nlohman and folly::dynamic), so that should give it a leg up. But those libraries also have a lot of bells and whistles, and it’s not clear how much that will matter to people.

While my company also has a custom implementation for some specific uses, we do still use folly::dynamic fairly heavily, and I would seriously consider migrating to Boost.JSON in our code-base instead.


 - Did you try to use the library?

Yes, I used it in non-standalone mode, and ran it through some tests we have in my employer’s code base.


- With which compiler(s)?

Gcc 7.3, C++17 mode.


- Did you have any problems?

Just a couple bugs that were quickly fixed by the authors.

One test failure I got was with the usage of scientific notation in serialized output, as has been noted by other reviewers. It’s not _wrong_, of course, but it’s unusual - and unusual is frequently bad for interoperability.

Also, I first tried it in standalone and was surprised it didn’t work even though we use C++17 - sadly, gcc 7.3 doesn’t have the pmr::memory_resource implementation. Gcc doesn’t have pmr until version 9+, which is a shame because incorporating it as standalone would be ideal; we do use boost heavily, but we almost always have issues when upgrading boost versions, so we don’t upgrade often.

I think being usable as standalone would increase the uptake for Boost.JSON… but requiring gcc 9+ negates that somewhat, perhaps.


 - How much effort did you put into your evaluation? A glance? A quick
reading? In-depth study?

I spent about a day testing it. I’m sorry I didn’t have more time this past week. Life happens. :(


 - Are you knowledgeable about the problem domain?

I’ve been using `folly::dynamic` from Facebook’s folly library since 2014, and have made changes to it for my company’s code base. It provides a similar variant-style structure based on JSON-ish types, with a parser and serializer for JSON encoding.

I’ve also written a custom JSON-based variant-structure for some of my company’s particular needs, with built-in schema support, JsonPointer and XPath-style query support, etc.

-hadriel


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [JSON][Review] Another Boost.JSON review

Boost - Dev mailing list
On Mon, Sep 21, 2020 at 8:47 PM Hadriel Kaplan via Boost
<[hidden email]> wrote:
> I vote to ACCEPT Boost.JSON into boost.

Thank you for your time and thoughtful review.

> Having yet another custom/purpose-built string implementation
> was surprising. It’s not an issue, but just surprising that yet another
> string type is being created.

I feel you here. What we want is a string type that has a very small
sizeof(), and the same sizeof() across implementations of the standard
library. And we want it to use the memory_resource / storage_ptr
system that Boost.JSON offers. This of course rules out std::string.
It is also nice to have a string type whose API is the same no matter
what version of C++ is used.

> It was also a bit surprising there wasn’t a function to pretty-print the
> serialized output (e..g, via an option arg for `serialize()`).

The current serializer prioritizes speed over anything else, but as
the library matures we will of course see other options.

> Boost.JSON has better performance and memory usage than some of the ones I know of

And this is an area of ongoing research and improvement - it will only
get better!

> unusual is frequently bad for interoperability.

Million dollar quote right here lol

>  Gcc doesn’t have pmr until version 9+, which is a shame
> ...
> I think being usable as standalone would increase the uptake
> for Boost.JSON… but requiring gcc 9+ negates that somewhat

Julien submitted a patch which is in develop and will get merged to master:

<https://github.com/CPPAlliance/json/commit/78c2bd301278bd9590b627451abcffe08b9ae67a>

This makes Boost.JSON use std::experimental::pmr::memory_resource on gcc 8.3.

Regards

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost