mobx-keystone icon indicating copy to clipboard operation
mobx-keystone copied to clipboard

Slowness with big model

Open hersentino opened this issue 2 years ago • 9 comments

Hello,

I use mobx-keystone in a project where i'm managing big amount of data that I get from api call. The model that I receive are not mobx-keystone snapshot but the api models match my mobx-keystone models, except for the $modeltype. When I instance my mobx-keystone models from the api data, it can be very long like more than 1 second. I've seen your fix on v0.67.0 version but in my case instantiations are still long with big models.

I've made a reproducible example here: https://github.com/hersentino/mobx-keystone-issue In this example you can find a big amount of data (that i really get from an api).

  • Clicking to the button "instance main model" you will be able to see the time taken by the instantiation of the model.
  • I also made a button that set a primitive variable in the store, this is to show that the heavier is the store, the longer the set will be (5ms with empty store, 25ms with big store), even if it just set a primitive variable.

I change all tProp by prop in models to save type checking time. Are I doing something wrong ? Thanks

hersentino avatar Mar 11 '22 17:03 hersentino

is your app going to mutate the models? if not, did you consider using frozen? https://mobx-keystone.js.org/frozen-data if you do, did you try transforming the data into a snapshot (and then using fromSnapshot instead of creating all the models)? or alternatively, if you use tProps you may ignore $modelType in snapshot in most cases since v0.64.0

xaviergonz avatar Mar 11 '22 21:03 xaviergonz

Btw, the times of time of setting are greatly reduced if you don't use the redux middleware.

With redux middleware: time of setting in the store : 187.5 ms time of change a primitive value in the store : 32.80000001192093 ms

Without redux middleware: time of setting in the store : 28.69999998807907 ms time of change a primitive value in the store : 0.699999988079071 ms

xaviergonz avatar Mar 11 '22 21:03 xaviergonz

or alternatively, if you use tProps you may ignore $modelType in snapshot in most cases since v0.64.0

Ok, I just tried that option and it is actually slower (1.4s vs 1.2s) due to the extra type checking involved to automatically detect the type.

xaviergonz avatar Mar 11 '22 21:03 xaviergonz

That being said I just published v0.67.1, which should make model creation around 25% faster when used with mobx 6. (In my case your example went down from 1000ms to around 750ms)

xaviergonz avatar Mar 11 '22 22:03 xaviergonz

Thanks for the quick responses !

I've just try without the reduxtools and with the fromSnapshot, like that :

const mainModel = fromSnapshot<MainModel>(data);

end the results are much quicker !

time of instanciation with fromGrpc : 289 ms time of setting in the store : 5.699999999953434 ms

if not, did you consider using frozen?

The problem is that I want to have models because I have modelAction and if I understand frozen are made for immutable data and the objects are not keystone models.

hersentino avatar Mar 11 '22 22:03 hersentino

const mainModel = fromSnapshot<MainModel>(data);

for that test to be accurate you'll need to either inject $modelType to your whole snapshot object tree or to make sure all model properties that reference models use a tProp and then use fromSnapshot(MainModel, data);

if you don't do that then you will just get a "simple" object without actions/etc.

The problem is that I want to have models because I have modelAction and if I understand frozen are made for immutable data and the objects are not keystone models.

Yes, that's not a good case for frozen then.

xaviergonz avatar Mar 11 '22 22:03 xaviergonz

if you don't do that then you will just get a "simple" object without actions/etc.

yep I did try and you're right I can't access to the modelAction.

for that test to be accurate you'll need to either inject $modelType

So I did try to modify my fromGrpc functions to inject the $modelType to the data and then use fromSnapshot<MainModel>(data) and the time of instantiation is pretty much the same (about 800ms), you can find a reproducible example in the branch "fromSnapshot" (https://github.com/hersentino/mobx-keystone-issue/tree/fromSnapshot).

Is it expected to have similar time with fromSnapshot ?

hersentino avatar Mar 18 '22 13:03 hersentino

I don't think fromSnapshot would help much, it basically does the same thing you do manually by creating sub-models.

the best way I could think of improving the speed is to either: a) use frozen for the stuff that doesn't need actions / is immutable b) use data models instead of actual "class" models (they should give you around the same speed as the from snapshot without $modelType, but they are a bit trickier to use)

xaviergonz avatar Mar 18 '22 17:03 xaviergonz

I'm trying the b solution and I start to have some good result, but i'm struggling to create reference with DataModel, can you confirm that is possible ?

Also I'm a chrome user so didn't notice yet but the example that I provide upside is about 2 times slower with firefox (without any extension, v98.0.2).

Result with chrome : time of instanciation : 765.4000000953674 ms

Result with firefox : time of instanciation : 2103 ms

hersentino avatar Apr 01 '22 07:04 hersentino