Skip to content

Commit

Permalink
Obsolete and cleanup some ObjectInstance access patterns (#1809)
Browse files Browse the repository at this point in the history
  • Loading branch information
@lahma
lahma authored Mar 16, 2024
1 parent afdc082 commit d306162
Show file tree
Hide file tree
Showing 7 changed files with 34 additions and 90 deletions.
61 changes: 18 additions & 43 deletions Jint/Engine.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -597,56 +597,31 @@ internal JsValue GetValue(Reference reference, bool returnReferenceToPool)
var v = baseObj.Get(property, reference.ThisValue);
return v;
}
else
{
// check if we are accessing a string, boxing operation can be costly to do index access
// we have good chance to have fast path with integer or string indexer
ObjectInstance? o = null;
if ((property._type & (InternalTypes.String | InternalTypes.Integer)) != InternalTypes.Empty
&& baseValue is JsString s
&& TryHandleStringValue(property, s, ref o, out var jsValue))
{
return jsValue;
}

if (o is null)
{
o = Runtime.TypeConverter.ToObject(Realm, baseValue);
}

if (reference.IsPrivateReference)
{
return o.PrivateGet((PrivateName) reference.ReferencedName);
}

var desc = o.GetProperty(property);
if (desc == PropertyDescriptor.Undefined)
{
return JsValue.Undefined;
}

if (desc.IsDataDescriptor())
{
return desc.Value;
}
// check if we are accessing a string, boxing operation can be costly to do index access
// we have good chance to have fast path with integer or string indexer
ObjectInstance? o = null;
if ((property._type & (InternalTypes.String | InternalTypes.Integer)) != InternalTypes.Empty
&& baseValue is JsString s
&& TryHandleStringValue(property, s, ref o, out var jsValue))
{
return jsValue;
}

var getter = desc.Get!;
if (getter.IsUndefined())
{
return JsValue.Undefined;
}
if (o is null)
{
o = Runtime.TypeConverter.ToObject(Realm, baseValue);
}

var callable = (ICallable) getter;
return callable.Call(baseValue, Arguments.Empty);
if (reference.IsPrivateReference)
{
return o.PrivateGet((PrivateName) reference.ReferencedName);
}
}

var record = baseValue as Environment;
if (record is null)
{
ExceptionHelper.ThrowArgumentException();
return o.Get(property, reference.ThisValue);
}

var record = (Environment) baseValue;
var bindingValue = record.GetBindingValue(reference.ReferencedName.ToString(), reference.Strict);

if (returnReferenceToPool)
Expand Down
13 changes: 1 addition & 12 deletions Jint/Native/Array/ArrayInstance.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -292,17 +292,6 @@ private bool DefineOwnProperty(uint index, PropertyDescriptor desc)
[MethodImpl(MethodImplOptions.AggressiveInlining)]
private JsNumber GetJsNumberLength() => _length is null ? JsNumber.PositiveZero : (JsNumber) _length._value!;

protected sealed override void AddProperty(JsValue property, PropertyDescriptor descriptor)
{
if (CommonProperties.Length.Equals(property ))
{
_length = descriptor;
return;
}

base.AddProperty(property, descriptor);
}

protected sealed override bool TryGetProperty(JsValue property, [NotNullWhen(true)] out PropertyDescriptor? descriptor)
{
if (CommonProperties.Length.Equals(property))
Expand Down Expand Up @@ -436,7 +425,7 @@ internal JsValue Get(uint index)
{
if (!TryGetValue(index, out var value))
{
value = UnwrapJsValue(Prototype?.GetProperty(JsString.Create(index)) ?? PropertyDescriptor.Undefined);
value = Prototype?.Get(JsString.Create(index)) ?? Undefined;
}

return value;
Expand Down
2 changes: 1 addition & 1 deletion Jint/Native/JsArguments.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ public override bool Set(JsValue property, JsValue value, JsValue receiver)
}

// property is an accessor or inherited
var desc = GetProperty(property);
var desc = GetOwnProperty(property);

if (desc.IsAccessorDescriptor())
{
Expand Down
33 changes: 7 additions & 26 deletions Jint/Native/Object/ObjectInstance.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -179,13 +179,6 @@ internal void SetProperty(Key property, PropertyDescriptor value)
_properties[property] = value;
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
internal void SetDataProperty(string property, JsValue value)
{
_properties ??= new PropertyDictionary();
_properties[property] = new PropertyDescriptor(value, PropertyFlag.ConfigurableEnumerableWritable);
}

[MethodImpl(MethodImplOptions.NoInlining)]
private void SetPropertyUnlikely(JsValue property, PropertyDescriptor value)
{
Expand Down Expand Up @@ -326,6 +319,7 @@ private List<JsValue> GetOwnPropertyKeysSorted(List<JsValue> initialOwnPropertyK

internal virtual IEnumerable<JsValue> GetInitialOwnStringPropertyKeys() => System.Linq.Enumerable.Empty<JsValue>();

[Obsolete("Will be removed")]
protected virtual void AddProperty(JsValue property, PropertyDescriptor descriptor)
{
SetProperty(property, descriptor);
Expand Down Expand Up @@ -453,9 +447,7 @@ protected internal virtual void SetOwnProperty(JsValue property, PropertyDescrip
SetProperty(property, desc);
}

/// <summary>
/// http://www.ecma-international.org/ecma-262/5.1/#sec-8.12.2
/// </summary>
[Obsolete("Use Get or GetOwnProperty")]
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public PropertyDescriptor GetProperty(JsValue property)
{
Expand All @@ -473,13 +465,8 @@ public bool TryGetValue(JsValue property, out JsValue value)
{
value = Undefined;
var desc = GetOwnProperty(property);
if (desc != null && desc != PropertyDescriptor.Undefined)
if (desc != PropertyDescriptor.Undefined)
{
if (desc == PropertyDescriptor.Undefined)
{
return false;
}

var descValue = desc.Value;
if (desc.WritableSet && !ReferenceEquals(descValue, null))
{
Expand All @@ -500,12 +487,7 @@ public bool TryGetValue(JsValue property, out JsValue value)
return true;
}

if (ReferenceEquals(Prototype, null))
{
return false;
}

return Prototype.TryGetValue(property, out value);
return Prototype?.TryGetValue(property, out value) == true;
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
Expand Down Expand Up @@ -654,7 +636,7 @@ internal bool CanPut(JsValue property)
return Extensible;
}

var inherited = Prototype.GetProperty(property);
var inherited = Prototype.GetOwnProperty(property);

if (inherited == PropertyDescriptor.Undefined)
{
Expand Down Expand Up @@ -1345,17 +1327,16 @@ internal virtual bool CreateMethodProperty(JsValue p, JsValue v)
}

/// <summary>
/// https://tc39.es/ecma262/#sec-createdatapropertyorthrow
/// https://tc39.es/ecma262/#sec-createdataproperty
/// </summary>
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public bool CreateDataProperty(JsValue p, JsValue v)
{
return DefineOwnProperty(p, new PropertyDescriptor(v, PropertyFlag.ConfigurableEnumerableWritable));
}


/// <summary>
/// https://tc39.es/ecma262/#sec-createdataproperty
/// https://tc39.es/ecma262/#sec-createdatapropertyorthrow
/// </summary>
[MethodImpl(MethodImplOptions.AggressiveInlining)]
internal bool CreateDataPropertyOrThrow(JsValue p, JsValue v)
Expand Down
2 changes: 1 addition & 1 deletion Jint/Runtime/Environments/GlobalEnvironment.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@ internal override JsValue GetBindingValue(Key name, bool strict)
}
else
{
desc = _global.GetProperty(name.Name);
desc = _global.GetOwnProperty(name.Name);
}

if (strict && desc == PropertyDescriptor.Undefined)
Expand Down
11 changes: 5 additions & 6 deletions Jint/Runtime/Environments/ObjectEnvironment.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,7 @@ internal override bool TryGetBinding(
return false;
}

var desc = _bindingObject.GetProperty(name.Value);
value = ObjectInstance.UnwrapJsValue(desc, _bindingObject);
value = _bindingObject.Get(name.Value);
return true;
}

Expand Down Expand Up @@ -154,13 +153,13 @@ internal override void SetMutableBinding(BindingName name, JsValue value, bool s

internal override JsValue GetBindingValue(Key name, bool strict)
{
var desc = _bindingObject.GetProperty(name.Name);
if (strict && desc == PropertyDescriptor.Undefined)
JsValue value;
if (!_bindingObject.TryGetValue(name.Name, out value) && strict)
{
ExceptionHelper.ThrowReferenceNameError(_engine.Realm, name);
ExceptionHelper.ThrowReferenceNameError(_engine.Realm, name.Name);
}

return ObjectInstance.UnwrapJsValue(desc, _bindingObject);
return value;
}

internal override bool DeleteBinding(Key name) => _bindingObject.Delete(name.Name);
Expand Down
2 changes: 1 addition & 1 deletion Jint/Runtime/Interpreter/Expressions/JintUnaryExpression.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ private JsValue EvaluateJsValue(EvaluationContext context)
ExceptionHelper.ThrowTypeError(engine.Realm, $"Cannot delete property '{referencedName}' of {o}");
}

if (StrictModeScope.IsStrictModeCode && !r.Base.AsObject().GetProperty(referencedName).Configurable)
if (StrictModeScope.IsStrictModeCode && !r.Base.AsObject().GetOwnProperty(referencedName).Configurable)
{
ExceptionHelper.ThrowTypeError(engine.Realm, $"Cannot delete property '{referencedName}' of {o}");
}
Expand Down

0 comments on commit d306162

Please sign in to comment.