Skip to content
This repository has been archived by the owner on Feb 17, 2023. It is now read-only.

Change JSON library from JSON:PP to JSON::XS #419

Closed
wants to merge 1 commit into from

Conversation

nishesj
Copy link
Contributor

@nishesj nishesj commented Jan 9, 2014

Change the json parser from PP to XS

@zzamboni
Copy link
Contributor

zzamboni commented Jan 9, 2014

What's the reason for this? We moved away from XS (which requires compilation) to be able to include the JSON modules with cf-sketch. --Diego

On Thu, Jan 9, 2014 at 12:15 PM, Nishes Joshi notifications@github.com
wrote:

Change the json parser from PP to XS
You can merge this Pull Request by running:
git pull https://github.com/nishesj/design-center json-xs
Or you can view, comment on it, or merge it online at:
#419
-- Commit Summary --

@nishesj
Copy link
Contributor Author

nishesj commented Jan 9, 2014

The dc Api is twice faster with the XS than PP. So if we can switch or better fallback to PP if XS is not found is better rather than hardcoding PP library or XS. And JSON lib that we are using i think does it by default as well..

@zzamboni
Copy link
Contributor

I thought this was already the case - @tzz?

On Jan 9, 2014, at 12:26 PM, Nishes Joshi notifications@github.com wrote:

The dc Api is twice faster with the XS than PP. So if we can switch or better fallback to PP if XS is not found is better.


Reply to this email directly or view it on GitHub.

@tzz
Copy link
Contributor

tzz commented Jan 10, 2014

Currently we use PP because XS can't handle bareword keys.

The speed difference is 10x so we should use XS if we can.

It may be smarter to try a XS parse, then drop to PP if the XS parse failed.

tzz added a commit to tzz/design-center that referenced this pull request Jan 21, 2014
cfengine_stdlib.cf: fix yum_rpm package method so that specifying arch is possible
tzz pushed a commit to tzz/design-center that referenced this pull request Jun 1, 2014
Updated enterprise api architecture image
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants