From abcb7f0d58b093fe813d90f19c19b7cffb9cbe78 Mon Sep 17 00:00:00 2001 From: Marko Lahma Date: Sat, 27 Apr 2024 20:53:01 +0300 Subject: [PATCH] Improve environment handling performance and cleanup API (#1845) --- Jint/Runtime/Debugger/DebugScope.cs | 2 +- .../Specialized/ClrAccessDescriptor.cs | 6 ++-- .../Environments/DeclarativeEnvironment.cs | 31 +++++++++++++------ Jint/Runtime/Environments/Environment.cs | 6 +--- .../Runtime/Environments/GlobalEnvironment.cs | 9 ++---- Jint/Runtime/Environments/JintEnvironment.cs | 13 +++----- .../Runtime/Environments/ModuleEnvironment.cs | 5 ++- .../Runtime/Environments/ObjectEnvironment.cs | 11 ++----- .../Expressions/JintAssignmentExpression.cs | 1 - .../Expressions/JintIdentifierExpression.cs | 4 +-- .../Expressions/JintMemberExpression.cs | 16 ++++------ .../Expressions/JintUpdateExpression.cs | 19 +++++------- .../Statements/JintForStatement.cs | 13 ++------ 13 files changed, 55 insertions(+), 81 deletions(-) diff --git a/Jint/Runtime/Debugger/DebugScope.cs b/Jint/Runtime/Debugger/DebugScope.cs index 6f89db0a31..711659dcb4 100644 --- a/Jint/Runtime/Debugger/DebugScope.cs +++ b/Jint/Runtime/Debugger/DebugScope.cs @@ -57,7 +57,7 @@ internal DebugScope(DebugScopeType type, Environment record, bool isTopLevel) /// Value of the binding public JsValue? GetBindingValue(string name) { - _record.TryGetBinding(new Environment.BindingName(name), strict: true, out _, out var result); + _record.TryGetBinding(new Environment.BindingName(name), out var result); return result; } diff --git a/Jint/Runtime/Descriptors/Specialized/ClrAccessDescriptor.cs b/Jint/Runtime/Descriptors/Specialized/ClrAccessDescriptor.cs index c190949242..1c72d05f61 100644 --- a/Jint/Runtime/Descriptors/Specialized/ClrAccessDescriptor.cs +++ b/Jint/Runtime/Descriptors/Specialized/ClrAccessDescriptor.cs @@ -31,14 +31,14 @@ public ClrAccessDescriptor( private JsValue DoGet(JsValue n) { - return _env.TryGetBinding(_name, false, out var binding, out _) - ? binding.Value + return _env.TryGetBinding(_name, out var value) + ? value : JsValue.Undefined; } private void DoSet(JsValue n, JsValue o) { - _env.SetMutableBinding(_name.Key, o, true); + _env.SetMutableBinding(_name.Key, o, strict: true); } } } diff --git a/Jint/Runtime/Environments/DeclarativeEnvironment.cs b/Jint/Runtime/Environments/DeclarativeEnvironment.cs index d9bbbd8efe..0fab1c8489 100644 --- a/Jint/Runtime/Environments/DeclarativeEnvironment.cs +++ b/Jint/Runtime/Environments/DeclarativeEnvironment.cs @@ -23,16 +23,16 @@ public DeclarativeEnvironment(Engine engine, bool catchEnvironment = false) : ba internal sealed override bool HasBinding(Key name) => _dictionary is not null && _dictionary.ContainsKey(name); - internal override bool TryGetBinding( - BindingName name, - bool strict, - out Binding binding, - [NotNullWhen(true)] out JsValue? value) + internal override bool TryGetBinding(BindingName name, [NotNullWhen(true)] out JsValue? value) { - binding = default; - var success = _dictionary is not null &&_dictionary.TryGetValue(name.Key, out binding); - value = success && binding.IsInitialized() ? binding.Value : default; - return success; + if (_dictionary?.TryGetValue(name.Key, out var binding) == true) + { + value = binding.Value; + return true; + } + + value = null; + return false; } internal void CreateMutableBindingAndInitialize(Key name, bool canBeDeleted, JsValue value) @@ -75,6 +75,7 @@ internal sealed override void SetMutableBinding(Key name, JsValue value, bool st { ExceptionHelper.ThrowReferenceNameError(_engine.Realm, name); } + CreateMutableBindingAndInitialize(name, canBeDeleted: true, value); return; } @@ -169,5 +170,17 @@ public void Clear() { _dictionary = null; } + + internal void TransferTo(List names, DeclarativeEnvironment env) + { + var source = _dictionary!; + var target = env._dictionary ??= new HybridDictionary(names.Count, checkExistingKeys: true); + for (var j = 0; j < names.Count; j++) + { + var bn = names[j]; + source.TryGetValue(bn, out var lastValue); + target[bn] = new Binding(lastValue.Value, canBeDeleted: false, mutable: true, strict: false); + } + } } } diff --git a/Jint/Runtime/Environments/Environment.cs b/Jint/Runtime/Environments/Environment.cs index 906c5e9018..6387e3ffb7 100644 --- a/Jint/Runtime/Environments/Environment.cs +++ b/Jint/Runtime/Environments/Environment.cs @@ -28,11 +28,7 @@ protected Environment(Engine engine) : base(InternalTypes.ObjectEnvironmentRecor internal abstract bool HasBinding(BindingName name); - internal abstract bool TryGetBinding( - BindingName name, - bool strict, - out Binding binding, - [NotNullWhen(true)] out JsValue? value); + internal abstract bool TryGetBinding(BindingName name, [NotNullWhen(true)] out JsValue? value); /// /// Creates a new mutable binding in an environment record. diff --git a/Jint/Runtime/Environments/GlobalEnvironment.cs b/Jint/Runtime/Environments/GlobalEnvironment.cs index d957518c90..3a1b2e14f0 100644 --- a/Jint/Runtime/Environments/GlobalEnvironment.cs +++ b/Jint/Runtime/Environments/GlobalEnvironment.cs @@ -69,19 +69,14 @@ internal override bool HasBinding(BindingName name) return _global.HasProperty(name.Value); } - internal override bool TryGetBinding( - BindingName name, - bool strict, - out Binding binding, - [NotNullWhen(true)] out JsValue? value) + internal override bool TryGetBinding(BindingName name, [NotNullWhen(true)] out JsValue? value) { - if (_declarativeRecord._dictionary is not null && _declarativeRecord.TryGetBinding(name, strict, out binding, out value)) + if (_declarativeRecord._dictionary is not null && _declarativeRecord.TryGetBinding(name, out value)) { return true; } // we unwrap by name - binding = default; value = default; // normal case is to find diff --git a/Jint/Runtime/Environments/JintEnvironment.cs b/Jint/Runtime/Environments/JintEnvironment.cs index 5d94397334..80aa73f9a0 100644 --- a/Jint/Runtime/Environments/JintEnvironment.cs +++ b/Jint/Runtime/Environments/JintEnvironment.cs @@ -9,7 +9,7 @@ internal static class JintEnvironment { internal static bool TryGetIdentifierEnvironmentWithBinding( Environment env, - in Environment.BindingName name, + Environment.BindingName name, [NotNullWhen(true)] out Environment? record) { record = env; @@ -34,8 +34,7 @@ record = record._outerEnv; internal static bool TryGetIdentifierEnvironmentWithBindingValue( Environment env, - in Environment.BindingName name, - bool strict, + Environment.BindingName name, [NotNullWhen(true)] out Environment? record, [NotNullWhen(true)] out JsValue? value) { @@ -44,16 +43,12 @@ record = env; if (env._outerEnv is null) { - return ((GlobalEnvironment) env).TryGetBinding(name, strict, out _, out value); + return ((GlobalEnvironment) env).TryGetBinding(name, out value); } while (!ReferenceEquals(record, null)) { - if (record.TryGetBinding( - name, - strict, - out _, - out value)) + if (record.TryGetBinding(name, out value)) { return true; } diff --git a/Jint/Runtime/Environments/ModuleEnvironment.cs b/Jint/Runtime/Environments/ModuleEnvironment.cs index 02f866bbb6..afb5027e61 100644 --- a/Jint/Runtime/Environments/ModuleEnvironment.cs +++ b/Jint/Runtime/Environments/ModuleEnvironment.cs @@ -47,16 +47,15 @@ internal override JsValue GetBindingValue(Key name, bool strict) return base.GetBindingValue(name, strict); } - internal override bool TryGetBinding(BindingName name, bool strict, out Binding binding, [NotNullWhen(true)] out JsValue? value) + internal override bool TryGetBinding(BindingName name, [NotNullWhen(true)] out JsValue? value) { if (_importBindings.TryGetValue(name.Key, out var indirectBinding)) { value = indirectBinding.Module._environment.GetBindingValue(indirectBinding.BindingName, strict: true); - binding = new(value, canBeDeleted: false, mutable: false, strict: true); return true; } - return base.TryGetBinding(name, strict, out binding, out value); + return base.TryGetBinding(name, out value); } /// diff --git a/Jint/Runtime/Environments/ObjectEnvironment.cs b/Jint/Runtime/Environments/ObjectEnvironment.cs index b2e6c8304a..46df2860e5 100644 --- a/Jint/Runtime/Environments/ObjectEnvironment.cs +++ b/Jint/Runtime/Environments/ObjectEnvironment.cs @@ -67,15 +67,9 @@ private bool HasProperty(JsValue property) return _bindingObject.HasProperty(property); } - internal override bool TryGetBinding( - BindingName name, - bool strict, - out Binding binding, - [NotNullWhen(true)] out JsValue? value) + internal override bool TryGetBinding(BindingName name, [NotNullWhen(true)] out JsValue? value) { // we unwrap by name - binding = default; - if (!HasProperty(name.Value)) { value = default; @@ -153,8 +147,7 @@ internal override void SetMutableBinding(BindingName name, JsValue value, bool s internal override JsValue GetBindingValue(Key name, bool strict) { - JsValue value; - if (!_bindingObject.TryGetValue(name.Name, out value) && strict) + if (!_bindingObject.TryGetValue(name.Name, out var value) && strict) { ExceptionHelper.ThrowReferenceNameError(_engine.Realm, name.Name); } diff --git a/Jint/Runtime/Interpreter/Expressions/JintAssignmentExpression.cs b/Jint/Runtime/Interpreter/Expressions/JintAssignmentExpression.cs index 2c782c7bd8..6b89c60f2f 100644 --- a/Jint/Runtime/Interpreter/Expressions/JintAssignmentExpression.cs +++ b/Jint/Runtime/Interpreter/Expressions/JintAssignmentExpression.cs @@ -49,7 +49,6 @@ protected override object EvaluateInternal(EvaluationContext context) if (_leftIdentifier is not null && JintEnvironment.TryGetIdentifierEnvironmentWithBindingValue( engine.ExecutionContext.LexicalEnvironment, _leftIdentifier.Identifier, - StrictModeScope.IsStrictModeCode, out var identifierEnvironment, out var temp)) { diff --git a/Jint/Runtime/Interpreter/Expressions/JintIdentifierExpression.cs b/Jint/Runtime/Interpreter/Expressions/JintIdentifierExpression.cs index 611b398a88..3eff83b4c3 100644 --- a/Jint/Runtime/Interpreter/Expressions/JintIdentifierExpression.cs +++ b/Jint/Runtime/Interpreter/Expressions/JintIdentifierExpression.cs @@ -54,14 +54,12 @@ public override JsValue GetValue(EvaluationContext context) return identifier.CalculatedValue; } - var strict = StrictModeScope.IsStrictModeCode; var engine = context.Engine; var env = engine.ExecutionContext.LexicalEnvironment; if (JintEnvironment.TryGetIdentifierEnvironmentWithBindingValue( env, identifier, - strict, out _, out var value)) { @@ -72,7 +70,7 @@ public override JsValue GetValue(EvaluationContext context) } else { - var reference = engine._referencePool.Rent(JsValue.Undefined, identifier.Value, strict, thisValue: null); + var reference = engine._referencePool.Rent(JsValue.Undefined, identifier.Value, StrictModeScope.IsStrictModeCode, thisValue: null); value = engine.GetValue(reference, returnReferenceToPool: true); } diff --git a/Jint/Runtime/Interpreter/Expressions/JintMemberExpression.cs b/Jint/Runtime/Interpreter/Expressions/JintMemberExpression.cs index f48968e69b..98db89d9d0 100644 --- a/Jint/Runtime/Interpreter/Expressions/JintMemberExpression.cs +++ b/Jint/Runtime/Interpreter/Expressions/JintMemberExpression.cs @@ -57,21 +57,18 @@ protected override object EvaluateInternal(EvaluationContext context) } JsValue? actualThis = null; - string? baseReferenceName = null; + object? baseReferenceName = null; JsValue? baseValue = null; - var isStrictModeCode = StrictModeScope.IsStrictModeCode; var engine = context.Engine; if (_objectExpression is JintIdentifierExpression identifierExpression) { var identifier = identifierExpression.Identifier; baseReferenceName = identifier.Key.Name; - var strict = isStrictModeCode; var env = engine.ExecutionContext.LexicalEnvironment; JintEnvironment.TryGetIdentifierEnvironmentWithBindingValue( env, identifier, - strict, out _, out baseValue); } @@ -96,13 +93,12 @@ protected override object EvaluateInternal(EvaluationContext context) } if (baseReference is Reference reference) { - baseReferenceName = reference.ReferencedName.ToString(); - baseValue = engine.GetValue(reference, false); - engine._referencePool.Return(reference); + baseReferenceName = reference.ReferencedName; + baseValue = engine.GetValue(reference, returnReferenceToPool: true); } else { - baseValue = engine.GetValue(baseReference, false); + baseValue = engine.GetValue(baseReference, returnReferenceToPool: false); } } @@ -117,7 +113,7 @@ protected override object EvaluateInternal(EvaluationContext context) // we can use base data types securely, object evaluation can mess things up var referenceName = property.IsPrimitive() ? TypeConverter.ToString(property) - : _determinedProperty?.ToString() ?? baseReferenceName; + : _determinedProperty?.ToString() ?? baseReferenceName?.ToString(); TypeConverter.CheckObjectCoercible(engine, baseValue, _memberExpression.Property, referenceName!); } @@ -127,7 +123,7 @@ protected override object EvaluateInternal(EvaluationContext context) return MakePrivateReference(engine, baseValue, property); } - return context.Engine._referencePool.Rent(baseValue, property, isStrictModeCode, thisValue: actualThis); + return context.Engine._referencePool.Rent(baseValue, property, StrictModeScope.IsStrictModeCode, thisValue: actualThis); } /// diff --git a/Jint/Runtime/Interpreter/Expressions/JintUpdateExpression.cs b/Jint/Runtime/Interpreter/Expressions/JintUpdateExpression.cs index b34570a3c4..cb3ef216c3 100644 --- a/Jint/Runtime/Interpreter/Expressions/JintUpdateExpression.cs +++ b/Jint/Runtime/Interpreter/Expressions/JintUpdateExpression.cs @@ -1,5 +1,6 @@ using Jint.Native; using Jint.Runtime.Environments; +using Environment = Jint.Runtime.Environments.Environment; namespace Jint.Runtime.Interpreter.Expressions { @@ -121,20 +122,16 @@ private JsValue UpdateNonIdentifier(EvaluationContext context) private JsValue? UpdateIdentifier(EvaluationContext context) { - var strict = StrictModeScope.IsStrictModeCode; var name = _leftIdentifier!.Identifier; - var engine = context.Engine; - var env = engine.ExecutionContext.LexicalEnvironment; if (JintEnvironment.TryGetIdentifierEnvironmentWithBindingValue( - env, - name, - strict, - out var environmentRecord, - out var value)) + context.Engine.ExecutionContext.LexicalEnvironment, + name, + out var environmentRecord, + out var value)) { - if (strict && _evalOrArguments) + if (_evalOrArguments && StrictModeScope.IsStrictModeCode) { - ExceptionHelper.ThrowSyntaxError(engine.Realm); + ExceptionHelper.ThrowSyntaxError(context.Engine.Realm); } var isInteger = value._type == InternalTypes.Integer; @@ -167,7 +164,7 @@ private JsValue UpdateNonIdentifier(EvaluationContext context) } } - environmentRecord.SetMutableBinding(name.Key.Name, newValue!, strict); + environmentRecord.SetMutableBinding(name.Key, newValue!, StrictModeScope.IsStrictModeCode); if (_prefix) { return newValue; diff --git a/Jint/Runtime/Interpreter/Statements/JintForStatement.cs b/Jint/Runtime/Interpreter/Statements/JintForStatement.cs index 30a42ad762..fe9ce7302f 100644 --- a/Jint/Runtime/Interpreter/Statements/JintForStatement.cs +++ b/Jint/Runtime/Interpreter/Statements/JintForStatement.cs @@ -170,17 +170,10 @@ private Completion ForBodyEvaluation(EvaluationContext context) private void CreatePerIterationEnvironment(EvaluationContext context) { var engine = context.Engine; - var lastIterationEnv = engine.ExecutionContext.LexicalEnvironment; - var lastIterationEnvRec = lastIterationEnv; - var outer = lastIterationEnv._outerEnv; - var thisIterationEnv = JintEnvironment.NewDeclarativeEnvironment(engine, outer); + var lastIterationEnv = (DeclarativeEnvironment) engine.ExecutionContext.LexicalEnvironment; + var thisIterationEnv = JintEnvironment.NewDeclarativeEnvironment(engine, lastIterationEnv._outerEnv); - for (var j = 0; j < _boundNames!.Count; j++) - { - var bn = _boundNames[j]; - var lastValue = lastIterationEnvRec.GetBindingValue(bn, true); - thisIterationEnv.CreateMutableBindingAndInitialize(bn, false, lastValue); - } + lastIterationEnv.TransferTo(_boundNames!, thisIterationEnv); engine.UpdateLexicalEnvironment(thisIterationEnv); }