Blazor ComponentBase.SetParametersAsync(ParameterView.Empty) still create costly WriterForType despite ParameterView.Empty

[sake402]

Is there an existing issue for this?

• I have searched the existing issues

Describe the bug

For performance reasons and faster startup, we have created a source generator that the automate the generation of Blazor Component's SetParameterAsync like this

    public partial class FooComponent
    {
        public override Task SetParametersAsync(ParameterView parameters)
        {
            foreach (var parameter in parameters)
            {
                switch (parameter.Name)
                {
                    case nameof(Value):
                    	Value = (System.Func<bool>)parameter.Value;
                    break;
                    case nameof(Show):
                    	Show = (LivingThing.Core.Frameworks.Common.Data.Subscription<bool>)parameter.Value;
                    break;
                    case nameof(Render):
                    	Render = (LivingThing.Core.Frameworks.Common.Data.Subscription<bool>)parameter.Value;
                    break;
                    case nameof(Control):
                    	Control = (LivingThing.Core.Frameworks.Client.Components.RenderControl)parameter.Value;
                    break;
                }
            }
            return base.SetParametersAsync(ParameterView.Empty);
        }
    }

After setting all our parameters, we call base.SetParametersAsync(ParameterView.Empty) so as to raise lifecycle event neccessary.

However, the codebase for ComponentBase.SetParametersAsync(ParameterView.Empty) is this

public virtual Task SetParametersAsync(ParameterView parameters)
        {
            parameters.SetParameterProperties(this);
            if (!_initialized)
            {
                _initialized = true;

                return RunInitAndSetParametersAsync();
            }
            else
            {
                return CallOnParametersSetAsync();
            }
        }

The line parameters.SetParameterProperties(this); calls into a codebase that instantiate the writer for this component, despite this is no longer needed and ends up wasting the precious time we are trying to save as well as memory (if you have a lot of components).

 var targetType = target.GetType();
            if (!_cachedWritersByType.TryGetValue(targetType, out var writers))
            {
                writers = new WritersForType(targetType); //This is very expensive as it does a lot of reflection
                _cachedWritersByType[targetType] = writers;
            }

I am proposing a pre-check on ParameterView in line parameters.SetParameterProperties(this); such that if the parameters is empty, there is no need to call parameters.SetParameterProperties(this); to avoid the startup cost

if (!parameters.IsEmpty){
parameters.SetParameterProperties(this);
}

Expected Behavior

No need to call parameters.SetParameterProperties(this); on empty ParameterView

Steps To Reproduce

No response

Exceptions (if any)

No response

.NET Version

No response

Anything else?

No response

[javiercn]

@sake402 thanks for contacting us.

We understand that it makes your case "faster", however that extra check is a tax on every other component when the vast majority of components will not take advantage of that optimization.

For performance reasons and faster startup, we have created a source generator that the automate the generation of Blazor Component's SetParameterAsync like this

If you are already generating the code, I would suggest you generate the code in your components to call the lifecycle methods in the same way we do.

FYI. We have in mind doing something like this (it is in our backlog)

[sake402]

@javiercn Thanks for your response. But I quite disagree with your assertion that extra check is a tax on every other component when the vast majority of components will not take advantage of that optimization. The cost of creating the WriterForType is rather very huge compared with checking a boolean variable.

I am currently running with a fork of the repo where I added this to ParameterView to check if it is constructed from ParameterView.Empty. The ParameterView.Empty is constructed from a static _emptyFrames refence so it's just a reference comparison here compared to the alternative of a heavy(of course one time) reflection job.

internal bool IsEmpty => _frames == _emptyFrames

[javiercn]

The cost of creating the WriterForType is rather very huge compared with checking a boolean variable.

Yes, but creating the writer is a one-time operation.

Also, most components have parameters, which is why the branch will not be taken in most cases. I'd rather us make a change that allows users to intercept and provide their own implementations of parameters.SetParameterProperties(this); rather than having to change the base class to allow for optimizations.

One such possibility likely involves having the component implement an interface and using that to set the parameters instead of doing so via reflection. The interface implementation can be generated via a SG and at that point you do not need to override anything nor change the base class for account for optimizations in the derived classes.

[sake402]

Also, most components have parameters, which is why the branch will not be taken in most cases. I'd rather us make a change that allows users to intercept and provide their own implementations of parameters.SetParameterProperties(this); rather than having to change the base class to allow for optimizations.

Well you do have a point here.

One such possibility likely involves having the component implement an interface and using that to set the parameters instead of doing so via reflection. The interface implementation can be generated via a SG and at that point you do not need to override anything nor change the base class for account for optimizations in the derived classes.

But the interface already implemented by ComponentBase.SetParameterAsync from IComponent was meant exactly for this. Instead of calling another interface method from ComponentBase.SetParametersAsync, perhaps a refractor from

public virtual Task SetParametersAsync(ParameterView parameters)
        {
            parameters.SetParameterProperties(this);
            if (!_initialized)
            {
                _initialized = true;

                return RunInitAndSetParametersAsync();
            }
            else
            {
                return CallOnParametersSetAsync();
            }
        }

to

protected Task RaiseParameterSetEventAsync()
{
            if (!_initialized)
            {
                _initialized = true;

                return RunInitAndSetParametersAsync();
            }
            else
            {
                return CallOnParametersSetAsync();
            }
}
public virtual Task SetParametersAsync(ParameterView parameters)
        {
            parameters.SetParameterProperties(this);
            return RaiseParametersSetEventAsync();
        }

Now if I decide to implement my own SetParametersAsync, I can still raise the lifecycle event by calling RaiseParametersSetEventAsync()

[MackinnonBuck]

This sounds like a decent suggestion, however we'll decide on the best path forward here when we implement #29550.