Data changed on mouseover even though data hasn't actually changed

[bheisler]

Take a seat, because this is a weird one.

Reproduction case: https://gist.github.com/bheisler/cdc7af6256c74e6e40d181248ebeab21 Druid version: Current master branch Platform: Windows (I don't think that matters for this one, but because it's so weird I'm not sure)

Running this program brings up a window. Mousing over the window results in a stream of messages to stdout saying that a change was detected; these are printed by the ChangeDetectedController in response to old_data.same(data) returning false. That is to say, there's an update caused by every window event even though (as far as I can tell) nothing in this code can actually change the data.

Expected behavior: The data should not be changed by mousing over the window.

Now, let me dig into why this is weird. This doesn't look like a minimal case, but it is. Just about every further reduction or change I can think of causes it to stop happening. A non-exhaustive list of things that cause this behavior to stop:

• Changing the inner type of UniqueId
• Removing the ThingId::Dummy variant (which is not used in this code)
• Changing the contents of the ThingId::Unique variant
• Removing or changing the type of any of Thing's fields, even though they also are not used in this code, except to instantiate a Thing.
• Not adding a Thing to the ThingEditor::things Vector
• Changing the type of ThingEditor::things
• Replacing the List with a Label

I've attached a debugger and found that the Vector::ptr_eq check in the Data impl for Vector is returning false, even though the contents of the vector are exactly the same.

I have no idea why this is happening, but I minimized this case out of my recipe manager app - it is happening in my actual app, and I can't find a way to make it stop happening with my real code without removing real functionality. My app uses old_data.same(new_data) to trigger an auto-save, so this is actually causing me a real problem.

[jpochyla]

This surprised me as well! It happens because things are iterated mutably in List::event, and Vector::iter_mut invalidates the vector. I think the issue disappears when the vector is really small, or the data is small, or if it hasn't been cloned yet (or sharing the structure with another vector).

[bheisler]

Huh. Okay, thanks for the information! I can probably work around it by boxing the elements of the vector.

Still, Druid ought to fix the Data impl for large Vectors in that case. Pointer equality implies sameness, but pointer inequality does not imply that the two values are not the same.

[jpochyla]

Pointer equality implies sameness, but pointer inequality does not imply that the two values are not the same.

I think that matches the semantics of Data. same values imply equality, but just being not the same does not imply being unequal. Maybe it's surprising and the docs could be improved.

My app uses old_data.same(new_data) to trigger an auto-save, so this is actually causing me a real problem.

Would it be possible to switch to old_data != new_data?

[bheisler]

Yeah, I probably could. I'm mostly just using Data there because it's implemented for f64.

Still, as I understand it, the purpose of Data is to be a fast approximate-equality test. Fair enough I suppose, but that means that a lot of updates will be triggered that don't need to be. At the least, that's an unexpected performance cliff, and kind of undermines the purpose of having a fast approximate-equality test.

[cmyr]

oooooh

this is a weird one.

I believe the situation here is that Vector does not heap allocate below some total size threshold, and the implementation of Vector in that case may cause problems with Data? I'm not totally sure if this is what's going on but it's my main suspect.

See https://github.com/linebender/druid/blob/master/druid/src/data.rs#L433 There's a test at https://github.com/linebender/druid/blob/master/druid/src/data.rs#L498, and I recall that I wrote this because I was getting confusing results with short vectors.

I don't think this quite explains the problem, but I would not be surprised if it isn't a factor.

[richard-uk1]

This isn't the only place that the idea of "rule out" comparison occurs - it also shows up in the bloom filters. They allow you to quickly check if an object isn't in a collection. The two returns are "definitely not" or "maybe".

[bheisler]

Okay, trying to work around this in my app.

Naively boxing the large enum variant doesn't seem to help. Also, apparently Data is not implemented for Box<T: Data> and it should be, Fortunately, I am allowed to implement Data for Box<MyType> myself. Using a Vector<Box<MyType>> does appear to suppress the issue, at least for sufficiently small Vectors. Given that it fails again for not-very-large vectors, this is unsuitable as a workaround.

Using Eq to detect changes rather than Data is not practical - my data contains floats, which do not implement Eq. Likewise, ordered_float::NotNan does not implement Data. I would need to write my own wrapper type around NotNan which could implement both. PartialEq could be used instead.

In the end, I've gone a bit farther and enhanced my auto-save to detect (using PartialEq) changes to the data that would be saved, rather than the representation of that data in the UI. This also helpfully avoids an unnecessary save when the user makes, then reverts, a change. In the process, I've discovered that it is apparently impossible to send a command through an ExtEventSink if the payload is Box<dyn Any> (or at least, I was unable to find any way to do so).

I'll admit that I'm somewhat frustrated at the difficulty of getting this all working, but I did get it working in my app so this is no longer blocking me.

[cmyr]

Yea, I get the frustration. I suspect this is some issue related specifically to im::Vector.

The reason we don't impl Data for Box is because we want to push folks to use Rc and Arc, which have fast comparisons based on pointer-equality. This might also help your ExtEventSink problem?

The Data impl for floats, iirc, uses bitwise equality, which is another useful trick.

[MrGibus]

+1 that this is a Vector thing. Using an Arc<Vec> does not show the same behaviour. gist

[jpochyla]

Using an Arc does not show the same behaviour.

Iterating Arc<Vec> mutably will clone the Vec anyway, but throw away the copy if nothing has changed during iteration. So either the sameness is checked in ListIter::for_each_mut, or later in update of children of List.

[richard-uk1]

Iterating Arc mutably will clone the Vec anyway

If you use make_mut, you will only clone when the ref count > 1 (strong + weak).