JooHyukKim commented on PR #20808:
URL: https://github.com/apache/pulsar/pull/20808#issuecomment-1647022782

   > Is this class on any critical path?
   
   This won't make a crucial improvment. But since stats run at fixed rate, 
there will be a small, but certain improvement.
   
   > Performance improvement often go with a convincible report instead of 
arbitrary best practice outside critical path - if these parts of code called 
only a few time during the whole application lifecycle, it's meaning less to 
improve the total time cost by a few CPU cycles.
   > 
   > Also, do you verify whether this class will be called in concurrent 
context? Stack is thread safe while ArrayDeque is not. Without such analysis 
I'd reject such unproven "improvement" at risk of breaking existing semantic.
   
   Right, I should have provided more context on this change. I added above in 
part  `C. analysis`  notes wrt to execution and concurrency. If explanation is 
not enough, feel free to close the PR!  thank you 🙏🏼
   
   
![image](https://github.com/apache/pulsar/assets/61615301/79f0d475-02ed-4c03-bf2a-e1ba3a5bcf03)
   
   /cc @tisonkun 


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to