From 0a67c664459eb7cde85b84890f8564462533572a Mon Sep 17 00:00:00 2001 From: Dirk Mueller Date: Sun, 22 Dec 2019 19:12:54 +0100 Subject: [PATCH 1/2] Reduce temporary string creation/reallocation in HTTPClient This improves both performance due to fewer memory allocations/copies as well as reduces code size by ~ 25% (150 bytes) --- .../src/ESP8266HTTPClient.cpp | 85 +++++++++++-------- 1 file changed, 50 insertions(+), 35 deletions(-) diff --git a/libraries/ESP8266HTTPClient/src/ESP8266HTTPClient.cpp b/libraries/ESP8266HTTPClient/src/ESP8266HTTPClient.cpp index c7a8a5259b..c49afbe201 100644 --- a/libraries/ESP8266HTTPClient/src/ESP8266HTTPClient.cpp +++ b/libraries/ESP8266HTTPClient/src/ESP8266HTTPClient.cpp @@ -1076,12 +1076,14 @@ String HTTPClient::errorToString(int error) void HTTPClient::addHeader(const String& name, const String& value, bool first, bool replace) { // not allow set of Header handled by code - if(!name.equalsIgnoreCase(F("Connection")) && - !name.equalsIgnoreCase(F("User-Agent")) && - !name.equalsIgnoreCase(F("Host")) && - !(name.equalsIgnoreCase(F("Authorization")) && _base64Authorization.length())){ - - String headerLine = name; + if (!name.equalsIgnoreCase(F("Connection")) && + !name.equalsIgnoreCase(F("User-Agent")) && + !name.equalsIgnoreCase(F("Host")) && + !(name.equalsIgnoreCase(F("Authorization")) && _base64Authorization.length())) { + + String headerLine; + headerLine.reserve(name.length() + value.length() + 4); + headerLine += name; headerLine += ": "; if (replace) { @@ -1094,13 +1096,12 @@ void HTTPClient::addHeader(const String& name, const String& value, bool first, headerLine += value; headerLine += "\r\n"; - if(first) { + if (first) { _headers = headerLine + _headers; } else { _headers += headerLine; } } - } void HTTPClient::collectHeaders(const char* headerKeys[], const size_t headerKeysCount) @@ -1225,7 +1226,17 @@ bool HTTPClient::sendHeader(const char * type) return false; } - String header = String(type) + ' ' + (_uri.length() ? _uri : F("/")) + F(" HTTP/1."); + String header; + // 168: Arbitrarily chosen to have enough buffer space for avoiding internal reallocations + header.reserve(_headers.length() + _base64Authorization.length() + _host.length() + 168); + header += type; + header += ' '; + if (_uri.length()) { + header += _uri; + } else { + header += '/'; + } + header += F(" HTTP/1."); if(_useHTTP10) { header += '0'; @@ -1233,33 +1244,31 @@ bool HTTPClient::sendHeader(const char * type) header += '1'; } - header += String(F("\r\nHost: ")) + _host; + header += F("\r\nHost: "); + header += _host; if (_port != 80 && _port != 443) { header += ':'; header += String(_port); } - header += String(F("\r\nUser-Agent: ")) + _userAgent + - F("\r\nConnection: "); - - if(_reuse) { - header += F("keep-alive"); - } else { - header += F("close"); - } - header += "\r\n"; + header += F("\r\nUser-Agent: "); + header += _userAgent; - if(!_useHTTP10) { - header += F("Accept-Encoding: identity;q=1,chunked;q=0.1,*;q=0\r\n"); + if (!_useHTTP10) { + header += F("\r\nAccept-Encoding: identity;q=1,chunked;q=0.1,*;q=0"); } - if(_base64Authorization.length()) { - header += F("Authorization: Basic "); + if (_base64Authorization.length()) { + header += F("\r\nAuthorization: Basic "); header += _base64Authorization; - header += "\r\n"; } - header += _headers + "\r\n"; + header += F("\r\nConnection: "); + header += _reuse ? F("keep-alive") : F("close"); + header += "\r\n"; + + header += _headers; + header += "\r\n"; DEBUG_HTTPCLIENT("[HTTP-Client] sending request header\n-----\n%s-----\n", header.c_str()); @@ -1290,20 +1299,23 @@ int HTTPClient::handleHeaderResponse() size_t len = _client->available(); if(len > 0) { String headerLine = _client->readStringUntil('\n'); - headerLine.trim(); // remove \r lastDataTime = millis(); DEBUG_HTTPCLIENT("[HTTP-Client][handleHeaderResponse] RX: '%s'\n", headerLine.c_str()); - if(headerLine.startsWith("HTTP/1.")) { - if(_canReuse) { + if (headerLine.startsWith(F("HTTP/1."))) { + if (_canReuse) { _canReuse = (headerLine[sizeof "HTTP/1." - 1] != '0'); } _returnCode = headerLine.substring(9, headerLine.indexOf(' ', 9)).toInt(); - } else if(headerLine.indexOf(':')) { - String headerName = headerLine.substring(0, headerLine.indexOf(':')); - String headerValue = headerLine.substring(headerLine.indexOf(':') + 1); + continue; + } + + int headerSeparator = headerLine.indexOf(':'); + if (headerSeparator > 0) { + String headerName = headerLine.substring(0, headerSeparator); + String headerValue = headerLine.substring(headerSeparator + 1); headerValue.trim(); if(headerName.equalsIgnoreCase(F("Content-Length"))) { @@ -1324,9 +1336,9 @@ int HTTPClient::handleHeaderResponse() _location = headerValue; } - for(size_t i = 0; i < _headerKeysCount; i++) { - if(_currentHeaders[i].key.equalsIgnoreCase(headerName)) { - if (_currentHeaders[i].value != "") { + for (size_t i = 0; i < _headerKeysCount; i++) { + if (_currentHeaders[i].key.equalsIgnoreCase(headerName)) { + if (!_currentHeaders[i].value.isEmpty()) { // Existing value, append this one with a comma _currentHeaders[i].value += ','; _currentHeaders[i].value += headerValue; @@ -1336,9 +1348,12 @@ int HTTPClient::handleHeaderResponse() break; // We found a match, stop looking } } + continue; } - if(headerLine == "") { + headerLine.trim(); // remove \r + + if (headerLine.isEmpty()) { DEBUG_HTTPCLIENT("[HTTP-Client][handleHeaderResponse] code: %d\n", _returnCode); if(_size > 0) { From 706204003f0f9565722fb30c15bf2e20949b412d Mon Sep 17 00:00:00 2001 From: Dirk Mueller Date: Fri, 27 Dec 2019 22:31:45 +0100 Subject: [PATCH 2/2] Add more correct reservation calculation --- libraries/ESP8266HTTPClient/src/ESP8266HTTPClient.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/libraries/ESP8266HTTPClient/src/ESP8266HTTPClient.cpp b/libraries/ESP8266HTTPClient/src/ESP8266HTTPClient.cpp index c49afbe201..f694dbbcfc 100644 --- a/libraries/ESP8266HTTPClient/src/ESP8266HTTPClient.cpp +++ b/libraries/ESP8266HTTPClient/src/ESP8266HTTPClient.cpp @@ -1227,8 +1227,9 @@ bool HTTPClient::sendHeader(const char * type) } String header; - // 168: Arbitrarily chosen to have enough buffer space for avoiding internal reallocations - header.reserve(_headers.length() + _base64Authorization.length() + _host.length() + 168); + // 128: Arbitrarily chosen to have enough buffer space for avoiding internal reallocations + header.reserve(_headers.length() + _uri.length() + + _base64Authorization.length() + _host.length() + _userAgent.length() + 128); header += type; header += ' '; if (_uri.length()) {