Thrashing heap on ESP8266

[symbioquine]

I have been using an ESP8266 and this awesome library to capture the key metrics from some photovoltaic equipment.

Unfortunately, I found with the number of metrics I was recording - approximately 1500 bytes/point metric line - that this library wasn't stable.

From what I can tell the heap gets fragmented really easily because this library internally does a ton of string concatenation on the heap so any other longer lived heap objects which are created concurrently with the batches being buffered will be scattered throughout the heap.

As a work-around/optimization I replaced the code which used this library with a global char[] buffer and some snprintf statements. Roughly as follows;

Before:

Point pointDevice("device_status");
pointDevice.addTag("location", HOSTNAME);

pointDevice.addField("rssi", WiFi.RSSI());

pointDevice.addField("uptime", millis() - setupMillis);
pointDevice.addField("freeHeap", ESP.getFreeHeap());
pointDevice.addField("heapFragmentation", ESP.getHeapFragmentation());
pointDevice.addField("maxFreeBlockSize", ESP.getMaxFreeBlockSize());

// ... lots more metrics recorded here

if (!influxClient.writePoint(point)) {
  Serial.printf("[E] Writing to influxdb failed\n");
  return;
}

After:

// globals

#define dataSize 2048
char data[dataSize];

// Later in loop code

int dataPos = snprintf(data, dataSize, "device_status,location=%s ", HOSTNAME);

dataPos += snprintf(&data[dataPos], dataSize - dataPos, "rssi=%di", WiFi.RSSI());
dataPos += snprintf(&data[dataPos], dataSize - dataPos, ",uptime=%di", millis() - setupMillis);
dataPos += snprintf(&data[dataPos], dataSize - dataPos, ",freeHeap=%di", ESP.getFreeHeap());
dataPos += snprintf(&data[dataPos], dataSize - dataPos, ",heapFragmentation=%di", ESP.getHeapFragmentation());
dataPos += snprintf(&data[dataPos], dataSize - dataPos, ",maxFreeBlockSize=%di", ESP.getMaxFreeBlockSize());

// ... lots more metrics recorded here

if(!httpClient.begin(wifiClient, influxUrl)) {
  Serial.printf("[E] Begin request to influxdb failed\n");
  return;
}

httpClient.addHeader(F("Content-Type"), F("text/plain"));

int statusCode = httpClient.POST((uint8_t*)data, dataPos);

if (statusCode != 204) {
  Serial.printf("[E] POST to influxdb failed\n");
  return;
}

httpClient.end();

This resulted in much more stable operation for my use-case as evidenced by the following graphs;

I don't know whether this sort of problem is "in scope" for this library, but I wanted to post this issue to create some visibility around the potential challenges that the String usage in this library could create.

At the very least I hope this issue can be helpful in case others have the same problem.

• https://hackingmajenkoblog.wordpress.com/2016/02/04/the-evils-of-arduino-strings/
• https://arduinoplusplus.wordpress.com/2019/03/01/are-strings-really-that-evil/
• https://learn.adafruit.com/memories-of-an-arduino/optimizing-sram

[vlastahajek]

@symbioquine, thanks for raising this topic. I'm aware of the possible memory problem. I did memory optimization that I was testing it for a long time with better result. However, before making this available, due to another reason I rewrote internal buffer handling logic and this also better handles memory. It is already available in the master. Could you, please, check it out?

[symbioquine]

Hi @vlastahajek Thanks for the quick reply!

Some of my testing was also done with a pretty recent version checked out from the master branch of this repository so I think the problem still exists.

At a glance I would guess the string concatenation happening in the following code might be at fault;

https://github.com/bonitoo-io/influxdb-client-for-arduino/blob/c135e9caae29428a8c75035495609b12580b5828/src/Point.cpp#L40-L74

I suspect Point::toLineProtocol() is the worst part in that regard since it builds up the largest String in many steps. I haven't profiled it yet, but a rewrite to something like the following would probably help;

String Point::toLineProtocol() const {
    String line  =  _measurement;
    // May allocate up to 3 extra bytes for unneeded separators - that could be avoided at the cost of more code complexity
    line.reserve(1 + _tags.length() + 1 + _fields.length() + 1 + _timestamp.length());
    if(hasTags()) {
        line += ",";
        line += _tags;
    }
    if(hasFields()) {
        line += " ";
        line += _fields;
    }
    if(hasTime()) {
        line += " ";
        line += _timestamp;
    }
    return line;
}

I think the way this was written before (e.g. with line += "," + _tags) it created an intermediate string with "," + _tags then added it to line. That means it was creating as many as 3 intermediate strings and possibly realloc'ing up to 6 times.