Rapidjson instead of cjson and dkjson
-
Easy enough just to sandbox the require function and substitute any references to other JSONs with RapidJson.
But it does need testing.
-
I just fixed the issue reported here https://smarthome.community/topic/409/car-integration/13 between myself and @toggledbits using rapidjson as I found this option here: https://github.com/xpol/lua-rapidjson/issues/23 which I have implemented in my fork of openluup.
Having had implemented rapidjson as my only json library for quite some time with a lot of success and any need of failover to DKjson, arguably this makes the API more compliant to what vera does:
openLuup/openLuup/requests.lua at development · rafale77/openLuup
a pure-Lua open-source emulation of the Vera Luup environment - rafale77/openLuup
I would suggest switching libraries and implementing this fix. Alternatively, an equivalent option exists with cjson but I still think rapidjson is a simpler, easier implementation since it forgoes the need for failover.
-
The openLuup facility to substitute one required module with another has existed since our original conversation prompted me to implement it. I’ve just never used it, but you’ll find a console page which lists all the modules required by plugins. It’s also possible to replace/add parameters to any specific call. So there’s never any need to do this sort of code modification.
Is there not a RapidJson module-wide option for this? ...or does it have to be done call-by-call?
-
From what I have found, it is an option which needs to be added for each call for which it is needed both for cjson and rapidjson. I also tried to apply this option to other openLuup modules and it seems to break openLuup so there are places where it appears that openLuup is expecting an empty object rather than an array. I therefore only applied this to the external facing calls in the request module.
-
It must be Cjson that breaks with the extra parameter – the built-in module doesn't examine a second parameter.
I would also like to add the
pretty = trueoption. It's vital if I'm going to substitute the internal module with RapidJson. Do you know if that breaks anything you use? -
@rafale77 said in Rapidjson instead of cjson and dkjson:
openLuup is expecting an empty object rather than an array
Just commenting because I'm puzzling over this comment... empty objects and empty arrays are the same thing in Lua... just an empty table. How would it know?
-
rafale77replied to akbooer on Apr 29, 2021, 5:00 PM last edited by rafale77 Apr 29, 2021, 3:16 PM
@akbooer said in Rapidjson instead of cjson and dkjson:
It must be Cjson that breaks with the extra parameter – the built-in module doesn't examine a second parameter.
Well, I am not using cjson at all on my production system (neither dkjson nor cjson are installed). I have completely replaced the built in json.lua module on my system with rapidjson everywhere and I have been running this way for almost one year.
I would also like to add the
pretty = trueoption. It's vital if I'm going to substitute the internal module with RapidJson. Do you know if that breaks anything you use?Yes it works fine, I played with it a little yesterday. The "prettying" adds an extra step to the encoding so its a tiny bit slower but rapidjson is much faster at doing this than other libraries.
@toggledbits said in Rapidjson instead of cjson and dkjson:
@rafale77 said in Rapidjson instead of cjson and dkjson:
openLuup is expecting an empty object rather than an array
Just commenting because I'm puzzling over this comment... empty objects and empty arrays are the same thing in Lua... just an empty table. How would it know?
I will have to test it again. I am learning here and I only attempted changing the jsonifying function in openLuup and openLuup failed to reload after that. I honestly have not spent too much time on it. It could have been a fluke. Edit: Yup probably a syntax error on my part, I retested and it works fine.
Edit2: I brute force enabled both pretty option and the empty field array for all json encoding function call on my openLuup installation and it works perfectly. No noticeable increase even in CPU utilization so far.
-
Any specific use for the prettying of the jsons you have in mind? I have done it now everywhere in openLuup and it doesn't break anything. I looked at my user-data.json file and it is pretty and bigger.
Since I don't typically visually read these json, I am not sure what benefit it gives and may prefer saving memory and storage wear.BTW, no more error on MSR! This was what I originally wanted to address!
-
A lot of the openLuup displays are raw encoded JSON (device user_data, scene definitions, ...) so need to be formatted.
I’ve added RapidJSON to the openLuup.json module, so that it will use it automatically with no code changes, and I’m testing that right now. I was initially seeing CPU drop by 30% (from 0.7% to 0.5%) but I have yet to add automatic substitution for those plugins which still use dkjson (AltUI, AltHue, ...) so there’s more to come... (not that it really matters.)
-
Just FYI, I have substituted the json library in the plugins I run for quite a long time now. ALTUI, ALTHue, SiteSensor, Harmony, IPhoneLocator... etc and have found nothing but improvement. I however did not bother formatting. My user-data.json goes from 1.2MB to 2.2MB when prettying. I have gone back to only added the empty table as array option to the openLuup calls only. So far it's been working great. Looking forward to see if you find any problem.
-
The user_data problem is easy to address: it does need to be formatted, but given that the default time interval is one hour, I’ll probably revert to the original openLuup encoder for that.
-
This is odd, As I said, I have used it "unformatted" with by rapidjson for the past 10months and have just reverted to doing it by removing the pretty option. What is the downside of doing it this way? I have not observed any difference from vera or dkjson actually for this specific file.
Since the json gets decoded, the output of decoding a pretty formatted and unformatted file is identical. You can actually see that ALTUI calls all its js/json cdn in "min" versions to save bandwidth. The difference between the "min" and the full versions is the pretty formatting. My understanding of the pretty formatting was that it was only for visual benefit. Am I missing something?
After tinkering and running it for all this time, I would highly recommend to use a single library for json decoding/encoding. It saves on resources and installation complexity. As noted higher, I completely got rid of dkjson and cjson on my installation. None of my plugins have had any problem whatsoever and even less so openLuup. The main benefit of rapidjson over cjson is its level of compliance which eliminates the need for failover code to DKjson and therefore also eliminates the need for an additional json module.
-
akbooerreplied to rafale77 on Apr 30, 2021, 3:31 PM last edited by akbooer Apr 30, 2021, 11:39 AM
@rafale77 said in Rapidjson instead of cjson and dkjson:
Am I missing something?
If you go to the openLuup console and look at user_data for any device, or json for any scene, then you need to see formatting. For the user_data itself, it's fundamental during my development to have this formatted. (It could be an option that I turn off in production, but experience shows that if you test things which are not the production version, then you get nasty surprises.)
I've just run a back-to-back test with a system comparing the RapidJSON (formatted) backup with an openLuup.json one:
- RapidJSON:
826 kb compressed to 111 kb (7.4:1) - openLuup:
540 kb compressed to 102 kb (5.3:1)
so, indeed, the generous whitespace formatting of RapidJSON adds nearly 300kB to the user_data file, but the LZAP compression used to store the backups does a great job in reducing them to nearly the same size (with a fairly impressive reduction factor.)
@rafale77 said in Rapidjson instead of cjson and dkjson:
I would highly recommend to use a single library for json decoding/encoding.
This is only possible for systems where you can install RapidJSON, and I have two examples of this being rather difficult (older systems where a pre-built library is not available.)
It seems to me that the latest implementation offers (nearly) the best of all worlds: if you have Cjson, it gets used (when it works), if you have RapidJSON, is gets used, if you have neither, that works too. No code changes, nothing extra to do. Works for me.
PS: my CPU has now dropped to 0.3%, down from a starting point this morning of 0.7%, so more than 50% reduction! (Again, not that it makes a huge difference to anything.) I think we are down to a point where we can say that there's really very, very little impact that openLuup has on a machine's free time.
- RapidJSON:
-
Ah ha! Thank you! Indeed I can see the problem in the device user_data lacking formatting. I am wondering though if it is possible to add formatting only to these specific calls.
-
Yes, I’ll get around to that...
...before moving it to development status!
-
Just finished tinkering with my installation and pushed to my fork. I am only now pretty formatting a few selected encoding calls from the "scene.lua" and "console.lua" files
