Performance optimization for #902 issue

[Opiumtm]

Updated method of MoreButton extraction from templated visual tree.

Replaced very expensive call XamlUtils.AllChildren<Control>(commandBar) with much optimized methods.

If EllipsisBehavior is applied to PageHeader control, used cached internal value (obtained and cached in overrided OnApplyTemplate protected method of PageHeader control). It is the most optimized method because WinRT call is performed only once when control template is applied.

If EllipsisBehavior is applied to CommandBar, used just 3 WinRT calls instead of very unoptimized manual visual tree walk.

Simple FindName works if called on templated visual tree root. No need for expensive manual tree walk with many WinRT interop calls.

if (VisualTreeHelper.GetChildrenCount(commandBar) > 0)  
{  
    var child = VisualTreeHelper.GetChild(commandBar, 0) as FrameworkElement; /* Templated root */  
    return child?.FindName("MoreButton") as Button;  
}  

[Opiumtm]

Tested this fix with real-life application on actual Lumia 950 device. UI performance radically improved. All visual lags and overall poor UI performance goes away.

[callummoffat]

My question would be: does this extensive optimization have any impact on stability? Speed shouldn't come at the expense of stability. I love the improved performance, but I'm a little worried about the potential cost to stability.

[Opiumtm]

@callummoffat Testing stability on actual device right now, not experienced crashes or incorrect work. Already tested in debugger. Anyway this fix is very conservative in nature. It doesn't change any high-level logic. Just technically improve performance bottleneck found by app profiling. If improved methods fail (they should not fail), fallback original logic is used. Leaved original method as fallback because of conservative approach in this fix.

[callummoffat]

That sounds... really good. I'm not seeing any problems with including it (from the perspective of a developer who knows little about T10's underlying code). Maximum performance for minimum risk is a good thing, no matter what way you look at it.

[Opiumtm]

@callummoffat Already wasted huge amount of time (couple of months to be honest) optimizing poor app performance to finally find this performance bottleneck. It doesn't critical on x86 desktop and somewhat tolerable on top tier Lumia 950. But on less powerful devices it performed very poor. It takes such a long time to investigate, exclude other possible factors, prepare correct testing conditions (not too easy on less affected x86 Intel desktop) and finally catch root cause in performance profiler, then investigate library code and find fundamentally unoptimized method called from a low-level system layout updating cycle on every visual tree update. It was a crazy adventure in coding and testing 😄

[callummoffat]

@JerryNixon,
This should definitely be included. Huge speed improvements with little or no impact on stability. It would be a shame not to include this.

[sebfrie]

I just wanted to second that. Please pull asap.

@Opiumtm Thanks for the adventure. :)

[JerryNixon]

Nice work.