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

Questions about module loading #169

Closed
JohnLudlow opened this issue Mar 5, 2020 · 4 comments
Closed

Questions about module loading #169

JohnLudlow opened this issue Mar 5, 2020 · 4 comments

Comments

@JohnLudlow
Copy link

JohnLudlow commented Mar 5, 2020

Hi,

I've been experimenting with module loading, in particular loading lodash (no particular reason for that module - it's just a good test).

This works:

using (var engine = new V8ScriptEngine(V8ScriptEngineFlags.EnableDynamicModuleImports))
{
    engine.DocumentSettings.AccessFlags |= DocumentAccessFlags.EnableFileLoading;
    var result = engine.Evaluate(new DocumentInfo { Category = ModuleCategory.Standard },
        @"import * as _ from 'd:\\project\\modules\\lodash-es-4.17.15\\package\\lodash.js';
        _.capitalize('tom')"
    );
}

This takes about 40s to run, and I see a lot of KeyNotFoundExceptions from DocumentSettings.FindSystemDocument. Could there be a check there to see whether the document is already in the map?

Now, we're interested in being able to import modules and allowing them to be used in non-module scripts and snippets:

using (var engine = new V8ScriptEngine(V8ScriptEngineFlags.EnableDynamicModuleImports))
{
    engine.DocumentSettings.AccessFlags |= DocumentAccessFlags.EnableFileLoading;
    var result = engine.Evaluate(new DocumentInfo { Category = ModuleCategory.Standard },
        @"import * as _ from 'd:\\project\\modules\\lodash-4.17.15\\package\\lodash.js';
    ");

    var result2 = engine.Evaluate("_.capitalize('tom')");
}

This also works, and it's quick - this took less than a tenth of a second to run in my test and only threw one KeyNotFoundException. But an eagle-eyed observer might notice an oddity - in one example, I use the lodash-es module (the ECMAScript standard module) and in one their core module (from https://www.npmjs.com/package/lodash). I can't seem to make one variant work for both examples.

  • Using lodash, import and use in the same Evaluate document:
    var result = engine.Evaluate(new DocumentInfo { Category = ModuleCategory.Standard },
        @"import * as _ from 'd:\\project\\modules\\lodash-4.17.15\\package\\lodash.js';
          _.capitalize('tom');
    ");

Result: _ is an empty object, capitalize is not a function.

  • Using lodash-es, import and use in the same Evaluate document:
    var result = engine.Evaluate(new DocumentInfo { Category = ModuleCategory.Standard },
        @"import * as _ from 'd:\\project\\modules\\lodash-es-4.17.15\\package\\lodash.js';
          _.capitalize('tom');
    ");

Result: Works, a bit slow, throws KeyNotFound a lot.

  • Using lodash, import and use in a separate Evaluate document
    var result = engine.Evaluate(new DocumentInfo { Category = ModuleCategory.Standard },
        @"import * as _ from 'd:\\project\\modules\\lodash-4.17.15\\package\\lodash.js';
    ");

    var result2 = engine.Evaluate("_.capitalize('tom')");

Result: Works really nicely

  • Using lodash-es, import and use in a separate
    var result = engine.Evaluate(new DocumentInfo { Category = ModuleCategory.Standard },
        @"import * as _ from 'd:\\project\\modules\\lodash-es-4.17.15\\package\\lodash.js';
    ");

    var result2 = engine.Evaluate("_.capitalize('tom')");

Result: Is a bit slow, throws KeyNotFound a lot, then, throws a ScriptEngineException ReferenceError: _ is not defined on the second Evaluate.

❓ What am I missing?

I've also been playing with system documents which give similar results.

@ClearScriptLib
Copy link
Collaborator

Hi @JohnLudlow,

First, we can't reproduce the poor performance you're seeing. In our testing, lodash-es runs in a second or two outside any debugger.

Second, we agree that the KeyNotFoundException issue can be annoying in the debugger despite providing the correct behavior. We'll eliminate the exception in the next ClearScript release. Thanks for reporting it!

When it comes to performance, it makes sense that lodash-es runs significantly slower than the non-module version, as the former loads over 300 modules from disk, whereas the latter is a single-file concatenation. Still, it shouldn't take very long unless the disk is very slow.

As for behavior, we believe that what you're seeing is all correct.

When you use import * as _ with lodash-es, the exports are stored in a module-local variable named _ that isn't accessible outside the module.

When you import the non-module lodash the same way, something interesting happens. The script creates a global variable named _ and puts its exports there, but in the calling module that variable is apparently hidden by the module-local variable created by the import statement, which remains empty since lodash.js has no module exports. Outside the module, the global _ becomes visible.

To use the Lodash exports in both places, you can do something like this:

engine.AddHostType(typeof(Console));
engine.Evaluate(new DocumentInfo { Category = ModuleCategory.Standard }, @"
    import * as _ from 'e:\\node_modules\\lodash-es\\lodash.js';
    Console.WriteLine(_.capitalize('tom'));
    Function('return this;')()._ = _; // store imports in global property
");
engine.Evaluate("Console.WriteLine(_.capitalize('jerry'))");

... or something like this:

engine.AddHostType(typeof(Console));
engine.Evaluate(new DocumentInfo { Category = ModuleCategory.Standard }, @"
    import 'e:\\node_modules\\lodash\\lodash.js'; // side effect import
    Console.WriteLine(_.capitalize('tom'));
");
engine.Evaluate("Console.WriteLine(_.capitalize('jerry'))");

You can also export the Lodash imports from your module, if that's something that makes sense in your application.

Good luck!

@JohnLudlow
Copy link
Author

Hi @ClearScriptLib,

Thanks for your reply.

I should have mentioned that the ~40s I noted was in debug. In release, it took about 1-3s to load lodash-es, which is reasonable an in line with your observation - were you running in debug or release? I can provide my test project if that would help, but I think the exceptions would have been a contributing factor and eliminating those would probably solve the debug performance issue I was seeing as well.

I wasn't aware that side effect importing would work. That's probably the best option for most of what we want to do.

Thanks also for the explanation for why importing the non-module lodash didn't do what I expect because that was slightly odd.

@ClearScriptLib
Copy link
Collaborator

In release, it took about 1-3s to load lodash-es, which is reasonable an in line with your observation - were you running in debug or release?

Those numbers are consistent with our tests, for which we used a release build. Note that the performance gap here isn't just due to disk I/O but also over 300 invocations of V8's script parser, each of which has its own overhead and global optimization impact.

We'll keep this issue open to track the KeyNotFoundException fix. Thanks again!

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 eliminates the troublesome exceptions.

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

2 participants