Openhab-addons: [rrd4j] Very poor performance on resource contrained systems

[ghys]

(Already discussed in https://github.com/openhab/openhab-addons/pull/8866#issuecomment-719944078)

I have noticed a huge performance degradation on the retrieval of data persisted in rrd4j
on a PINE A64 2GB in recent snapshots (#1998), when the same operation on the same data used to be fast on the M1
release.

I'm attaching an example of RRD file where this can be reproduced:
ElectricityMeter_Current (Number:Current)
FGDW002WindowSensor_Temperature (Number:Temperature)
rrd-examples.zip

The performance is as follows - these are exactly the same requests except in the second case the rrd4j persistence addon was uninstalled and replaced with the .jar from the Milestone 1 release:

3.0.0-SNAPSHOT (distribution version)

time curl -o /dev/null http://openhab:8080/rest/persistence/items/ElectricityMeter_Current?starttime=2020-10-31T10%3A47%3A36.624Z
real    0m2.996s   < !!!
user    0m0.030s
sys     0m0.012s

time curl -o /dev/null http://openhab:8080/rest/persistence/items/FGDW002WindowSensor_Temperature?starttime=2020-10-31T10%3A47%3A36.624Z
real    0m0.618s   < !!
user    0m0.031s
sys     0m0.013s

3.0.0.M1

time curl -o /dev/null http://openhab:8080/rest/persistence/items/ElectricityMeter_Current?starttime=2020-10-31T10%3A47%3A36.624Z
real    0m0.071s
user    0m0.023s
sys     0m0.018s

time curl -o /dev/null http://openhab:8080/rest/persistence/items/FGDW002WindowSensor_Temperature?starttime=2020-10-31T10%3A47%3A36.624Z
real    0m0.067s
user    0m0.019s
sys     0m0.021s

[wborn]

What might help is to only get the item once from the registry instead of for every historic item in the loop. I.e. remove it from mapToState and cache the item in a variable. WDYT @kaikreuzer?

[wborn]

Can you test https://github.com/wborn/openhab-addons/commit/982640decf892f0a7d957c9b37e497206bec8a7e @ghys?

[kaikreuzer]

Sounds like an excellent idea!

[J-N-K]

It this a regression that should block M2?

[ghys]

Can you test wborn@982640d @ghys?

I will but I don't think this is it.
I think the poor performance occurs when mapToState returns a QuantityType here:
https://github.com/openhab/openhab-addons/blob/7850904c71652f59dd60929263be8ee2f3c83e66/bundles/org.openhab.persistence.rrd4j/src/main/java/org/openhab/persistence/rrd4j/internal/RRD4jPersistenceService.java#L400

I have tested with a Number item without a dimension and there's no performance issue, same thing with a Switch item.

My guess is that it thinks that the stored value is not in the same unit as the one requested so there's some conversion going on.
(note that the data being queried was persisted before #8866).

[kaikreuzer]

It this a regression that should block M2?

Hard to say. At least I'll wait for the moment. Unfortunately, I am not at home and thus can't help.

[ghys]

Can you test wborn@982640d @ghys?

Done and it doesn't help:

270 │ Active │  80 │ 3.0.0.202011011228      │ openHAB Add-ons :: Bundles :: Persistence Service :: RRD4j

time curl http://openhab:8080/rest/persistence/items/ElectricityMeter_Current?starttime=2020-10-31T10%3A47%3A36.624Z
real    0m3.094s
user    0m0.025s
sys     0m0.016s

however I also reverted a change of #8866 based on your branch:
https://github.com/openhab/openhab-addons/commit/52b4caa960d9d4cef8e337467ce41716b138cd2c
and it solves the performance problem;

271 │ Active │  80 │ 3.0.0.202011011233      │ openHAB Add-ons :: Bundles :: Persistence Service :: RRD4j

time curl http://openhab:8080/rest/persistence/items/ElectricityMeter_Current?starttime=2020-10-31T10%3A47%3A36.624Z
real    0m0.097s
user    0m0.026s
sys     0m0.012s

I know it's more correct to return a QuantityType but IMHO it isn't worth the performance impact.

[wborn]

Well it was worth a try. So we can choose for the M2 to either have bad performance (this issue) or reintroduce the issue of the QuantityType not properly restored (https://github.com/openhab/openhab-addons/issues/8809). I have don't have a strong preference for both. Or does anyone have a 3rd option (except for postponing the M2 until both are fixed)?

[wborn]

The bad performance probably has more impact because it causes issues all the time whereas the restore on startup behavior only occurs on startup... which occurs less often. WDYT?

[ghys]

the QuantityType not properly restored (#8809)

Wouldn't it be fine to handle this particular case (#8809) in the code when the state is restored on startup? Maybe around here:
https://github.com/openhab/openhab-core/blob/0ac14b9f8f03ab8201c599b27800e39ceb575537/bundles/org.openhab.core.persistence/src/main/java/org/openhab/core/persistence/internal/PersistenceManagerImpl.java#L287

[J-N-K]

Are you sure mapToState is the problem? There is not much code differrence between the instantiation of a DecimalType and a QuantityType. Maybe the problem occurs later, when the state is converted again for the REST output?

[wborn]

I created https://github.com/openhab/openhab-addons/pull/8929, but I have some other things to do now. But you can merge it if you want to. :-)

[ghys]

Maybe the problem occurs later, when the state is converted again for the REST output?

Yes, possibly.

The bad performance probably has more impact because it causes issues all the time whereas the restore on startup behavior only occurs on startup... which occurs less often. WDYT?

Not my decision, but I wouldn't ship M2 with this regression. It will probably affect more users who run openHAB on SoCs after an upgrade from M1 to M2 (rrd4j is the persistence service installed by default), than relieve those who already suffered from #8809 in M1 anyway. And there are workarounds until it's fixed properly, like using MapDB to restore states on startup instead of rrd4j.

[kaikreuzer]

Are you sure mapToState is the problem? There is not much code differrence between the instantiation of a DecimalType and a QuantityType.

I'd also actually first like to understand why this makes a difference, it does not appear logical.

[cpmeister]

I wonder if it has to do with how the core is creating the QuantityType instances. It looks like they are converting numbers to strings and back every time you create a QuantityType. I could imagine this would be very slow.

https://github.com/openhab/openhab-core/blob/5a9c5e7d87716406302d5b1eb1e5966bab8195b8/bundles/org.openhab.core/src/main/java/org/openhab/core/library/types/QuantityType.java#L113

[kaikreuzer]

it does not appear logical.

Ok, I think I found a logical explanation and fixed it with https://github.com/openhab/openhab-addons/pull/8938.