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

Defer dynamic export of type members until first access #192

Merged
merged 1 commit into from
Jan 26, 2024

Conversation

jasongin
Copy link
Member

Previously, when dynamically invoking .NET APIs from JS (as in the semantic-kernel example) loading of types within an assembly/namespace was deferred, but whenever a type was loaded, marshalling code was generated and compiled for all members of the type, and all types referenced by any of the members, cascading on to the referenced types' members and so on. This meant for a nontrivial application thousands of types and members could be processed at startup time, the vast majority of which were never actually used by the app.

This change defers the marshalling code generation/compilation for members (constructors, properties, and methods) of loaded types, until the first time each individual member is accessed. And then, there is no need to load referenced types until that time either.

This change has two major benefits:

  1. It significantly improves startup time for the dynamic invocation scenario. (The SK example runs over 2X faster, not counting the network call.)
  2. It prevents any marshalling code-gen bugs with APIs deep in the type dependency tree (unused by the application) from blocking the application startup. While there has been a lot of work to fix crashes while processing all different kinds of .NET APIs, whether they are fully supported for marshalling or not, there are probably a few edge cases still lurking.

Summary of changes:

  • In TypeExporter when exporting properties, initially define the property with getter/setter callbacks that load the actual property getter/setter and then redefine the property before calling it.
  • Enhance JSCallbackOverload to support deferred loading of overloaded constructor/method callbacks, using a Lazy<T> to invoke the loader once (which generates and compiles the marshalling code).
  • In TypeExporter, use JSCallbackOverload to defer loading of constructors and methods.
    • For non-overload constructors and methods this may have a slight perf impact since there is now an additional level of indirection. With more work it could be possible to redefine the method properties (but not the constructors). But it might not be worth the extra code.

Related diagnostic improvements:

  • Update tracing related to exporting type members.
  • Rename existing TRACE and DEBUG environment variables to use a consistent NODE_API_ prefix, and update README-dev.md.
  • Make the new delayed export behavior conditional on a NODE_API_DELAYLOAD variable, enabled by default. (I'm not sure it's needed, but it's possible it could cause some unforeseen problems.)

@jasongin jasongin requested a review from vmoroz January 26, 2024 19:26
Copy link
Member

@vmoroz vmoroz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

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

Successfully merging this pull request may close these issues.

2 participants