Improve interop performance against BCL list types (#1792)

Jint.Benchmark/ListInteropBenchmark.cs

@@ -0,0 +1,114 @@
+using System.Collections;
+using BenchmarkDotNet.Attributes;
+using Jint.Native;
+using Jint.Runtime.Interop;
+
+namespace Jint.Benchmark;
+
+[MemoryDiagnoser]
+public class ListInteropBenchmark
+{
+    private static bool IsArrayLike(Type type)
+    {
+        if (typeof(IDictionary).IsAssignableFrom(type))
+        {
+            return false;
+        }
+
+        if (typeof(ICollection).IsAssignableFrom(type))
+        {
+            return true;
+        }
+
+        foreach (var typeInterface in type.GetInterfaces())
+        {
+            if (!typeInterface.IsGenericType)
+            {
+                continue;
+            }
+
+            if (typeInterface.GetGenericTypeDefinition() == typeof(IReadOnlyCollection<>)
+                || typeInterface.GetGenericTypeDefinition() == typeof(ICollection<>))
+            {
+                return true;
+            }
+        }
+
+        return false;
+    }
+
+    private const int Count = 10_00;
+    private Engine _engine;
+    private JsValue[] _properties;
+
+    [GlobalSetup]
+    public void Setup()
+    {
+        _engine = new Engine(options =>
+        {
+            options
+                .SetWrapObjectHandler((engine, target, type) =>
+                {
+                    var instance = new ObjectWrapper(engine, target);
+                    var isArrayLike = IsArrayLike(instance.Target.GetType());
+                    if (isArrayLike)
+                    {
+                        instance.Prototype = engine.Intrinsics.Array.PrototypeObject;
+                    }
+
+                    return instance;
+                })
+                ;
+        });
+
+        _properties = new JsValue[Count];
+        var input = new List<Data>(Count);
+        for (var i = 0; i < Count; ++i)
+        {
+            input.Add(new Data { Category = new Category { Name = i % 2 == 0 ? "Pugal" : "Beagle" } });
+            _properties[i] = JsNumber.Create(i);
+        }
+
+        _engine.SetValue("input", input);
+        _engine.SetValue("CONST", new { category = "Pugal" });
+    }
+
+    [Benchmark]
+    public void Filter()
+    {
+        var value = (Data) _engine.Evaluate("input.filter(i => i.category?.name === CONST.category)[0]").ToObject();
+        if (value.Category.Name != "Pugal")
+        {
+            throw new InvalidOperationException();
+        }
+    }
+
+    [Benchmark]
+    public void Indexing()
+    {
+        _engine.Evaluate("for (var i = 0; i < input.length; ++i) { input[i]; }");
+    }
+
+    [Benchmark]
+    public void HasProperty()
+    {
+        var input = (ObjectWrapper) _engine.GetValue("input");
+        for (var i = 0; i < _properties.Length; ++i)
+        {
+            if (!input.HasProperty(_properties[i]))
+            {
+                throw new InvalidOperationException();
+            }
+        }
+    }
+
+    private class Data
+    {
+        public Category Category { get; set; }
+    }
+
+    private class Category
+    {
+        public string Name { get; set; }
+    }
+}

Jint/Native/Array/ArrayOperations.cs

@@ -3,6 +3,7 @@
 using Jint.Native.Object;
 using Jint.Native.String;
 using Jint.Runtime;
+using Jint.Runtime.Interop;
 
 namespace Jint.Native.Array
 {
@@ -46,6 +47,15 @@ public static ArrayOperations For(ObjectInstance instance, bool forWrite)
                 return new JsTypedArrayOperations(typedArrayInstance);
             }
 
+            if (instance is ObjectWrapper wrapper)
+            {
+                var descriptor = wrapper._typeDescriptor;
+                if (descriptor.IsArrayLike && wrapper.Target is ICollection)
+                {
+                    return new IndexWrappedOperations(wrapper);
+                }
+            }
+
             return new ObjectOperations(instance);
         }
 
@@ -121,7 +131,9 @@ public JsValue Current
                 {
                     return !_initialized
                         ? JsValue.Undefined
-                        : _obj.TryGetValue(_current, out var temp) ? temp : JsValue.Undefined;
+                        : _obj.TryGetValue(_current, out var temp)
+                            ? temp
+                            : JsValue.Undefined;
                 }
             }
 
@@ -356,7 +368,7 @@ public override void DeletePropertyOrThrow(ulong index)
             public override bool HasProperty(ulong index) => _target.HasProperty(index);
         }
 
-                private sealed class JsStringOperations : ArrayOperations
+        private sealed class JsStringOperations : ArrayOperations
         {
             private readonly Realm _realm;
             private readonly JsString _target;
@@ -459,6 +471,70 @@ public override bool TryGetValue(ulong index, out JsValue value)
 
             public override void DeletePropertyOrThrow(ulong index) => throw new NotSupportedException();
         }
+
+        private sealed class IndexWrappedOperations : ArrayOperations
+        {
+            private readonly ObjectWrapper _target;
+            private readonly ICollection _collection;
+            private readonly IList _list;
+
+            public IndexWrappedOperations(ObjectWrapper wrapper)
+            {
+                _target = wrapper;
+                _collection = (ICollection) wrapper.Target;
+                _list = (IList) wrapper.Target;
+            }
+
+            public override ObjectInstance Target => _target;
+
+            public override ulong GetSmallestIndex(ulong length) => 0;
+
+            public override uint GetLength() => (uint) _collection.Count;
+
+            public override ulong GetLongLength() => GetLength();
+
+            public override void SetLength(ulong length) => throw new NotSupportedException();
+
+            public override void EnsureCapacity(ulong capacity)
+            {
+            }
+
+            public override JsValue Get(ulong index) => index < (ulong) _collection.Count ? ReadValue((int) index) : JsValue.Undefined;
+
+            public override bool TryGetValue(ulong index, out JsValue value)
+            {
+                if (index < (ulong) _collection.Count)
+                {
+                    value = ReadValue((int) index);
+                    return true;
+                }
+
+                value = JsValue.Undefined;
+                return false;
+            }
+
+            private JsValue ReadValue(int index)
+            {
+                if (_list is not null)
+                {
+                    return (uint) index < _list.Count ? JsValue.FromObject(_target.Engine, _list[index]) : JsValue.Undefined;
+                }
+
+                // via reflection is slow, but better than nothing
+                return JsValue.FromObject(_target.Engine, _target._typeDescriptor.IntegerIndexerProperty!.GetValue(Target, [index]));
+            }
+
+            public override bool HasProperty(ulong index) => index < (ulong) _collection.Count;
+
+            public override void CreateDataPropertyOrThrow(ulong index, JsValue value)
+                => _target.CreateDataPropertyOrThrow(index, value);
+
+            public override void Set(ulong index, JsValue value, bool updateLength = false, bool throwOnError = true)
+                => _target[(int) index] = value;
+
+            public override void DeletePropertyOrThrow(ulong index)
+                => _target.DeletePropertyOrThrow(index);
+        }
     }
 
     /// <summary>

Jint/Runtime/Descriptors/Specialized/ReflectionDescriptor.cs

@@ -3,63 +3,81 @@
 using Jint.Runtime.Interop;
 using Jint.Runtime.Interop.Reflection;
 
-namespace Jint.Runtime.Descriptors.Specialized
+namespace Jint.Runtime.Descriptors.Specialized;
+
+internal sealed class ReflectionDescriptor : PropertyDescriptor
 {
-    internal sealed class ReflectionDescriptor : PropertyDescriptor
+    private readonly Engine _engine;
+    private readonly ReflectionAccessor _reflectionAccessor;
+    private readonly object _target;
+    private readonly string _propertyName;
+
+    private JsValue? _get;
+    private JsValue? _set;
+
+    public ReflectionDescriptor(
+        Engine engine,
+        ReflectionAccessor reflectionAccessor,
+        object target,
+        string propertyName,
+        bool enumerable)
+        : base((enumerable ? PropertyFlag.Enumerable : PropertyFlag.None) | PropertyFlag.CustomJsValue)
     {
-        private readonly Engine _engine;
-        private readonly ReflectionAccessor _reflectionAccessor;
-        private readonly object _target;
+        _flags |= PropertyFlag.NonData;
+        _engine = engine;
+        _reflectionAccessor = reflectionAccessor;
+        _target = target;
+        _propertyName = propertyName;
+    }
 
-        public ReflectionDescriptor(
-            Engine engine,
-            ReflectionAccessor reflectionAccessor,
-            object target,
-            bool enumerable)
-            : base((enumerable ? PropertyFlag.Enumerable : PropertyFlag.None) | PropertyFlag.CustomJsValue)
+    public override JsValue? Get
+    {
+        get
         {
-            _flags |= PropertyFlag.NonData;
-            _engine = engine;
-            _reflectionAccessor = reflectionAccessor;
-            _target = target;
-
-            if (reflectionAccessor.Writable && engine.Options.Interop.AllowWrite)
+            if (_reflectionAccessor.Readable)
             {
-                Set = new SetterFunction(_engine, DoSet);
+                return _get ??= new GetterFunction(_engine, DoGet);
             }
-            if (reflectionAccessor.Readable)
+
+            return null;
+        }
+    }
+
+    public override JsValue? Set
+    {
+        get
+        {
+            if (_reflectionAccessor.Writable && _engine.Options.Interop.AllowWrite)
             {
-                Get = new GetterFunction(_engine, DoGet);
+                return _set ??= new SetterFunction(_engine, DoSet);
             }
-        }
 
-        public override JsValue? Get { get; }
-        public override JsValue? Set { get; }
+            return null;
+        }
+    }
 
+    protected internal override JsValue? CustomValue
+    {
+        get => DoGet(thisObj: null);
+        set => DoSet(thisObj: null, value);
+    }
 
-        protected internal override JsValue? CustomValue
-        {
-            get => DoGet(null);
-            set => DoSet(null, value);
-        }
+    private JsValue DoGet(JsValue? thisObj)
+    {
+        var value = _reflectionAccessor.GetValue(_engine, _target, _propertyName);
+        var type = _reflectionAccessor.MemberType;
+        return JsValue.FromObjectWithType(_engine, value, type);
+    }
 
-        private JsValue DoGet(JsValue? thisObj)
+    private void DoSet(JsValue? thisObj, JsValue? v)
+    {
+        try
         {
-            var value = _reflectionAccessor.GetValue(_engine, _target);
-            var type = _reflectionAccessor.MemberType;
-            return JsValue.FromObjectWithType(_engine, value, type);
+            _reflectionAccessor.SetValue(_engine, _target, _propertyName, v!);
         }
-
-        private void DoSet(JsValue? thisObj, JsValue? v)
+        catch (TargetInvocationException exception)
         {
-            try
-            {
-                _reflectionAccessor.SetValue(_engine, _target, v!);
-            }
-            catch (TargetInvocationException exception)
-            {
-                ExceptionHelper.ThrowMeaningfulException(_engine, exception);
-            }
+            ExceptionHelper.ThrowMeaningfulException(_engine, exception);
         }
     }
 }