Skip to content

Commit 6b32fda

Browse files
mathieucarboumathieucarbou
committed
Further optimizations based on data fragment position and fixed message_opcode non set for non fragmented frames
1 parent ce6f7b2 commit 6b32fda

2 files changed

Lines changed: 60 additions & 34 deletions

File tree

‎src/AsyncWebSocket.cpp‎

Lines changed: 59 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -565,7 +565,7 @@ void AsyncWebSocketClient::_onData(void *pbuf, size_t plen) {
565565
uint8_t *data = (uint8_t *)pbuf;
566566

567567
while (plen > 0) {
568-
async_ws_log_v("WS[%" PRIu32 "]: _onData: plen=%" PRIu32 ", _pstate=%" PRIu8 ", _status=%" PRIu8, _clientId, plen, _pstate, static_cast<uint8_t>(_status));
568+
async_ws_log_v("WS[%" PRIu32 "] _onData: plen=%" PRIu32 ", _pstate=%" PRIu8 ", _status=%" PRIu8, _clientId, plen, _pstate, static_cast<uint8_t>(_status));
569569

570570
if (_pstate == STATE_FRAME_START) {
571571
const uint8_t *fdata = data;
@@ -593,7 +593,7 @@ void AsyncWebSocketClient::_onData(void *pbuf, size_t plen) {
593593
}
594594

595595
async_ws_log_v(
596-
"WS[%" PRIu32 "]: _pinfo: index: %" PRIu64 ", final: %" PRIu8 ", opcode: %" PRIu8 ", masked: %" PRIu8 ", len: %" PRIu64, _clientId, _pinfo.index,
596+
"WS[%" PRIu32 "] _pinfo: index: %" PRIu64 ", final: %" PRIu8 ", opcode: %" PRIu8 ", masked: %" PRIu8 ", len: %" PRIu64, _clientId, _pinfo.index,
597597
_pinfo.final, _pinfo.opcode, _pinfo.masked, _pinfo.len
598598
);
599599

@@ -606,7 +606,7 @@ void AsyncWebSocketClient::_onData(void *pbuf, size_t plen) {
606606
if (plen == 0) {
607607
// Safari close frame edge case: masked bit set but no mask data
608608
if (_pinfo.opcode == WS_DISCONNECT) {
609-
async_ws_log_v("WS[%" PRIu32 "]: close frame with incomplete mask, treating as unmasked", _clientId);
609+
async_ws_log_v("WS[%" PRIu32 "] close frame with incomplete mask, treating as unmasked", _clientId);
610610
_pinfo.masked = 0;
611611
_pinfo.index = 0;
612612
_pinfo.len = 0;
@@ -616,7 +616,7 @@ void AsyncWebSocketClient::_onData(void *pbuf, size_t plen) {
616616

617617
//wait for more data
618618
_pstate = STATE_FRAME_MASK;
619-
async_ws_log_v("WS[%" PRIu32 "]: waiting for more mask data: read=%" PRIu8 "/4", _clientId, _pinfo.masked - 1);
619+
async_ws_log_v("WS[%" PRIu32 "] waiting for more mask data: read=%" PRIu8 "/4", _clientId, _pinfo.masked - 1);
620620
return;
621621
}
622622

@@ -632,7 +632,7 @@ void AsyncWebSocketClient::_onData(void *pbuf, size_t plen) {
632632

633633
// restore masked to 1 for backward compatibility
634634
if (_pinfo.masked >= 5) {
635-
async_ws_log_v("WS[%" PRIu32 "]: mask read complete", _clientId);
635+
async_ws_log_v("WS[%" PRIu32 "] mask read complete", _clientId);
636636
_pinfo.masked = 1;
637637
}
638638

@@ -644,30 +644,37 @@ void AsyncWebSocketClient::_onData(void *pbuf, size_t plen) {
644644
}
645645
}
646646

647-
if ((datalen + _pinfo.index) < _pinfo.len) {
648-
_pstate = STATE_FRAME_DATA;
649-
async_ws_log_v("WS[%" PRIu32 "]: processing next fragment index=%" PRIu64 ", len=%" PRIu32 "", _clientId, _pinfo.index, (uint32_t)datalen);
650-
651-
if (_pinfo.index == 0) {
652-
if (_pinfo.opcode) {
653-
_pinfo.message_opcode = _pinfo.opcode;
654-
_pinfo.num = 0;
655-
}
647+
if (_pinfo.index == 0) { // first fragment of the frame
648+
// init message_opcode for this frame
649+
// note: For next WS_CONTINUATION frames, they have opcode 0, so message_opcode will stay like the first frame
650+
if (_pinfo.opcode == WS_TEXT || _pinfo.opcode == WS_BINARY) {
651+
_pinfo.message_opcode = _pinfo.opcode;
656652
}
653+
// init frame number to 0 if only 1 frame or if this is the first frame of a fragmented message
654+
if (_pinfo.final || datalen < _pinfo.len) {
655+
_pinfo.num = 0;
656+
}
657+
}
658+
659+
if ((datalen + _pinfo.index) < _pinfo.len) { // more fragments to read for this frame
660+
_pstate = STATE_FRAME_DATA;
657661

658662
if (datalen > 0) {
659-
_handleDataEvent(data, datalen);
663+
async_ws_log_v(
664+
"WS[%" PRIu32 "] processing next fragment of %s frame %" PRIu32 ", index: %" PRIu64 ", len: %" PRIu32 "", _clientId,
665+
(_pinfo.message_opcode == WS_TEXT) ? "text" : "binary", _pinfo.num, _pinfo.index, (uint32_t)datalen
666+
);
667+
_handleDataEvent(data, datalen, datalen == plen); // datalen == plen means that we are processing the last part of the current TCP packet
660668
}
661669

662670
// track index for next fragment
663671
_pinfo.index += datalen;
664672

665-
} else if ((datalen + _pinfo.index) == _pinfo.len) {
673+
} else if ((datalen + _pinfo.index) == _pinfo.len) { // this is the last fragment for this frame
666674
_pstate = STATE_FRAME_START;
667-
async_ws_log_v("WS[%" PRIu32 "]: processing final fragment index=%" PRIu64 ", len=%" PRIu32 "", _clientId, _pinfo.index, (uint32_t)datalen);
668675

669676
if (_pinfo.opcode == WS_DISCONNECT) {
670-
async_ws_log_v("WS[%" PRIu32 "]: processing disconnect", _clientId);
677+
async_ws_log_v("WS[%" PRIu32 "] processing disconnect", _clientId);
671678

672679
if (datalen) {
673680
uint16_t reasonCode = (uint16_t)(data[0] << 8) + data[1];
@@ -690,20 +697,23 @@ void AsyncWebSocketClient::_onData(void *pbuf, size_t plen) {
690697
}
691698

692699
} else if (_pinfo.opcode == WS_PING) {
693-
async_ws_log_v("WS[%" PRIu32 "]: processing ping", _clientId);
700+
async_ws_log_v("WS[%" PRIu32 "] processing ping", _clientId);
694701
_server->_handleEvent(this, WS_EVT_PING, NULL, NULL, 0);
695702
_queueControl(WS_PONG, data, datalen);
696703

697704
} else if (_pinfo.opcode == WS_PONG) {
698-
async_ws_log_v("WS[%" PRIu32 "]: processing pong", _clientId);
705+
async_ws_log_v("WS[%" PRIu32 "] processing pong", _clientId);
699706
if (datalen != AWSC_PING_PAYLOAD_LEN || memcmp(AWSC_PING_PAYLOAD, data, AWSC_PING_PAYLOAD_LEN) != 0) {
700707
_server->_handleEvent(this, WS_EVT_PONG, NULL, NULL, 0);
701708
}
702709

703710
} else if (_pinfo.opcode < WS_DISCONNECT) { // continuation or text/binary frame
704-
async_ws_log_v("WS[%" PRIu32 "]: processing data frame num=%" PRIu32 "", _clientId, _pinfo.num);
711+
async_ws_log_v(
712+
"WS[%" PRIu32 "] processing final fragment of %s frame %" PRIu32 ", index: %" PRIu64 ", len: %" PRIu32 "", _clientId,
713+
(_pinfo.message_opcode == WS_TEXT) ? "text" : "binary", _pinfo.num, _pinfo.index, (uint32_t)datalen
714+
);
705715

706-
_handleDataEvent(data, datalen);
716+
_handleDataEvent(data, datalen, datalen == plen); // datalen == plen means that we are processing the last part of the current TCP packet
707717

708718
if (_pinfo.final) {
709719
_pinfo.num = 0;
@@ -731,7 +741,7 @@ void AsyncWebSocketClient::_onData(void *pbuf, size_t plen) {
731741
}
732742
}
733743

734-
void AsyncWebSocketClient::_handleDataEvent(uint8_t *data, size_t len) {
744+
void AsyncWebSocketClient::_handleDataEvent(uint8_t *data, size_t len, bool endOfPaquet) {
735745
// ------------------------------------------------------------
736746
// Issue 384: https://github.com/ESP32Async/ESPAsyncWebServer/issues/384
737747
// Discussion: https://github.com/ESP32Async/ESPAsyncWebServer/pull/383#discussion_r2760425739
@@ -740,21 +750,37 @@ void AsyncWebSocketClient::_handleDataEvent(uint8_t *data, size_t len) {
740750
// This was a bit hacky but it was working and it was documented, although completely incorrect because it was modifying a byte outside of the data buffer.
741751
// So to fix this behavior and to avoid breaking existing client code that may be relying on this behavior, we now have to copy the data to a temporary buffer that has an extra byte for the null terminator.
742752
// ------------------------------------------------------------
743-
// Optimization note:
753+
//
754+
// Optimization notes:
755+
//
756+
// 1) opcodes
757+
//
744758
// - info->opcode stores the current WS frame type (binary, text, continuation)
745759
// - info->message_opcode stores the WS frame type of the first frame of the message, which is used for fragmented messages to know the message type when processing subsequent frame with opcode 0 (continuation)
746760
// So we can use info->message_opcode to avoid copying the data for non-text frames, and only copy the data for text frames when we need to add a null terminator for client code convenience.
761+
//
762+
// 2) data copy vs data backup/restore
763+
// - endOfPaquet: is true when datalen == plen. plen is the remaining bytes in the current TCP packet, so if datalen == plen, it means that we are processing the last part of the current TCP packet.
764+
// In that case, we have to copy since we cannot backup/restore the byte after the data buffer.
765+
// Otherwise we can backup the byte and restore since we know that the byte after is owned by the current TCP packet (same pointer).
747766
if (_pinfo.message_opcode == WS_TEXT) {
748-
std::unique_ptr<uint8_t[]> copy(new (std::nothrow) uint8_t[len + 1]());
749-
if (copy) {
750-
memcpy(copy.get(), data, len);
751-
copy[len] = 0;
752-
_server->_handleEvent(this, WS_EVT_DATA, (void *)&_pinfo, copy.get(), len);
753-
} else {
754-
async_ws_log_e("Failed to allocate");
755-
if (_client) {
756-
_client->abort();
767+
if (endOfPaquet) {
768+
std::unique_ptr<uint8_t[]> copy(new (std::nothrow) uint8_t[len + 1]());
769+
if (copy) {
770+
memcpy(copy.get(), data, len);
771+
copy[len] = 0;
772+
_server->_handleEvent(this, WS_EVT_DATA, (void *)&_pinfo, copy.get(), len);
773+
} else {
774+
async_ws_log_e("Failed to allocate");
775+
if (_client) {
776+
_client->abort();
777+
}
757778
}
779+
} else {
780+
uint8_t backup = data[len];
781+
data[len] = 0;
782+
_server->_handleEvent(this, WS_EVT_DATA, (void *)&_pinfo, data, len);
783+
data[len] = backup;
758784
}
759785
} else {
760786
_server->_handleEvent(this, WS_EVT_DATA, (void *)&_pinfo, data, len);

‎src/AsyncWebSocket.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -237,7 +237,7 @@ class AsyncWebSocketClient {
237237
void _clearQueue();
238238

239239
// this function is called when a text message is received, in order to copy the buffer and place a null terminator at the end of the buffer for easier handling of text messages.
240-
void _handleDataEvent(uint8_t *data, size_t len);
240+
void _handleDataEvent(uint8_t *data, size_t len, bool endOfPaquet);
241241

242242
public:
243243
void *_tempObject;

0 commit comments

Comments
 (0)