Skip to content
This repository has been archived by the owner on May 1, 2024. It is now read-only.

[android] remove Anticipator.cs for now #8858

Merged

Conversation

jonathanpeppers
Copy link
Member

Description of Change

Context: https://github.com/jonathanpeppers/anticipator
Context: #8245

I was profiling the Blank Xamarin.Forms app template and saw this:

Method call summary
Total(ms) Self(ms)      Calls Method name
   40095        0         11 (wrapper runtime-invoke) <Module>:runtime_invoke_void__this___object (object,intptr,intptr,intptr)
   40094        0          8 System.Threading.ThreadHelper:ThreadStart (object)
   40094        0          8 System.Threading.ExecutionContext:Run (System.Threading.ExecutionContext,System.Threading.ContextCallback,object)
   40094        0          8 System.Threading.ExecutionContext:Run (System.Threading.ExecutionContext,System.Threading.ContextCallback,object,bool)
   40094        0          8 System.Threading.ExecutionContext:RunInternal (System.Threading.ExecutionContext,System.Threading.ContextCallback,object,bool)
   40092        0          8 System.Threading.ThreadHelper:ThreadStart_Context (object)
   40092        0          8 Xamarin.Forms.Platform.Android.Anticipator:Loop (object)
   40035        0         16 System.Threading.WaitHandle:WaitOne (System.TimeSpan)
   40035        0         16 System.Threading.WaitHandle:WaitOne (System.TimeSpan,bool)
   40035        0         16 System.Threading.WaitHandle:WaitOne (long,bool)
   40035        0         16 System.Threading.WaitHandle:InternalWaitOne (System.Runtime.InteropServices.SafeHandle,long,bool,bool)
   40034        0         16 System.Threading.WaitHandle:WaitOneNative (System.Runtime.InteropServices.SafeHandle,uint,bool,bool)
   40034    40034         16 (wrapper managed-to-native) System.Threading.WaitHandle:Wait_internal (intptr*,int,bool,int)

The Anticipator class stuck out, so I profiled it separately in a
blank app (not using Xamarin.Forms at all). This project isolates the
Anticipator, in timing specifically Build.VERSION.SdkInt and the
Resource.designer.cs class. This launches an app 10 times and
averages the result.

The results were that it hurt performance in every scenario:

HAXM Emulator / Debug
    Anticipator: 141.273
    No Anticipator: 108.182
HAXM Emulator / Release
    Anticipator: 86.182
    No Anticipator: 63.545
Pixel 3XL / Debug
    Anticipator: 101.545
    No Anticipator: 93.182
Pixel 3XL / Release
    Anticipator: 45.818
    No Anticipator: 38.455
Pixel 3XL / Release+AOT
    Anticipator: 33.636
    No Anticipator: 30.545

Feel free to try out my sample as additional verification.

I would recommend removing this class for now, and only bring it back
when we have some numbers showing it improves things. Sorry!

Let me know if I need to target a branch other than master, thanks!

Issues Resolved

n/a

API Changes

None

Platforms Affected

  • Android

Behavioral/Visual Changes

None

Before/After Screenshots

Not applicable, I think

Testing Procedure

Feel free to the project here, for additional verification: https://github.com/jonathanpeppers/anticipator

I will work on getting a script like this in Xamarin.Forms itself, documentation, etc.

PR Checklist

  • Targets the correct branch
  • Tests are passing (or failures are unrelated)

@jonathanpeppers
Copy link
Member Author

rebased to fix merge conflict

Copy link
Contributor

@kingces95 kingces95 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please do not delete Anticipator.cs as I'm editing it and removing file will cause me merge headaches. Feel free to use use preprocessor directives to exclude it if you'd like.

@jonathanpeppers
Copy link
Member Author

jonathanpeppers commented Jan 7, 2020

@kingces95 I can use #if 👍

Should this also retarget 4.4.0? I opened this PR before I understood how the branching is done here.

@kingces95
Copy link
Contributor

You've removed the call sites so simplest thing would be to simply undelete the file.

Context: https://github.com/jonathanpeppers/anticipator
Context: xamarin#8245

I was profiling the Blank Xamarin.Forms app template and saw this:

    Method call summary
    Total(ms) Self(ms)      Calls Method name
       40095        0         11 (wrapper runtime-invoke) <Module>:runtime_invoke_void__this___object (object,intptr,intptr,intptr)
       40094        0          8 System.Threading.ThreadHelper:ThreadStart (object)
       40094        0          8 System.Threading.ExecutionContext:Run (System.Threading.ExecutionContext,System.Threading.ContextCallback,object)
       40094        0          8 System.Threading.ExecutionContext:Run (System.Threading.ExecutionContext,System.Threading.ContextCallback,object,bool)
       40094        0          8 System.Threading.ExecutionContext:RunInternal (System.Threading.ExecutionContext,System.Threading.ContextCallback,object,bool)
       40092        0          8 System.Threading.ThreadHelper:ThreadStart_Context (object)
       40092        0          8 Xamarin.Forms.Platform.Android.Anticipator:Loop (object)
       40035        0         16 System.Threading.WaitHandle:WaitOne (System.TimeSpan)
       40035        0         16 System.Threading.WaitHandle:WaitOne (System.TimeSpan,bool)
       40035        0         16 System.Threading.WaitHandle:WaitOne (long,bool)
       40035        0         16 System.Threading.WaitHandle:InternalWaitOne (System.Runtime.InteropServices.SafeHandle,long,bool,bool)
       40034        0         16 System.Threading.WaitHandle:WaitOneNative (System.Runtime.InteropServices.SafeHandle,uint,bool,bool)
       40034    40034         16 (wrapper managed-to-native) System.Threading.WaitHandle:Wait_internal (intptr*,int,bool,int)

The `Anticipator` class stuck out, so I profiled it separately in a
blank app (not using Xamarin.Forms at all). This project isolates the
`Anticipator`, in timing specifically `Build.VERSION.SdkInt` and the
`Resource.designer.cs` class. This launches an app 10 times and
averages the result.

The results were that it hurt performance in every scenario:

    HAXM Emulator / Debug
        Anticipator: 141.273
        No Anticipator: 108.182
    HAXM Emulator / Release
        Anticipator: 86.182
        No Anticipator: 63.545
    Pixel 3XL / Debug
        Anticipator: 101.545
        No Anticipator: 93.182
    Pixel 3XL / Release
        Anticipator: 45.818
        No Anticipator: 38.455
    Pixel 3XL / Release+AOT
        Anticipator: 33.636
        No Anticipator: 30.545

Feel free to try out my sample as additional verification.

I would recommend removing this class for now, and only bring it back
when we have some numbers showing it improves things. Sorry!
@jonathanpeppers jonathanpeppers changed the base branch from master to 4.4.0 January 7, 2020 22:12
@kingces95 kingces95 merged commit d3877e3 into xamarin:4.4.0 Jan 8, 2020
@jonathanpeppers jonathanpeppers deleted the remove-anticipator-for-now branch January 8, 2020 21:34
@samhouts samhouts added this to the 4.4.0 milestone Jan 9, 2020
@samhouts samhouts added the t/housekeeping ♻︎ Internal only changes, won't be included in release notes label Jan 10, 2020
tuyen-vuduc pushed a commit to NAXAM/Xamarin.Forms that referenced this pull request Jan 14, 2020
* 'master' of github.com:xamarin/Xamarin.Forms: (330 commits)
  [Android] Fix color filter usage on API29 (xamarin#9180)
  Add null check to GetIconColor (xamarin#9172)
  Apply cecil fixes to make packages 2017 compatible (xamarin#9145)
  [UWP] Attempt to resolve entry on UWP not correctly calculating the correct height when in a scroll view (xamarin#8214)
  Fixes 7992 Changes UWP DatePicker to show the picker flyout on DatePickerFocus() (xamarin#8056)
  Update bug_report.md (xamarin#8688)
  [android/ios] improve perf when not using Application.Properties (xamarin#8887)
  The great Androidx IF Def'ing of 2019 (xamarin#8898)
  Added IconColor property for managing navigation icon color (xamarin#5185)
  Add UWP display prompt (xamarin#8720)
  fix bad merge
  Fix 8743 - now using specific style in SearchBar [UWP] (xamarin#8773)
  Added the SwipeView tag to the Core Gallery samples (xamarin#8819)
  [Tizen] Shell: FlyoutBackgroundImage, FlyoutBackgroundImageAspect (xamarin#8905)
  [Core] remove array covariant cast for UWP (xamarin#9135)
  [android] remove Anticipator.cs for now (xamarin#8858)
  Update the CarouselView Position setting the CurrentItem (xamarin#7946) fixes xamarin#7924
  send remove events (xamarin#9124)
  [platform] improve perf of PropertyChangedEventArgsExtensions (xamarin#9084)
  Fix SeachBarRenderer CreateNativeControl issue (xamarin#8946)
  ...

# Conflicts:
#	Xamarin.Forms.Core/FontImageSource.cs
jonathanpeppers added a commit to jonathanpeppers/Xamarin.Forms that referenced this pull request Jan 16, 2020
Context: xamarin#8858

After merging from 4.4.0, etc. this class came back.

@kingces95 did not want to remove the `Anticipator.cs` file. Since
`Xamarin.Forms.Platform.Android.csproj` is now a short-form MSBuild
project, you have to add an explicit:

`<Compile Remove="Anticipator.cs"/>` to remove it.
samhouts pushed a commit that referenced this pull request Jan 17, 2020
Context: #8858

After merging from 4.4.0, etc. this class came back.

@kingces95 did not want to remove the `Anticipator.cs` file. Since
`Xamarin.Forms.Platform.Android.csproj` is now a short-form MSBuild
project, you have to add an explicit:

`<Compile Remove="Anticipator.cs"/>` to remove it.
@jonpryor jonpryor mentioned this pull request Mar 12, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
a/performance p/Android t/housekeeping ♻︎ Internal only changes, won't be included in release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants