Perf: Cache inventory calculation on dashboard #4939
Conversation
| end | ||
|
|
||
| private | ||
|
|
||
| attr_reader :current_organization | ||
| attr_reader :current_organization, :inventory |
There was a problem hiding this comment.
Do we need this to be readable from outside the class?
There was a problem hiding this comment.
It isn't readable from outside the class because it's defined below the private keyword:
There was a problem hiding this comment.
Then why do you need an attr_reader declaration at all?
There was a problem hiding this comment.
I'm using it on line 30 of num_locations_with_inventory. It isn't necessary as I could just access the instance variable directly. I chose this route just to follow the current pattern in this class (similar to current_organization which also doesn't really need an attr_reader).
My plan is to submit another PR refactoring OrganizationStats to use a call class method instead, as well as improve it's performance in terms of queries. But I figured this PR is just about refactoring to calculate the inventory once (which I imagine could provide a meaningful performance improvement in prod in some situations), so the changes here are as minimal as possible.
But if you prefer I can totally get rid of the attr_reader for inventory and access the @inventory instance variable directly.
Doesn't resolve any issue.
Description
Noticed a number of duplicate queries in the dashboard, so here's a PR to remove them.
Type of change
How Has This Been Tested?
Test suite and some manually testing locally.
Screenshots
These are just on my local machine with the seed database so YMMV
Before:
~70 ms response time
32 queries (11 cached)
After:
~56 ms response time
27 queries (6 cached)