microsoft / ClearScript

A library for adding scripting to .NET applications. Supports V8 (Windows, Linux, macOS) and JScript/VBScript (Windows).
https://microsoft.github.io/ClearScript/
MIT License
1.77k stars 148 forks source link

Performance issue with DataRow in 7.3.4 #433

Closed rouvian closed 1 year ago

rouvian commented 2 years ago

Here is a sample script which accesses a DataRow

var ds = server.ExecuteDataSet(context.connectionString, context.queryText, context);

var dt = ds.Tables(0);
var drs = dt.Rows;

for(var i = 0; i < drs.Count; i++) {
    var dr = drs(i);
    var value = dr(0);
    logger.LogMessage(MessageType.Information, value, "");
}

The line value = dr(0) cause OperationCanceledException which is caught internally. However, this causes a big performance issue in the loop. The issue is in TypeHelpers.cs in the source code of ClearScript:

if (argIndex >= args.Length)
{
    if (!param.IsOptional && !param.HasDefaultValue)
    {
        throw new OperationCanceledException();
    }

    continue;
}

The same issue does not occur in 7.3.1.

ClearScriptLib commented 2 years ago

Hi @rouvian,

We've reproduced the performance regression and will address it in the next release.

By the way, the JavaScript syntax obj(arg) is rarely used for invoking managed indexers, but it requires fewer host calls than the more common obj.Item(arg) or obj.Item.get(arg), so it's faster.

However, you could still do better by doing the indexing in managed code:

engine.Script.getRow = new Func<DataRowCollection, int, DataRow>((drs, i) => drs[i]);
engine.Script.getCol = new Func<DataRow, int, object>((dr, i) => dr[i]);

And then, in JavaScript:

const drs = dt.Rows;
const count = drs.Count;
for (let i = 0; i < count; ++i) {
    const dr = getRow(drs, i);
    const value = getCol(dr, 0);
    // ...
}

The performance regression you discovered doesn't affect this technique, so it should yield a significant boost in all recent ClearScript versions.

Thanks for reporting this issue!

ClearScriptLib commented 1 year ago

Version 7.3.5 addressed the performance regression.