PropertyDescriptor improvements

[viruscamp]

PropertyDescriptor improvements
add PropertyFlag.NonData
now PropertyDescriptors for CLR Field, Property and Indexer are accessor property descriptors.

By treating CLR Property as accessor property, it will reduce the time of call to Property Getter.

[viruscamp]

Here is a test, which fail in the main branch current:

    class TestClass
    {
        public static readonly TestClass Instance = new TestClass();

        private int x = 1;
        public int PropertySideEffect => x++;
    }

    [Fact]
    public void ClrPropertySideEffect()
    {
        _engine.SetValue("testClass", TestClass.Instance);
        _engine.Execute("""
            const handler = {
              get(target, property, receiver) {
                return 2; // the same value to second call, make data property check happy
              }
            };
            const p = new Proxy(testClass, handler);
        """);

        Assert.Equal(1, TestClass.Instance.PropertySideEffect); // first call to PropertySideEffect
        Assert.Equal(2, _engine.Evaluate("p.PropertySideEffect").AsInteger()); // no call to PropertySideEffect
        Assert.Equal(2, TestClass.Instance.PropertySideEffect); // second call to PropertySideEffect
    }

[viruscamp]

IMO, PropertyFlag.CustomJsValue and PropertyFlag.NonData should not be flags, they should be a readonly Property.

class PropertyDescriptor {
  public bool CustomJsValue => false;
  public bool NonData=> false;
}

class ReflectionDescriptor {
  public bool CustomJsValue => true;
  public bool NonData=> true;
}

[lahma]

So if this is about performance (and to validate there's not regressions), could you show show some benchmark results before/after?

[viruscamp]

So if this is about performance (and to validate there's not regressions), could you show show some benchmark results before/after?

It's not about performance mainly. It changed the behavior by making CLR Property and Indexer to be js accessor descriptor instead of data descriptor. So, the PropertyDescriptor.IsDataDescriptor method won't call the CLR Property/Indexer Getter to avoid the side effect in the CLR Property/Indexer Getter.

I think PropertyDescriptor.IsDataDescriptor is mainly used in JsProxy get/set, it may acclerate them for GetSetPropertyDescriptor and ReflectionDescriptor a little.
In a data descriptor(the original PropertyDescriptor), I added an additional check to NonData flag.

It's maybe a problem that CLR Field is also js accessor descriptor now.

[viruscamp]

cd Jint.Tests.Test262
LC_ALL=C dotnet test

property-descriptor-nondata 64ee20f

Passed!  - Failed:     0, Passed: 56295, Skipped: 11583, Total: 67878, Duration: 1 m 31 s - Jint.Tests.Test262.dll (net6.0)
Passed!  - Failed:     0, Passed: 56295, Skipped: 11583, Total: 67878, Duration: 1 m 30 s - Jint.Tests.Test262.dll (net6.0)
Passed!  - Failed:     0, Passed: 56295, Skipped: 11583, Total: 67878, Duration: 1 m 31 s - Jint.Tests.Test262.dll (net6.0)

main 8ebdc34

Passed!  - Failed:     0, Passed: 56295, Skipped: 11583, Total: 67878, Duration: 1 m 31 s - Jint.Tests.Test262.dll (net6.0)
Passed!  - Failed:     0, Passed: 56295, Skipped: 11583, Total: 67878, Duration: 1 m 30 s - Jint.Tests.Test262.dll (net6.0)
Passed!  - Failed:     0, Passed: 56295, Skipped: 11583, Total: 67878, Duration: 1 m 30 s - Jint.Tests.Test262.dll (net6.0)

[lahma]

Generally we can't tell much about performance by reading test run timings, there's a lot more going on there. But this seems quite safe and covered with tests, thank you for your contribution.