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

[lhotari]

ArrayDeque takes an initial capacity as the parameter. The purpose is maxReadsInFlightAcquireQueueSize is to limit the capacity. It doesn't make sense to allocate the buffer upfront. Simply remove the maxReadsInFlightAcquireQueueSize parameter.