Dynamic dispatch for storage?

[Frando]

Hi,
I've been thinking about whether it would make sense to remove the generic Storage<T> field on the Feed struct and instead have a Box<dyn DynStorage> object on the Feed. DynStorage would be a trait with all public methods of Storage<T>. Thus, Feed<T> would just be Feed.

Benefits:

• More ergonomic to work with in code that uses Feed. This becomes more apparent on the branch that removes failure (From Failure into std::error::Error #102) where the generic type argument becomes really long and has to be carried through all code that uses Feed.
• Allows to combine both in-memory and disk-backed Feeds in a container. Currently, that wouldn't be possible.

Drawbacks:

• Cost of dynamic dispatch

I would guess that compiler optimizations might oftenly reduce the cost of dynamic dispatch if you'd only use one implementation in your code paths, but I'm not sure about it.

Opinions?

[Frando]

@yoshuawuyts do you have an opinion on this?

[Frando]

We could even have a dyn Storage for each storage (keypair, data, bitfield etc). The Node impl allows to have different storages and e.g. corestore uses this to have a different key storage for feeds it manages. I'm not sure what the cost of dynamic dispatch actually is though.

[yoshuawuyts]

@Frando big fan of erasing types in cases like these. Think this is a reasonable direction to explore.

[bltavares]

Great! @Frando if you haven't already started, I could take a look. I am planning to do some love coding this Sunday and I could box the types Em qua, 13 de mai de 2020 08:23, Yoshua Wuyts <[email protected]> escreveu:

…

@Frando <https://github.com/Frando> big fan of erasing types in cases like these. Think this is a reasonable direction to explore. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#109 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAA2XIVHM5BZC5FDQRPB4ZDRRJ7JTANCNFSM4MD3MMFA> .

[Frando]

@bltavares very cool. I did start a bit on this some time but didn't publish yet. I opened #113 with what I have. It's unfinished. Please feel very free to continue there and just push to the branch dircectly, and/or do a new clean start. I ran into an issue wrt to boxing the individual RandomAccess instances, it's explained in a comment on #113. If you have an idea on how to solve that that would be amazing. Otherwise I think we can also continue with the approach in #113.