Framed/stream grants clone the queue handle, costing a refcount per grant

[cf-rhett]

Summary

Framed (and stream) grants store an owned handle, so every grant clones the BbqHandle::Target. With Arc<BBQueue<..>> that is an atomic refcount increment on acquire and a decrement on release. In an SPSC setup the producer and consumer live on different threads, so those writes land on a cacheline both cores are touching.

The clone does not appear to buy anything: FramedProducer already holds a handle for its entire lifetime, so the queue is guaranteed alive for any grant it hands out. A grant that borrowed the producer's handle would be equally sound and free.

Where

FramedGrantW / FramedGrantR hold bbq: Q::Target:

https://github.com/jamesmunns/bbqueue/blob/main/source/bbq2/src/prod_cons/framed.rs#L63-L88

and the grant constructors fill it by cloning:

Ok(FramedGrantW {
    bbq: self.bbq.clone(),
    ...
})

BbqHandle for Arc<BBQueue<..>> implements bbq_ref as self.clone(), whereas the &BBQueue impl returns *self, which is why a static-backed queue pays nothing here.

Measurement

Ring of 512 KiB with BoxedSlice storage, AtomicCoord, and the maitake notifier, 8-byte (usize) length headers, producer and consumer on separate tokio worker threads, grants sized to a full 64 KiB MTU and committed down to the packet size (mirroring a tun driver loop). 2048 packets per iteration, criterion.

packet size	Arc<BBQueue>	&'static BBQueue	delta
64 B	4.8 M pkt/s	6.3 M pkt/s	50 ns/packet
1500 B	3.6 M pkt/s	4.0 M pkt/s	27 ns/packet
9000 B	2.7 M pkt/s	2.9 M pkt/s	37 ns/packet

The delta is largest at 64 B, where there is no backpressure and the grant rate is highest.

To confirm the refcount is the whole story rather than the Arc indirection, I added a third handle whose Target is a Copy pointer wrapper, changing nothing else. It recovered the entire gap, landing on the &'static numbers with the same heap storage.

Possible direction

A lifetime-parameterised grant that borrows the producer/consumer's handle rather than owning a clone, e.g. FramedGrantW<'a, Q, H> holding &'a Q::Target. That would keep Arc-backed queues free of per-grant refcount traffic while leaving the existing owned-handle behaviour available for callers that need a detached 'static grant.

Happy to put up a PR if that shape sounds reasonable, or if you would rather it be an additional API than a change to the existing one.

Context

Found while moving a userspace VPN data plane off a private bbqueue fork onto 0.7. Not urgent for us -- the wake path costs ~4 us per packet, so 30 ns is noise at our packet rates -- but the queue itself is fast enough that the refcount is measurable, so it seemed worth reporting.

[titaniumtraveler]

Considering that all relevant methods are implemented both for Arc<BBQueue> and &'a BBQueue wouldn't it be viable (and arguably easier) to just explicitly deref the Arc<_> to the underlying &'a BBQueue and then use that in the rest of the function?

The reason that the Arc is being cloned, is that the resulting Arc<BBQueue>-handle (and all dependent handles like the Read/Write grants) don't need to keep a borrow to the queue around and therefore can be easily passed to other threads etc

[cf-rhett]

That works if the grant never escapes the Arc borrow. Ours does: poll_read/poll_get_lease return owned leases that outlive the call, so &BBQueue would require either permanently leaking two 512 KiB rings per connection (another 1 MiB on every reconnect) or returning to the old process-global singleton pair.

This is measurable for us: with the leak fixed, Arc-owned grants are 57–90 ns/packet slower than borrowed grants, reducing ring-only throughput by 16–34%. Would you be open to an additional borrowed-grant API while retaining the current owned API?

[titaniumtraveler]

I think that could probably best solved by letting you derive a FramedGrantW<&'_ BBQueue,_> from a FramedProducer<Arc<BBQueue>,_>, where the grant is then borrowing from the queue inside of the FramedProducer.
Which should be easy enough to achieve, though I'm kinda unsure how to best surface that as API.
@jamesmunns do you have an opinion on this?

The alternative (which doesn't need wouldn't need any new API) would be to clone (and implicitly pin) an Arc<BBQueue> at the start of your top-level Future (the one that you tokio::spawn()) and then only ever use the FramedProducer<&'_ BBQueue,_> created from a borrow of that BBQueue.

Something like this:

async fn producer(bbq: Arc<BBQueue>) {
    let bbq: &BBQueue = &bbq;
    let framed_producer = bbq.framed_producer();

    loop {
      let grant = framed_producer.wait_grant().await;
      // Do something with the wait grant ...
    }
}

That would also make the code clearer about when the ref counts being incremented/decremented.
(In this case the increment happens when the bbq: Arc<BBQueue> is initially cloned and the decrement happens at the end of this future, which would drop the bbq: Arc<BBQueue> again)

[cf-rhett]

I dug into this further, and the top-level future approach works for us. Each driver future can own the Arc and create its producer or consumer through the existing BbqHandle for &BBQueue implementation. Grants that cross our completion API boundary still need an owned handle because they can outlive the poll call.

I also found a separate issue in the async wait path: the wait_* methods enter the notifier before checking whether the queue operation is already ready. #132 adds an immediate grant/read attempt across the framed and stream APIs, while retaining the existing subscribe-before-recheck path when that attempt fails.

The benchmark methodology I used earlier did not meet its repeatability criterion once task placement was controlled, so I removed the figures and am not drawing a performance conclusion from them.

With the existing reference handle support, I don't think we need the additional borrowed-grant API I proposed. I'll keep #131 limited to the owned-handle leak. I think this issue can be closed.

[titaniumtraveler]

Thanks for pointing me toward the simpler approach. I think this issue can be closed.

It's all about resource lifetimes!
The future needs to be able to borrow the BBQueue anyways, so using the borrow directly works wonderfully.
That is kinda why I love rust.
It is an incredible language to learn the details of how the lifecycles of the objects you are using work!

[cf-rhett]

Following up with better numbers.

I rebuilt the benchmark so each endpoint runs on its own persistent OS thread with a current-thread runtime, which removes the task-placement variance that made my earlier figures unreliable. It is ring-only, no I/O. Ratios are computed within each run so scheduling noise stays common-mode, and I collected 40 runs across two independently built sets. No runs are excluded.

Isolating #132 as the single-commit delta against its parent:

Frame size	Driver produces	Driver consumes
64 B	55.1% faster (40/40)	26.1% faster (40/40)
1500 B	22.6% faster (39/40)	35.8% faster (39/40)
9000 B	~9% faster (31/40)	~9% faster (35/40)

The two columns are the two driver shapes I use: one where the driver produces into the queue and the API side holds the owned consumer, and one the other way round. The counts are how many runs agree on the direction of the effect. I only claim the first two rows. At 9000 B the median favours the pre-check but the agreement is too weak to assert.

I also measured #131 on its own, and it is performance-neutral at every frame size: medians between 0.97 and 1.10, with direction agreement no better than a coin flip. That seemed worth reporting given it adds an Arc decrement to every commit and release.

One observation while I was in there, offered as data rather than a request. At 9000-byte frames, stock 0.7 is roughly 18-23% slower per packet than the 0.6-era fork we are migrating from, and #132 recovers about half of that. At 64 and 1500 bytes 0.7 is comfortably ahead either way. Jumbo frames are not something we need to support, so I am not chasing it, but if that gap is surprising to you it might be worth a look.

The figures in my earlier comment came from the harness with uncontrolled task placement and are withdrawn in favour of these.