Feature: branch pruning in calculation engine

[frantzmiccoli]

This is:

- [ ] a bugfix
- [x] a new feature

Checklist:

• Changes are covered by unit tests
• Code style is respected
• Commit message explains why the change is made (see /~https://github.com/erlang/otp/wiki/Writing-good-commit-messages)
• CHANGELOG.md contains a short summary of the change
• Documentation is updated as necessary

Why this change is needed?

Calculation engine was resolving every function by first resolving its arguments including IFs, this was causing significant over evaluation when IFs were used as it meant for every case to be evaluated.

I have tested against 5 files made by 4 different people to ensure that this was not introducing regression, I have observed none. It generates speed improvement from 0% to 80% on those files.

[PowerKiKi]

@frantzmiccoli That seems like a very interesting improvement, but also very risky of course. As it seems you are still actively working on it, please rebase, squash and force push when you think you are done.

I has only a very quick look, but noticed a few new public methods. If possible it would be best to not grow the public API. I am not saying it can be avoidable in this case, but please keep it in mind.

@MarkBaker I added you as reviewer as it is a significant change and to see if that would conflict in some ways with your new calculation engine ?

[PowerKiKi]

Also, try as much as possible to split in private methods, rather that expanding even more the already ginormous methods.

[frantzmiccoli]

Thanks for your comments and advices.

I am done with the polishing, all those last commits were desperate tries to make the CI silent (maybe testing from my Travis would have avoided the commit spam).

Yes, this is touching the core of the calculation engine, so, it would not be to surprising to introduce regression. I did my best to thoroughly test it, if you see extra tests to perform let me know.

As per the extra public methods:

• Stack::getStackItem(): enables code factorization as Calculation::_parseFormula() doesn't manipulate tokens only through a stack and was also manually creating the arrays representing tokens.
• Stack::__toString(): was really convenient to debug I would recommend to leave it but I could truncate it.
• I followed the result caching logic which introduced a few extra methods in Calculation: setBranchPruningEnabled(), enableBranchPruning(), disableBranchPruning() and clearBranchStore().
• Nothing to do with this pull request but I am wondering if some functions are not public just to enable testing like processTokenStack(), I usually prefer to use the reflection API in my unit tests.

A side point about CalculationTest::testBranchPruningFormulaParsing, this test was pretty thick. I could not use a data provider as the expected result testing is way non trivial. So I did split the test in many different functions.

As per the code splitting in the calculation engine itself, the _parseFormula() and processTokenStack() functions are not using context objects that could be pass around to subrountines. I could spend some time splitting those huge methods as I think I have gained some understanding of the calculation engine inner workings but I think this should be for another pull request. I don't think it would deeply impact execution speed.

[PowerKiKi]

Closed by mistake, re-opening.

@MarkBaker, with your work on the engine, should this be merged as soon as possible ? Or will your work replace everything soon enough anyway so this shouldn't be merged at all?

[PowerKiKi]

I realised this PR has been closed because the develop branch was permanently deleted.

@frantzmiccoli, would you be able to rebase on master ? Or maybe we should wait for @MarkBaker inputs before you do extra work ?