zumba/json-serializer

Small json_encode() improvement suggestion

AnrDaemon opened this issue · 4 comments

Instead of wasting CPU cycles in JsonSerializer::calculateEncodeOptions(), just use numeric value.

1856 == JSON_UNESCAPED_SLASHES | JSON_UNESCAPED_UNICODE | JSON_PARTIAL_OUTPUT_ON_ERROR | JSON_PRESERVE_ZERO_FRACTION

or

1344 == JSON_UNESCAPED_SLASHES | JSON_UNESCAPED_UNICODE | JSON_PRESERVE_ZERO_FRACTION

Hi @AnrDaemon. Thanks for suggesting that, but I was doing some performance tests comparing the proposed solution with the current implemented and the difference is not significative.

I copied the class with some changes to allow override the json_encode options and to use regular exception (not triggered on the test, but to make sure it is runnable).
I used 3v4l for the tests, so it is not only my computer or a specific version of PHP and here is the result: https://3v4l.org/AiosH

Resuming each version by the latest version, comparing original options vs proposed options:

  • 5.5.32: 1.0839021205902 vs 1.0706119537354 (1.22% faster)
  • 5.6.18: 0.97606897354126 vs 0.98555397987366 (0.97% slower)
  • 7.0.3: 0.48688697814941 vs 0.49011421203613 (0.66% slower)

So, based on the results above, it doesn't increase the performance, except on 5.5 by a little. Also, changing the json_encode options could cause backward incompatibility that I would like to avoid if it doesn't increase considerably the performance. Also, my test was running 10k iterations, so the best case (on 5.5), it would save 0.001ms in on serialization.

PS: I didn't include JSON_PRESERVE_ZERO_FRACTION because I know for sure it decreases the performance (I implemented it on php core :D) and to have a better comparison on PHP 5.5 (where the option is not supported). Also, it is your suggestion and on the original code, so it wouldn't affect the test results.

Thanks for your suggestion, but I guess we will pass on this one.

Since json_encode options is a bit set, it cannot cause backward incompatibilities.
I have a script that runs with same bit mask from 5.3 to 7.0, and the code is much cleaner that way.
Anyway, that was merely a suggestion.
BTW, did I say that your class was a great inspiration? Thanks much for your share!

The BC issue I mention is people expecting one output and receiving another. It can be fine for PHP, but can cause issues if they are using this output with another app in different language or using a different library.

I'm planning to make changes on the library to allow the json options to be passed to the serialize method. Kind like I did on the demo in 3v4l. Allowing you to send the options that you want without breaking/extending anything. The default would be the options from the library.

Thanks for saying it was an inspiration for you. It inspires us from sharing and contributing even more things. 👍

When I'm producing some output that is intended to be consumed by some foreign application, I'm trying to stick to standards as much as possible. If standard says "this is not recommended" or "this is optional", I read that as strictly forbidden, and so on.
Under this light, I don't see it feasible to let application set encoding options.