Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support HostItemFlags.DirectAccess for .NET objects #161

Closed
Nir1812 opened this issue Jan 29, 2020 · 12 comments
Closed

Support HostItemFlags.DirectAccess for .NET objects #161

Nir1812 opened this issue Jan 29, 2020 · 12 comments
Assignees

Comments

@Nir1812
Copy link

Nir1812 commented Jan 29, 2020

I have spent 3 days converting my application to successfully use ClearScript instead of the old MSScriptControl (which is limited to 32 bit).
Unfortunately, after sending my application to the QA team, I have found out that the ClearScript engine is MUCH slower than the MSScriptControl!

Take a look at the profiler output, which shows the amount of CPU it takes for all the reflection calls.
Is there any way to bypass these checks?

image

And for comparison: Same application, same script but using MSScriptControl:
image

The script code that runs on both engines use a while loop over a recordset with 3000 records, and calls a method in a host object for each record.

@ClearScriptLib
Copy link
Collaborator

ClearScriptLib commented Jan 29, 2020

Hi @Nir1812,

The script control limits itself to COM, whereas ClearScript concerns itself with .NET, whose type system is significantly richer (e.g., it supports statics, generics, constructors, extension methods, etc.).

Is there any way to bypass these checks?

For Windows script engines (JScript and VBScript), ClearScript supports HostItemFlags.DirectAccess, which allows the engine to interact with a host object without ClearScript's involvement.

This feature is currently restricted to COM objects, but we've just confirmed that it also works for any .NET object as long as it's ComVisible. We'll remove the COM restriction in the next ClearScript release.

Thanks!

@ClearScriptLib ClearScriptLib changed the title Much slower than using 'Microsoft Script Control' Support HostItemFlags.DirectAccess for .NET objects Jan 29, 2020
@Nir1812
Copy link
Author

Nir1812 commented Jan 29, 2020

Thank you for the answer.
If my host objects are COM visible (I was using MS ScriptControl until recently), your suggestion is to use the DirectAccess flag and add the objects with AddHostObject right?
Or should I use AddCOMObject? (Note that my objects are not really COM... They are just COM-Visible)

@ClearScriptLib
Copy link
Collaborator

Hi @Nir1812,

AddHostObject with HostItemFlags.DirectAccess is the correct approach. Unfortunately it won't work today, as ClearScript supports direct access only for COM objects. The next release will support it for COM-visible objects as well.

In the meantime, if you're building ClearScript yourself (we've noticed your fork), you can simply remove the IsCOMObject check here. That should work for you as a temporary solution.

Good luck!

@Nir1812
Copy link
Author

Nir1812 commented Jan 29, 2020

Thank you for the direction.
I have made the change, built the DLL and used the Direct flag for my objects, and it worked!
I will use my build for the time being, and will wait for the next release.
Any expected date for it?

ClearScriptLib added a commit that referenced this issue Apr 13, 2020
…Hub Issue #160); fixed 64-bit V8 initialization on Azure App Service (GitHub Issue #166); enabled DirectAccess for ComVisible .NET objects (GitHub Issue #161); added ScriptEngine.ExposeHostObjectStaticMembers (GitHub Issue #152); added ScriptEngine.UndefinedImportValue and made Undefined.Value public (GitHub Issue #154); enhanced ExtendedHostFunctions.typeLibEnums to pull in external enumerations; added thread affinity enforcement in WindowsScriptEngine.Dispose; eliminated KeyNotFoundException when checking for system documents (GitHub Issue #169); enabled the use of the application's root folder as a backup for its local data folder (GitHub Issue #171); reduced minimum CPU profile and heap size sampling intervals to 125 ms; updated deployment and API documentation, resolving GitHub Issues #158 and #159. Tested with V8 8.1.307.28.
@ClearScriptLib
Copy link
Collaborator

Version 6.0.1 expands DirectAccess support to all ComVisible objects.

@Brain2000
Copy link

Brain2000 commented Aug 5, 2021

I'm having a slow issue as well (using the V8 engine with ClearScript version 7.1.5).

It seems like the speed is also related to the number of properties/methods on the .net object. I set up the same readonly property on two objects, one had just 1 property and the other one is a giant library we use with ... 50,000 properties and methods? (it's a rather large library)...

The library with just one property can run about 10,000 marshals per second, where the giant library could only run about 1,000 marshals per second, given the same property.

I could not find much of a difference in speed with the DirectAccess flag enabled or not. Both .NET classes have the ComVisible(true) attribute.

Something is definitely amiss here. Any suggestions?

@ClearScriptLib
Copy link
Collaborator

Hi @Brain2000,

Sorry, just trying to make sure we understand, you’re adding this property to an object that has 50,000 others?

Thanks!

@Brain2000
Copy link

I believe I have narrowed it down to the class size, though perhaps it could be something else. Here is the test in more detail:

The two classes:

[ComVisible(true)]
public partial class BigLib  <----- this is the existing class with over 50,000 methods and properties
{
    public string speedtest
    {
        get
        {
            return "test";
        }
    }
}

[ComVisible(true)]
public class tiny   <----- this is the class that will only contain the one property
{
    public string speedtest
    {
        get
        {
            return "test";
        }
    }
}

The setup:

var JsRuntime = new V8ScriptEngine();
JsRuntime.AddHostObject("BigLib", HostItemFlags.DirectAccess, new BigLib());
JsRuntime.AddHostObject("tiny", HostItemFlags.DirectAccess, new tiny());
JsRuntime.Execute("BigLib.Log('big start');
  for (let i = 0; i < 20000; i++) {
    var x = BigLib.speedtest;
  }
  BigLib.Log('big end');
  
  BigLib.Log('tiny start');
  for (let i = 0; i < 20000; i++) {
    var x = tiny.speedtest;
  }
  BigLib.Log('tiny end');");

The output:

8/6/2021 12:55:36 PM - big start
8/6/2021 12:55:43 PM - big end

8/6/2021 12:55:43 PM - tiny start
8/6/2021 12:55:44 PM - tiny end

Note that the big class took around 7 seconds and the tiny class took around 1 second, to retrieve the speedtest property 20,000 times.

@Brain2000
Copy link

Running the CPU diagnostics against the process yielded this as the function taking all the time:

Function Name	Total CPU [unit, %]	Self CPU [unit, %]	Module	Category
[External Call] Microsoft.ClearScript.V8.SplitProxy.V8SplitProxyManaged.GetHostObjectNamedPropertyWithCacheability(int, Ptr, Ptr, ref bool)	8260 (75.91%)	8212 (75.46%)	Multiple modules	IO | JIT | Kernel

@Brain2000
Copy link

Brain2000 commented Aug 6, 2021

It looks like it is using reflection to retrieve all members, and then enumerates through them all using the Linq "FirstOrDefault" to find the property (i.e. in this file: https://github.com/microsoft/ClearScript/blob/041b5b4ca5395c5ba63ef7a4f3215d433074e941/ClearScript/Util/COM/DispatchWrappers.cs)

What do you think of putting all the members into a dictionary? Perhaps something like this in DispatchWrapers.cs:
(note ** = added/modified line of code)

**      private Dictionary<string, DispatchMember> members;

        public DynamicDispatchWrapper(HostItem hostItem, IDispatch dispatch)
        {
            this.hostItem = hostItem;
            this.dispatch = dispatch;
**          this.members = dispatch.GetMembers().ToDictionary(item => item.Name, item => item);
        }

        public object GetProperty(string name, out bool isCacheable, params object[] args)
        {
            isCacheable = false;

            DispatchMember member = null;
            if (args.Length < 1)
            {
                // some objects crash on attempt to retrieve a method as a property

**              if (GetMembers().TryGetValue(name, out member) && member.DispatchFlags == DispatchFlags.Method)
                {
                    return new HostMethod(hostItem, name);
                }
            }

            try
            {
                var result = dispatch.GetProperty(name, args);
                return result;
            }
            catch (Exception)
            {
                if (args.Length < 1)
                {
                    if (member == null)
                    {
**                      GetMembers().TryGetValue(name, out member);
                    }

                    if ((member != null) && !member.DispatchFlags.HasFlag(DispatchFlags.Method))
                    {
                        return new HostIndexedProperty(hostItem, name);
                    }

                    return new HostMethod(hostItem, name);
                }

                throw;
            }
        }

@ClearScriptLib
Copy link
Collaborator

Hi @Brain2000.

We've confirmed the slowdown and are tracking it as Issue #281.

ClearScript supports scenarios that necessitate enumerating the entire property set, but we believe it should be possible to optimize that away for simple cases that don't involve indexed or renamed properties. In the meantime, consider exposing a wrapper that provides access to a subset of the main object's properties.

To be honest, we've never tested with a type that comes anywhere near 50,000 properties. Our first attempt to reproduce your issue failed, as no current .NET runtime seems to support 50,000 get-set properties. We got it working by converting the properties to get-only.

By the way, V8ScriptEngine doesn't use COM to interact with host objects, so DirectAccess has no effect.

Thanks for reporting this issue!

@Brain2000
Copy link

Brain2000 commented Aug 9, 2021

Thank you! I will check Issue #281.

Also, you are correct. This is a large foundation class that our dev department has been adding to for the last 15 years. I thought it had closer to 50,000 properties/methods, but after double checking the class itself has less than 10,000. The rest are in subclasses, which I should not have been counting.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants