You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@qpid.apache.org by "cliffjansen (via GitHub)" <gi...@apache.org> on 2024/10/24 05:37:40 UTC

[PR] Rotate02 [qpid-proton]

cliffjansen opened a new pull request, #435:
URL: https://github.com/apache/qpid-proton/pull/435

   the first commit does fewer buffer defrag/rotates in the case of outgoing deliveries.
   
   the second takes advantage of code of first to promote more fairness in scheduling
   
   third makes many calls to buffer defrag more performant.  The first commit ensures that an outgoing delivery is never wrapped, so this optimization always applies.  The special case of cj-sender runs > 10x faster, but streaming messages should in aggregate enjoy a performance boost or reduced cpu load.


-- 
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@qpid.apache.org

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


Re: [PR] Rotate02 [qpid-proton]

Posted by "astitcher (via GitHub)" <gi...@apache.org>.
astitcher commented on PR #435:
URL: https://github.com/apache/qpid-proton/pull/435#issuecomment-2445046125

   There are 2 use cases for ```pn_buffer_t```: 
   1. As an intermediate buffer in the output logic chain. In this usage I'm pretty sure that we can stop automatically defragging the buffer and only do it on a specific request. In this world the ```pn_buffer_bytes``` API can obviously only return one contiguous segment of the buffer, but there can only be 2 of them so instead of using ```pn_buffer_bytes``` once to get the whole buffer using it twice in succession will get both segments if the buffer has wrapped.
   2. As a hacky way to manage a grow only buffer that is never in practice defragged anyway - I think all uses of ```pn_buffer_memory`` are like this. We should just introduce a new buffer api for these cases.


-- 
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@qpid.apache.org

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


Re: [PR] Rotate02 [qpid-proton]

Posted by "cliffjansen (via GitHub)" <gi...@apache.org>.
cliffjansen commented on PR #435:
URL: https://github.com/apache/qpid-proton/pull/435#issuecomment-2448973270

   Thanks for taking a look.
   
   I will try to rework this as suggested.


-- 
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@qpid.apache.org

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


Re: [PR] Rotate02 [qpid-proton]

Posted by "astitcher (via GitHub)" <gi...@apache.org>.
astitcher commented on code in PR #435:
URL: https://github.com/apache/qpid-proton/pull/435#discussion_r1821300422


##########
c/src/core/buffer.c:
##########
@@ -236,7 +236,10 @@ static void pn_buffer_rotate (pn_buffer_t *buf, size_t sz) {
 
 static inline int pn_buffer_defrag(pn_buffer_t *buf)
 {
-  pn_buffer_rotate(buf, buf->start);
+  if (pni_buffer_wrapped(buf))
+    pn_buffer_rotate(buf, buf->start);
+  else
+    memmove(buf->bytes, buf->bytes + buf->start, buf->size);

Review Comment:
   I think this logic is actually part of the _rotate function and should be there.
   
   Also our code formatting has the braces explicitly present for this format of ```if```:
   ```
   if (...) {
     ...
   } else {
     ...
   }'''



-- 
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@qpid.apache.org

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