Replace expensive node json calls by jq #1342
Conversation
|
I now realize that since --arg AT_LATEST "`date -u +'%Y-%m-%dT%H:%M:%S.000Z' --date '4 minutes'`"\
--arg AT_EARLIEST "`date -u +'%Y-%m-%dT%H:%M:%S.000Z' --date '-9 minutes'`"\instead. I imagine this is not very important |
|
I am not familiar with Line 14 in acb0150 |
Note: probably still overkill
Dune-jr
changed the title
|
Patched the other location + another call that could be replaced identically by jq, saving a lot of time: pi@pancreas:~/myopenaps $ time json -f upload/latest-treatments.json -a created_at eventType
time jq -r '.[] |.created_at + " " + .eventType' upload/latest-treatments.json
2019-12-31T15:48:28+02:00 Temp Basal
2019-12-31T15:32:57+02:00 Temp Basal
2019-12-31T15:15:30+02:00 Temp Basal
2019-12-31T14:05:56+02:00 Temp Basal
2019-12-31T13:49:31+02:00 Correction Bolus
real 0m44.844s
user 0m15.323s
sys 0m0.401s
pi@pancreas:~/myopenaps $ time jq -r '.[] |.created_at + " " + .eventType' upload/latest-treatments.json
2019-12-31T15:48:28+02:00 Temp Basal
2019-12-31T15:32:57+02:00 Temp Basal
2019-12-31T15:15:30+02:00 Temp Basal
2019-12-31T14:05:56+02:00 Temp Basal
2019-12-31T13:49:31+02:00 Correction Bolus
real 0m0.161s
user 0m0.039s
sys 0m0.012sNote that it is probably still overkill for what it does. There is one last call in but I do not use a g4 so I cannot possibly test that one. |
|
This will break the date handling in OpenAPS further from how it works now, by assuming a specific date format for Nightscout CGM data. Nightscout only guarantees the CGM and treatment dates comply with ISO 8601 and can be both normalised to UTC or use the local time zoned format, with or without millisecond precision. You might see this code work depending on the data sources you have for the data in Nightscout, but this will not work for all data sources. Please do not add more code that doesn't use a real date parser - I can see how making the system more performant is good, but this shouldn't make OpenAPS less compatible with external data sources. |
@sulkaharo so this assumption was wrong. It seems the parsing job falls back on OpenAPS :(
This is a bit of information I was looking for. I see two issues with this implementation:
I imagine there is no way to cover what Nightscout could update with Performance is a big concern to me, the pi0 spends all its time at 1000Mhz and loop times aren't ideal. |
|
Ok, so thinking about this, what we could do is have the code that loads the treatments from Nightscout translate the dates to whatever format Openaps wants to use internally and then have the scripts that need to be as performant as possible to use that format. This will probably slow down the data load from Nightscout, but would allow the rest of the code to trust the date formats. For this, what’s the ideal internal format? We could probably speed things up even more by using something like Javascript’s milliseconds from 1980 ( Date.now() ) which would be inherently compatible with what the oref JavaScript code does as well |
|
@sulkaharo Sounds like a good way to go about it to me. If there is a need for retro-compatibility (which surely there is?), what about an additional Unless you see a way to go about it that does not need backwards compatibility. |
|
If we go with global solution to #1348 than no need to contine here. On the other hand, if we don't than do it, by all means... |
|
@tzachi-dar 100% agree |
|
Just to make sure we are all aligned up here: This PR wanted to replace two parts of the code:
Both of this exists in #1352 Please let me know if anyone sees a problem with this approach. |
|
I believe this is probably unnecessary given #1411 , since the root cause of |
Since I'm having CPU load issues on my pi0, I went to check for some expensive calls that could be optimized away. I found this call:
oref0/bin/oref0-ns-loop.sh
Lines 94 to 97 in acb0150
node /usr/local/bin/json --helptakes 14 seconds on my rasp, so I gladly replaced it with ajqcall. It's quite inexpensive and I did some testing to make sure it outputs the exact same string.The only caveat is that it assumes that the time in
cgm/ns-glucose.jsonis formatted like"%Y-%M-%DT%H:%M:%S.000Z"which I believe is safe.
Here is some output (with -100 instead of -10):
CLICK ME
CLICK ME
Note: I wanted to use jq's strptime and strftime (https://stackoverflow.com/a/49263320/3660320) but it is not necessary since the dates are formatted in ISO 8601 (jqlang/jq#1056 (comment))
TODO: I did not test this in a real loop.