openfoodfacts / openfoodfacts-server

Open Food Facts database, API server and web interface - 🐪🦋 Perl, CSS and JS coders welcome 😊 For helping in Python, see Robotoff or taxonomy-editor
http://openfoodfacts.github.io/openfoodfacts-server/
GNU Affero General Public License v3.0
646 stars 374 forks source link

Which JSON reading/writing module should we use? JSON::PP, JSON::XS, JSON::Create / Parse, others? #5081

Closed stephanegigandet closed 1 month ago

stephanegigandet commented 3 years ago

What

We currently use JSON::PP to write JSON files (e.g. for the API). It may be a good thing to revisit this, to see if we should switch to JSON::XS (there was an issue in 2016, but hopefully it has been fixed since), or possibly to JSON::Create. There is also a Cpanel::JSON::XS fork.

One way to test them could be to test them on all the .sto files for the products, make a write and read round trip, and see if any data is different (or if it crashed).

It would be interesting to test the performance too. JSON::PP is pure Perl, so the others should be much faster.

Part of

svensven commented 3 years ago

ah, you're already ahead of me :D

svensven commented 3 years ago

for the record, https://metacpan.org/pod/JSON#DESCRIPTION says

This module is a thin wrapper for JSON::XS-compatible modules with a few additional features. All the backend modules convert a Perl data structure to a JSON text and vice versa. This module uses JSON::XS by default, and when JSON::XS is not available, falls back on JSON::PP, which is in the Perl core since 5.14. If JSON::PP is not available either, this module then falls back on JSON::backportPP (which is actually JSON::PP in a different .pm file) bundled in the same distribution as this module. You can also explicitly specify to use Cpanel::JSON::XS, a fork of JSON::XS by Reini Urban.

svensven commented 3 years ago

https://perlmaven.com/comparing-the-speed-of-json-decoders (don't bother watching the video)

stephanegigandet commented 3 years ago

This is the bug that made me remove JSON::XS in 2016, but I have never investigated it since: #469

svensven commented 3 years ago

per https://metacpan.org/changes/distribution/JSON-XS

4.0 Fri Nov 16 00:06:54 CET 2018

  • SECURITY IMPLICATION: this release enables allow_nonref by default for compatibility with RFC 7159 and newer. See "old" vs. "new" JSON under SECURITY CONSIDERATIONS.
CharlesNepote commented 3 years ago

I'm not sure that this is relevant here but some people are yelling that the JSON results are not always sorted the same way. I also think it's interesting to have the fields always sorted the same way: it makes it easier to find a field, for example at the beginning or at the end of the document. Here are two examples of the same product, I just reloaded the page:

image

image

svensven commented 3 years ago

I'm not sure that this is relevant here but some people are yelling that the JSON results are not always sorted the same way. I also think it's interesting to have the fields always sorted the same way: it makes it easier to find a field, for example at the beginning or at the end of the document.

Looks like there's an option to sort the keys, but it adds overhead: https://metacpan.org/pod/JSON#canonical

stephanegigandet commented 3 years ago

"This is adding a comparatively high overhead." --> it's worth testing how much overhead it adds, it may not matter much (especially as we are not using JSON::XS yet)

stephanegigandet commented 3 years ago

I got a strange message while installing JSON::XS on a server through cpan:

*** The stability canary says: (nothing, it was driven away by harsh weather)


It seems you are running perl version 5.028001, likely the "official" or "standard" version. While there is nothing wrong with doing that, standard perl versions 5.022 and up are not supported by JSON::XS. While this might be fatal, it might also be all right - if you run into problems, you might want to downgrade your perl or switch to the stability branch.


*** If everything works fine, you can ignore this message.

stephanegigandet commented 3 years ago

arf

https://stackoverflow.com/questions/46802396/is-this-canarystability-rant-a-legitimate-concern

github-actions[bot] commented 8 months ago

This issue has been open 90 days with no activity. Can you give it a little love by linking it to a parent issue, adding relevant labels and projets, creating a mockup if applicable, adding code pointers from https://github.com/openfoodfacts/openfoodfacts-server/blob/main/.github/labeler.yml, giving it a priority, editing the original issue to have a more comprehensive description… Thank you very much for your contribution to 🍊 Open Food Facts

hangy commented 1 month ago

I think we can close this as done, as we now use JSON::MaybeXS #10578