Rapidjson instead of cjson and dkjson
-
@akbooer, have you thought about using rapidjson instead of cjson?
I am testing it on ALTUI and doesn't appear to be as strict as cjson while being even faster.
Benchmarks comparing dkjson, cjson and rapidjson halfway down the page:
performance/nulls.json: (x10000) module decoding encoding dkjson 1.5754740238 0.0276830196 cjson 0.0551309586 0.0487358570 rapidjson 0.0545330048 0.0497119427 performance/booleans.json: (x10000) module decoding encoding dkjson 1.4916000366 0.5738999844 cjson 0.0556819439 0.0495460033 rapidjson 0.0491378307 0.0377278328 performance/guids.json: (x10000) module decoding encoding dkjson 2.3570721149 3.0556299686 cjson 0.1626739502 0.1092689037 rapidjson 0.1135668755 0.0830221176 performance/paragraphs.json: (x10000) module decoding encoding dkjson 8.5023150444 31.5211300850 cjson 0.8555159569 0.6739630699 rapidjson 0.3181121349 0.1543619633 performance/floats.json: (x10000) module decoding encoding dkjson 2.1173479557 1.7756278515 cjson 0.1096220016 0.3009288311 rapidjson 0.0733149052 0.1116719246 performance/integers.json: (x10000) module decoding encoding dkjson 1.7672340870 1.3564231396 cjson 0.0811710358 0.2808339596 rapidjson 0.0581030846 0.0484278202 performance/mixed.json: (x10000) module decoding encoding dkjson 16.8118801117 13.2526481152 cjson 1.3187069893 0.7380268574 rapidjson 1.3779308796 0.4688320160
Edit: I have implemented it across all of my plugins without the openLuup.json wrapper and have been running error free. Will let you know in a couple of days if I would recommend this but it is looking very good so far... It can completely and seemlessly replace dkjson.
installation instruction:
luarocks install rapidjson --server https://luarocks.org/dev
Then run a replace of dkjson and cjson to rapidjson in every lua file in the openluup and plugin lua file folder.
-
It seems like there is a module within openLuup which is not handling the response from rapidjson well. I have not gotten to the bottom of it but I will. Otherwise all the plugins I tested work fine (ALTUI, ALTHUE, Harmony, SiteSensor, Darksky, DEM etc...)
Edit: found it. It was a logging feature specifically calling the wrapper with a function which obviously doesn't exist with the library. I got the whole thing to now work with dkjson completely replaced and without any wrapper. Will post on my openLuup fork on GitHub.
-
You can see from this screenshot of the CPU utilization in light blue the difference it makes:
The baseline is not really changed even if you can see a difference of ~0.2-0.3%.
The 3 spikes on the left are caused by a browser opening the openluup ALTUI page and you can see a spike to 10-13%.
The middle area is me running testing and implementing rapidjson gradually to the whole openLuup instance. The right most spike is now the height of the spike when I open an openLuup instance. It only goes up to 7% instead of the previous 10-13%.The actual main advantage of this approach is actually really to avoid having a wrapper to decide between two different json encode/parser/decoder option depending on which one works. We can only use one unique one which works across the board which is also marginally faster than cjson but over 10x faster than dkjson.
-
Thanks for pointing this out.
- the whole idea of a module wrapper is that you don't have to change code anywhere else.
- the whole idea of a pure Lua stand-alone version is to simplify system installs.
- the speed tests don't show a lot of difference between it and Cjson, frankly...
...but I do like the fact that sorting and pretty-printing is an option (as in the original openLuup version.)
I'll add another check within the wrapper to see when it's available, and use it, but I haven't any plans to remove the original, or the Cjson option.
-
I am somewhat at a loss.
To make use of the wrapper, people need to change the code anyway. Whether calling the wrapper or the library directly makes very little difference in the code change, it is a replacement to dkjson. Most vera plugins don't call the wrapper today and the wrapper does not exist on the vera... @mrFarmer 's Harmony plugin is an example where a test and switch was added to the plugin allowing switching between the two library and it doesn't make use of the wrapper.
I though the idea of using the wrapper was to make up for the fact that cjson was not 100% compliant with dkjson. You can see from the benchmark test that it is indeed only 67% conforming to standards vs. 100% for rapidjson. This is the reason why cjson can't handle everything dkjson can.
I rapidjson as a better alternative to cjson, not just due to the very little performance improvement but because it eliminates the need to maintain 2 json handlers.
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...
So I am puzzled... With all due respect, it really makes little sense to maintain a wrapper to manage two json handlers to make things run faster when you can do the same thing by using a drop in replacement, or worst case, modify the code like @mrFarmer did which one would have to do to use the wrapper anyway.
This makes the system much simpler smaller and lighter and as you can see, faster as well, not for the calls already using cjson but for all the remaining ones which are still using dkjson and relying on the wrapper... -
I am even thinking if one could use a symbolic link to replace dkjson with rapidjson to avoid changing any lua code at all...
-
@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. -
rafale77wrote on Jun 15, 2020, 4:21 AM last edited by rafale77 Jun 16, 2020, 11:06 AM
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.
-
-
This is rather good news!
-
-
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:
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 = true
option. It's vital if I'm going to substitute the internal module with RapidJson. Do you know if that breaks anything you use?
5/31