You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@guacamole.apache.org by "mike-jumper (via GitHub)" <gi...@apache.org> on 2023/05/17 23:11:26 UTC
[GitHub] [guacamole-server] mike-jumper commented on a diff in pull request #429: GUACAMOLE-1776: Batch up base64 encoding to reduce syscalls.
mike-jumper commented on code in PR #429:
URL: https://github.com/apache/guacamole-server/pull/429#discussion_r1197146239
##########
src/libguac/socket.c:
##########
@@ -286,56 +308,70 @@ ssize_t __guac_socket_write_base64_triplet(guac_socket* socket,
output[3] = '=';
}
- /* At this point, 4 base64 bytes have been written */
- if (guac_socket_write(socket, output, 4))
- return -1;
-
- /* If no second byte was provided, only one byte was written */
- if (b < 0)
- return 1;
-
- /* If no third byte was provided, only two bytes were written */
- if (c < 0)
- return 2;
+ return 0;
+}
- /* Otherwise, three bytes were written */
- return 3;
+ssize_t guac_socket_flush_base64(guac_socket* socket) {
+ const unsigned char* src = socket->__ready_buf;
-}
+ int encodedCount = 0;
+ int remaining = socket->__ready;
-ssize_t __guac_socket_write_base64_byte(guac_socket* socket, int buf) {
+ // Encode bytes in groups of three
+ while(remaining > 2) {
Review Comment:
Space between `while` and `(` please (see surrounding code).
##########
src/libguac/socket.c:
##########
@@ -286,56 +308,70 @@ ssize_t __guac_socket_write_base64_triplet(guac_socket* socket,
output[3] = '=';
}
- /* At this point, 4 base64 bytes have been written */
- if (guac_socket_write(socket, output, 4))
- return -1;
-
- /* If no second byte was provided, only one byte was written */
- if (b < 0)
- return 1;
-
- /* If no third byte was provided, only two bytes were written */
- if (c < 0)
- return 2;
+ return 0;
+}
- /* Otherwise, three bytes were written */
- return 3;
+ssize_t guac_socket_flush_base64(guac_socket* socket) {
+ const unsigned char* src = socket->__ready_buf;
-}
+ int encodedCount = 0;
+ int remaining = socket->__ready;
-ssize_t __guac_socket_write_base64_byte(guac_socket* socket, int buf) {
+ // Encode bytes in groups of three
+ while(remaining > 2) {
+ __guac_socket_encode_base64(src[0], src[1], src[2], socket->__encoded_buf + encodedCount);
- int* __ready_buf = socket->__ready_buf;
+ remaining -= 3;
+ src += 3;
+ encodedCount += 4;
+ }
- int retval;
+ // Take care of partial remnants
+ if(remaining == 2) {
+ __guac_socket_encode_base64(src[0], src[1], -1, socket->__encoded_buf + encodedCount);
+ encodedCount += 4;
+ } else if(remaining == 1) {
+ __guac_socket_encode_base64(src[0], -1, -1, socket->__encoded_buf + encodedCount);
+ encodedCount += 4;
+ }
- __ready_buf[socket->__ready++] = buf;
+ // Write buffer to socket
+ int retval = guac_socket_write(socket, socket->__encoded_buf, encodedCount);
+ if (retval < 0)
+ return retval;
- /* Flush triplet */
- if (socket->__ready == 3) {
- retval = __guac_socket_write_base64_triplet(socket, __ready_buf[0], __ready_buf[1], __ready_buf[2]);
- if (retval < 0)
- return retval;
+ socket->__ready = 0;
- socket->__ready = 0;
- }
+ return 0;
- return 1;
}
ssize_t guac_socket_write_base64(guac_socket* socket, const void* buf, size_t count) {
+ const unsigned char* src = (const unsigned char*)buf;
+ size_t remaining = count;
+ int len;
int retval;
- const unsigned char* char_buf = (const unsigned char*) buf;
- const unsigned char* end = char_buf + count;
+ while (remaining > 0) {
+ // Fill ready buffer as much as possible
+ len = GUAC_SOCKET_BASE64_READY_BUFFER_SIZE - socket->__ready;
+ if(remaining < len) {
+ len = remaining;
+ }
- while (char_buf < end) {
+ memcpy(socket->__ready_buf + socket->__ready, src, len);
- retval = __guac_socket_write_base64_byte(socket, *(char_buf++));
- if (retval < 0)
- return retval;
+ socket->__ready += len;
+ src += len;
+ remaining -= len;
+ // Flush ready buffer when full
+ if(socket->__ready == GUAC_SOCKET_BASE64_READY_BUFFER_SIZE) {
Review Comment:
Here, as well: space between `if` and `(`.
##########
src/libguac/socket.c:
##########
@@ -286,56 +308,70 @@ ssize_t __guac_socket_write_base64_triplet(guac_socket* socket,
output[3] = '=';
}
- /* At this point, 4 base64 bytes have been written */
- if (guac_socket_write(socket, output, 4))
- return -1;
-
- /* If no second byte was provided, only one byte was written */
- if (b < 0)
- return 1;
-
- /* If no third byte was provided, only two bytes were written */
- if (c < 0)
- return 2;
+ return 0;
+}
- /* Otherwise, three bytes were written */
- return 3;
+ssize_t guac_socket_flush_base64(guac_socket* socket) {
+ const unsigned char* src = socket->__ready_buf;
-}
+ int encodedCount = 0;
+ int remaining = socket->__ready;
-ssize_t __guac_socket_write_base64_byte(guac_socket* socket, int buf) {
+ // Encode bytes in groups of three
+ while(remaining > 2) {
+ __guac_socket_encode_base64(src[0], src[1], src[2], socket->__encoded_buf + encodedCount);
- int* __ready_buf = socket->__ready_buf;
+ remaining -= 3;
+ src += 3;
+ encodedCount += 4;
+ }
- int retval;
+ // Take care of partial remnants
+ if(remaining == 2) {
+ __guac_socket_encode_base64(src[0], src[1], -1, socket->__encoded_buf + encodedCount);
+ encodedCount += 4;
+ } else if(remaining == 1) {
Review Comment:
1. Space between `else if` and `(` please (see surrounding code).
2. Please do not cuddle the `else if` (see https://guacamole.apache.org/guac-style/#braces)
##########
src/libguac/socket.c:
##########
@@ -286,56 +308,70 @@ ssize_t __guac_socket_write_base64_triplet(guac_socket* socket,
output[3] = '=';
}
- /* At this point, 4 base64 bytes have been written */
- if (guac_socket_write(socket, output, 4))
- return -1;
-
- /* If no second byte was provided, only one byte was written */
- if (b < 0)
- return 1;
-
- /* If no third byte was provided, only two bytes were written */
- if (c < 0)
- return 2;
+ return 0;
+}
- /* Otherwise, three bytes were written */
- return 3;
+ssize_t guac_socket_flush_base64(guac_socket* socket) {
+ const unsigned char* src = socket->__ready_buf;
-}
+ int encodedCount = 0;
+ int remaining = socket->__ready;
-ssize_t __guac_socket_write_base64_byte(guac_socket* socket, int buf) {
+ // Encode bytes in groups of three
+ while(remaining > 2) {
+ __guac_socket_encode_base64(src[0], src[1], src[2], socket->__encoded_buf + encodedCount);
- int* __ready_buf = socket->__ready_buf;
+ remaining -= 3;
+ src += 3;
+ encodedCount += 4;
+ }
- int retval;
+ // Take care of partial remnants
+ if(remaining == 2) {
+ __guac_socket_encode_base64(src[0], src[1], -1, socket->__encoded_buf + encodedCount);
+ encodedCount += 4;
+ } else if(remaining == 1) {
+ __guac_socket_encode_base64(src[0], -1, -1, socket->__encoded_buf + encodedCount);
+ encodedCount += 4;
+ }
- __ready_buf[socket->__ready++] = buf;
+ // Write buffer to socket
+ int retval = guac_socket_write(socket, socket->__encoded_buf, encodedCount);
+ if (retval < 0)
+ return retval;
- /* Flush triplet */
- if (socket->__ready == 3) {
- retval = __guac_socket_write_base64_triplet(socket, __ready_buf[0], __ready_buf[1], __ready_buf[2]);
- if (retval < 0)
- return retval;
+ socket->__ready = 0;
- socket->__ready = 0;
- }
+ return 0;
- return 1;
}
ssize_t guac_socket_write_base64(guac_socket* socket, const void* buf, size_t count) {
+ const unsigned char* src = (const unsigned char*)buf;
+ size_t remaining = count;
+ int len;
int retval;
- const unsigned char* char_buf = (const unsigned char*) buf;
- const unsigned char* end = char_buf + count;
+ while (remaining > 0) {
+ // Fill ready buffer as much as possible
+ len = GUAC_SOCKET_BASE64_READY_BUFFER_SIZE - socket->__ready;
+ if(remaining < len) {
Review Comment:
`if(` -> `if (`
##########
src/libguac/socket.c:
##########
@@ -286,56 +308,70 @@ ssize_t __guac_socket_write_base64_triplet(guac_socket* socket,
output[3] = '=';
}
- /* At this point, 4 base64 bytes have been written */
- if (guac_socket_write(socket, output, 4))
- return -1;
-
- /* If no second byte was provided, only one byte was written */
- if (b < 0)
- return 1;
-
- /* If no third byte was provided, only two bytes were written */
- if (c < 0)
- return 2;
+ return 0;
+}
- /* Otherwise, three bytes were written */
- return 3;
+ssize_t guac_socket_flush_base64(guac_socket* socket) {
+ const unsigned char* src = socket->__ready_buf;
-}
+ int encodedCount = 0;
+ int remaining = socket->__ready;
-ssize_t __guac_socket_write_base64_byte(guac_socket* socket, int buf) {
+ // Encode bytes in groups of three
+ while(remaining > 2) {
+ __guac_socket_encode_base64(src[0], src[1], src[2], socket->__encoded_buf + encodedCount);
- int* __ready_buf = socket->__ready_buf;
+ remaining -= 3;
+ src += 3;
+ encodedCount += 4;
+ }
- int retval;
+ // Take care of partial remnants
+ if(remaining == 2) {
Review Comment:
Space between `if` and `(` please (see surrounding code).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org