Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Static vs Dynamic bindings performance #597

Open
xperiandri opened this issue Apr 11, 2024 · 13 comments
Open

Static vs Dynamic bindings performance #597

xperiandri opened this issue Apr 11, 2024 · 13 comments

Comments

@xperiandri
Copy link

For the dynamic binding view model we provide a binding list once and it is created only once.
But what about the static binding view model? As I understand the logic provided in samples creates bindings for each call before execution, is that true?

@xperiandri
Copy link
Author

We need some solution that allows bindings to be created only once. This is what I do now

module Bindings =

    let private viewModel = Unchecked.defaultof<AutoSuggestAddressViewModel>

    let suggestedAddressesBinding =
        Binding.oneWaySeq ((fun m -> m.SuggestedAddresses), (=), id)
        <| nameof viewModel.SuggestedAddresses

    let suggestedCitiesBinding =
        Binding.oneWaySeq ((fun m -> m.SuggestedCities), (=), id)
        <| nameof viewModel.SuggestedCities

    let suggestedStatesBinding =
        Binding.oneWaySeq ((fun m -> m.SuggestedStates), (=), id)
        <| nameof viewModel.SuggestedStates

    let streetBinding =
        Binding.TwoWayT.id
        >> Binding.mapModel (fun m -> m.Street)
        >> Binding.mapMsg (fun s -> StreetChanged s)
        >> Binding.alterMsgStream throttle
        <| nameof viewModel.Street

    let cityBinding =
        Binding.TwoWayT.id
        >> Binding.mapModel (fun m -> m.City)
        >> Binding.mapMsg (fun s -> CityChanged s)
        >> Binding.alterMsgStream throttle
        <| nameof viewModel.City

    let stateBinding =
        Binding.TwoWayT.id
        >> Binding.mapModel (fun m -> m.State)
        >> Binding.mapMsg (fun s -> StateChanged s)
        >> Binding.alterMsgStream throttle
        <| nameof viewModel.State

    let zipCodeBinding =
        Binding.TwoWayT.id
        >> Binding.mapModel (fun m -> m.ZipCode)
        >> Binding.mapMsg (fun s -> ZipCodeChanged s)
        >> Binding.alterMsgStream throttle
        <| nameof viewModel.ZipCode

    let searchAddressCommandBinding =
        Binding.CmdParamT.model
            (fun street m ->
                String.IsNullOrWhiteSpace street
                && String.Equals (street, m.Street, StringComparison.InvariantCultureIgnoreCase)
            )
            (fun street _ -> StreetChanged street)
        <| nameof viewModel.SearchAddressCommand

    let addressSelectedCommandBinding =
        Binding.CmdParamT.model
            (fun address m ->
                match m.SelectedAddress with
                | ValueSome a -> a <> address
                | _ -> true
            )
            (fun address _ -> AddressSelected address)
        <| nameof viewModel.AddressSelectedCommand

    let searchCityCommandBinding =
        Binding.CmdParamT.model
            (fun city m ->
                String.IsNullOrWhiteSpace city
                && String.Equals (city, m.City, StringComparison.InvariantCultureIgnoreCase)
            )
            (fun city _ -> CityChanged city)
        <| nameof viewModel.SearchCityCommand

    let citySelectedCommandBinding =
        Binding.CmdParamT.model
            (fun city m ->
                not
                <| (String.Equals (city.Name, m.City, StringComparison.InvariantCultureIgnoreCase)
                    && String.Equals (city.State, m.State, StringComparison.InvariantCultureIgnoreCase))
            )
            (fun city _ -> CitySelected city)
        <| nameof viewModel.CitySelectedCommand

    let searchStateCommandBinding =
        Binding.CmdParamT.model
            (fun state m ->
                String.IsNullOrWhiteSpace state
                && String.Equals (state, m.State, StringComparison.InvariantCultureIgnoreCase)
            )
            (fun state _ -> StateChanged state)
        <| nameof viewModel.SearchStateCommand

    let stateSelectedCommandBinding =
        Binding.CmdParamT.model
            (fun state m -> not <| String.Equals (state, m.State, StringComparison.InvariantCultureIgnoreCase))
            (fun state _ -> StateSelected state)
        <| nameof viewModel.StateSelectedCommand

type AutoSuggestAddressViewModel (args) =
    inherit ViewModelBase<Model, Msg> (args)

    member _.SuggestedAddresses = base.Get (Bindings.suggestedAddressesBinding)
    member _.SuggestedCities = base.Get (Bindings.suggestedCitiesBinding)
    member _.SuggestedStates = base.Get (Bindings.suggestedStatesBinding)

    member _.Street
        with get () = base.Get<string> (Bindings.streetBinding)
        and set (value) = base.Set<string> (Bindings.streetBinding, value)

    member _.City
        with get () = base.Get<string> (Bindings.cityBinding)
        and set (value) = base.Set<string> (Bindings.cityBinding, value)

    member _.State
        with get () = base.Get<string> (Bindings.stateBinding)
        and set (value) = base.Set<string> (Bindings.stateBinding, value)

    member _.ZipCode
        with get () = base.Get<string> (Bindings.zipCodeBinding)
        and set (value) = base.Set<string> (Bindings.zipCodeBinding, value)

    member _.SearchAddressCommand = base.Get (Bindings.searchAddressCommandBinding)
    member _.AddressSelectedCommand = base.Get (Bindings.addressSelectedCommandBinding)
    member _.SearchCityCommand = base.Get (Bindings.searchCityCommandBinding)
    member _.CitySelectedCommand = base.Get (Bindings.citySelectedCommandBinding)
    member _.SearchStateCommand = base.Get (Bindings.searchStateCommandBinding)
    member _.StateSelectedCommand = base.Get (Bindings.stateSelectedCommandBinding)

@YkTru
Copy link

YkTru commented Apr 19, 2024

(Hi, sorry to interfere with your main question, but I was wondering)

Where did you get the "CmdParamT" ? Did you wrote it? Would you mind sharing the code (and other similar helpers (CmdIfT, CmdParamIfT, etc. and all "Elmish.WPF bindings" equivalent) for statically type VM?

Thank you

@xperiandri
Copy link
Author

@YkTru here it is https://github.com/eCierge/Elmish.Uno/blob/3b526cf1e8124a5cb6bf7f5bdc22eed69a634914/src/Elmish.Uno/Binding.fs#L229
Follow my changes in that repo, I have several additions there

@YkTru
Copy link

YkTru commented Apr 19, 2024

Thanks a lot; I use WPF but the helpers all seem compatible

I don't know the details of Uno (I assume it uses, contrary to WPF "Binding", "x:Bind" like UWP/WinUI?), but is there a reason you don't have "CmdIfT/CmdParamIfT"?

@xperiandri
Copy link
Author

It has pretty much the same XAML. It has run-time Binding and compile-time x:Bind (which is OneTime by default) and does not have OneWayToSource
I've just forgotten about it. I work on the project right now and add what is missing step-by-step

@YkTru
Copy link

YkTru commented Apr 19, 2024

@marner2 Do you think some of his helpers for better static VM should be merge with Elmish.WPF core library? Static VM are very useful and fun to work with versus "bindings"

@marner2
Copy link
Collaborator

marner2 commented Apr 22, 2024

@xperiandri to answer your question simply, the bindings are run only once per instance of the static type (ie, they are reused in each subsequent call). This is the same behavior as the "lists" of bindings from the dynamic version. Under the hood, Elmish.WPF stores a dictionary of the bindings mapped to the static view model property names as strings using the CallerMemberName attribute (that's why all of the examples use base.Get () (Binding.X.id)).

I suppose an optimization would be to store those statically on the type rather than on each instance, but that's not something I saw as a huge performance bottleneck as those functions are supposed to be pure and fat-free.

@marner2
Copy link
Collaborator

marner2 commented Apr 22, 2024

@YkTru I initially dismissed the idea of using Bindings.TwoWayT but I'm seeing now that it can be quite useful to use the same binding for the get and the set, even if only half is used in each instance (maybe I should rethink the way I'm separating it out).

I'm in the process of adding OneWaySeqT.

Also the CmdT.id actually passes the parameter as the message (that's why the message is obj). Looks like I need to document it better. All of the other functions are just helpers that discard the parameter (which means I probably should clean it up).

@marner2
Copy link
Collaborator

marner2 commented Apr 22, 2024

Huh, I see the new F# editor helpers give the wrong parameter name in this specific case:
image

memberName= is actually done with the preceding () in all of those Gets. You can see it's correct when there is another value to pass into that first argument (green circle) in the Set case.

@YkTru
Copy link

YkTru commented Apr 22, 2024

@marner2 Again thank you very much for the effort you are putting on this library

I'm trying to understand the Elmish.WPF source code as much as I can (it'll take me a few more weeks/months, for sure; coming from C#+MVVM, it's pretty obscure to me).

Yesterday, I was trying to use an option in a two-way binding for a staticVM without a OneWayToSourceOptT, but ended up getting stuck for hours trying to "get it to work" (using Option.Map, converters, etc.) without success. I couldn't get the setter to work. Same thing as I was trying to pass a param (in a typical "AddItem" to a list command), but without a CmdParamT I'm clueless on how to do this for a staticVM.

Is there any (recommended) way other than using a OneWayToSourceOptT (or TwoWayT; why did you dismissed the idea? for more flexibility?), or do I need to extend Elmish.WPF, or wait for an update? I also don't know how to represent those helpers that already exist for bindings, but not for staticVM:

  • oneWay/ToSource/twoWay + option/validate/lazy
  • cmd + Param/If/ParamIf
  • is this relevant for selected + Index/Item?
  • etc.

Is this the right mindset for staticVM (ie to create the helpers needed)? What do you recommend? @xperiandri?

Thanks again, this library is great, and I think if efforts are made to get a full staticVM experience, it could interest a lot more people and help them switch from MVVM. I'd love to be able to help, one day I hope, that's for sure.

@xperiandri
Copy link
Author

@YkTru
I've implemented BindingT static class with all that stuff that exists in Binding.
See my latest commit

@marner2 it would be nice to backport what I've done to this repo.
All I've changed was according to usage in my business app.

@marner2
Copy link
Collaborator

marner2 commented Apr 23, 2024

@YkTru I think the best way to start understanding the new stuff is to look at the implementations of the corresponding untyped versions. The biggest difference usually is createBinding vs createBindingT.

The "general philosophy" of the new bindings is to allow them to be simply composed rather than trying to maintain a huge list of helpers (ie, the Static Bindings Helpers which are from that line and below - 3,500 lines of helpers!). I'm open to opinions on this.

The optional T helpers stuff has a bit of a complication compared to its untyped counterpart. When it's untyped, you are just return "obj" everywhere which .NET recognizes as inherently nullable. However there's a problem with trying to unbox the type for the target type (what if it's a non-nullable type like a primitive or a struct?) I can't just assume it can be nullable. You'd have to use Nullable<T> for the output type of non-nullable types, while reference types could just use the source type (I haven't looked into how to add the nullable type check for reference types). So you would want something like:

Binding.mapModel ValueOption.toNullable >> Binding.mapMsg ValueOption.ofNullable

if you want to make a primitive type handle nulls, or

Binding.mapModel ValueOption.toObj >> Binding.mapMsg ValueOption.ofObj

if you already have a nullable type.

The "correct" thing to do in the general case where the strong typed version of something doesn't exist is to just use the untyped version of it. You won't get nice autocomplete with that particular property, but everything else will work at runtime.

@marner2
Copy link
Collaborator

marner2 commented Apr 23, 2024

@YkTru see PR #602. Is that what you need? I'm still a little unsure if I want to keep the helpers like this, but this is the basic idea.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants