Optimize Xamarin Forms DryIoc adapter performance

[dadhi]

Hi, I am an author of DryIoc.

Looking at code, it seems over-complecated and therefore minimizes the wins provided by DryIoc performance.

The goal of this issue (for me) is to gather more info about IoC conventions used by Prism Application. At the moment I am unable to use Xamarin (dev license issues), and cannot PR directly. But I can do educated guesses :-) and list the ideas for improvements here. Then me, when I have license, or may be someone else may try them.

[dansiegel]

@dadhi if you want to join the Slack channel we can easily discuss there what improvements should be made to optimize the DryIoc implementation. The specific function you're pointing to we have a requirement for the ViewModelLocator to construct the ViewModel with an instance of the INavigationService that is aware of the page it will be used on (assuming that it is needed).

[brianlagunas]

The INavigationService is not a singleton and is specific to a particular page. This means an instance of the nav service must be created, properties set, and then injected into each ViewModel manually. Is there a better way?

[brianlagunas]

By the way, I am considering making DryIoc the new "official" container of Prism. So any improvements you can make to the implementation the better :)

[dadhi]

Thanks for responding!

What I see in this fragment and in other places is using of Service Locator instead of Dependency Injection. Instead of injecting the required service, the container/locator itself is injected, and then the service is Resolved from it. Not so much benefits from DI, more like double work.

Ideally, the fragment in question should look like this:

 ViewModelLocationProvider.SetDefaultViewModelFactory((view, vmType) =>
        {
            var page = view as Page;
            if(page != null)
            {
                var getVM = Container.Resolve<Func<Page, object>(vmType);

                // Page will passed to vm or its dependencies, including nav service, if any.
                return getVM(page);
            }
            return Container.Resolve(type);
        });

The problem is that current nav service accepts Page as property. DryIoc won't pass Func arguments to properties (at the moment). Then why not to add Page to nav service constructor?

As I understood nav service is transient and bound to page?

[brianlagunas]

For each page that is created, an INavigationService instance must be created and the Page property must be set. Then that INavigationService instance must be injected into the page's ViewModel ctor if the INavigationService interface is defined as a dependency in the VM ctor. I'm not apposed to adding a Page parameter to the ctor as long as it works :)

[brianlagunas]

At first glance this does seem to work. However, if we add the Page parameter to the ctor, a Page object will always be created even when it's not wanted. Sometimes it needs to be null so that we access the Application's MainPage: https://github.com/PrismLibrary/Prism/blob/master/Source/Xamarin/Prism.Forms/Navigation/PageNavigationService.cs#L492

Now, you'll probably say that to turn off WithAutoConcreteTypeResolution, but we need this in order for other concrete objects to be created when required. Such as the XF NavigationPage.

[brianlagunas]

Would this be better than what we have now?

                if (page != null)
                {
                    var navService = CreateNavigationService(page);
                    var getVM = Container.Resolve<Func<INavigationService, object>>(type);
                    return getVM(navService);
                }

[dadhi]

I have created the simple model here to experiment with. May be wrong though.

Now, you'll probably say that to turn off WithAutoConcreteTypeResolution, but we need this in order for other concrete objects to be created when required. Such as the XF NavigationPage.

It is better to avoid such convention in framework itself. If user wants such magic in the App services, it is his choice.

[brianlagunas]

It is better to avoid such convention in framework itself. If user wants such magic in the App services, it is his choice.

You will never win this argument :). If this option is turned off, Prism breaks and is completely unusable as none of the ViewModels would be created.

[dadhi]

Just a heads up that I am working on DryIoc v3 which will released in upcoming weeks. May expect perf improvements and removal of some integration pain points.

[brianlagunas]

Great to hear!

[brianlagunas]

Hey @dadhi, I was wondering why this code doesn't work when providing a serviceKey:

static void Main(string[] args)
        {
            const string navigationServiceName = "navigationService";

            var container = new Container();            

            container.Register<INavigationService, PageNavigationService>(serviceKey: navigationServiceName);
            container.Register<INavigationService>(
                made: Made.Of(() => SetPage(Arg.Of<INavigationService>(navigationServiceName), Arg.Of<Page>())),
                setup: Setup.DecoratorWith(r => navigationServiceName.Equals(r.ServiceKey)));

            container.Register<ViewModel>();

            var getVM = container.Resolve<Func<Page, object>>(typeof(ViewModel));
            var result = getVM(new Page());
        }

        internal static INavigationService SetPage(INavigationService navigationService, Page page)
        {
            if (navigationService is IPageAware pageAware)
                pageAware.Page = page;

            return navigationService;
        }

        internal interface INavigationService { }

        internal class Page { }

        internal interface IPageAware
        {
            Page Page { get; set; }
        }

        internal class PageNavigationService : INavigationService, IPageAware
        {
            public Page Page { get; set; }
        }

        internal class ViewModel
        {
            public INavigationService NavService { get; }
            public ViewModel(INavigationService navigationService)
            {
                NavService = navigationService;
            }
        }

Is this supported?

[brianlagunas]

Never mind, I think I understand what is going on and why this doesn't work. The serviceKey is not the parameter name when the INavigationService is being resolved as a dependency.

[dadhi]

Ok, I need to run it to figure out.

Btw, the next DryIoc v3 will support (already in preview) the Resolve(blah, object[] args), so this Func mangling may be minimized.

[dadhi]

@brianlagunas ,

In your sample when resolving ViewModel you need to specify navigation service key, as it is used everywhere:

        container.Register<ViewModel>(
            made: Parameters.Of.Type<INavigationService>(serviceKey: navigationServiceName));

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.