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

Improve registry startup and lookup performance #674

Merged
merged 2 commits into from Jan 12, 2017

Conversation

Projects
None yet
6 participants
@hartez
Member

hartez commented Jan 10, 2017

Optimize search for Effect resolution name
Avoid multiple collection of assemblies during startup
Cache handler lookups

Description of Change

Don't resolve Effect resolution short name unless the assembly actually has effects.

If the Dependency Service hasn't already been initialized, pass the assembly list to it for initialization (so the Dependency Service doesn't have to compile that list again for itself).

When looking up handler types, cache null results to avoid traversing the class hierarchy on subsequent lookups.

Bugs Fixed

  • None

API Changes

  • None

Behavioral Changes

  • None

PR Checklist

  • Has tests (if omitted, state reason in description)
  • Rebased on top of master at time of PR
  • Changes adhere to coding standard
  • Consolidate commits as makes sense
Optimize search for Effect resolution name
Avoid multiple collection of assemblies during startup
Cache handler lookups
@StephaneDelcroix

How much time does this shave ?

internal static void Initialize(Assembly[] assemblies)
{
if (s_initialized)

This comment has been minimized.

@StephaneDelcroix

StephaneDelcroix Jan 11, 2017

Member

why don't you do this check in the parameterless Initialize() ?

@StephaneDelcroix

StephaneDelcroix Jan 11, 2017

Member

why don't you do this check in the parameterless Initialize() ?

This comment has been minimized.

@hartez

hartez Jan 11, 2017

Member

The only path to the parameterless Initialize() already checks s_initialized before calling it.

That said, checking it inside of Initialize() would be clearer; I'll update it.

@hartez

hartez Jan 11, 2017

Member

The only path to the parameterless Initialize() already checks s_initialized before calling it.

That said, checking it inside of Initialize() would be clearer; I'll update it.

@hartez

This comment has been minimized.

Show comment
Hide comment
@hartez

hartez Jan 11, 2017

Member

@StephaneDelcroix It doesn't do much - on an Android device for an app with enough assemblies, it can shave a few milliseconds off of startup.

Member

hartez commented Jan 11, 2017

@StephaneDelcroix It doesn't do much - on an Android device for an app with enough assemblies, it can shave a few milliseconds off of startup.

@rmarinho rmarinho merged commit 1616413 into master Jan 12, 2017

6 checks passed

Android-UITests-C8 Finished TeamCity Build Xamarin.Forms :: Debug :: Cycle 8 :: UI Tests :: OSX Test Cloud Package - Run Android 6.0.1 : Tests passed: 351, i…
Details
OSX-Debug-C8 Finished TeamCity Build Xamarin.Forms :: Debug :: Cycle 8 :: OSX Debug : Running
Details
Windows-Debug-C8 Finished TeamCity Build Xamarin.Forms :: Debug :: Cycle 8 :: Windows Debug : Tests passed: 3695, ignored: 10
Details
iOS10-UITests-C8 Finished TeamCity Build Xamarin.Forms :: Debug :: Cycle 8 :: UI Tests :: OSX Test Cloud Package - Run iOS Unified iOS10 : Tests passed: 34…
Details
iOS8-UITests-C8 Finished TeamCity Build Xamarin.Forms :: Debug :: Cycle 8 :: UI Tests :: OSX Test Cloud Package - Run iOS Unified IOS8 : Tests passed: 344…
Details
iOS9-UITests-C8 Finished TeamCity Build Xamarin.Forms :: Debug :: Cycle 8 :: UI Tests :: OSX Test Cloud Package - Run iOS Unified iOS9 : Tests passed: 346…
Details

@rmarinho rmarinho referenced this pull request May 3, 2017

Merged

Fix bugzilla55542 #898

0 of 4 tasks complete

@hartez hartez deleted the registry-perf-enhancements branch May 16, 2017

@samhouts samhouts added this to the 2.3.4 milestone Jun 27, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment