[improve][ml] Use lock-free queue in InflightReadsLimiter since there's no concurrent access

[guan46]

Motivation

• Because all changes to queuedHandles are in the lock, there is no need to use thread-safe collections

Modifications

• Replace SpscArrayQueue with ArrayDeque.

Documentation

• doc
• doc-required
• doc-not-needed
• doc-complete

Matching PR in forked repository

PR in forked repository: guan46#7

[lhotari]

@guan46 Just curious to know: how do you know that this improves performance? What's your use case and what are you measuring?

[lhotari]

This PR is correct, but just interested to know more about the motivation behind this PR.
The original reason to use SpscArrayQueue was due to a plan to remove synchronization from InflightReadsLimiter, but I didn't end up doing that. It should be fine to make the change that's in this PR.

@guan46 Please run CI in your own fork as a first verification step. That ensures that this change doesn't break something and you'll be able to work on the changes without needing someone to spend time in checking if this PR is ok or not.

[guan46]

SpscArrayQueue has a memory barrier to ensure data consistency in concurrent situations, whereas ArrayDeque has no memory barrier. ArrayDeque can improve performance in non-concurrent situations @lhotari

[lhotari]

SpscArrayQueue has a memory barrier to ensure data consistency in concurrent situations, whereas ArrayDeque has no memory barrier. ArrayDeque can improve performance in non-concurrent situations @lhotari

@guan46 Yes, I agree on that. I'm not against the changes in this PR.
Please follow-up on my previous comment, #23962 (comment) . I don't see a link to the build in your fork.

If you are looking for future contribution opportunities in Pulsar, you can always ask for suggestions on the dev channel in Pulsar Slack. Joining the Pulsar bi-weekly community meeting is a place where contributions are also discussed. I can help to find contribution opportunies with high impact.

Regarding the "performance improvement" aspect: This PR won't improve performance so that you'd be able to measure the difference. One reason for this is that the queue is only used when permits are all used.

[guan46]

All the tests of my repo have passed, please take a look.Thank you! @lhotari

[guan46]

I can't get into pulsar's workspace.Could you help me? @lhotari

[lhotari]

I can't get into pulsar's workspace.Could you help me? @lhotari

@guan46 you will be able to join with the link at https://pulsar.apache.org/community/#section-discussions or

[guan46]

All the tests of my repo have passed, please take a look. @lhotari

[lhotari]

Please check the latest review comment.

[guan46]

…. @guan46 you will be able to join with the link at https://pulsar.apache.org/community/#section-discussions or
I can access this link and have joined the pulsar community.And the code's been fixed like you said.@lhotari

[lhotari]

LGTM, good work @guan46

[lhotari]

/pulsarbot rerun-failure-checks

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 74.26%. Comparing base (bbc6224) to head (f933447).
Report is 930 commits behind head on master.

Additional details and impacted files

@@             Coverage Diff              @@
##             master   #23962      +/-   ##
============================================
+ Coverage     73.57%   74.26%   +0.69%     
+ Complexity    32624    31929     -695     
============================================
  Files          1877     1854      -23     
  Lines        139502   143881    +4379     
  Branches      15299    16352    +1053     
============================================
+ Hits         102638   106858    +4220     
+ Misses        28908    28645     -263     
- Partials       7956     8378     +422     

Flag	Coverage Δ
inttests	26.75% <16.66%> (+2.17%)	⬆️
systests	23.18% <16.66%> (-1.14%)	⬇️
unittests	73.80% <100.00%> (+0.95%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
...eeper/mledger/impl/cache/InflightReadsLimiter.java	82.99% <100.00%> (+0.85%)	⬆️

... and 1042 files with indirect coverage changes