Fix stock performance issues #5143
Conversation
jniles
force-pushed
the
fix-stock-performance-issues
branch
5 times, most recently
from
bc839b2 to
3ce70f7
Compare
jniles
force-pushed
the
fix-stock-performance-issues
branch
2 times, most recently
from
e315c58 to
fbd5b08
Compare
|
@mbayopanda can I get a review? I discovered that on master, the months of stock for the CMM was 5, according to the |
jniles
force-pushed
the
fix-stock-performance-issues
branch
2 times, most recently
from
f522586 to
a7a1e35
Compare
|
@mbayopanda can this be merged? |
|
I would like to review this PR, but I'm unclear how to evaluate its performance improvement. |
|
@jmcameron, stock exit is the best example of this. Here are the things to look for:
|
|
Another thing you can do, I suppose is to print the DB logs ( |
jniles
force-pushed
the
fix-stock-performance-issues
branch
from
a7a1e35 to
48e6c25
Compare
Caches the results for the stock components to ensure that we do not overwhelm our servers with HTTP requests.
Removes extraneous code paths from the CMM calculation and combines the duplicated code into a single function to calculate the CMM. Also remove the redundant calls to getCMM().
Removed unused indicators from the stock core reporting.
jniles
force-pushed
the
fix-stock-performance-issues
branch
from
48e6c25 to
abd0a3c
Compare
There was a problem hiding this comment.
Looks good. My review of the code was more like a skim of changes, but the changes seem okay. I tested the latest BHIMA and IMCK data with and without this PR. The number of calls to getCMM() is definitely reduced with this PR. But there are still possibly some duplicate calls. I noticed a definite speed up. With the IMCK data, the "loading data" part of the Stock exit page took 2-3 seconds with master and was less than a second with this PR. So this is a worthwhile refactor.
| @@ -4,27 +4,27 @@ | |||
|
|
|||
| <table | |||
| class="table table-condensed table-bordered" | |||
| ng-show="$ctrl.soldOutInventories.length > 0"> | |||
| ng-show="$ctrl.stockOutInventories.length > 0"> | |||
Fixes the fact that we have some months with 30 days and others with 31 days.
|
@jmcameron thanks for your review! I'm going to include this in the quick fix release we do at IMCK today, since our production users will hammer it and let us know if there are any breaking issues. Much better to find that our while we can work with them here than have them call us from afar... |
|
bors r+ |
|
Build succeeded: |
This PR addresses two performance issues:
stock_consumptionin the stock controller and merged the calls togetCMMinto a single function. Then we optimized the function to only callgetCMM()for unique depot/inventory pairs.While doing so, I've refactored the CMM calculations in stock/core.js to make the more streamlined and rename generic sounding names to more meaningful names . There are two basic paths through core.js:
getBulkInventoryCMM()->computeInventoryIndicators()getBulkInventoryCMM()->computeInventoryIndicators()->computeLotIndicators()The first path computes S_MAX, S_MIN, and other inventory indicators that relate to the change of stock out. This is good enough for the "Items in Stock" registry and other reports that need to see the data on an inventory level.
The second path does everything in the first, but also computes the S_RISK and S_RISK_QUANTITY and other lot indicators at are related to the risk of expiration of an individual lot.
Closes #4711
Partially addresses #5127
Partially addresses #5073