Implement Data for Vec<Data>

[chris-zen]

This PR implements the Data trait for a Vec<T> where T: Data, which solves the following case without having to use #[data(same_fn = "PartialEq::eq")]:

#[derive(Data)]
struct A {
  // ...
}

#[derive(Data)]
struct B {
  v: Vec<A>
  // ...
}

[xStrom]

This seems useful. Thanks!

[cmyr]

so long-term we don't want data for vec; we want data-aware collection types that are cheap to clone; and currently, if you're using a vec for data, it should probably be in an Arc.

[xStrom]

Should this be reverted? 🤔

[luleyleo]

Well, I think it is nice for ergonomics, as Arc<Vec<T>> is quite annoying, especially when ending up with nesting Arc<Vec<Arc<Vec<T>>>>.

But some good collection types, maybe similar to the Im ones would be a much better solution, as using Vec<T> could lead to surprisingly* bad performance.

Thus I'm not to happy about this change as it pretty much will have to be reverted at some point.

*Mostly for people new to druid

[xStrom]

Somewhat tangential, but I'm also wondering under what scenario the data goes on multiple threads that it needs an Arc instead of Rc. 🤔 I haven't looked at the code too deeply with this question in mind, but most of the platform code which generates events seems pretty single-threaded.

[luleyleo]

Yes, I think there is a note somewhere about the possibility of using Rc but that there is no guarantee for it to work .
And something like 'I haven't looked at the code too deeply' even was the reasoning for the note, I believe.

[xStrom]

After giving it some further thought, even if the underlying platform is single-threaded, druid could potentially do its own event handling in a fan-out way. Global events could be ran on a separate thread per window. Even a single window could split up the widget tree, running the event on half of the tree on one thread and the other half on another.

It's not a simple change though and it's an open question how big a widget tree would have to be for multi-threaded work to be faster. This deserves its seperate topic, probably on zulip to start with.

[cmyr]

Yea, the choice of Arc is that it should be no slower than Rc in the uncontested case, and it sets us up for a future where we're doing multi-threaded stuff. 🤷. (I'm imagining more passing things back and forth explicitly to offload work).

My gut feeling is that this should be reverted, but maybe also that we should speed up work on adding some immutable collection types; we've also had performance issues with people using String in their Data.

medium term I'd like to discourage use of String (and bring over the rope from xi) and I'd also like good general purpose immutable set and list collections; ideally this would be based on some ecosystem crate like im.rs.

[cmyr]

Looking at source, it seems like the im crate has ptr_eq methods on its collections, so it should be trivial to implement Data for these types, and encourage their use in druid? We could even reexport them if we wanted, since we'll need to import them anyway?

[richard-uk1]

I think some notes in the docs would be good, as people will instinctively use Vec for their Data impls. Maybe it could go in the Data section of the book?

[chris-zen]

Thanks for the discussion, I understand now why I should avoid direct use of Vec. I'll update my code to use a clone-friendly data structure. Feel free to revert.
Keep the great work 💪

[cmyr]

@chris-zen Thanks for actually lighting the fire to get us to bring in some druid-appropriate collections. ;)