search

moosetechnology / MooseIDE

New Tools for Moose
MIT License
8 stars 19 forks source link

Selecting a model in the model browser is slow #976

Closed jecisc closed 9 months ago

jecisc commented 9 months ago

I have an image with one model loaded (the image is 1.73Go big so it is a big model). If I open the model browser and click on this model then my image does not respond for more than 20sec. This seems a lot just to click on a model and doing nothing.

jecisc commented 9 months ago

I tried to profil and 84% of the time is spent in full GC. This seems a lot because we already customize the GC in Moose to not run as much as the default Pharo

jecisc commented 9 months ago

One location in the browsers that seems to cause this is MiModelsBrowserModel>>#selectedModel. This method will create collections such as moose groups and those intermediate collections might cause GC when we want to clean them

jecisc commented 9 months ago

The filtering happening in this method is supposed to be for propagation. Couldn't we just filter for the propagation instead of filtering when we ask the selected model?

jecisc commented 9 months ago

Or we should save the filtered item to not recompute it each time we are asking the selected model

NicolasAnquetil commented 9 months ago

you could also do some cleaning because MiModelsBrowser>>miSelectedModel does not call MiModelsBrowserModel>>miSelectedModel

 MiModelsBrowser>> miSelectedModel

    "similar to #miSelectedItem but ignore the setting #filterStubsSetting"

    ^ specModel selected

whereas MiModelsBrowser>>miSelectedModel does:

 MiModelsBrowser>> miSelectedItem

    ^ specModel selectedModel

This is very counter intuitive. So there are two methods, we just need to call the right one at the right time

jecisc commented 9 months ago

I did some optimization on #asMooseGroup that makes things faster. But the fact that we copy the whole model each time we request #miSelectedModel is still a big problem IMO when we have a big model

NicolasAnquetil commented 9 months ago

Why do we copy the model ? This is clearly not a good idea. Is that linked to the "Without stubs" setting ? If so, this is a mistake and it has been corrected in the past ... (integration problem ?)

jecisc commented 9 months ago

Yes, this is because of this setting. By default instead of returning the model we copy it without the stubs. So it copy the model at each access

NicolasAnquetil commented 9 months ago

The filtering happening in this method is supposed to be for propagation. Couldn't we just filter for the propagation instead of filtering when we ask the selected model?.

But I think we "ask the selected model" in only 2 cases: propagation and inspection, so this is already working as expected

jecisc commented 9 months ago

The model is requested for example to know how to display some buttons in the toolbars or menus of some browsers if I'm not mistaken. This means that for the buttons that request the model, we do a copy of this model

NicolasAnquetil commented 9 months ago

Yes, this is because of this setting. By default instead of returning the model we copy it without the stubs. So it copy the model at each access

The logger does that caching. Everything propagated is logged and can be re-accessed later. May be we should always open the logger so that the user knows it exists and understands what it does?

NicolasAnquetil commented 9 months ago

The model is requested for example to know how to display some buttons in the toolbars or menus of some browsers if I'm not mistaken. This means that for the buttons that request the model, we do a copy of this model

Yes, that might be the case. You are right