Sort hash keys on JSON encode

[pmb0]

Make sure JSON result stays the same for a given hash. This is necessary
for caching.

[jhthorsen]

@boxi0: In what case does the order of the JSON keys matter for caching? Caching should be handled with HTTP cache headers, not the body.

https://developers.google.com/speed/docs/best-practices/caching

Also, it will under no circumstances be possible for us to take a pull request without any tests.

[pmb0]

I'm using the response body's JSON string to calculate an ETag hash for this response. So in this case the order of the JSON keys matter.

[kraih]

Allright, i might have been a bit too quick here, so the change could go away again. Sorting costs quite a bit of performance and we might be better off just recommending sorted hashes as a solution to the rare caching problem.

[kraih]

Sorry for the confusion, my patch now has two 👎 votes, so i'm reverting it. fadc41e

[daoswald]

If the decision to include/not include this feature is based on performance concerns we should decide what is an acceptable performance hit, and then test to see if this surpasses that threshold.

I grabbed the JSON from a GitHub API search, generated a structure from it, and then timed runs with the 'sort' enabled, and with it not (the current code). Here is a gist showing the test:

https://gist.github.com/daoswald/005ef45f06d8bafc5519

I ran the test ten times, first with no sort, and then with sort. Then took the average of the ten runs, and finally calculated a percent change.

The change was a 2.4% performance penalty for sorting the JSON, using the data demonstrated in the gist. The data set was about 162KB of JSON, decoded to a structure, and then encoded repeatedly in a benchmark.

I don't have any idea whether this is an acceptable performance hit to take for the feature, but hopefully this will provide useful information to those giving +1 / -1 to the proposal.

[kraih]

@daoswald Thanks for the detailed benchmark.

[jhthorsen]

I'm 👎

I think you should cache based on input, not output. Or find something else that is unique with the request.

[kraih]

I'm neutral on this issue, 2.4% is not that much of a performance hit, but the use case presented here doesn't convince me either.

[pmb0]

I can solve my problem doing other things, yes. But I think a canonical option like in JSON::JS would be smart.

[pmb0]

Typo: JSON::XS

[kraih]

On the plus side, you might enjoy this new caching helper that also resulted from this discussion. 😉 http://mojolicio.us/perldoc/Mojolicious/Plugin/DefaultHelpers#is_fresh

[pmb0]

I do, thank you. :)