Investigate performance issues

[TimLariviere]

Android is slow when debugging (several seconds before displaying another screen), especially with Program.withConsoleTrace. We should find a way to speed things up a little.

Also in Release mode, Android (and maybe iOS) still feels a little slow.

[cmeeren]

I notice that ViewElementExtensions.ViewElement.With has 236 optional properties. Does that mean that each single call to With will allocate 236 Option objects? Might that impact performance? (I guess not that much, since AFAIK they are short-lived and will be collected in gen0, but I haven't done any measurements.)

[dsyme]

I notice that ViewElementExtensions.ViewElement.With has 236 optional properties. Does that mean that each single call to With will allocate 236 Option objects? Might that impact performance? (I guess not that much, since AFAIK they are short-lived and will be collected in gen0, but I haven't done any measurements.)

Looks like that is very rarely called (only one caller in this codebase)

But the method should be marked inline for sure. Then at each call site nearly everything will be reduced away. I suppose this very large number of optionals may cause problems for the F# optimizer, it will be interesting to see if that's the case, though I suppose it should reduce things fairly promptly

[dsyme]

Then at each call site nearly everything will be reduced away

BTW it's worth examining the optimized code for uses of these inline methods from Xamarin.Forms.Core.fs - it's important that the F# optimizer do its work correctly for these

[Zaid-Ajaj]

BTW it's worth examining the optimized code for uses of these inline methods

Can you please point the direction in which we can examine the correctness of the inlining?

[dsyme]

If you take this view code:

      View.Label(textColor=Color.Black)

then in Release code you get the equivalent of:

let attrs = AttributesBuilder(1)
attrs.Add(ViewAttributes.TextColorAttribKey(), Some(Color.Black).Value)
ViewElement.Create(ViewBuilders::CreateFuncLabel, ViewBuilders::UpdateFuncLabel, attrs)

So the creation code has been inlined and reduced down to ViewElement.Create, all optional argument None values have been eliminated, the count of the attributes has been pre-computed (= 1). There is still a Some(Black).Value construct which could be remove - that looks a bug in the F# optimizer.

[dsyme]

In more detail the inline of the call to View.Label becomes this:

        ViewBuilders.ConstructLabel(?text=text,
                               ?horizontalTextAlignment=horizontalTextAlignment,
                               ?verticalTextAlignment=verticalTextAlignment,
                               ?fontSize=fontSize,
                               ?fontFamily=fontFamily,
                               ?fontAttributes=fontAttributes,
                               ?textColor=textColor,
                               ?formattedText=formattedText,
                               ?lineBreakMode=lineBreakMode,
                               ?lineHeight=lineHeight,
                               ?maxLines=maxLines,
                               ?textDecorations=textDecorations,
                               ?horizontalOptions=horizontalOptions,
                               ?verticalOptions=verticalOptions,
                               ?margin=margin,
                               ?gestureRecognizers=gestureRecognizers,
                               ?anchorX=anchorX,
                               ?anchorY=anchorY,
                               ?backgroundColor=backgroundColor,
                               ?heightRequest=heightRequest,
                               ?inputTransparent=inputTransparent,
                               ?isEnabled=isEnabled,
                               ?isVisible=isVisible,
                               ?minimumHeightRequest=minimumHeightRequest,
                               ?minimumWidthRequest=minimumWidthRequest,
                               ?opacity=opacity,
                               ?rotation=rotation,
                               ?rotationX=rotationX,
                               ?rotationY=rotationY,
                               ?scale=scale,
                               ?style=style,
                               ?styleClass=styleClass,
                               ?translationX=translationX,
                               ?translationY=translationY,
                               ?widthRequest=widthRequest,
                               ?resources=resources,
                               ?styles=styles,
                               ?styleSheets=styleSheets,
                               ?isTabStop=isTabStop,
                               ?scaleX=scaleX,
                               ?scaleY=scaleY,
                               ?tabIndex=tabIndex,
                               ?childrenReordered=childrenReordered,
                               ?measureInvalidated=measureInvalidated,
                               ?focused=focused,
                               ?sizeChanged=sizeChanged,
                               ?unfocused=unfocused,
                               ?classId=classId,
                               ?styleId=styleId,
                               ?automationId=automationId,
                               ?created=created,
                               ?ref=ref)

with nearly all the parameters None. This is reduced, and in turn becomes this:

        let attribBuilder = ViewBuilders.BuildLabel(0,
                               ?text=text,
                               ?horizontalTextAlignment=horizontalTextAlignment,
                               ?verticalTextAlignment=verticalTextAlignment,
                               ?fontSize=fontSize,
                               ?fontFamily=fontFamily,
                               ?fontAttributes=fontAttributes,
                               ?textColor=textColor,
                               ?formattedText=formattedText,
                               ?lineBreakMode=lineBreakMode,
                               ?lineHeight=lineHeight,
                               ?maxLines=maxLines,
                               ?textDecorations=textDecorations,
                               ?horizontalOptions=horizontalOptions,
                               ?verticalOptions=verticalOptions,
                               ?margin=margin,
                               ?gestureRecognizers=gestureRecognizers,
                               ?anchorX=anchorX,
                               ?anchorY=anchorY,
                               ?backgroundColor=backgroundColor,
                               ?heightRequest=heightRequest,
                               ?inputTransparent=inputTransparent,
                               ?isEnabled=isEnabled,
                               ?isVisible=isVisible,
                               ?minimumHeightRequest=minimumHeightRequest,
                               ?minimumWidthRequest=minimumWidthRequest,
                               ?opacity=opacity,
                               ?rotation=rotation,
                               ?rotationX=rotationX,
                               ?rotationY=rotationY,
                               ?scale=scale,
                               ?style=style,
                               ?styleClass=styleClass,
                               ?translationX=translationX,
                               ?translationY=translationY,
                               ?widthRequest=widthRequest,
                               ?resources=resources,
                               ?styles=styles,
                               ?styleSheets=styleSheets,
                               ?isTabStop=isTabStop,
                               ?scaleX=scaleX,
                               ?scaleY=scaleY,
                               ?tabIndex=tabIndex,
                               ?childrenReordered=childrenReordered,
                               ?measureInvalidated=measureInvalidated,
                               ?focused=focused,
                               ?sizeChanged=sizeChanged,
                               ?unfocused=unfocused,
                               ?classId=classId,
                               ?styleId=styleId,
                               ?automationId=automationId,
                               ?created=(match created with None -> None | Some createdFunc -> Some (fun (target: obj) ->  createdFunc (unbox<Xamarin.Forms.Label> target))),
                               ?ref=(match ref with None -> None | Some (ref: ViewRef<Xamarin.Forms.Label>) -> Some ref.Unbox))

        ViewElement.Create<Xamarin.Forms.Label>(ViewBuilders.CreateFuncLabel, ViewBuilders.UpdateFuncLabel, attribBuilder)

again with nearly all None. The call to BuildLabel becomes this:

        let attribCount = match text with Some _ -> attribCount + 1 | None -> attribCount
        let attribCount = match horizontalTextAlignment with Some _ -> attribCount + 1 | None -> attribCount
        let attribCount = match verticalTextAlignment with Some _ -> attribCount + 1 | None -> attribCount
        let attribCount = match fontSize with Some _ -> attribCount + 1 | None -> attribCount
        let attribCount = match fontFamily with Some _ -> attribCount + 1 | None -> attribCount
        let attribCount = match fontAttributes with Some _ -> attribCount + 1 | None -> attribCount
        let attribCount = match textColor with Some _ -> attribCount + 1 | None -> attribCount
        let attribCount = match formattedText with Some _ -> attribCount + 1 | None -> attribCount
        let attribCount = match lineBreakMode with Some _ -> attribCount + 1 | None -> attribCount
        let attribCount = match lineHeight with Some _ -> attribCount + 1 | None -> attribCount
        let attribCount = match maxLines with Some _ -> attribCount + 1 | None -> attribCount
        let attribCount = match textDecorations with Some _ -> attribCount + 1 | None -> attribCount

        let attribBuilder = ViewBuilders.BuildView(attribCount, ?horizontalOptions=horizontalOptions, ?verticalOptions=verticalOptions, ?margin=margin, ?gestureRecognizers=gestureRecognizers, ?anchorX=anchorX, ?anchorY=anchorY, ?backgroundColor=backgroundColor, ?heightRequest=heightRequest, ?inputTransparent=inputTransparent, ?isEnabled=isEnabled, ?isVisible=isVisible, ?minimumHeightRequest=minimumHeightRequest, ?minimumWidthRequest=minimumWidthRequest, ?opacity=opacity, ?rotation=rotation, ?rotationX=rotationX, ?rotationY=rotationY, ?scale=scale, ?style=style, ?styleClass=styleClass, ?translationX=translationX, ?translationY=translationY, ?widthRequest=widthRequest, ?resources=resources, ?styles=styles, ?styleSheets=styleSheets, ?isTabStop=isTabStop, ?scaleX=scaleX, ?scaleY=scaleY, ?tabIndex=tabIndex, ?childrenReordered=childrenReordered, ?measureInvalidated=measureInvalidated, ?focused=focused, ?sizeChanged=sizeChanged, ?unfocused=unfocused, ?classId=classId, ?styleId=styleId, ?automationId=automationId, ?created=created, ?ref=ref)
        match text with None -> () | Some v -> attribBuilder.Add(ViewAttributes.TextAttribKey, (v)) 
        match horizontalTextAlignment with None -> () | Some v -> attribBuilder.Add(ViewAttributes.HorizontalTextAlignmentAttribKey, (v)) 
        match verticalTextAlignment with None -> () | Some v -> attribBuilder.Add(ViewAttributes.VerticalTextAlignmentAttribKey, (v)) 
        match fontSize with None -> () | Some v -> attribBuilder.Add(ViewAttributes.FontSizeAttribKey, makeFontSize(v)) 
        match fontFamily with None -> () | Some v -> attribBuilder.Add(ViewAttributes.FontFamilyAttribKey, (v)) 
        match fontAttributes with None -> () | Some v -> attribBuilder.Add(ViewAttributes.FontAttributesAttribKey, (v)) 
        match textColor with None -> () | Some v -> attribBuilder.Add(ViewAttributes.TextColorAttribKey, (v)) 
        match formattedText with None -> () | Some v -> attribBuilder.Add(ViewAttributes.FormattedTextAttribKey, (v)) 
        match lineBreakMode with None -> () | Some v -> attribBuilder.Add(ViewAttributes.LineBreakModeAttribKey, (v)) 
        match lineHeight with None -> () | Some v -> attribBuilder.Add(ViewAttributes.LineHeightAttribKey, (v)) 
        match maxLines with None -> () | Some v -> attribBuilder.Add(ViewAttributes.MaxLinesAttribKey, (v)) 
        match textDecorations with None -> () | Some v -> attribBuilder.Add(ViewAttributes.TextDecorationsAttribKey, (v)) 
        attribBuilder

and the match are reduced to the above code.

[dsyme]

I added an F# compiler bug for the issue where there was a Some allocation remaining, and [likely fix](remaining https://github.com/Microsoft/visualfsharp/pull/6533).

With this in place the code

      View.Label(textColor=Color.Black)

becomes

let attrs = AttributesBuilder(1)
attrs.Add(ViewAttributes.TextColorAttribKey(), Color.Black)
ViewElement.Create(ViewBuilders.CreateFuncLabel, ViewBuilders.UpdateFuncLabel, attrs)

which is, I believe, three allocations per view element

1. the AttributesBuilder
2. the attributes array inside the AttributesBuilder
3. the ViewElement

plus the lookups on static fields for the behaviour of the view element.

The allocation of the temporary AttributesBuilder could be removed either by making it a struct or simply by directly allocating and filling in the array in the generated code for Xamarin.Forms.Core.fs. This could apply to in any generated since we can trust the attribCount in generated code of Xamarin.Forms.Core.fs (but not necessarily for user extensions). That would reduce things to a minimum of two allocations per view element. I don't think it's possible to do better than that.

But note that none of this will matter if other things dominate, e.g.

• the process of doing differential update checking is slow for some reason, or
• the view is very large and has no dependsOn performance tips, or
• the view is being recomputed way too often (too many messages)

[cmeeren]

Will be interesting to see if this can improve #383!

[cmeeren]

I notice that dependsOn uses structural hashing. If using dependsOn with complex records or large lists of records, I guess it would be much more performant to use reference hashing. Could we give the user the option to specify the equality comparison to use?

For example:

let (==) = LanguagePrimitives.PhysicalEquality
dependsOnWith 
  (model.Count, model.Entities)
  (fun (c1, es1) (c2, es2) -> c1 = c2 && es1 == es2)
  (fun model (count, entities) -> ...)

[dsyme]

Yes, no reason not to support that

[dsyme]

(need a hash code too, i.e. IEqualityComparer)

[cmeeren]

Sorry, lacking context for understanding that last comment. Why/where do you need a hash code?

[dsyme]

If you use a reference-identity equality you should use a matching reference-identity hash. Otherwise your hash will be structural and hence more expensive, e.g.

let (==) = LanguagePrimitives.PhysicalEquality
let hashq = LanguagePrimitives.PhysicalHash

dependsOnWith 
  (model.Count, model.Entities)
  (fun (c1, es1) (c2, es2) -> c1 = c2 && es1 == es2)
  (fun (c, es) -> hash c + hashq es)
  (fun model (count, entities) -> ...)

or if you like add a HashIdentity.pair and something like this:

dependsOnWith 
  (model.Count, model.Entities)
  (HashIdentity.pair HashIdentity.Structural HashIdentity.Reference)
  (fun model (count, entities) -> ...)

It's not ideal though.

[cmeeren]

Hm, is that required because the hash is needed internally by Fabulous?

If so, it might be better to just have to supply the hash function, not the equality. The hash function is simpler (only one set of params), and AFAIK it's sufficient for Fabulous to compare hashes, without needing a user-supplied equality comparison.

[dsyme]

Yes, dependsOn uses a weak hash table to record results. https://github.com/fsprojects/Fabulous/blob/master/src/Fabulous.Core/ViewHelpers.fs#L13

I'd encourage people to experiment with other ways to do this. One approach is to allocate optimization points like this:

let optimizeA = viewOptimizationWith (fun (v1,u1) (v2, u2) -> ...)
let view model dispatch = 
    ...
    optimizeA (model.X, model.Y) (fun model (x,y) -> ...)
    ...

with library code:

let viewOptimizationWith eq = 
    let mutable prevKey = None 
    fun newKey computeValue -> 
          match prevKey with 
          | Some (WeakHandle k) when  eq k newKey -> etc.  following existing `dependsOn` code

However TBH it's slightly frustrating to need the separate top-level declaration to get the cache point allocated which is why dependsOn uses the ugly global dictionary. It feels like there is an F# language suggestion lurking here.

[cmeeren]

There are many ways to skin this cat. I think you'd get far with the following:

1. Implement dependsOnWith (subModel: 'm) (getHash: 'm -> int) (view: DoNotUse -> 'm -> ...)

2. Add a convenience memberwiseRefHash function that, using reflection (once, then caching the resulting fast property accessors), computes the reference hash of each member of the record/tuple sub-model.

This allows people to simply call

dependsOnWith
  (model.Count, model.Entities)
  memberwiseRefHash
  (fun model (count, entities) -> ...)

and they'd get reference equality check for all elements of the tuple.

Could that work?

---

FWIW, I think the global dictionary is a nice way to solve a difficult problem to the benefit of the user. If an F# language suggestion could alleviate this, that'd be great - I can imagine then being able to memoize function calls inline without having to define top-level memoized versions of functions.

---

For inspiration of other possible ways to solve this, check out Elmish's lazyView (a separate React component, constructed inline), or how Elmish.WPF uses per-binding ref cells to cache computed values (not sure how well that translates to Fabulous; Elmish with static views have its own unique set of challenges).

[cmeeren]

I am toying with the following equality comparer for Elmish.WPF. Could something like that work here, or perhaps a hash function (instead of equality comparer) based on the same approach?

open System
open System.Linq.Expressions
open System.Reflection

/// Returns a fast, untyped getter for the property specified by the PropertyInfo.
/// The getter takes an instance and returns a property value.
let private buildUntypedGetter (propertyInfo: PropertyInfo) : obj -> obj =
  let method = propertyInfo.GetMethod
  let objExpr = Expression.Parameter(typeof<obj>, "o")
  let expr =
    Expression.Lambda<Func<obj, obj>>(
      Expression.Convert(
        Expression.Call(
          Expression.Convert(objExpr, method.DeclaringType), method),
          typeof<obj>),
      objExpr)
  let action = expr.Compile()
  fun target -> action.Invoke(target)

/// Reference/physical equality. Also see elmEq.
let refEq = LanguagePrimitives.PhysicalEquality

/// Memberwise equality where value-typed members and string members are
/// compared using structural comparison, and reference-typed members
/// (except strings) are compared using reference equality. This is a useful
/// default for oneWayLazy etc. since all parts of the Elm model (i.e., all members
/// of the arguments to this function) are normally immutable. For a direct reference
/// equality check (not memberwise), see refEq (which should be used when passing
/// a single non-string object from the model).
let elmEq<'a> : 'a -> 'a -> bool =
  let gettersAndEq =
    typeof<'a>.GetProperties()
    |> Array.map (fun pi ->
        let getter = buildUntypedGetter pi
        let eq =
          if pi.PropertyType.IsValueType || pi.PropertyType = typeof<string>
          then (fun (a, b) -> a = b)
          else obj.ReferenceEquals
        getter, eq
    )
  fun x1 x2 ->
    gettersAndEq |> Array.forall (fun (get, eq) -> eq (get (box x1), get (box x2)))

(I think it's fast, but I'm no performance expert, let me know if I've missed something obvious.)

[TimLariviere]

I've started to run some perf measurements to find bottlenecks in Fabulous.

If you're interested, you can run your own tests with this branch : https://github.com/TimLariviere/Fabulous/tree/performance (ElmishContacts.Android won't work for the moment -- missing dependencies)

I use ElmishContacts as a reference, because that's where I noticed some big slowdowns when navigating from page to page. This test has been run on an iPhone 5s simulator on a Mac Book Pro, so it's supposed to be very good compared to an actual device.

For now, I've only tested navigation from the empty home page to the contact creation page.

Fabulous determines this is a new page and almost no control can be reused (except maybe the NavigationPage root) https://github.com/TimLariviere/ElmishContacts/blob/686d9598bc8a07dc52facb69dd1cfd4b111e7cb6/ElmishContacts/EditPage.fs#L206-L277

What I can see from the full log is that Fabulous is doing great almost everywhere, except on some controls.

• let update msg model is done in approximately 8 milliseconds (I found also 1ms on another test)
• let view model dispatch is done in 162 milliseconds (some improvements can be made here)
• (Fabulous) canReuseChild is instantaneous (the whole page changed so it was expected)
• (Fabulous) newPageElement.UpdateIncremental is done in 354ms

That last one is quite slow I think, given it ran on a powerful iOS simulator. It can easily take seconds on an actual low-end Android device.

Most of the controls are almost instantaneous. Only StackLayout starts to take a significant time (129ms), only to be top by NavigationPage (with 353ms, of which 129 is from StackLayout)

I will try to check what's going on with the layout controls

Full log:

Update done in 8 ms / 80054 ticks View done in 162 ms / 1624492 ticks canReuseChild done in 0 ms / 237 ticks UpdateFuncElement done in 0 ms / 52 ticks UpdateFuncVisualElement done in 0 ms / 3917 ticks UpdateFuncPage done in 0 ms / 7043 ticks 2019-05-11 11:19:45.843428+0200 iOS[6571:163757] Updating NavigationPage, prevCount = 1, newCount = 2 Create Xamarin.Forms.ContentPage done in 0 ms / 492 ticks UpdateFuncElement done in 0 ms / 28 ticks UpdateFuncVisualElement done in 0 ms / 2910 ticks Create Xamarin.Forms.ToolbarItem done in 0 ms / 29 ticks UpdateFuncElement done in 0 ms / 35 ticks UpdateFuncMenuItem done in 0 ms / 3393 ticks UpdateFuncToolbarItem done in 0 ms / 9971 ticks UpdateFuncPage done in 2 ms / 23731 ticks Create Xamarin.Forms.ScrollView done in 0 ms / 8901 ticks UpdateFuncElement done in 0 ms / 31 ticks UpdateFuncVisualElement done in 0 ms / 4235 ticks UpdateFuncView done in 0 ms / 7919 ticks UpdateFuncLayout done in 1 ms / 11518 ticks Create Xamarin.Forms.StackLayout done in 2 ms / 27116 ticks UpdateFuncElement done in 0 ms / 34 ticks UpdateFuncVisualElement done in 0 ms / 2872 ticks UpdateFuncView done in 1 ms / 12802 ticks UpdateFuncLayout done in 2 ms / 25781 ticks Create Xamarin.Forms.Grid done in 1 ms / 19515 ticks UpdateFuncElement done in 0 ms / 39 ticks UpdateFuncVisualElement done in 0 ms / 4217 ticks UpdateFuncView done in 0 ms / 9077 ticks UpdateFuncLayout done in 1 ms / 12638 ticks Create Xamarin.Forms.RowDefinition done in 0 ms / 4102 ticks UpdateFuncRowDefinition done in 0 ms / 8092 ticks Create Xamarin.Forms.RowDefinition done in 0 ms / 13 ticks UpdateFuncRowDefinition done in 0 ms / 88 ticks Create Xamarin.Forms.ColumnDefinition done in 0 ms / 4617 ticks UpdateFuncColumnDefinition done in 0 ms / 2150 ticks Create Xamarin.Forms.ColumnDefinition done in 0 ms / 16 ticks UpdateFuncColumnDefinition done in 0 ms / 762 ticks Create Xamarin.Forms.Button done in 1 ms / 16358 ticks UpdateFuncElement done in 0 ms / 43 ticks UpdateFuncVisualElement done in 0 ms / 7102 ticks UpdateFuncView done in 1 ms / 13248 ticks UpdateFuncButton done in 7 ms / 71112 ticks Create ElmishContacts.Controls.BorderedEntry done in 1 ms / 12292 ticks UpdateFuncElement done in 0 ms / 43 ticks UpdateFuncVisualElement done in 0 ms / 4907 ticks UpdateFuncView done in 0 ms / 8838 ticks UpdateFuncInputView done in 1 ms / 16047 ticks UpdateFuncEntry done in 10 ms / 105434 ticks Create ElmishContacts.Controls.BorderedEntry done in 0 ms / 361 ticks UpdateFuncElement done in 0 ms / 36 ticks UpdateFuncVisualElement done in 0 ms / 9855 ticks UpdateFuncView done in 1 ms / 11964 ticks UpdateFuncInputView done in 1 ms / 14320 ticks UpdateFuncEntry done in 1 ms / 18076 ticks UpdateFuncGrid done in 71 ms / 718283 ticks Create Xamarin.Forms.StackLayout done in 0 ms / 351 ticks UpdateFuncElement done in 0 ms / 45 ticks UpdateFuncVisualElement done in 0 ms / 4901 ticks UpdateFuncView done in 1 ms / 10357 ticks UpdateFuncLayout done in 1 ms / 14169 ticks Create Xamarin.Forms.Label done in 0 ms / 435 ticks UpdateFuncElement done in 0 ms / 40 ticks UpdateFuncVisualElement done in 0 ms / 3545 ticks UpdateFuncView done in 0 ms / 6523 ticks UpdateFuncLabel done in 1 ms / 10166 ticks Create Xamarin.Forms.Switch done in 0 ms / 8581 ticks UpdateFuncElement done in 0 ms / 43 ticks UpdateFuncVisualElement done in 0 ms / 5115 ticks UpdateFuncView done in 0 ms / 9834 ticks UpdateFuncSwitch done in 2 ms / 23601 ticks UpdateFuncStackLayout done in 16 ms / 162795 ticks Create Xamarin.Forms.Label done in 0 ms / 265 ticks UpdateFuncElement done in 0 ms / 37 ticks UpdateFuncVisualElement done in 0 ms / 3189 ticks UpdateFuncView done in 0 ms / 6161 ticks UpdateFuncLabel done in 1 ms / 10001 ticks Create ElmishContacts.Controls.BorderedEntry done in 0 ms / 352 ticks UpdateFuncElement done in 0 ms / 27 ticks UpdateFuncVisualElement done in 0 ms / 2914 ticks UpdateFuncView done in 0 ms / 5467 ticks UpdateFuncInputView done in 0 ms / 8544 ticks UpdateFuncEntry done in 1 ms / 12167 ticks Create Xamarin.Forms.Label done in 0 ms / 237 ticks UpdateFuncElement done in 0 ms / 22 ticks UpdateFuncVisualElement done in 0 ms / 2877 ticks UpdateFuncView done in 0 ms / 5789 ticks UpdateFuncLabel done in 0 ms / 8578 ticks Create ElmishContacts.Controls.BorderedEntry done in 0 ms / 272 ticks UpdateFuncElement done in 0 ms / 25 ticks UpdateFuncVisualElement done in 0 ms / 3190 ticks UpdateFuncView done in 0 ms / 5928 ticks UpdateFuncInputView done in 0 ms / 8979 ticks UpdateFuncEntry done in 1 ms / 11921 ticks Create Xamarin.Forms.Label done in 0 ms / 154 ticks UpdateFuncElement done in 0 ms / 21 ticks UpdateFuncVisualElement done in 0 ms / 3347 ticks UpdateFuncView done in 0 ms / 5969 ticks UpdateFuncLabel done in 0 ms / 8901 ticks Create Xamarin.Forms.Editor done in 0 ms / 9492 ticks UpdateFuncElement done in 0 ms / 40 ticks UpdateFuncVisualElement done in 0 ms / 8508 ticks UpdateFuncView done in 1 ms / 13274 ticks UpdateFuncInputView done in 1 ms / 16243 ticks UpdateFuncEditor done in 3 ms / 32108 ticks Create Xamarin.Forms.Button done in 0 ms / 262 ticks UpdateFuncElement done in 0 ms / 38 ticks UpdateFuncVisualElement done in 0 ms / 5859 ticks UpdateFuncView done in 0 ms / 9736 ticks UpdateFuncButton done in 1 ms / 17127 ticks UpdateFuncStackLayout done in 129 ms / 1291587 ticks UpdateFuncScrollView done in 135 ms / 1359928 ticks UpdateFuncContentPage done in 148 ms / 1489594 ticks 2019-05-11 11:19:45.993460+0200 iOS[6571:163757] PushAsync, page number 1 UpdateFuncNavigationPage done in 353 ms / 3539693 ticks newPageElement.UpdateIncremental done in 354 ms / 3544155 ticks

[TimLariviere]

After a deep dive into the generated code thanks to (or due to, depending on the view) implementing Shell support, I saw that the current view diffing algorithm is highly inefficient both performance-wise and memory-wise because it eagerly walk the whole UI tree, boxing and unboxing way too much.

I think we can resolve this issue by making a diffing algorithm upfront. Instead of walking the UI tree just to find there's no work needed, we could precompute the actual delta between the previous state and the current one (previous: ViewElement -> current: ViewElement -> DeltaViewElement).

This way, we could efficiently walk the UI tree to update what really needs updating.

[cmeeren]

I guess that comment wasn't intended for me, but I'd like to understand this a bit better (not least since I'm working on a seemingly promising dynamic Elmish WPF framework, and performance testing is always challenging, so prior art is always appreciated).

What do you mean when you say "walk the whole UI tree"? Doesn't the diffing algorithm short-circuit if a child ViewElement type has changed?

And how does a DeltaViewElement help?

[TimLariviere]

What do you mean when you say "walk the whole UI tree"? Doesn't the diffing algorithm short-circuit if a child ViewElement type has changed?

To some extents, yes there is short-circuits... but only in the case of strict reference equality (that's where dependsOn comes in handy) Or value equality if we know the type can be compared (e.g. string, int, etc.). This is determined dynamically by the Generator.

Fabulous doesn't compute the diff beforehand. Instead it goes through each property to check if it has been added/removed/updated. This leads to walking the UI tree, Fabulous will first access the UI control and then checks if its properties need updates.

It will do so until it reaches the end of the ViewElement tree, which should be similar to the UI tree depth.

static member UpdateContentPage (prevOpt: ViewElement voption, curr: ViewElement, target: Xamarin.Forms.ContentPage) =
    let mutable prevContentOpt = ValueNone
    let mutable currContentOpt = ValueNone
    (...)
    match prevContentOpt, currContentOpt with
    | ValueSome prevValue, ValueSome newValue when identical prevValue newValue ->
        // Reference equality, we do nothing
        ()
    | ValueNone, ValueNone ->
        // Nothing to do
        ()
    | ValueSome prevValue, ValueSome newValue when canReuseChild prevValue newValue ->
        // Only applicable with ViewElement-typed property value
        // We access the content UI control and will call its related Update function
        newValue.UpdateIncremental(prevValue, target.Content)
    | _, ValueSome newValue ->
        // The ContentPage didn't have content before, we're creating it now
        target.Content <- (newValue.Create() :?> Xamarin.Forms.View)
    | ValueSome _, ValueNone ->
        // There was a value, now there isn't
        target.Content <- null

At the highest level, Fabulous only checks if the previous UI is reusable for updates (is it the same control type?) https://github.com/fsprojects/Fabulous/blob/02e1a4d67d8588e84cefbf69b06405936d554c66/src/Fabulous.Core/ElmishProgram.fs#L91-L97

So if you do

View.ContentPage(
    content=View.Grid(
        View.Label("Some text")
    )
)

Then you send a message but don't change the UI. Without memoizations, Fabulous will access the following controls:

• ContentPage
  • unbox obj -> ContentPage
  • unbox obj -> TemplatedPage
  • unbox obj -> Page
  • unbox obj -> VisualElement
  • unbox obj -> NavigableElement
  • unbox obj -> Element
• Grid (ContentPage.Content)
  • unbox obj -> Grid
  • unbox obj -> Layout
  • unbox obj -> View
  • unbox obj -> VisualElement
  • unbox obj -> NavigableElement
  • unbox obj -> Element
• Label (Grid.Children.[0])
  • unbox obj -> Label
  • unbox obj -> View
  • unbox obj -> VisualElement
  • unbox obj -> NavigableElement
  • unbox obj -> Element

The unboxing is the step before calling the related Update* method. Fabulous will go through each of the inherited types to update the properties specific to those types.

So it did a lot of high cost operations where it should have done nothing...

And how does a DeltaViewElement help?

Precomputing the delta between the previous state and the new one enables us to construct a "plan of actions". DeltaViewElement will contain only a set of instructions to apply to the UI (Add a new child to a list, Remove a control from another, Update a property with a new value, etc.)

It is far more reliable than reference equality, which most of the times will return false without memoizations (ViewElement is a reference type, functions as well command=(fun () -> dispatch ...))

Also computing a tree diff between 2 dictionaries is way more efficient than going through the whole UI tree to check there if something needs to be done.

[TimLariviere]

Constructing this “plan of actions” also has some benefits:

• Easier to debug (Why this property isn’t updating?)
• Remove the responsibility of diffing from the platform renderer (Today Fabulous mixes diffing and view creation. But Fabulous should handle only the diffing, other devs can implement frameworks like Avalonia with just the instructions to update)

[cmeeren]

Thanks for the clarification! I guess what I don't understand is how the DeltaViewElement can be created. After all, the view function produces a whole new ViewElement hierarchy on each call (excepting any dependsOn etc.), so how can a delta be created without traversing the whole hierarchy and checking each property in the same way?

[TimLariviere]

The hierarchy will be traversed one way or another.

The difference here is that the diffing won’t need to pro-actively access the underlying controls first, most of the time for nothing. That’s this access combined with boxing/unboxing that takes a long time, even more on sensitive systems like Android.

The new diffing algorithm will reduce this access to the underlying controls by doing a first pass between the previous and new ViewElement. The DeltaViewElement (surely a bad name) will only contain instructions on changed properties. E.g. [ (“FontSize”, Added 50.), (“Text”, Updated “My Title”), (“FontAttributes”, Removed) ] And nothing, if the value hasn’t changed.

In the example with ContentPage, if the developer doesn’t use dependsOn at each node, changing a property on ContentPage will trigger a scan down to the label (which will access the underlying control, boxing and unboxing for each inherited types), where it’s not necessary.

[cmeeren]

The difference here is that the diffing won’t need to pro-actively access the underlying controls first ... trigger a scan down to the label (which will access the underlying control, boxing and unboxing for each inherited types)

When you say "control" here, are you referring to the actual UI controls?

If so, I think I've managed to stay clear of this in "Elmish.WPF.Dynamic". As long as the ViewElement type doesn't change, I have to traverse the inheritance hierarchy and check each property for modifications, but I only access the actual UI controls when setting any changed property values (which I do right after I check each property).

If you referred to something other than the actual UI controls, I'm afraid I didn't understand.

[TimLariviere]

When you say "control" here, are you referring to the actual UI controls?

Yes.

I have to traverse the inheritance hierarchy and check each property for modifications, but I only access the actual UI controls when setting any changed property values (which I do right after I check each property).

Do you have an example of how you do this?

[cmeeren]

For example (I have removed a lot of properties from this):

type UIElement(setProps: UIElement -> unit) as this =
  inherit Visual()

  // Default values
  static let _DefAllowDrop = System.Windows.UIElement.AllowDropProperty.DefaultMetadata.DefaultValue :?> bool
  static let _DefCacheMode = System.Windows.UIElement.CacheModeProperty.DefaultMetadata.DefaultValue :?> System.Windows.Media.CacheMode

  do setProps this

  new() = UIElement(fun _ -> ())

  // The cached view
  [<DefaultValue>] val mutable internal __View: System.Windows.UIElement voption

  // All ViewElement property setters - some are simple values, some represent events,
  // and some (not shown here) are child ViewElements

  [<DefaultValue>] val mutable private _AllowDrop: bool voption
  member this.AllowDrop with set x = this._AllowDrop <- ValueSome x

  // CacheMode is another ViewElement
  [<DefaultValue>] val mutable private _CacheMode: CacheMode voption
  member this.CacheMode with set x = this._CacheMode <- ValueSome x

  [<DefaultValue>] val mutable private _DragEnter: EventHandlerWrapper<System.Windows.DragEventArgs> voption
  member this.DragEnter with set x = this._DragEnter <- EventHandlerWrapper(this, x) |> ValueSome

  [<DefaultValue>] val mutable private _DragLeave: EventHandlerWrapper<System.Windows.DragEventArgs> voption
  member this.DragLeave with set x = this._DragLeave <- EventHandlerWrapper(this, x) |> ValueSome

  member internal this.SetInitialProps (v: System.Windows.UIElement) =
    base.SetInitialProps v
    this.SilenceEvents <- true  // This is from the base ViewElement class
    this._AllowDrop |> ValueOption.iter (fun x -> v.AllowDrop <- x)
    this._CacheMode |> ValueOption.iter (fun x -> v.CacheMode <- x.RenderNew () :?> System.Windows.Media.CacheMode)
    this._DragEnter |> ValueOption.iter (fun x -> x.Subscription <- v.DragEnter.Subscribe x.Fn)
    this._DragLeave |> ValueOption.iter (fun x -> x.Subscription <- v.DragLeave.Subscribe x.Fn)
    this.SilenceEvents <- false

  member internal this.UpdateProps (prev: UIElement, v: System.Windows.UIElement) =
    base.UpdateProps (prev, v)
    this.SilenceEvents <- true
    // See below for updateValue and updateFn definitions
    updateValue _DefAllowDrop prev._AllowDrop this._AllowDrop id (fun x -> v.AllowDrop <- x)
    updateValue _DefCacheMode prev._CacheMode this._CacheMode id (fun x -> v.CacheMode <- x)
    updateFn prev._DragEnter this._DragEnter v.DragEnter
    updateFn prev._DragLeave this._DragLeave v.DragLeave
    this.SilenceEvents <- false

  /// Renders and returns a new view
  override this.RenderNew () : System.Windows.DependencyObject =
    let v = new System.Windows.UIElement()
    this.__View <- ValueSome v
    this.SetInitialProps v
    upcast v

  /// Attempts to incrementally update an existing view. If successful,
  /// returns ValueNone. Otherwise renders and returns the new view.
  override this.UpdateIncremental (prev: ViewElement) : System.Windows.DependencyObject voption =
    if System.Object.ReferenceEquals(this, prev) then ValueNone
    else
      match prev with
      | :? UIElement as prev ->
          match prev.__View with
          | ValueNone -> 
              this.RenderNew () |> ValueSome
          | ValueSome v ->
              this.__View <- ValueSome v
              this.UpdateProps (prev, v)
              ValueNone
      | _ -> 
          this.RenderNew () |> ValueSome

Some relevant helpers from the code above:

type EventHandlerWrapper<'a>(owner: Node, f: 'a -> unit) =
  let id = f.GetType()
  let fn = fun x -> if owner.SilenceEvents then () else f x
  member __.Fn = fn
  member __.Id = id
  member val Subscription = 
    { new IDisposable with member __.Dispose () = () }
    with get, set

// Note that I will write separate variants of this: One where I know 'a to be
// a ViewElement, one where I know it to not be a ViewElement, and one where
// 'a is e.g. obj and thus might or might not be a ViewElement. That would allow
// me to avoid boxing/unboxing. I have the logic in place to do it, I just haven't
// written the helpers yet.
let updateValue 
    (defVal: 'raw)
    (prev: 'a voption)
    (curr: 'a voption)
    (map: 'b -> 'raw)
    (setValue: 'raw -> unit) =
  match ValueOption.map box prev, ValueOption.map box curr with
  | ValueNone, ValueNone -> 
      ()
  | ValueSome _, ValueNone -> 
      setValue defVal
  | ValueSome (:? Node as prev), ValueSome (:? Node as curr) ->
      curr.UpdateIncremental prev |> ValueOption.iter (unbox<'b> >> map >> setValue)
  | ValueSome prev, ValueSome curr when prev = curr -> 
      ()
  | _, ValueSome (:? Node as curr) -> 
      curr.RenderNew () |> unbox<'b> |> map |> setValue
  | _, ValueSome curr -> 
      curr |> unbox<'b> |> map |> setValue

let updateFn (prev: EventHandlerWrapper<'a> voption) (curr: EventHandlerWrapper<'a> voption) (event: IEvent<'sender, 'a>) =
  match prev, curr with
  | ValueNone, ValueNone -> 
      ()
  | ValueSome prev', ValueSome curr' when prev'.Id = curr'.Id -> 
      curr'.Subscription <- prev'.Subscription
  | ValueSome prev', ValueSome curr' -> 
      prev'.Subscription.Dispose ()
      curr'.Subscription <- event.Subscribe curr'.Fn
  | ValueSome prev, ValueNone -> 
      prev.Subscription.Dispose ()
  | ValueNone, ValueSome curr -> 
      curr.Subscription <- event.Subscribe curr.Fn

[dsyme]

When you say "control" here, are you referring to the actual UI controls?

Yes.

I don't think we really access the underlying control when doing the diff, only to apply the updates, e.g.

        match prevHorizontalOptionsOpt, currHorizontalOptionsOpt with
        | ValueSome prevValue, ValueSome currValue when prevValue = currValue -> ()
        | _, ValueSome currValue -> target.HorizontalOptions <-  currValue
        | ValueSome _, ValueNone -> target.HorizontalOptions <- Xamarin.Forms.LayoutOptions.Fill
        | ValueNone, ValueNone -> ()

Perhaps the list diffing does look at the existing control. But logically speaking it should be able to do everything off the previous-current view diff.

Regarding making the diff explicit: in the current setup it would only be possible to construct a diff object by doing the same comparison process that we are doing today. Then you would walk the diff and apply it. So the current code is effectively the merge these two phases and the elimination of the intermediate diff object.

So I'm not yet convinced making the diff object explicit will intrinsically help perf (though the code may be cleaner, which may be important).

Unboxing is a type test, is it showing up in the performance profiles as the most expensive micro-operation?

It's possible that for back-and-forth navigation pages then keeping multiple (detached) "previous" control trees with matching "previous" ViewElement descriptions might work, effectively capturing the different modalities of the UI in stored, applied collection of controls ready to apply a small diff to and display. A declarative hint could help here, and it could be automatically active for different pages of NavigationPage?

[TimLariviere]

I don't think we really access the underlying control when doing the diff, only to apply the updates

The issue is not really with primitive properties like HorizontalOptions, it's more about content properties (Content, Children). We're not accessing the real native controls, but we still need to access a significant part of the created XF controls just to make sure they don't need any update.

A typical page would be:

View.ContentPage(
    content=View.Grid(
        View.StackLayout(orientation=LayoutOptions.Horizontal,
            children=[
                View.Label("Some text")
                View.Label("Some other text")
                View.Image(src="...")
            ]
        View.Label("Some text")
    )
)

Without proper dependsOn, any message will trigger a complete scan down to the deepest control, most of the times for nothing (either no UI change or not this deep).

And we can't require people to use dependsOn everywhere.

That's because ViewElements are references and not equatable, so any time you create a new one (View.Label) you will need to check all of its properties (only with prev/curr values), including scanning its children (its children's children, etc.).

So we spend some times doing operations that can be avoided by only comparing previous/current ViewElement before even using the XF controls. We will still scan down to the deepest level of the ViewElements, but the XF controls won't be involved (no need for boxing/unboxing, only direct comparison of dictionaries).

And if I learned one thing about Android since I do Xamarin, it can be really susceptible to a huge amount of rapid operations.

Though I agree, it is a little too big to say "That's our performance problem right here!". It's certain some other parts represent a good chunk of the current problem.

But I think precomputing the diff would greatly reduce those unneeded operations like boxing/unboxing, accessing content properties (Content, Children).

See my tentative for a new way of reacting to changes: https://github.com/TimLariviere/Fabulous/blob/new-diff-algorithm/src/Fabulous.Core/DummyView.fs

Still need to really benchmark things though.

[TimLariviere]

Performance has been drastically improved in v2 :)