Rapidjson instead of cjson and dkjson
-
@rafale77 said in Rapidjson instead of cjson and dkjson:
I am even thinking if one could use a symbolic link to replace dkjson with rapidjson to avoid changing any lua code at all...
Yes, I’d thought that too. A solution entirely outside of the code.
-
@rafale77 said in Rapidjson instead of cjson and dkjson:
Simplifying the system install is also doable as it can be treated like any other lua module (example: like luasocket) and add it to the install script of openLuup. It is two lines to be added (one is to install luarocks) and is no different very different from installing cjson...
Well, that may be true if pre-built libraries are available. However, I’ve had openLuup running on a wide variety of machine architectures (including Windows PCs) so it’s not a given that a particular module is available.
-
Hmm you are right. I am a bit too linux focused... The same problem applies to cjson though but a failover to dkjson is probably necessary to cover these cases.
That being said, using the wrapper, so far, there is really no need for cjson any more. -
So after two days of testing, I can confirm that I see a drop by about 10% of my openLuup CPU load from 3.1% to 2.8%.
I poll z-way every 500ms and these numbers are without loading ALTUI which in itself causes a CPU load spike. I have had 0 problem with it having bypassed the wrapper and deleted all my dkjson files and cjson library from my installation. I suspect the 10% gain to be from plugins which previously were running dkjson to now be able to switch to rapidjson rather than a performance difference with cjson.
It isn't the huge benefit that cjson provided over dkjson but I would strongly advocate to replace cjson with rapidjson for both the performance gain and the degree of compliance allowing it, within the openluup json wrapper, to have to switch to dkjson if rapidjson is installed.Edit: And after testing loading ALTUI on browsers for the past couple of hours, the ALTUI CPU spikes are also now significantly and consistently lower, now rarely going over 5% when it used to hit 10% frequently before. See below the spikes on the left are due to ALTUI opening on the browser before the tweaks. On the right, after 6/13 midnight are the same actions ALTUI spikes with rapidjson.
-
So this morning I tried really hard to overload ALTUI and cause openLuup to hang... and look at this:
The CPU spikes are really tiny compared to what they were before on the left, before rapidjson implementation!
And no I still could not get it to hang on socket forced closure... -
rafale77replied to akbooer on Jun 29, 2020, 7:58 PM last edited by rafale77 Jun 29, 2020, 3:59 PM
I am so happy with this discovery... See how the CPU utilization never goes above 5% anymore? As a reminder, previously with DKjson and even with the openLuup json wrapper with cjson, any opening of an ALTUI page would jack the CPU utilization to 10-12%. An update from Homewave would send it up to 8% both up from the same baseline of 3-3.2%, so up 9% and 5% respectively from the base. You can see here that Homewave poll gets the CPU to ~4% from 2.8% (1.2% increase), while ALTUI gets it to go up to 4.7% (1.9% increase).
ALTUI DKjson -> Rapidjson: 9% -> 1.9% --> 79% CPU load reduction
Homewave DKjson -> Rapidjson: 5% ->1.2% --> 76% CPU load reductionNeedless to say that the page loading/screen update speed on the UI and Homewave are very noticeably faster as well.
-
Yes, haven’t forgotten, but it needs a bit of work on my side that’s more substantial than the simple updates I’ve done very recently. Will get to it ASAP.
-
Yes, you would need to replace all the references to dkjson in every code outside the json wrapper to refer to the wrapper and then inside the wrapper to replace cjson with rapidjson. openLuup not the only thing though as all the plugins then need to load the wrapper instead of dkjson when they see they are using openLuup. I manually changed code on my entire openLuup installation... including the lua files in the cgi folder... easy to forget these.
The alternative would be to use a symbolic link? By adding a luarocks and rapidjson install in the openluup installation file and then deleting the dkjson file for openLuup and replacing it with a symbolic link to rapidjson? Not sure if this would work everywhere.
-
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.
