flexmeasures icon indicating copy to clipboard operation
flexmeasures copied to clipboard

[WIP] Speed up assets and accounts pages

Open victorgarcia98 opened this issue 4 months ago • 6 comments

Description

Running against the simulation DB with 196 Assets and 59 users, the existing code takes about 134s to load the asset page. With the changes introduced in this PR, the asset pages loading time is brought down to 53s, which is still pretty high.

Further Improvements

  • Time profiling to identify slow routines.

Related Items

Closes https://github.com/FlexMeasures/flexmeasures/issues/964


  • [X] I agree to contribute to the project under Apache 2 License.
  • [X] To the best of my knowledge, the proposed patch is not based on code under GPL or other license that is incompatible with FlexMeasures

victorgarcia98 avatar Feb 26 '24 12:02 victorgarcia98

The loading time for the asset page is still unacceptable. How we process our internal API responses needs some work, I believe. For example, for assets, we are going through considerable trouble to recreate Asset objects, from what our API gives us, without querying our database and keeping them out of our db session. But then we also assign to it an owner (via a db query), child assets (via recursively processing our internal API response), a parent (via a db query) and sensors (via a db query). This is all costing considerable time when loading the assets page.

I see a couple of options:

  1. It would be a lot quicker to just query the asset table and have the objects live in the session.
  2. Bypass the internal API altogether.
  3. Stop loading owners, children and parents unless the UI needs them (the assets page doesn't use them), and have the response of the internal API include a sensor count so we don't need to query the sensor table.

Before you start work, know that I did a tech spike on these options, so I can quickly push some code once we decided.

@nhoening your input is desired.

Flix6x avatar Mar 11 '24 11:03 Flix6x

All I want to maintain is that if possible we call the internal API, as then we know we use our one layer where authorization is defined.

One or two internal API calls per UI page load should suffice, of course. We can definitely change anything after that. We should study if that approach is feasible for what the asset and account page needs to do.

nhoening avatar Mar 12 '24 13:03 nhoening

I implemented option 1, such that we still call the internal API for the auth check, but then reload the Assets from the db instead of separately querying its parents, owner, children and sensors. This further reduces the loading times on the assets page (I saw loading times of 17 seconds now).

Flix6x avatar Mar 18 '24 15:03 Flix6x

Should we run a profiler to check where the 17 seconds are spent?

nhoening avatar Mar 18 '24 16:03 nhoening

I noticed that there are fewer assets in the /assets listing compared to before this PR, so there must still be something wrong. Also, the asset icons (which are based on the asset type) are gone.

I tried it out locally and found the same. It turns out that we were only considering the assets of the current_user and the PUBLIC assets were missing, as well. Added these two and updated the tests.

victorgarcia98 avatar Mar 27 '24 22:03 victorgarcia98

Reminder: this PR requires an API changelog entry.

victorgarcia98 avatar Apr 02 '24 15:04 victorgarcia98