From 7b74a7e725e75bf20c93ebb169294fb33882b56a Mon Sep 17 00:00:00 2001 From: Thays Grazia Date: Tue, 16 Nov 2021 15:14:37 -0300 Subject: [PATCH] [wasm][debugger] Improvement of memory consumption while Evaluating Expressions (#61470) * Fix memory consumption. * Fix debugger-tests * Fix compilation. * Addressing @lewing PR. * Address @lewing comment * Addressing @radical comment. * Addressing comments. * Addressing @radical comments. * missing return. * Addressing @radical comments * Adding test case Co-authored-by: Larry Ewing * Fixing tests. * Adding another test case. Thanks @lewing. * Reuse the script. Co-authored-by: Larry Ewing --- .../BrowserDebugProxy/BrowserDebugProxy.csproj | 1 + .../BrowserDebugProxy/EvaluateExpression.cs | 198 ++++++++------------- .../DebuggerTestSuite/EvaluateOnCallFrameTests.cs | 7 +- .../tests/debugger-test/debugger-evaluate-test.cs | 5 + 4 files changed, 86 insertions(+), 125 deletions(-) diff --git a/src/mono/wasm/debugger/BrowserDebugProxy/BrowserDebugProxy.csproj b/src/mono/wasm/debugger/BrowserDebugProxy/BrowserDebugProxy.csproj index 169856b..0c5d37b 100644 --- a/src/mono/wasm/debugger/BrowserDebugProxy/BrowserDebugProxy.csproj +++ b/src/mono/wasm/debugger/BrowserDebugProxy/BrowserDebugProxy.csproj @@ -7,6 +7,7 @@ + diff --git a/src/mono/wasm/debugger/BrowserDebugProxy/EvaluateExpression.cs b/src/mono/wasm/debugger/BrowserDebugProxy/EvaluateExpression.cs index 6d1367f..5545a26 100644 --- a/src/mono/wasm/debugger/BrowserDebugProxy/EvaluateExpression.cs +++ b/src/mono/wasm/debugger/BrowserDebugProxy/EvaluateExpression.cs @@ -12,17 +12,28 @@ using System.Threading; using System.Threading.Tasks; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Scripting; using Microsoft.CodeAnalysis.CSharp.Syntax; using Microsoft.CodeAnalysis.Emit; +using Microsoft.CodeAnalysis.Scripting; using Newtonsoft.Json; using Newtonsoft.Json.Linq; +using System.Text.RegularExpressions; namespace Microsoft.WebAssembly.Diagnostics { internal static class EvaluateExpression { + internal static Script script = CSharpScript.Create( + "", + ScriptOptions.Default.WithReferences( + typeof(object).Assembly, + typeof(Enumerable).Assembly, + typeof(JObject).Assembly + )); private class FindVariableNMethodCall : CSharpSyntaxWalker { + private static Regex regexForReplaceVarName = new Regex(@"[^A-Za-z0-9_]", RegexOptions.Singleline); public List identifiers = new List(); public List methodCall = new List(); public List memberAccesses = new List(); @@ -32,6 +43,7 @@ namespace Microsoft.WebAssembly.Diagnostics private int visitCount; public bool hasMethodCalls; public bool hasElementAccesses; + internal List variableDefinitions = new List(); public void VisitInternal(SyntaxNode node) { @@ -97,7 +109,7 @@ namespace Microsoft.WebAssembly.Diagnostics { // Generate a random suffix string suffix = Guid.NewGuid().ToString().Substring(0, 5); - string prefix = ma_str.Trim().Replace(".", "_"); + string prefix = regexForReplaceVarName.Replace(ma_str, "_"); id_name = $"{prefix}_{suffix}"; memberAccessToParamName[ma_str] = id_name; @@ -114,7 +126,7 @@ namespace Microsoft.WebAssembly.Diagnostics { // Generate a random suffix string suffix = Guid.NewGuid().ToString().Substring(0, 5); - string prefix = iesStr.Trim().Replace(".", "_").Replace("(", "_").Replace(")", "_"); + string prefix = regexForReplaceVarName.Replace(iesStr, "_"); id_name = $"{prefix}_{suffix}"; methodCallToParamName[iesStr] = id_name; } @@ -138,7 +150,7 @@ namespace Microsoft.WebAssembly.Diagnostics return SyntaxFactory.IdentifierName(id_name); }); - var paramsSet = new HashSet(); + var localsSet = new HashSet(); // 2. For every unique member ref, add a corresponding method param if (ma_values != null) @@ -151,7 +163,7 @@ namespace Microsoft.WebAssembly.Diagnostics throw new Exception($"BUG: Expected to find an id name for the member access string: {node_str}"); } memberAccessValues[id_name] = value; - root = UpdateWithNewMethodParam(root, id_name, value); + AddLocalVariableWithValue(id_name, value); } } @@ -159,7 +171,7 @@ namespace Microsoft.WebAssembly.Diagnostics { foreach ((IdentifierNameSyntax idns, JObject value) in identifiers.Zip(id_values)) { - root = UpdateWithNewMethodParam(root, idns.Identifier.Text, value); + AddLocalVariableWithValue(idns.Identifier.Text, value); } } @@ -172,7 +184,7 @@ namespace Microsoft.WebAssembly.Diagnostics { throw new Exception($"BUG: Expected to find an id name for the member access string: {node_str}"); } - root = UpdateWithNewMethodParam(root, id_name, value); + AddLocalVariableWithValue(id_name, value); } } @@ -185,96 +197,73 @@ namespace Microsoft.WebAssembly.Diagnostics { throw new Exception($"BUG: Expected to find an id name for the element access string: {node_str}"); } - root = UpdateWithNewMethodParam(root, id_name, value); + AddLocalVariableWithValue(id_name, value); } } return syntaxTree.WithRootAndOptions(root, syntaxTree.Options); - CompilationUnitSyntax UpdateWithNewMethodParam(CompilationUnitSyntax root, string id_name, JObject value) + void AddLocalVariableWithValue(string idName, JObject value) { - var classDeclaration = root.Members.ElementAt(0) as ClassDeclarationSyntax; - var method = classDeclaration.Members.ElementAt(0) as MethodDeclarationSyntax; - - if (paramsSet.Contains(id_name)) - { - // repeated member access expression - // eg. this.a + this.a - return root; - } - - argValues.Add(ConvertJSToCSharpType(value)); - - MethodDeclarationSyntax updatedMethod = method.AddParameterListParameters( - SyntaxFactory.Parameter( - SyntaxFactory.Identifier(id_name)) - .WithType(SyntaxFactory.ParseTypeName(GetTypeFullName(value)))); - - paramsSet.Add(id_name); - root = root.ReplaceNode(method, updatedMethod); - - return root; + if (localsSet.Contains(idName)) + return; + localsSet.Add(idName); + variableDefinitions.Add(ConvertJSToCSharpLocalVariableAssignment(idName, value)); } } - private object ConvertJSToCSharpType(JToken variable) + private string ConvertJSToCSharpLocalVariableAssignment(string idName, JToken variable) { + string typeRet; + object valueRet; JToken value = variable["value"]; string type = variable["type"].Value(); string subType = variable["subtype"]?.Value(); - + string objectId = variable["objectId"]?.Value(); switch (type) { case "string": - return value?.Value(); + { + var str = value?.Value(); + str = str.Replace("\"", "\\\""); + valueRet = $"\"{str}\""; + typeRet = "string"; + break; + } case "number": - return value?.Value(); + valueRet = value?.Value(); + typeRet = "double"; + break; case "boolean": - return value?.Value(); + valueRet = value?.Value().ToLower(); + typeRet = "bool"; + break; case "object": - return variable; + valueRet = "Newtonsoft.Json.Linq.JObject.FromObject(new {" + + $"type = \"{type}\"" + + $", description = \"{variable["description"].Value()}\"" + + $", className = \"{variable["className"].Value()}\"" + + (subType != null ? $", subtype = \"{subType}\"" : "") + + (objectId != null ? $", objectId = \"{objectId}\"" : "") + + "})"; + typeRet = "object"; + break; case "void": - return null; - } - throw new Exception($"Evaluate of this datatype {type} not implemented yet");//, "Unsupported"); - } - - private string GetTypeFullName(JToken variable) - { - string type = variable["type"].ToString(); - string subType = variable["subtype"]?.Value(); - object value = ConvertJSToCSharpType(variable); - - switch (type) - { - case "object": - { - if (subType == "null") - return variable["className"].Value(); - else - return "object"; - } - case "void": - return "object"; + valueRet = "Newtonsoft.Json.Linq.JObject.FromObject(new {" + + $"type = \"object\"," + + $"description = \"object\"," + + $"className = \"object\"," + + $"subtype = \"null\"" + + "})"; + typeRet = "object"; + break; default: - return value.GetType().FullName; + throw new Exception($"Evaluate of this datatype {type} not implemented yet");//, "Unsupported"); } - throw new ReturnAsErrorException($"GetTypefullName: Evaluate of this datatype {type} not implemented yet", "Unsupported"); + return $"{typeRet} {idName} = {valueRet};"; } } - private static SyntaxNode GetExpressionFromSyntaxTree(SyntaxTree syntaxTree) - { - CompilationUnitSyntax root = syntaxTree.GetCompilationUnitRoot(); - ClassDeclarationSyntax classDeclaration = root.Members.ElementAt(0) as ClassDeclarationSyntax; - MethodDeclarationSyntax methodDeclaration = classDeclaration.Members.ElementAt(0) as MethodDeclarationSyntax; - BlockSyntax blockValue = methodDeclaration.Body; - ReturnStatementSyntax returnValue = blockValue.Statements.ElementAt(0) as ReturnStatementSyntax; - ParenthesizedExpressionSyntax expressionParenthesized = returnValue.Expression as ParenthesizedExpressionSyntax; - - return expressionParenthesized?.Expression; - } - private static async Task> ResolveMemberAccessExpressions(IEnumerable member_accesses, MemberReferenceResolver resolver, CancellationToken token) { @@ -335,8 +324,6 @@ namespace Microsoft.WebAssembly.Diagnostics return values; } - [UnconditionalSuppressMessage("SingleFile", "IL3000:Avoid accessing Assembly file path when publishing as a single file", - Justification = "Suppressing the warning until gets fixed, see https://github.com/dotnet/runtime/issues/51202")] internal static async Task CompileAndRunTheExpression(string expression, MemberReferenceResolver resolver, CancellationToken token) { expression = expression.Trim(); @@ -344,23 +331,13 @@ namespace Microsoft.WebAssembly.Diagnostics { expression = "(" + expression + ")"; } - SyntaxTree syntaxTree = CSharpSyntaxTree.ParseText(@" - using System; - public class CompileAndRunTheExpression - { - public static object Evaluate() - { - return " + expression + @"; - } - }", cancellationToken: token); + SyntaxTree syntaxTree = CSharpSyntaxTree.ParseText(expression + @";", cancellationToken: token); - SyntaxNode expressionTree = GetExpressionFromSyntaxTree(syntaxTree); + SyntaxNode expressionTree = syntaxTree.GetCompilationUnitRoot(token); if (expressionTree == null) throw new Exception($"BUG: Unable to evaluate {expression}, could not get expression from the syntax tree"); - FindVariableNMethodCall findVarNMethodCall = new FindVariableNMethodCall(); findVarNMethodCall.VisitInternal(expressionTree); - // this fails with `"a)"` // because the code becomes: return (a)); // and the returned expression from GetExpressionFromSyntaxTree is `a`! @@ -388,7 +365,7 @@ namespace Microsoft.WebAssembly.Diagnostics if (findVarNMethodCall.hasMethodCalls) { - expressionTree = GetExpressionFromSyntaxTree(syntaxTree); + expressionTree = syntaxTree.GetCompilationUnitRoot(token); findVarNMethodCall.VisitInternal(expressionTree); @@ -400,7 +377,7 @@ namespace Microsoft.WebAssembly.Diagnostics // eg. "elements[0]" if (findVarNMethodCall.hasElementAccesses) { - expressionTree = GetExpressionFromSyntaxTree(syntaxTree); + expressionTree = syntaxTree.GetCompilationUnitRoot(token); findVarNMethodCall.VisitInternal(expressionTree); @@ -409,48 +386,21 @@ namespace Microsoft.WebAssembly.Diagnostics syntaxTree = findVarNMethodCall.ReplaceVars(syntaxTree, null, null, null, elementAccessValues); } - expressionTree = GetExpressionFromSyntaxTree(syntaxTree); + expressionTree = syntaxTree.GetCompilationUnitRoot(token); if (expressionTree == null) throw new Exception($"BUG: Unable to evaluate {expression}, could not get expression from the syntax tree"); - MetadataReference[] references = new MetadataReference[] - { - MetadataReference.CreateFromFile(typeof(object).Assembly.Location), - MetadataReference.CreateFromFile(typeof(Enumerable).Assembly.Location) - }; - - CSharpCompilation compilation = CSharpCompilation.Create( - "compileAndRunTheExpression", - syntaxTrees: new[] { syntaxTree }, - references: references, - options: new CSharpCompilationOptions(OutputKind.DynamicallyLinkedLibrary)); + try { + var newScript = script.ContinueWith( + string.Join("\n", findVarNMethodCall.variableDefinitions) + "\nreturn " + syntaxTree.ToString()); - SemanticModel semanticModel = compilation.GetSemanticModel(syntaxTree); - CodeAnalysis.TypeInfo TypeInfo = semanticModel.GetTypeInfo(expressionTree, cancellationToken: token); + var state = await newScript.RunAsync(cancellationToken: token); - using (var ms = new MemoryStream()) + return JObject.FromObject(ConvertCSharpToJSType(state.ReturnValue, state.ReturnValue.GetType())); + } + catch (Exception) { - EmitResult result = compilation.Emit(ms, cancellationToken: token); - if (!result.Success) - { - var sb = new StringBuilder(); - foreach (Diagnostic d in result.Diagnostics) - sb.Append(d.ToString()); - - throw new ReturnAsErrorException(sb.ToString(), "CompilationError"); - } - - ms.Seek(0, SeekOrigin.Begin); - Assembly assembly = Assembly.Load(ms.ToArray()); - Type type = assembly.GetType("CompileAndRunTheExpression"); - - object ret = type.InvokeMember("Evaluate", - BindingFlags.InvokeMethod | BindingFlags.Static | BindingFlags.Public, - null, - null, - findVarNMethodCall.argValues.ToArray()); - - return JObject.FromObject(ConvertCSharpToJSType(ret, TypeInfo.Type)); + throw new ReturnAsErrorException($"Cannot evaluate '{expression}'.", "CompilationError"); } } @@ -462,7 +412,7 @@ namespace Microsoft.WebAssembly.Diagnostics typeof(float), typeof(double) }; - private static object ConvertCSharpToJSType(object v, ITypeSymbol type) + private static object ConvertCSharpToJSType(object v, Type type) { if (v == null) return new { type = "object", subtype = "null", className = type.ToString(), description = type.ToString() }; diff --git a/src/mono/wasm/debugger/DebuggerTestSuite/EvaluateOnCallFrameTests.cs b/src/mono/wasm/debugger/DebuggerTestSuite/EvaluateOnCallFrameTests.cs index 524ff68..ae471cf 100644 --- a/src/mono/wasm/debugger/DebuggerTestSuite/EvaluateOnCallFrameTests.cs +++ b/src/mono/wasm/debugger/DebuggerTestSuite/EvaluateOnCallFrameTests.cs @@ -461,7 +461,7 @@ namespace DebuggerTests await EvaluateOnCallFrameFail(id, ("me.foo", "ReferenceError"), - ("this", "ReferenceError"), + ("this", "CompilationError"), ("this.NullIfAIsNotZero.foo", "ReferenceError")); }); @@ -542,6 +542,11 @@ namespace DebuggerTests ("this.CallMethodWithParmBool(true)", TString("TRUE")), ("this.CallMethodWithParmBool(false)", TString("FALSE")), ("this.CallMethodWithParmString(\"concat\")", TString("str_const_concat")), + ("this.CallMethodWithParmString(\"\\\"\\\"\")", TString("str_const_\"\"")), + ("this.CallMethodWithParmString(\"🛶\")", TString("str_const_🛶")), + ("this.CallMethodWithParmString(\"\\uD83D\\uDEF6\")", TString("str_const_🛶")), + ("this.CallMethodWithParmString(\"🚀\")", TString("str_const_🚀")), + ("this.CallMethodWithParmString_λ(\"🚀\")", TString("λ_🚀")), ("this.CallMethodWithParm(10) + this.a", TNumber(12)), ("this.CallMethodWithObj(null)", TNumber(-1)), ("this.CallMethodWithChar('a')", TString("str_const_a"))); diff --git a/src/mono/wasm/debugger/tests/debugger-test/debugger-evaluate-test.cs b/src/mono/wasm/debugger/tests/debugger-test/debugger-evaluate-test.cs index 74153fa..b8124c6 100644 --- a/src/mono/wasm/debugger/tests/debugger-test/debugger-evaluate-test.cs +++ b/src/mono/wasm/debugger/tests/debugger-test/debugger-evaluate-test.cs @@ -378,6 +378,11 @@ namespace DebuggerTests return str + parm; } + public string CallMethodWithParmString_λ(string parm) + { + return "λ_" + parm; + } + public string CallMethodWithParmBool(bool parm) { if (parm) -- 2.7.4