You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@qpid.apache.org by GitBox <gi...@apache.org> on 2021/05/26 13:33:08 UTC

[GitHub] [qpid-dispatch] kgiusti commented on a change in pull request #1229: DISPATCH-2142: buffering changes for tcp adaptor

kgiusti commented on a change in pull request #1229:
URL: https://github.com/apache/qpid-dispatch/pull/1229#discussion_r639729717



##########
File path: src/adaptors/tcp_adaptor.c
##########
@@ -216,59 +226,39 @@ void qdr_tcp_q2_unblocked_handler(const qd_alloc_safe_ptr_t context)
     sys_mutex_unlock(tc->activation_lock);
 }
 
-
 // Extract buffers and their bytes from raw connection.
-// * Proton decides how many buffers are to be taken.
-// * Buffers with no data are freed.
-// * Buffers with data are appended to caller's buffers list.
 // * Add received byte count to connection stats
 // * Return the count of bytes in the buffers list
 static int handle_incoming_raw_read(qdr_tcp_connection_t *conn, qd_buffer_list_t *buffers)
 {
-    pn_raw_buffer_t raw_buffers[READ_BUFFERS];
-
-    size_t n;
-    int count = 0;
-    int free_count = 0;
-    const bool was_open = conn->bytes_unacked < TCP_MAX_CAPACITY;
-
-    while ((conn->raw_closed_write || count + conn->bytes_unacked < TCP_MAX_CAPACITY)
-           && (n = pn_raw_connection_take_read_buffers(conn->pn_raw_conn, raw_buffers, READ_BUFFERS)) ) {
-
-        for (size_t i = 0; i < n && raw_buffers[i].bytes; ++i) {
-            qd_buffer_t *buf = (qd_buffer_t*) raw_buffers[i].context;
-            qd_buffer_insert(buf, raw_buffers[i].size);
-            count += raw_buffers[i].size;
-
-            assert(raw_buffers[i].size == qd_buffer_size(buf));
-            if (raw_buffers[i].size > 0) {
-                DEQ_INSERT_TAIL(*buffers, buf);
-            } else {
-                qd_buffer_free(buf);
-                free_count++;
-            }
-        }
+    pn_raw_buffer_t raw_buffer;
+    if ( conn->bytes_unacked >= TCP_MAX_CAPACITY || !pn_raw_connection_take_read_buffers(conn->pn_raw_conn, &raw_buffer, 1)) {
+        return 0;
     }
+    int result = raw_buffer.size;
+    qd_log(tcp_adaptor->log_source, QD_LOG_DEBUG,
+        "[C%"PRIu64"] pn_raw_connection_take_read_buffers() took buffer with %zu bytes",
+        conn->conn_id, result);
 
-    if (count > 0) {
+    if (buffers) {
+        qd_buffer_list_append(buffers, (uint8_t*) (raw_buffer.bytes + raw_buffer.offset), raw_buffer.size);
+    }
+    //reset buffer for further reads
+    conn->read_buffer.size = 0;
+    conn->read_buffer.offset = 0;
+    conn->read_pending = false;
+    if (result > 0) {
         // account for any incoming bytes just read
         conn->last_in_time = tcp_adaptor->core->uptime_ticks;
-        conn->bytes_in      += count;
-        conn->bytes_unacked += count;
+        conn->bytes_in      += result;
+        conn->bytes_unacked += result;

Review comment:
       Question:  if the received bytes are being flushed (buffers == NULL) they will never be sent to the destination - in this case should conn->bytes_unacked not be incremented?




-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org