State reduced to equal value still triggers a state changed notification

[thierryrama]

We are using a record type as a state and when the reducer produces a new state with an equal value as the previous state, then the component injected with IState<> still gets notified.

We think that the issue is with the Fluxor.State<TState>.DefaultObjectReferenceEquals which always performs a reference equality.

private static bool DefaultObjectReferenceEquals(TState x, TState y) =>
    object.ReferenceEquals(x, y);

Replacing it with same value as Fluxor.StateSelection<TState,TValue>.DefaultValueEquals prevents the false positive difference.

private static bool DefaultValueEquals(TValue x, TValue y) =>
    object.ReferenceEquals(x, y)
    || (x as IEquatable<TValue>)?.Equals(y) == true
    || object.Equals(x, y);

We tested it on 5.0.0, 5.1.0-Beta1, and the latest 5.1 branch.

Note: Thank you for this awesome library. We started using the IStateSelection and are really looking forward to the 5.1 release. We currently have to build our project against the latest 5.1 branch because we noticed that, in 5.1.0-Beta1, the components don't automatically trigger the StateHasChanged when using the IStateSelection. But this issue has already been fixed by #271. 👍

[mrpmorris]

Hi

Why is the instance difference if the state is unchanged? I think this is possibly highlighting an inefficiency in your code where you are creating new objects to represent the same state?

[thierryrama]

Thanks for your quick reply. We could have indeed tested for the condition that would produce 2 equal states in the reducer and not generate a new state instance. We are still in the learning mode so we are not implementing everything as it should.

As a newbie to Flux and Fluxor, it did not come as a reflex to test for those conditions before generating a new state from the action data. I definitely could have tested for example that the equality of unique id in the current state and action data and just return the current state.

It may still be useful though for the framework to check for the value equality instead of just the reference equality to not further amplify the inefficiency.

[mrpmorris]

Comparing records is a deep process. It will check every property, every enumerable, every property of every instance in those enumerates, etc.

It won't be nearly as fast as comparing references, and if your state hasn't changed you won't need a new instance. Not creating new instances when state hasn't changed is another optimisation.

[uhfath]

Could we just use IEquatable or overriden Equals for this? In this case a developer would be responsible for how states are compared.

[mrpmorris]

I don't think using Equals is a good idea because it would check a lot of data in complex record structures and that would be slow.

I expect adding an optional Func<T, T, bool> would be an option, but that seems like a lot of manual work for users to implement.

I would say the issue is that you are reducing state that doesn't need to be reduced. So instead of this

=> state with { Name == action.Name} do this => state.Name == action.Name ? state : state with { Name == action.Name}

This is more difficult when you need to change an item in a collection within state. You could try out Reducible, it's designed to aid with this exact scenario - https://www.nuget.org/packages/Reducible/

[uhfath]

Thanks for the suggestion. But my idea was a bit different.

I was referring to IEquatable<T> implementations for states. In other words if a state implements IEquatable<T> then use that. If not, use ReferenceEquals which I suppose is used now. And perhaps before calling IEquatable<T> methods first check GetHashCode like hash collections do.

This way we can keep the backward compatibility with current code and keep the performance to be controlled by developers where they need to tune equality checks.

Does that makes sense?

[mrpmorris]

Records implement IEquatable - so it would be slow by default.

[uhfath]

Completely forgot about that. So ReferenceEquals it is since states are immutable anyway. But I'll take a look at Reducible as you've suggested. Thank you.

[iguanaware]

I ran into a similar problem. On State Change I was checking if a record was changed. It took almost 200ms for the equals check on one collection. After serval features where added dispatch took over 1000ms. The UI and dispatching actions started lagging.

Our state had 250,000 items in various collections.

If you can't rely on ReferenceEquals, we are considering adding a version to every point where we need to monitor subset of state

return state with {  
Customers =  state.Customers with { Version = state.Customers.Version +1, etc, etc},
Version = state.Version + 1
}

if(lastcustomerversion != state.Value.Customers.Version)

@mrpmorris it would be great to see how you envision how best to leverage Reducible with Fluxor

[mrpmorris]

Why would you have 250k items in memory at once?

[iguanaware]

We have a custom analytics software like Power BI. Our model has 36 billion rows containing all medical claims for the US. The 250,000 items are various search facet constraints than could be used to filter the data, for example city state. The only trick we had to use was a custom ReadOnlyRecordList. Immutable list with immutable items allow for a quick dirty check with ReferenceEquals. It’s not as much memory as you would think. All the strings are automatically deduped by the runtime. It ends up being a couple million references. The Fluxor state keeps track of all the possible parameters that could be part of the predicates for the underlying queries. So long as we stay under 100ms for the round trip of the actions the application feels very responsive. This is our first Fluxor application we are still learning what to do in an effect async vs in the action. It seems very straightforward, until you realize you did it wrong and the application is running too slow. A simple object = object on a giant record can kill performance. BTW: Thank you for your github contributions

[mrpmorris]

I suspect your bottleneck might be the blazor render tree.

Make sure you use @key when generating content in a loop, and override ShouldRender to prevent rendering if an individual component's state has not changed.