`VelloFont` is `Arc<Arc<[u8]>>`

[BD103]

The VelloFont asset can be simplified down to Arc<Arc<[u8]>>. It's generally bad practice to wrap an Arc in another Arc because it requires two pointer dereferences on every access, which can hurt caching.

pub struct VelloFont {
    pub font: Arc<peniko::Font>,
}

pub struct peniko::Font {
    pub data: Blob<u8>,
    pub index: u32,
}

pub struct Blob<T> {
    data: Arc<dyn AsRef<[T]>>,
    id: u64,
}

// Or in other words
pub struct VelloFont {
    pub font: Arc<peniko::Font>,
}

pub struct VelloFont {
    pub font: Arc<Blob<u8>>,
}

pub struct VelloFont {
    pub font: Arc<Arc<dyn AsRef<[u8]>>>,
}

I propose VelloFont no longer wraps peniko::Font in an Arc and depends on Font to keep it shareable.

[BD103]

Removing the Arc will increase the size of VelloFont from 8 bytes to 32 bytes on a 64-bit computer, but the performance gains from removing the extra pointer abstraction may be worth it. This probably needs testing :)

[simbleau]

I don't think this is a valid issue. For one, I have no control over what a peniko::Font is, because that comes from a different crate. Secondly, if I start to subsume the data it has, I'm replacing and maintaining it.

I think Arc<Arc<T>> is totally fine in this case. It's a trade of design over performance.

[DJMcNab]

I'm confused by this response, because peniko::Font is Clone. What other property of the font being an Arc are you using?

[BD103]

I think the concern is more what if peniko::Font changes its internal representation to no longer be an Arc, though a clarification would be nice.

Furthermore, that kind of change would likely be breaking, so you would probably catch it while upgrading.

[simbleau]

I'm confused by this response, because peniko::Font is Clone. What other property of the font being an Arc are you using?

The original poster was saying that a peniko::Font uses Arc to store data. See "Blob".

pub struct peniko::Font {
    pub data: Blob<u8>,
    pub index: u32,
}

pub struct Blob<T> {
    data: Arc<dyn AsRef<[T]>>,
    id: u64,
}

In bevy_vello, we store a VelloFont as newtype of Arc<peniko::Font>.

Their argument was something along the lines of "Arc<Arc<T>> is bad", which is only valid in a vacuum of other details. I believe their intention was to suggest VelloFont change to this:

pub struct VelloFont {
    pub data: Arc<[u8]>,
    pub index: u32,
}

Something like that.

I'm pushing back, it makes no good sense to me. This essentially means we'd be making maintaining our own font definition. Keep that in peniko, not in bevy_vello. This is an optimization solution without a problem.

[BD103]

Oh, my apologies! What I meant to say was that VelloFont should be changed to this:

pub struct VelloFont {
    pub font: peniko::Font,
}

peniko can absolutely still be used here, I didn't mean to suggest that it should be removed. I opened up #62 which implements the change I suggested.

[simbleau]

Ah, that makes a lot more sense now.

[simbleau]

This was put into #61 with co-author credit @BD103 . Thanks again!

[BD103]

Sounds great, I'm glad we could get it figured out!