Jace icon indicating copy to clipboard operation
Jace copied to clipboard

Fix performance issue with CaseSensitivity enabled

Open aviita opened this issue 3 years ago • 1 comments

When CaseSensitive is set to true from JaceOptions, performance should be significantly better than without case sensitivity. Case sensitivity setting was not passed to FunctionRegistry and ConstantRegistry constructors, which caused them to do extra lower case conversions in case variable dictionary was passed to the formula. Fixes issue #75.

New benchmark was created to verify the fix. Benchmark needs to have variables which are provided to CalculationEngine.Calculate(), so VerifyVariableNames() gets called, which causes the extra calls to ToLowerFast().

Below results show ~200 ms improvement when Case Sensitive is true. Results table was created by running benchmark separately with and without fix and copying results to one table.

Engine Case Sensitive Formula Total Iteration Total Duration (fix) Total Duration (no fix)
Interpreted False something2 - (var1 + var2 * 3)/(2+3) 1000000 00:00:01.6005267 00:00:01.5919016
Interpreted True something2 - (var1 + var2 * 3)/(2+3) 1000000 00:00:00.6069845 00:00:00.8390435
Compiled False something2 - (var1 + var2 * 3)/(2+3) 1000000 00:00:01.5865326 00:00:01.5770084
Compiled True something2 - (var1 + var2 * 3)/(2+3) 1000000 00:00:00.5930012 00:00:00.8189981

Additionally:

  • Unit test which tests case sensitivity enabled throws when variable case does not match
  • Benchmark uses dictionary copy constructor to avoid throwing due to dictionary being modified with constants and then failing on VerifyVariableNames().

aviita avatar Aug 10 '21 09:08 aviita

@pieterderycke Any chance of getting this merged?

aviita avatar Sep 02 '21 18:09 aviita