Fixes #20998: Performance of compliance computation in ComplianceLevel can be improved #4237
Conversation
ncharles
added
Trigger test
WIP
|
PR updated with a new commit |
|
PR updated with a new commit |
|
i'm unsure this does anything meaningfull at this point, I have only one working test and no valid support of precision |
|
PR updated with a new commit |
|
PR updated with a new commit |
|
PR updated with a new commit |
|
PR updated with a new commit |
ncharles
added
Trigger test
and removed
Trigger test
WIP
|
PR updated with a new commit |
|
PR updated with a new commit |
|
what s the status of that PR ? There is some commented out tests, should I still review it now ? |
|
You can review it. The tests are commented because they fail, but they failed also before (well, they didn't exist) |
| def pc_for(i:Int) : Double = { | ||
| if(i == 0) { | ||
| // these depends on the precision | ||
| val diviser = divisers(precision) |
There was a problem hiding this comment.
@fanf do you know what's the cost of doing an assert ? I'll need to assert that prevision is <=5
There was a problem hiding this comment.
create a case class precision with case object Level1 to Level5 and its value setted. It should not be perf impacting (object are global, and accessing a static field is very quick, and the traduction to level is only done at border like API so not important)
|
PR updated with a new commit |
|
This PR is not mergeable to upper versions. |
|
OK, squash merging this PR |
…l can be improved
… not correct, and performance is not correct
ncharles
force-pushed
the
bug_20998/performance_of_compliance_computation_in_compliancelevel_can_be_improved
branch
from
b68962b
to
970667c
Compare
|
OK, merging this PR |
https://issues.rudder.io/issues/20998
This PR revisits the way ComplianceLevel are computed, to make it faster, and to fix issue in specific cases (ex, 200 succes, 199 repaired, 1 error, 1 missing, 1 unexpected , 1 non compliancere gives 47% success and 50% repaired with 0 precision)
All in all, the computation is ~4 times faster, mainly by using Long computation rather than BigDecimal
The drawback is that we can't go further than precision 5 (I can't seem to see which use case would need more than that), so computation are made using Long, by multiplying with the power of ten, and then truncating.
It also introduces a method to compute compliance directly without pending result, so that we can skip the duplication of ComplianceLevel and the creation of a CompliantPercent
It is not enough to make the display of the Nodes list instantaneous in the Rule detail page, but my tests show it's twice as fast as before. Unfortunately the way it's done computed twice by level the compliance (once without pending, and one with pending), and it's made for the Rule, and then each Directives and Nodes, and then for each components, etc etc.
The API answers on my test platform in 2.4s (from 4.5s before) - removing all compliance computation makes it answer in 1s (which is the cost of getting the Groups and Rules)