SensESP icon indicating copy to clipboard operation
SensESP copied to clipboard

Review all DynamicJsonDocument sizes, adjust as appropriate

Open ba58smith opened this issue 4 years ago • 9 comments

When we switched to ArduinoJson 6, we made all of the DynamicJsonDocuments the default size of 1024. Most of those in SensESP are fixed size, and can be made smaller, but there are some that are variable in size, such as the Json for all of the configuration data, that need to by dynamic.

Review all of these: make the variable ones vary, and make the fixed sized ones an appropriate size.

ba58smith avatar Dec 12 '20 14:12 ba58smith

@ba58smith - I noticed an anomaly while adjusting the DynamicJsonDocument json_doc(1024) for the message emitted with the attitude data. I had calculated that the message could be up to 152 chars max, so I figured making it 200 would be fine. In a fit of curiousity though, I tried a value of 100 first, knowing that the message is actually longer than that and something bad should happen. However I didn't immediately see any effects and the code seemed to run OK.

I've got it on my todo list to investigate further (I could for example have been mistaken about the actual msg size). For now I'm using 200 chars in the orientation code. Just thought I'd toss this into the mix...

BjarneBitscrambler avatar Jan 01 '21 00:01 BjarneBitscrambler

@BjarneBitscrambler - I seem to remember that this came up when @mairas and @joelkoz were discussing the best way to handle this. (That it doesn't cause an error if the doc isn't big enough.) If you can do some additional testing, and come up with a way to know if/when we ever have a doc that's too small, that would be great... but certainly not critical.

ba58smith avatar Jan 02 '21 14:01 ba58smith

A bit of reading about the ArduinoJson library and some testing clarified what was happening: the StaticJsonDocument and DynamicJsonDocument objects are well-protected against overwriting memory that isn't theirs. Any attempt to add too many entries to the JsonDocument causes entries to be omitted from the final string. I suspect it's the last-added items that are left out, but I haven't confirmed that yet. Anyway, the library guarantees that it will not trash memory if one under-allocates for a doc.

There's a method JsonDocument::overflowed() that tells whether the JsonDocument was large enough to store all values. Details are at https://arduinojson.org/v6/api/jsondocument/overflowed/ (it's new in version 6.17.0 of the library). There are also calls like JsonDocument::memoryUsage() to indicate the current number of bytes used in the JsonDocument.

It would add a bit of size to SensESP code, but one could call overflowed() after formulating the JSON message to check whether one has lost values. Not sure of what to do with the information - certainly it could be printed to the debug port, or a fancier approach would be to increment a variable used in the DynamicJsonDocument doc( size_needed ) call - one would omit data from the first few Signal K messages but eventually the variable would reach a value where there were no more overflows.

I'm next going to check whether a straightforward count of all the chars in a maximum-sized message is an accurate prediction of the size of JsonDocument to allocate. I'd assume so... :-)

BTW, you may already know this, the difference between StaticJsonDocument and DynamicJsonDocument is that the former allocates from the stack and is faster than the latter, which allocates from the heap. Neither one is resizable - if you want a bigger object, a new one has to be created.

BjarneBitscrambler avatar Jan 03 '21 02:01 BjarneBitscrambler

Nice research! Like you, I'm not sure what to do with the info, though.

ba58smith avatar Jan 03 '21 02:01 ba58smith

Thanks! I spent a bit more quality time with one of my d1_minis today, and clarified how the JsonDocument allocates memory. I have a formula that predicts what size is needed in the constructor for DynamicJsonDocument(). Hope this is useful :-)

Summary

To estimate how many bytes to request in the call to DynamicJsonDocument(), add the following:

  • 16 bytes for each key - value pair
  • 16 bytes for each parent node of a nested key - value (regardless of how many values are children of the node)
  • number of characters + 1 for each key or value that is specified using a String variable (as opposed to a constant like json_doc["path"] = ...)

It's a bit of a pain to apply the formula if you want an exact answer. The main point is that the amount of memory one needs to ask for in the DynamicJsonDocument() call is only loosely correlated to the length of the resulting json string. The amount of memory needed is more related to the number of key/value pairs, whereas the length of the json string is more determined by the length of the contents.

I did find the json_doc.memoryUsage() call useful though, as it tells what is actually used. So one approach to optimizing SensESP JsonDocs is to insert a call to memoryUsage() for debugging purposes and record this number. Then inspect the code to locate any variable strings that are used as keys or values, and adjust the number upward by the expected maximum length of these variables.

Example

Here's an example with 6 key/value pairs, 1 parent node, and two strings used for keys or values, where the expected size needed is 131. The comments for each key/value illustrate the numbers. Running this example and examining the output reveals that the estimate is correct.

void testjson( int docsize ) {
  //creates a JsonDocument object of specified docsize
  //fills it with an example and prints the json string and various debugging info

  Serial.printf_P(PSTR("heap BEFORE: %d. "), ESP.getFreeHeap());
  int val = 6;
  String my_string = "14 char string";
  String my_key = "key";

  DynamicJsonDocument json_doc(docsize);  //expect to need 7 * 16 + 19 = 131 bytes see below.
  json_doc["A"] = (char*)0;               // 16 bytes for key - value pair
  json_doc["B"] = val;                    // 16 bytes for key - value pair
  JsonObject nest = json_doc.createNestedObject("nest"); // 16 bytes for parent node
    nest["X"] = 3;                        // 16 bytes for key - value pair
    nest["Y"] = 4;                        // 16 bytes for key - value pair
  json_doc["C"] = my_string;              // 16 bytes for key - value pair + 15 bytes for my_string
  json_doc[my_key] = "Some long string";  // 16 bytes for key - value pair + 4 bytes for my_key
  String json;
  serializeJson(json_doc, json);
  Serial.printf("Docsize %d.  String len %u. C-str len %u. ", 
                docsize,
                json.length(), 
                strlen(json.c_str()));
  Serial.printf("Mem use %d. Overflow=%d for string: %s",
                json_doc.memoryUsage(), 
                json_doc.overflowed(), 
                json.c_str());

  Serial.printf_P(PSTR("heap AFTER: %d\n"), ESP.getFreeHeap());

}

Conclusions

Overrunning the allocated memory causes the resulting json string to be trimmed. Trimming happens on a last-added, first-removed basis, and one key-value pair is removed at a time. (More accurately, I suspect that any calls to add key-value pairs after the existing memory is fully used, are ignored).

It makes no difference in the memory requirement how long a numeric or boolean or null value is, and whether that value is expressed using a variable or a constant. However, string variables (including the const String my_string = "xyz" kind) used for either keys or values need to have their lengths added to the total.

Trimming always leaves the json string properly formed - however it just might be missing the key/value that you are looking for. The call json_doc.overflowed() indicates whether trimming has happened or not.

BjarneBitscrambler avatar Jan 04 '21 02:01 BjarneBitscrambler

Nice work, @BjarneBitscrambler! You should publish that somewhere that Google will find it more easily, as I'm sure we're not the only people who have ever wondered about it. For SensESP, would a simple approach be something like if (json_doc.overflowed()) { DebugE("DynamicJsonDocument size too small") }, everywhere we use a DynamicJsonDoc? We could get trickier, comparing json_doc.memoryUsage() to doc_size, and having a debugE message like "Your DynamicJsonDocument doc_size it too small by XXX", which would make it easier to fix it, I suppose, and not any more difficult.

ba58smith avatar Jan 04 '21 13:01 ba58smith

Your simple approach sounds fine; that should quickly catch any under-estimates of the needed size. I'm going to add this to the orientation example. The trickier approach unfortunately wouldn't work, as memoryUsage() only reports on what's actually stored in the JsonDoc - anything that's been trimmed off because it won't fit isn't counted. So memoryUsage() will never report a number larger than the original doc_size.

Indeed, others have wondered about this question! In fact, it's been answered (https://arduinojson.org/v6/how-to/determine-the-capacity-of-the-jsondocument/) by the ArduinoJson author. Naturally, I only found this after poking around myself :-) Benoit has a good description of how to arrive at size estimates - and there's even a couple of macros and an online tool to help with the estimates.

BjarneBitscrambler avatar Jan 04 '21 16:01 BjarneBitscrambler

It should be known that @mairas and I discussed this issue offline several weeks ago. It is important to note that part of the "solution" is to prevent memory from becoming fragmented. That amount of "free memory" is very different from the amount of "contiguous free memory", of which DynamicJsonDocument needs the later.

We are familiar with the "how to determine capacity" page, and the most important section of that page is the very end, that reads:

Don’t overthink this problem
Keep things simple.
If your program computes the capacity at run-time, 
you’re trying to be too smart.

You should not be looking for the exact memory consumption in each 
possible configuration. Instead, you should pick one size that is big 
enough to support all valid configurations.

joelkoz avatar Jan 04 '21 18:01 joelkoz

@joelkoz - are you saying this Issue has been dealt with, or just that when it is dealt with, it doesn't need to be too complicated?

ba58smith avatar Jan 07 '21 12:01 ba58smith