Only store Arc<Glyph> if 'druid' feature is enabled

[cmyr]

This was motivated by seeing some code where a user (@ctrlcctrlv) was
jumping through hoops to get around the fact that we were using Arc.

Arc was only used originally because it is important for druid, but
I'm not sure it makes sense as a more general design.

With this patch, we will only use Arc if the user enables the 'druid'
feature.

The default API now operates on Glyph objects directly; where needed
there are new _raw methods, behind the 'druid' feature, that operaate on
Arc.

Does this makes sense to folks? @chrissimpkins @ctrlcctrlv @simoncozens are your lives improved in any way by this change?

[github-actions]

🗜 Bloat check ⚖️

Comparing 891a3f3 against aca74c2

target	old size	new size	difference
target/release/examples/load_save	1.9 MB	1.9 MB	632 Bytes (0.03%)
target/debug/examples/load_save	8.57 MB	8.57 MB	-8.57 KB (-0.10%)

[chrissimpkins]

I don't believe that ref counted pointers (positively or negatively) influence anything that I am doing with the crate at this stage.

Any idea of the performance impact of the change? Intuitively looks like maybe very small benefit with single glyph methods like the glyph getters, but perhaps something more significant with the methods that required RC pointer management across the entire glyph repertoire?

[cmyr]

in theory it means we save one allocation per glyph, and also there is one less pointer jump when accessing any glyph field. In practice I would expect the difference to be negligible.

[ctrlcctrlv]

Cool change…on the plus side, I learned a lot about the Arc type while I did my work, and finally understood the difference between Arc::clone and Clone::clone(Deref<Arc<T>>) so it was not at all a total loss from my perspective, considering a lot of MFEK is multithreaded (since most modules use mfek-ipc to open a filesystem watcher thread so they can react to files being rewritten in the currently open UFO, a feature Norad doesn't have/maybe shouldn't have, but necessary for MFEKglif).

[cmyr]

@ctrlcctrlv does this change represent an improvement for you?

[ctrlcctrlv]

@cmyr yes, it simplifies my code a lot, i no longer have to think about whether an API returns an Arc<Glyph> (Arc which my use case doesn't need) or a Glyph, and, if i do need an Arc, i'd prefer to do the wrapping myself than have you do it, because Rust code is so wrapped up in ownership that the less ownership decisions API's make the better.

[ctrlcctrlv]

i may have learned something by doing it the more complex way, but clearly in Rust when you're writing out a fully qualified trait call like Clone::clone(…) something is inefficient in the API to make you do that, you should almost never need to write out a fully qualified trait procedure call.

in my case I could not use make_mut because that would give me a &mut to data owned by norad, i was worried that the changes i was making to the Glyph structs, (intended to be temporary / throwaway,) would be written into the norad Font and might affect a Font::save.

so that's why i came up with Clone::clone(&**glyph), the bizarre code which got you to write this, to guarantee i owned a copy of the Arc<Glyph> norad did not know about. Arc::clone is insufficient as that just bumps the reference count, i wanted a bare type detached from what norad had hehe.

https://github.com/MFEK/metadata/blob/c51c9f045485a0713ff1abb8f2e2fe60d5c399d4/src/glyphs.rs#L120

[cmyr]

@ctrlcctrlv You shouldn't need to worry about ownership; the whole point of make_mut is that it will always create a new clone of the data if it's not unique, and it can't be unique if you got it from norad. So it won't be saved out unless you explicitly add your changed glyph back into the layer.

This is the fun thing with Arc; it lets you share things, but only when they're identical. As soon as you make a change, you have your own copy of the data, and you cannot modify the original.

The only exception here is if you use the glyph_mut method on layer; this does mutate in place.

[cmyr]

In any case I understand sometimes just wanting a raw Glyph instead, and your approach (and there's lots of ways you could write it) works for that. :)

[github-actions]

🗜 Bloat check ⚖️

Comparing cc72511 against 61999a6

target	old size	new size	difference
target/release/examples/load_save	1.9 MB	1.9 MB	4.1 KB (0.21%)
target/debug/examples/load_save	8.6 MB	8.59 MB	-8.33 KB (-0.09%)

[github-actions]

🗜 Bloat check ⚖️

Comparing d2fd64e against 61999a6

target	old size	new size	difference
target/release/examples/load_save	1.9 MB	1.9 MB	4.1 KB (0.21%)
target/debug/examples/load_save	8.6 MB	8.59 MB	-8.33 KB (-0.09%)