From 6819d8cf7b625c5ccfbca9ff4615cd3502e0b152 Mon Sep 17 00:00:00 2001 From: gumbarros Date: Fri, 8 Mar 2024 15:37:14 -0300 Subject: [PATCH 1/4] Optimize built-in functions to don't have unnecessary type unboxing to string and created ExecuteBuiltInFunction method. --- src/NCalc/Domain/EvaluationVisitor.cs | 341 +++++++------------------- 1 file changed, 90 insertions(+), 251 deletions(-) diff --git a/src/NCalc/Domain/EvaluationVisitor.cs b/src/NCalc/Domain/EvaluationVisitor.cs index 188df26..de09c7a 100644 --- a/src/NCalc/Domain/EvaluationVisitor.cs +++ b/src/NCalc/Domain/EvaluationVisitor.cs @@ -29,7 +29,8 @@ public override void Visit(LogicalExpression expression) throw new Exception("The method or operation is not implemented."); } - private static readonly Type[] CommonTypes = { typeof(double), typeof(long), typeof(bool), typeof(string), typeof(decimal) }; + private static readonly Type[] CommonTypes = + [typeof(double), typeof(long), typeof(bool), typeof(string), typeof(decimal)]; /// /// Gets the the most precise type. @@ -98,122 +99,126 @@ public override void Visit(BinaryExpression expression) { // simulate Lazy> behavior for late evaluation object leftValue = null; - Func left = () => + + object Left() { if (leftValue == null) { expression.LeftExpression.Accept(this); leftValue = Result; } + return leftValue; - }; + } // simulate Lazy> behavior for late evaluations object rightValue = null; - Func right = () => - { - if (rightValue == null) - { - expression.RightExpression.Accept(this); - rightValue = Result; - } - return rightValue; - }; switch (expression.Type) { case BinaryExpressionType.And: - Result = Convert.ToBoolean(left(), cultureInfo) && Convert.ToBoolean(right(), cultureInfo); + Result = Convert.ToBoolean(Left(), cultureInfo) && Convert.ToBoolean(Right(), cultureInfo); break; case BinaryExpressionType.Or: - Result = Convert.ToBoolean(left(), cultureInfo) || Convert.ToBoolean(right(), cultureInfo); + Result = Convert.ToBoolean(Left(), cultureInfo) || Convert.ToBoolean(Right(), cultureInfo); break; case BinaryExpressionType.Div: - Result = IsReal(left()) || IsReal(right()) - ? Numbers.Divide(left(), right(), cultureInfo) - : Numbers.Divide(Convert.ToDouble(left(), cultureInfo), right(), cultureInfo); + Result = IsReal(Left()) || IsReal(Right()) + ? Numbers.Divide(Left(), Right(), cultureInfo) + : Numbers.Divide(Convert.ToDouble(Left(), cultureInfo), Right(), cultureInfo); break; case BinaryExpressionType.Equal: // Use the type of the left operand to make the comparison - Result = CompareUsingMostPreciseType(left(), right()) == 0; + Result = CompareUsingMostPreciseType(Left(), Right()) == 0; break; case BinaryExpressionType.Greater: // Use the type of the left operand to make the comparison - Result = CompareUsingMostPreciseType(left(), right()) > 0; + Result = CompareUsingMostPreciseType(Left(), Right()) > 0; break; case BinaryExpressionType.GreaterOrEqual: // Use the type of the left operand to make the comparison - Result = CompareUsingMostPreciseType(left(), right()) >= 0; + Result = CompareUsingMostPreciseType(Left(), Right()) >= 0; break; case BinaryExpressionType.Lesser: // Use the type of the left operand to make the comparison - Result = CompareUsingMostPreciseType(left(), right()) < 0; + Result = CompareUsingMostPreciseType(Left(), Right()) < 0; break; case BinaryExpressionType.LesserOrEqual: // Use the type of the left operand to make the comparison - Result = CompareUsingMostPreciseType(left(), right()) <= 0; + Result = CompareUsingMostPreciseType(Left(), Right()) <= 0; break; case BinaryExpressionType.Minus: - Result = Numbers.Subtract(left(), right(), cultureInfo); + Result = Numbers.Subtract(Left(), Right(), cultureInfo); break; case BinaryExpressionType.Modulo: - Result = Numbers.Modulo(left(), right(), cultureInfo); + Result = Numbers.Modulo(Left(), Right(), cultureInfo); break; case BinaryExpressionType.NotEqual: // Use the type of the left operand to make the comparison - Result = CompareUsingMostPreciseType(left(), right()) != 0; + Result = CompareUsingMostPreciseType(Left(), Right()) != 0; break; case BinaryExpressionType.Plus: - if (left() is string) + if (Left() is string) { - Result = string.Concat(left(), right()); + Result = string.Concat(Left(), Right()); } else { - Result = Numbers.Add(left(), right(), cultureInfo); + Result = Numbers.Add(Left(), Right(), cultureInfo); } break; case BinaryExpressionType.Times: - Result = Numbers.Multiply(left(), right(), cultureInfo); + Result = Numbers.Multiply(Left(), Right(), cultureInfo); break; case BinaryExpressionType.BitwiseAnd: - Result = Convert.ToUInt16(left(), cultureInfo) & Convert.ToUInt16(right(), cultureInfo); + Result = Convert.ToUInt16(Left(), cultureInfo) & Convert.ToUInt16(Right(), cultureInfo); break; case BinaryExpressionType.BitwiseOr: - Result = Convert.ToUInt16(left(), cultureInfo) | Convert.ToUInt16(right(), cultureInfo); + Result = Convert.ToUInt16(Left(), cultureInfo) | Convert.ToUInt16(Right(), cultureInfo); break; case BinaryExpressionType.BitwiseXOr: - Result = Convert.ToUInt16(left(), cultureInfo) ^ Convert.ToUInt16(right(), cultureInfo); + Result = Convert.ToUInt16(Left(), cultureInfo) ^ Convert.ToUInt16(Right(), cultureInfo); break; case BinaryExpressionType.LeftShift: - Result = Convert.ToUInt16(left(), cultureInfo) << Convert.ToUInt16(right(), cultureInfo); + Result = Convert.ToUInt16(Left(), cultureInfo) << Convert.ToUInt16(Right(), cultureInfo); break; case BinaryExpressionType.RightShift: - Result = Convert.ToUInt16(left(), cultureInfo) >> Convert.ToUInt16(right(), cultureInfo); + Result = Convert.ToUInt16(Left(), cultureInfo) >> Convert.ToUInt16(Right(), cultureInfo); break; case BinaryExpressionType.Exponentiation: - Result = Math.Pow(Convert.ToDouble(left(), cultureInfo), Convert.ToDouble(right(), cultureInfo)); + Result = Math.Pow(Convert.ToDouble(Left(), cultureInfo), Convert.ToDouble(Right(), cultureInfo)); break; } + + object Right() + { + if (rightValue == null) + { + expression.RightExpression.Accept(this); + rightValue = Result; + } + + return rightValue; + } } public override void Visit(UnaryExpression expression) @@ -276,333 +281,186 @@ public override void Visit(Function function) return; } - switch (function.Identifier.Name.ToLower()) - { - #region Abs - case string n when n.Equals("abs", StringComparison.OrdinalIgnoreCase): + ExecuteBuiltInFunction(function); + } - CheckCase("Abs", function.Identifier.Name); + private void ExecuteBuiltInFunction(Function function) + { + var functionName = function.Identifier.Name.ToLower(); + switch (functionName) + { + case "abs": + CheckCase("Abs", function.Identifier.Name); if (function.Expressions.Length != 1) throw new ArgumentException("Abs() takes exactly 1 argument"); - - Result = Math.Abs(Convert.ToDecimal( - Evaluate(function.Expressions[0]), cultureInfo) - ); - + Result = Math.Abs(Convert.ToDecimal(Evaluate(function.Expressions[0]), cultureInfo)); break; - #endregion - - #region Acos - case string n when n.Equals("acos", StringComparison.OrdinalIgnoreCase): - + case "acos": CheckCase("Acos", function.Identifier.Name); - if (function.Expressions.Length != 1) throw new ArgumentException("Acos() takes exactly 1 argument"); - Result = Math.Acos(Convert.ToDouble(Evaluate(function.Expressions[0]), cultureInfo)); - break; - #endregion - - #region Asin - case string n when n.Equals("asin", StringComparison.OrdinalIgnoreCase): - + case "asin": CheckCase("Asin", function.Identifier.Name); - if (function.Expressions.Length != 1) throw new ArgumentException("Asin() takes exactly 1 argument"); - Result = Math.Asin(Convert.ToDouble(Evaluate(function.Expressions[0]), cultureInfo)); - break; - #endregion - - #region Atan - case string n when n.Equals("atan", StringComparison.OrdinalIgnoreCase): - + case "atan": CheckCase("Atan", function.Identifier.Name); - if (function.Expressions.Length != 1) throw new ArgumentException("Atan() takes exactly 1 argument"); - Result = Math.Atan(Convert.ToDouble(Evaluate(function.Expressions[0]), cultureInfo)); - break; - #endregion - - #region Atan2 - case string n when n.Equals("atan2", StringComparison.OrdinalIgnoreCase): - + case "atan2": CheckCase("Atan2", function.Identifier.Name); - if (function.Expressions.Length != 2) throw new ArgumentException("Atan2() takes exactly 2 argument"); - - Result = Math.Atan2(Convert.ToDouble(Evaluate(function.Expressions[0]), cultureInfo), Convert.ToDouble(Evaluate(function.Expressions[1]), cultureInfo)); - + Result = Math.Atan2(Convert.ToDouble(Evaluate(function.Expressions[0]), cultureInfo), + Convert.ToDouble(Evaluate(function.Expressions[1]), cultureInfo)); break; - #endregion - - #region Ceiling - case string n when n.Equals("ceiling", StringComparison.OrdinalIgnoreCase): - + case "ceiling": CheckCase("Ceiling", function.Identifier.Name); - if (function.Expressions.Length != 1) throw new ArgumentException("Ceiling() takes exactly 1 argument"); - Result = Math.Ceiling(Convert.ToDouble(Evaluate(function.Expressions[0]), cultureInfo)); - break; - #endregion - - #region Cos - - case string n when n.Equals("cos", StringComparison.OrdinalIgnoreCase): - + case "cos": CheckCase("Cos", function.Identifier.Name); - if (function.Expressions.Length != 1) throw new ArgumentException("Cos() takes exactly 1 argument"); - Result = Math.Cos(Convert.ToDouble(Evaluate(function.Expressions[0]), cultureInfo)); - break; - #endregion - - #region Exp - case string n when n.Equals("exp", StringComparison.OrdinalIgnoreCase): - + case "exp": CheckCase("Exp", function.Identifier.Name); - if (function.Expressions.Length != 1) throw new ArgumentException("Exp() takes exactly 1 argument"); - Result = Math.Exp(Convert.ToDouble(Evaluate(function.Expressions[0]), cultureInfo)); - break; - #endregion - - #region Floor - case string n when n.Equals("floor", StringComparison.OrdinalIgnoreCase): - + case "floor": CheckCase("Floor", function.Identifier.Name); - if (function.Expressions.Length != 1) throw new ArgumentException("Floor() takes exactly 1 argument"); - Result = Math.Floor(Convert.ToDouble(Evaluate(function.Expressions[0]), cultureInfo)); - break; - #endregion - - #region IEEERemainder - case string n when n.Equals("ieeeremainder", StringComparison.OrdinalIgnoreCase): - + case "ieeeremainder": CheckCase("IEEERemainder", function.Identifier.Name); - if (function.Expressions.Length != 2) throw new ArgumentException("IEEERemainder() takes exactly 2 arguments"); - - Result = Math.IEEERemainder(Convert.ToDouble(Evaluate(function.Expressions[0])), Convert.ToDouble(Evaluate(function.Expressions[1]), cultureInfo)); - + Result = Math.IEEERemainder(Convert.ToDouble(Evaluate(function.Expressions[0])), + Convert.ToDouble(Evaluate(function.Expressions[1]), cultureInfo)); break; - #endregion - - #region Ln - case string n when n.Equals("ln", StringComparison.OrdinalIgnoreCase): - + case "ln": CheckCase("Ln", function.Identifier.Name); - if (function.Expressions.Length != 1) throw new ArgumentException("Ln() takes exactly 1 argument"); - Result = Math.Log(Convert.ToDouble(Evaluate(function.Expressions[0]), cultureInfo)); - break; - #endregion - - #region Log - case string n when n.Equals("log", StringComparison.OrdinalIgnoreCase): - + case "log": CheckCase("Log", function.Identifier.Name); - if (function.Expressions.Length != 2) throw new ArgumentException("Log() takes exactly 2 arguments"); - - Result = Math.Log(Convert.ToDouble(Evaluate(function.Expressions[0]), cultureInfo), Convert.ToDouble(Evaluate(function.Expressions[1]), cultureInfo)); - + Result = Math.Log(Convert.ToDouble(Evaluate(function.Expressions[0]), cultureInfo), + Convert.ToDouble(Evaluate(function.Expressions[1]), cultureInfo)); break; - #endregion - - #region Log10 - case string n when n.Equals("log10", StringComparison.OrdinalIgnoreCase): - + case "log10": CheckCase("Log10", function.Identifier.Name); - if (function.Expressions.Length != 1) throw new ArgumentException("Log10() takes exactly 1 argument"); - Result = Math.Log10(Convert.ToDouble(Evaluate(function.Expressions[0]), cultureInfo)); - break; - #endregion - - #region Pow - case string n when n.Equals("pow", StringComparison.OrdinalIgnoreCase): - + case "pow": CheckCase("Pow", function.Identifier.Name); - if (function.Expressions.Length != 2) throw new ArgumentException("Pow() takes exactly 2 arguments"); - - Result = Math.Pow(Convert.ToDouble(Evaluate(function.Expressions[0]), cultureInfo), Convert.ToDouble(Evaluate(function.Expressions[1]), cultureInfo)); - + Result = Math.Pow(Convert.ToDouble(Evaluate(function.Expressions[0]), cultureInfo), + Convert.ToDouble(Evaluate(function.Expressions[1]), cultureInfo)); break; - #endregion - - #region Round - case string n when n.Equals("round", StringComparison.OrdinalIgnoreCase): - + case "round": CheckCase("Round", function.Identifier.Name); - if (function.Expressions.Length != 2) throw new ArgumentException("Round() takes exactly 2 arguments"); - - MidpointRounding rounding = (options & EvaluateOptions.RoundAwayFromZero) == EvaluateOptions.RoundAwayFromZero ? MidpointRounding.AwayFromZero : MidpointRounding.ToEven; - - Result = Math.Round(Convert.ToDouble(Evaluate(function.Expressions[0]), cultureInfo), Convert.ToInt16(Evaluate(function.Expressions[1]), cultureInfo), rounding); - + MidpointRounding rounding = + (options & EvaluateOptions.RoundAwayFromZero) == EvaluateOptions.RoundAwayFromZero + ? MidpointRounding.AwayFromZero + : MidpointRounding.ToEven; + Result = Math.Round(Convert.ToDouble(Evaluate(function.Expressions[0]), cultureInfo), + Convert.ToInt16(Evaluate(function.Expressions[1]), cultureInfo), rounding); break; - #endregion - - #region Sign - case string n when n.Equals("sign", StringComparison.OrdinalIgnoreCase): - + case "sign": CheckCase("Sign", function.Identifier.Name); - if (function.Expressions.Length != 1) throw new ArgumentException("Sign() takes exactly 1 argument"); - Result = Math.Sign(Convert.ToDouble(Evaluate(function.Expressions[0]), cultureInfo)); - break; - #endregion - - #region Sin - case string n when n.Equals("sin", StringComparison.OrdinalIgnoreCase): - + case "sin": CheckCase("Sin", function.Identifier.Name); - if (function.Expressions.Length != 1) throw new ArgumentException("Sin() takes exactly 1 argument"); - Result = Math.Sin(Convert.ToDouble(Evaluate(function.Expressions[0]), cultureInfo)); - break; - #endregion - - #region Sqrt - case string n when n.Equals("sqrt", StringComparison.OrdinalIgnoreCase): - + case "sqrt": CheckCase("Sqrt", function.Identifier.Name); - if (function.Expressions.Length != 1) throw new ArgumentException("Sqrt() takes exactly 1 argument"); - Result = Math.Sqrt(Convert.ToDouble(Evaluate(function.Expressions[0]), cultureInfo)); - break; - #endregion - - #region Tan - case string n when n.Equals("tan", StringComparison.OrdinalIgnoreCase): - + case "tan": CheckCase("Tan", function.Identifier.Name); - if (function.Expressions.Length != 1) throw new ArgumentException("Tan() takes exactly 1 argument"); - Result = Math.Tan(Convert.ToDouble(Evaluate(function.Expressions[0]), cultureInfo)); - break; - #endregion - - #region Truncate - case string n when n.Equals("truncate", StringComparison.OrdinalIgnoreCase): - + case "truncate": CheckCase("Truncate", function.Identifier.Name); - if (function.Expressions.Length != 1) throw new ArgumentException("Truncate() takes exactly 1 argument"); - Result = Math.Truncate(Convert.ToDouble(Evaluate(function.Expressions[0]), cultureInfo)); - break; - #endregion - - #region Max - case string n when n.Equals("max", StringComparison.OrdinalIgnoreCase): - + case "max": CheckCase("Max", function.Identifier.Name); - if (function.Expressions.Length != 2) throw new ArgumentException("Max() takes exactly 2 arguments"); - object maxleft = Evaluate(function.Expressions[0]); object maxright = Evaluate(function.Expressions[1]); - Result = Numbers.Max(maxleft, maxright, cultureInfo); break; - #endregion - - #region Min - case string n when n.Equals("min", StringComparison.OrdinalIgnoreCase): - + case "min": CheckCase("Min", function.Identifier.Name); - if (function.Expressions.Length != 2) throw new ArgumentException("Min() takes exactly 2 arguments"); - object minleft = Evaluate(function.Expressions[0]); object minright = Evaluate(function.Expressions[1]); - Result = Numbers.Min(minleft, minright, cultureInfo); break; - #endregion - - #region ifs - case string n when n.Equals("ifs", StringComparison.OrdinalIgnoreCase): - + case "ifs": CheckCase("ifs", function.Identifier.Name); - if (function.Expressions.Length < 3 || function.Expressions.Length % 2 != 1) throw new ArgumentException("ifs() takes at least 3 arguments, or an odd number of arguments"); - foreach (var eval in function.Expressions.Where((_, i) => i % 2 == 0)) { var index = Array.IndexOf(function.Expressions, eval); @@ -618,39 +476,23 @@ public override void Visit(Function function) break; - #endregion - #region if - case string n when n.Equals("if", StringComparison.OrdinalIgnoreCase): - + case "if": CheckCase("if", function.Identifier.Name); - if (function.Expressions.Length != 3) throw new ArgumentException("if() takes exactly 3 arguments"); - bool cond = Convert.ToBoolean(Evaluate(function.Expressions[0]), cultureInfo); - Result = cond ? Evaluate(function.Expressions[1]) : Evaluate(function.Expressions[2]); break; - #endregion - - - #region in - case string n when n.Equals("in", StringComparison.OrdinalIgnoreCase): - + case "in": CheckCase("in", function.Identifier.Name); - if (function.Expressions.Length < 2) throw new ArgumentException("in() takes at least 2 arguments"); - object parameter = Evaluate(function.Expressions[0]); - bool evaluation = false; - - // Goes through any values, and stop whe one is found for (int i = 1; i < function.Expressions.Length; i++) { - object argument = Evaluate(function.Expressions[i]); + var argument = Evaluate(function.Expressions[i]); if (CompareUsingMostPreciseType(parameter, argument) == 0) { evaluation = true; @@ -661,11 +503,8 @@ public override void Visit(Function function) Result = evaluation; break; - #endregion - default: - throw new ArgumentException("Function not found", - function.Identifier.Name); + throw new ArgumentException("Function not found", function.Identifier.Name); } } From 31fc6bf49a7ea5bd93fc7e7237244051b5f79e3e Mon Sep 17 00:00:00 2001 From: gumbarros Date: Fri, 8 Mar 2024 15:38:57 -0300 Subject: [PATCH 2/4] Put 'Left' local function after return --- src/NCalc/Domain/EvaluationVisitor.cs | 34 ++++++++++++++------------- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/src/NCalc/Domain/EvaluationVisitor.cs b/src/NCalc/Domain/EvaluationVisitor.cs index de09c7a..0045c9d 100644 --- a/src/NCalc/Domain/EvaluationVisitor.cs +++ b/src/NCalc/Domain/EvaluationVisitor.cs @@ -100,17 +100,6 @@ public override void Visit(BinaryExpression expression) // simulate Lazy> behavior for late evaluation object leftValue = null; - object Left() - { - if (leftValue == null) - { - expression.LeftExpression.Accept(this); - leftValue = Result; - } - - return leftValue; - } - // simulate Lazy> behavior for late evaluations object rightValue = null; @@ -209,13 +198,26 @@ object Left() break; } + return; + + object Left() + { + if (leftValue != null) + return leftValue; + + expression.LeftExpression.Accept(this); + leftValue = Result; + + return leftValue; + } + object Right() { - if (rightValue == null) - { - expression.RightExpression.Accept(this); - rightValue = Result; - } + if (rightValue != null) + return rightValue; + + expression.RightExpression.Accept(this); + rightValue = Result; return rightValue; } From 10d3d7eac4fd7a89697d69c9cf863eeeba318d88 Mon Sep 17 00:00:00 2001 From: gumbarros Date: Sat, 9 Mar 2024 10:01:18 -0300 Subject: [PATCH 3/4] Use ConcurrentDictionary and generic WeakReference instead of manually controlling thread-safety --- src/NCalc/Expression.cs | 132 +++++++++++++++------------------------- 1 file changed, 48 insertions(+), 84 deletions(-) diff --git a/src/NCalc/Expression.cs b/src/NCalc/Expression.cs index aa90eba..0a46987 100644 --- a/src/NCalc/Expression.cs +++ b/src/NCalc/Expression.cs @@ -1,11 +1,11 @@ using System; using System.Collections; +using System.Collections.Concurrent; using System.Collections.Generic; using System.Diagnostics; using System.Globalization; using System.Linq; using System.Text; -using System.Threading; using NCalc.Domain; using Antlr4.Runtime; @@ -108,8 +108,7 @@ public Expression(LogicalExpression expression, EvaluationVisitor evaluationVisi #region Cache management private static bool _cacheEnabled = true; - private static Dictionary _compiledExpressions = new(); - private static readonly ReaderWriterLock Rwl = new(); + private static readonly ConcurrentDictionary> _compiledExpressions = new(); public static bool CacheEnabled { @@ -121,7 +120,7 @@ public static bool CacheEnabled if (!CacheEnabled) { // Clears cache - _compiledExpressions = new Dictionary(); + _compiledExpressions.Clear(); } } } @@ -129,119 +128,84 @@ public static bool CacheEnabled /// /// Removed unused entries from cached compiled expression /// - private static void CleanCache() + private static void ClearCache() { - var keysToRemove = new List(); - - try + foreach (var kvp in _compiledExpressions) { - Rwl.AcquireWriterLock(Timeout.Infinite); - foreach (var de in _compiledExpressions) - { - if (!de.Value.IsAlive) - { - keysToRemove.Add(de.Key); - } - } + if (kvp.Value.TryGetTarget(out _)) + continue; - - foreach (var key in keysToRemove) + if (_compiledExpressions.TryRemove(kvp.Key, out _)) { - _compiledExpressions.Remove(key); - Trace.TraceInformation("Cache entry released: " + key); + Trace.TraceInformation("Cache entry released: " + kvp.Key); } } - finally - { - Rwl.ReleaseReaderLock(); - } } #endregion public static LogicalExpression Compile(string expression, bool nocache) { - LogicalExpression logicalExpression = null; + LogicalExpression logicalExpression; if (_cacheEnabled && !nocache) { - try + if (_compiledExpressions.TryGetValue(expression, out var wr)) { - Rwl.AcquireReaderLock(Timeout.Infinite); + Trace.TraceInformation("Expression retrieved from cache: " + expression); - if (_compiledExpressions.TryGetValue(expression, out var wr)) + if (wr.TryGetTarget(out var target)) { - Trace.TraceInformation("Expression retrieved from cache: " + expression); - logicalExpression = wr.Target as LogicalExpression; - - if (wr.IsAlive && logicalExpression != null) - { - return logicalExpression; - } + return target; } } - finally - { - Rwl.ReleaseReaderLock(); - } } - if (logicalExpression == null) - { - var lexer = new NCalcLexer(new AntlrInputStream(expression)); - var errorListenerLexer = new ErrorListenerLexer(); - lexer.AddErrorListener(errorListenerLexer); + var lexer = new NCalcLexer(new AntlrInputStream(expression)); + var errorListenerLexer = new ErrorListenerLexer(); + lexer.AddErrorListener(errorListenerLexer); - var parser = new NCalcParser(new CommonTokenStream(lexer)); - var errorListenerParser = new ErrorListenerParser(); - parser.AddErrorListener(errorListenerParser); + var parser = new NCalcParser(new CommonTokenStream(lexer)); + var errorListenerParser = new ErrorListenerParser(); + parser.AddErrorListener(errorListenerParser); - try - { - logicalExpression = parser.ncalcExpression().retValue; - } - catch(Exception ex) - { - var message = new StringBuilder(ex.Message); - if (errorListenerLexer.Errors.Count != 0) - { - message.AppendLine(); - message.AppendLine(string.Join(Environment.NewLine, errorListenerLexer.Errors.ToArray())); - } - if (errorListenerParser.Errors.Count != 0) - { - message.AppendLine(); - message.AppendLine(string.Join(Environment.NewLine, errorListenerParser.Errors.ToArray())); - } - - throw new EvaluationException(message.ToString()); - } + try + { + logicalExpression = parser.ncalcExpression().retValue; + } + catch(Exception ex) + { + var message = new StringBuilder(ex.Message); if (errorListenerLexer.Errors.Count != 0) { - throw new EvaluationException(string.Join(Environment.NewLine, errorListenerLexer.Errors.ToArray())); + message.AppendLine(); + message.AppendLine(string.Join(Environment.NewLine, errorListenerLexer.Errors.ToArray())); } if (errorListenerParser.Errors.Count != 0) { - throw new EvaluationException(string.Join(Environment.NewLine, errorListenerParser.Errors.ToArray())); + message.AppendLine(); + message.AppendLine(string.Join(Environment.NewLine, errorListenerParser.Errors.ToArray())); } - if (!_cacheEnabled || nocache) - return logicalExpression; - - try - { - Rwl.AcquireWriterLock(Timeout.Infinite); - _compiledExpressions[expression] = new WeakReference(logicalExpression); - } - finally - { - Rwl.ReleaseWriterLock(); - } + throw new EvaluationException(message.ToString()); + } + if (errorListenerLexer.Errors.Count != 0) + { + throw new EvaluationException(string.Join(Environment.NewLine, errorListenerLexer.Errors.ToArray())); + } + if (errorListenerParser.Errors.Count != 0) + { + throw new EvaluationException(string.Join(Environment.NewLine, errorListenerParser.Errors.ToArray())); + } - CleanCache(); + if (!_cacheEnabled || nocache) + return logicalExpression; + + _compiledExpressions[expression] = new WeakReference(logicalExpression); + + ClearCache(); - Trace.TraceInformation("Expression added to cache: " + expression); - } + Trace.TraceInformation("Expression added to cache: " + expression); return logicalExpression; } From e7cd2d745218a4cf477bfc529a41d168975743b6 Mon Sep 17 00:00:00 2001 From: gumbarros Date: Mon, 18 Mar 2024 08:58:39 -0300 Subject: [PATCH 4/4] Merge with main and use ToUpperInvariant to checking --- src/NCalc/Domain/EvaluationVisitor.cs | 52 +++++++++--------- src/NCalc/Domain/Parameter.cs | 3 +- src/NCalc/Expression.cs | 77 +++++++-------------------- 3 files changed, 46 insertions(+), 86 deletions(-) diff --git a/src/NCalc/Domain/EvaluationVisitor.cs b/src/NCalc/Domain/EvaluationVisitor.cs index dec831b..cbce283 100644 --- a/src/NCalc/Domain/EvaluationVisitor.cs +++ b/src/NCalc/Domain/EvaluationVisitor.cs @@ -307,39 +307,39 @@ public override void Visit(Function function) private void ExecuteBuiltInFunction(Function function) { - var functionName = function.Identifier.Name.ToLower(); + var functionName = function.Identifier.Name.ToUpperInvariant(); switch (functionName) { - case "abs": + case "ABS": CheckCase("Abs", function.Identifier.Name); if (function.Expressions.Length != 1) throw new ArgumentException("Abs() takes exactly 1 argument"); Result = Math.Abs(Convert.ToDecimal(Evaluate(function.Expressions[0]), cultureInfo)); break; - case "acos": + case "ACOS": CheckCase("Acos", function.Identifier.Name); if (function.Expressions.Length != 1) throw new ArgumentException("Acos() takes exactly 1 argument"); Result = Math.Acos(Convert.ToDouble(Evaluate(function.Expressions[0]), cultureInfo)); break; - case "asin": + case "ASIN": CheckCase("Asin", function.Identifier.Name); if (function.Expressions.Length != 1) throw new ArgumentException("Asin() takes exactly 1 argument"); Result = Math.Asin(Convert.ToDouble(Evaluate(function.Expressions[0]), cultureInfo)); break; - case "atan": + case "ATAN": CheckCase("Atan", function.Identifier.Name); if (function.Expressions.Length != 1) throw new ArgumentException("Atan() takes exactly 1 argument"); Result = Math.Atan(Convert.ToDouble(Evaluate(function.Expressions[0]), cultureInfo)); break; - case "atan2": + case "ATAN2": CheckCase("Atan2", function.Identifier.Name); if (function.Expressions.Length != 2) throw new ArgumentException("Atan2() takes exactly 2 argument"); @@ -347,35 +347,35 @@ private void ExecuteBuiltInFunction(Function function) Convert.ToDouble(Evaluate(function.Expressions[1]), cultureInfo)); break; - case "ceiling": + case "CEILING": CheckCase("Ceiling", function.Identifier.Name); if (function.Expressions.Length != 1) throw new ArgumentException("Ceiling() takes exactly 1 argument"); Result = Math.Ceiling(Convert.ToDouble(Evaluate(function.Expressions[0]), cultureInfo)); break; - case "cos": + case "COS": CheckCase("Cos", function.Identifier.Name); if (function.Expressions.Length != 1) throw new ArgumentException("Cos() takes exactly 1 argument"); Result = Math.Cos(Convert.ToDouble(Evaluate(function.Expressions[0]), cultureInfo)); break; - case "exp": + case "EXP": CheckCase("Exp", function.Identifier.Name); if (function.Expressions.Length != 1) throw new ArgumentException("Exp() takes exactly 1 argument"); Result = Math.Exp(Convert.ToDouble(Evaluate(function.Expressions[0]), cultureInfo)); break; - case "floor": + case "FLOOR": CheckCase("Floor", function.Identifier.Name); if (function.Expressions.Length != 1) throw new ArgumentException("Floor() takes exactly 1 argument"); Result = Math.Floor(Convert.ToDouble(Evaluate(function.Expressions[0]), cultureInfo)); break; - case "ieeeremainder": + case "IEEEREMAINDER": CheckCase("IEEERemainder", function.Identifier.Name); if (function.Expressions.Length != 2) throw new ArgumentException("IEEERemainder() takes exactly 2 arguments"); @@ -383,14 +383,14 @@ private void ExecuteBuiltInFunction(Function function) Convert.ToDouble(Evaluate(function.Expressions[1]), cultureInfo)); break; - case "ln": + case "LN": CheckCase("Ln", function.Identifier.Name); if (function.Expressions.Length != 1) throw new ArgumentException("Ln() takes exactly 1 argument"); Result = Math.Log(Convert.ToDouble(Evaluate(function.Expressions[0]), cultureInfo)); break; - case "log": + case "LOG": CheckCase("Log", function.Identifier.Name); if (function.Expressions.Length != 2) throw new ArgumentException("Log() takes exactly 2 arguments"); @@ -398,14 +398,14 @@ private void ExecuteBuiltInFunction(Function function) Convert.ToDouble(Evaluate(function.Expressions[1]), cultureInfo)); break; - case "log10": + case "LOG10": CheckCase("Log10", function.Identifier.Name); if (function.Expressions.Length != 1) throw new ArgumentException("Log10() takes exactly 1 argument"); Result = Math.Log10(Convert.ToDouble(Evaluate(function.Expressions[0]), cultureInfo)); break; - case "pow": + case "POW": CheckCase("Pow", function.Identifier.Name); if (function.Expressions.Length != 2) throw new ArgumentException("Pow() takes exactly 2 arguments"); @@ -413,7 +413,7 @@ private void ExecuteBuiltInFunction(Function function) Convert.ToDouble(Evaluate(function.Expressions[1]), cultureInfo)); break; - case "round": + case "ROUND": CheckCase("Round", function.Identifier.Name); if (function.Expressions.Length != 2) throw new ArgumentException("Round() takes exactly 2 arguments"); @@ -424,42 +424,42 @@ private void ExecuteBuiltInFunction(Function function) break; - case "sign": + case "SIGN": CheckCase("Sign", function.Identifier.Name); if (function.Expressions.Length != 1) throw new ArgumentException("Sign() takes exactly 1 argument"); Result = Math.Sign(Convert.ToDouble(Evaluate(function.Expressions[0]), cultureInfo)); break; - case "sin": + case "SIN": CheckCase("Sin", function.Identifier.Name); if (function.Expressions.Length != 1) throw new ArgumentException("Sin() takes exactly 1 argument"); Result = Math.Sin(Convert.ToDouble(Evaluate(function.Expressions[0]), cultureInfo)); break; - case "sqrt": + case "SQRT": CheckCase("Sqrt", function.Identifier.Name); if (function.Expressions.Length != 1) throw new ArgumentException("Sqrt() takes exactly 1 argument"); Result = Math.Sqrt(Convert.ToDouble(Evaluate(function.Expressions[0]), cultureInfo)); break; - case "tan": + case "TAN": CheckCase("Tan", function.Identifier.Name); if (function.Expressions.Length != 1) throw new ArgumentException("Tan() takes exactly 1 argument"); Result = Math.Tan(Convert.ToDouble(Evaluate(function.Expressions[0]), cultureInfo)); break; - case "truncate": + case "TRUNCATE": CheckCase("Truncate", function.Identifier.Name); if (function.Expressions.Length != 1) throw new ArgumentException("Truncate() takes exactly 1 argument"); Result = Math.Truncate(Convert.ToDouble(Evaluate(function.Expressions[0]), cultureInfo)); break; - case "max": + case "MAX": CheckCase("Max", function.Identifier.Name); if (function.Expressions.Length != 2) throw new ArgumentException("Max() takes exactly 2 arguments"); @@ -468,7 +468,7 @@ private void ExecuteBuiltInFunction(Function function) Result = Numbers.Max(maxleft, maxright, cultureInfo); break; - case "min": + case "MIN": CheckCase("Min", function.Identifier.Name); if (function.Expressions.Length != 2) throw new ArgumentException("Min() takes exactly 2 arguments"); @@ -477,7 +477,7 @@ private void ExecuteBuiltInFunction(Function function) Result = Numbers.Min(minleft, minright, cultureInfo); break; - case "ifs": + case "IFS": CheckCase("ifs", function.Identifier.Name); if (function.Expressions.Length < 3 || function.Expressions.Length % 2 != 1) throw new ArgumentException("ifs() takes at least 3 arguments, or an odd number of arguments"); @@ -496,7 +496,7 @@ private void ExecuteBuiltInFunction(Function function) break; - case "if": + case "IF": CheckCase("if", function.Identifier.Name); if (function.Expressions.Length != 3) throw new ArgumentException("if() takes exactly 3 arguments"); @@ -504,7 +504,7 @@ private void ExecuteBuiltInFunction(Function function) Result = cond ? Evaluate(function.Expressions[1]) : Evaluate(function.Expressions[2]); break; - case "in": + case "IN": CheckCase("in", function.Identifier.Name); if (function.Expressions.Length < 2) throw new ArgumentException("in() takes at least 2 arguments"); diff --git a/src/NCalc/Domain/Parameter.cs b/src/NCalc/Domain/Parameter.cs index 9c6f8e7..776db00 100644 --- a/src/NCalc/Domain/Parameter.cs +++ b/src/NCalc/Domain/Parameter.cs @@ -3,8 +3,7 @@ namespace NCalc.Domain; public class Identifier(string name) : LogicalExpression { public string Name { get; set; } = name; - - + public override void Accept(LogicalExpressionVisitor visitor) { visitor.Visit(this); diff --git a/src/NCalc/Expression.cs b/src/NCalc/Expression.cs index bae6289..8d44d7f 100644 --- a/src/NCalc/Expression.cs +++ b/src/NCalc/Expression.cs @@ -1,11 +1,11 @@ using System; using System.Collections; +using System.Collections.Concurrent; using System.Collections.Generic; using System.Diagnostics; using System.Globalization; using System.Linq; using System.Text; -using System.Threading; using NCalc.Domain; using Antlr4.Runtime; @@ -136,8 +136,8 @@ public Expression(LogicalExpression expression, EvaluationVisitor evaluationVisi #region Cache management private static bool _cacheEnabled = true; - private static Dictionary _compiledExpressions = new(); - private static readonly ReaderWriterLock Rwl = new(); + private static readonly ConcurrentDictionary> _compiledExpressions = new(); + public static bool CacheEnabled { @@ -149,7 +149,7 @@ public static bool CacheEnabled if (!CacheEnabled) { // Clears cache - _compiledExpressions = new Dictionary(); + _compiledExpressions.Clear(); } } } @@ -157,31 +157,15 @@ public static bool CacheEnabled /// /// Removed unused entries from cached compiled expression /// - private static void CleanCache() + private static void ClearCache() { - var keysToRemove = new List(); - - try + foreach (var kvp in _compiledExpressions) { - Rwl.AcquireWriterLock(Timeout.Infinite); - foreach (var de in _compiledExpressions) - { - if (!de.Value.IsAlive) - { - keysToRemove.Add(de.Key); - } - } - + if (kvp.Value.TryGetTarget(out _)) + continue; - foreach (var key in keysToRemove) - { - _compiledExpressions.Remove(key); - Trace.TraceInformation("Cache entry released: " + key); - } - } - finally - { - Rwl.ReleaseReaderLock(); + if (_compiledExpressions.TryRemove(kvp.Key, out _)) + Trace.TraceInformation("Cache entry released: " + kvp.Key); } } @@ -194,33 +178,18 @@ private static void CleanCache() public static LogicalExpression Compile(string expression, EvaluateOptions options) { - LogicalExpression logicalExpression = null; + LogicalExpression logicalExpression; if (_cacheEnabled && !options.HasOption(EvaluateOptions.NoCache)) { - try + if (_compiledExpressions.TryGetValue(expression, out var wr)) { - Rwl.AcquireReaderLock(Timeout.Infinite); + Trace.TraceInformation("Expression retrieved from cache: " + expression); - if (_compiledExpressions.TryGetValue(expression, out var wr)) - { - Trace.TraceInformation("Expression retrieved from cache: " + expression); - logicalExpression = wr.Target as LogicalExpression; - - if (wr.IsAlive && logicalExpression != null) - { - return logicalExpression; - } - } - } - finally - { - Rwl.ReleaseReaderLock(); + if (wr.TryGetTarget(out var target)) + return target; } } - - if (logicalExpression != null) - return logicalExpression; var lexer = new NCalcLexer(new AntlrInputStream(expression)); var errorListenerLexer = new ErrorListenerLexer(); @@ -266,20 +235,12 @@ public static LogicalExpression Compile(string expression, EvaluateOptions optio if (!_cacheEnabled || options.HasOption(EvaluateOptions.NoCache)) return logicalExpression; - try - { - Rwl.AcquireWriterLock(Timeout.Infinite); - _compiledExpressions[expression] = new WeakReference(logicalExpression); - } - finally - { - Rwl.ReleaseWriterLock(); - } - - CleanCache(); + _compiledExpressions[expression] = new WeakReference(logicalExpression); + + ClearCache(); Trace.TraceInformation("Expression added to cache: " + expression); - + return logicalExpression; }