Skip to content
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

Startup/Scan speed optimisation #409

Open
oatsoda opened this issue Mar 14, 2023 · 15 comments
Open

Startup/Scan speed optimisation #409

oatsoda opened this issue Mar 14, 2023 · 15 comments

Comments

@oatsoda
Copy link

oatsoda commented Mar 14, 2023

Hi

In a large Blazor Wasm project, I'm seeing the startup scan taking 500-700ms on average, and then the Store Initialiser taking approx 200-300ms.

I have the <Fluxor.Blazor.Web.StoreInitializer /> in my App component and I have the following startup registration:

services.AddFluxor(
     options =>
     {
        options.ScanAssemblies(typeof(SomeStoreState).Assembly);
      });

Presumably the large-ish assembly is the cause of this as it scans looking for all the registrations?

Is there anything I can do to optimise (aside from split the assembly up)? I notice there is a TypesToScan on the startup options - would this be more targeted? Is it just the Store types I need or is it all Store, Reducer and Effect types required?

Any clues would be gratefully received!

@mrpmorris
Copy link
Owner

mrpmorris commented Mar 14, 2023

It only scans public classes, so if you make other classes internal it should help.

@oatsoda
Copy link
Author

oatsoda commented Mar 17, 2023

Thanks. If I specified all of the TypesToScan can I avoid an Assembly scan completely?

@mrpmorris
Copy link
Owner

I don't think so, but you can try it :)

@oatsoda
Copy link
Author

oatsoda commented Mar 20, 2023

Just completed an experiment moving all the Stores/Effects/Reducers to their own project and it didn't affect the scan time or Store Initializer.

Which I think implies it wasn't due to the speed of having to scan the whole assembly, but actually the registration of the types themselves?

These are the total stores I have:

43 [FeatureState] attributes
301 [ReducerMethod] attributes
146 [EffectMethod] attributes

Would this be what you would expect for these numbers?

I'm not really sure where to go with it next, except to dive into the Fluxor code and understand where the time is lost?

@oatsoda
Copy link
Author

oatsoda commented Mar 23, 2023

FYI, I tried using the ScanTypes instead of the ScanAssemblies option, but it was similar ballpark timings.

Interestingly, if I run my startup code though a unit test, the timing is approx 50-100ms - so the issue is with the startup performance specifically in the browser/on wasm.

@oatsoda
Copy link
Author

oatsoda commented Mar 24, 2023

It looks like the main culprit is the AssemblyScanSettings.FilterMethods method. I guess because the reflection is slow in Web Assembly.

So probably the solution would require providing an alternative mechanism where I could make the service registrations without the need for reflection. I could then potentially use source generators to do the reflection to generate the registration code.

I note that a similar approach was mentioned here.

Anyone who uses a Wasm project and has a decent number of Types and Methods is likely going to see this degradation: which is not insignificant for a browser-based web app.

@mrpmorris
Copy link
Owner

If your assembly is purely store classes, then giving Fluxor a list of classes to scan isn't going to be any quicker, because it'll scan the same classes.

Even a code gen that creates a list of methods to scan at compile time won't speed it up, because it'll still need to fetch the MethodInfo.

The problem isn't that there are too many methods being scanned that aren't matches, but that there are so many that are matches.

@mrpmorris
Copy link
Owner

I've added a benchmark app. It seems that having 100 classes each with 1 reducer or effect method takes about 4 times the amount of time as it does to have a single class with 100 reducer or effect methods in it.

I'm not sure if I am going to be able to do anything about this or not.

At the moment I wrap reducer/effect methods with instances of Effect and Reducer descendant classes at runtime and then call through to those methods. I might be able to speed things up using code-gen where I create those classes at compile time, and then decorate the assembly with

[FluxorClasses(typeof(State1), typeof(Reducers1), typeof(Effects1), typeof(State2), etc);

Then the reflection would only have to scan attributes on the assembly and be given all of them in one go.

I don't know how much this will bloat the app. If it bloats the app too much then it will make the first ever download take longer.

By the way, my timings were

  • 100 states
  • Each with 100 Reducer methods and 100 Effect methods

1.5 seconds.

@oatsoda
Copy link
Author

oatsoda commented Mar 26, 2023

Appreciate your input and time on this @mrpmorris. So that benchmarking app is running on Wasm? If so, I think those timings look comparable with my specific case.

Then the reflection would only have to scan attributes on the assembly and be given all of them in one go. I don't know how much this will bloat the app.

Interesting. That could work - what would the bloat be from? Presumably the source generator creating the single, large, attribute would be in a separate assembly?

Hard for me to assist with too many specifics as I'm not yet familiar with how Fluxor works internally - I only really have a high-level overview.

I suppose I was thinking more radically about pre-creating what appear to be delegates which currently are created from reflection (using Delegate.CreateDelegate), e.g.

Given I have

[FeatureState]
public record UserStore(IReadOnlyList<User> Users);

public class UserStoreReducers 
{
   [ReducerMethod]
   public async Task ReduceUserAdded(UserStore state, UserAdded action) {  // etc. }   
}

public class UserStoreEffects 
{
   [EffectMethod]
   public async Task UserAddedEffect(IDispatcher dispatcher, UserAdded action) { // etc. }
}

[FeatureState]
public record PaymentStore(IReadOnlyList<Payment> Payments);

public static class PaymentStoreReducers 
{
   [ReducerMethod]
   public async Task ReducePaymentUpdated(PaymentStore state, PaymentUpdated action) { // etc. }
}

Then I could declaratively define them:

fluxorOptions.AddState<UserStore>(s => 
   {
      s.WithReducerClass<UserStoreReducers>()
         .WithReducer<UserAdded>(usr => usr.ReduceUserAdded);

      s.WithEffectClass<UserStoreEffects>()
         .WithEffect<UserAddedEffect>(use => use.UserAddedEffect);
   }
);

fluxorOptions.AddState<PaymentStore>(s => 
   s.WithStaticReducer<PaymentUpdated>(PaymentStoreReducers.PaymentUpdated);
);

The idea being that I should then be able create a source generator to do the reflection over my Features/Reducers/Effects and generate this declarative code (in fact, the logic for the source generator could live in Fluxor as a "helper" method which people could just create the "shell" of the source generator and just call this one method to have them generated).

Appreciate this is a pretty different approach and I'm not familiar enough with all the different internal things to consider - attributes vs interface Features/Reducers/Effects, middleware etc.

@mrpmorris
Copy link
Owner

mrpmorris commented Mar 26, 2023

Reducers are actually written like this...

public class IncrementActionReducer : IReducer<CounterState>
{
  public bool ShouldReduceStateForAction(object action) => action is IncrementAction;
  public CounterState Reduce(CounterState state, object action) => state with { Counter = Counter + 1 };
}

Effects are written in a similar way.

When I encounter a [ReducerMethod] or [EffectMethod] I create an instance of a wrapper class that simply calls the decorated method.

Which means that if you use those attributes, there will always be a Type.GetMethod reflection call on that method

@mrpmorris
Copy link
Owner

PS: Changing the code in the way I suggested changed a 1.6 MB DLL to 2.4 MB.

I don't suppose that's so bad seeing as it gets downloaded and cached. Adding 1 second for a one-off download is better than waiting an extra 1 second each time the app starts I suppose.

@mrpmorris
Copy link
Owner

I've made some improvements in the Benchmarks branch.

Before : ScanAssemblies 31.47 ms
After: ScanAssemblies 27.21 ms

Not fantastic, but an improvement. Please give it a try and let me know.

@oatsoda
Copy link
Author

oatsoda commented Mar 31, 2023

Thanks @mrpmorris. I've tried my app against the Benchmarks branch (up to this commit).

It may have had a small benefit - possibly making it more consistent - but I am still seeing approx 500ms for the startup.

BTW, the benchmark app will measure the speed of your code relative to changes you make, but if you want to see the true speed you'll need to have a test WASM app with those test Features/Effects/Reducers.

@oatsoda
Copy link
Author

oatsoda commented Mar 31, 2023

Also, if you're looking for any other micro-optimisations, ReflectionScanner has a few cases of multiple-enumerating some IEnumerable<>s. Change the public Scan method and private GetCandidateTypes to take arrays instead - the origin types for AssembliesToScan and TypesToScan are arrays as defined in FluxorOptions, and you can enumerate scanIncludeList as an array the point it is created.

@mrpmorris
Copy link
Owner

@oatsoda I believe everything is passed as an array. Isn't that the case?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants