-
Notifications
You must be signed in to change notification settings - Fork 150
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
Non-allocating fast path for compiling code #615
base: master
Are you sure you want to change the base?
Conversation
@@ -224,17 +224,36 @@ public static UIntPtr GetDigest(this string code) | |||
return (UIntPtr.Size == 4) ? (UIntPtr)code.GetDigestAsUInt32() : (UIntPtr)code.GetDigestAsUInt64(); | |||
} | |||
|
|||
public static UIntPtr GetDigest(IntPtr code, int length) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would prefer to use Span<byte>
, but it is not available in .NET 4.5.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ClearScript 7.5 will use Span
, at least internally. To do that on .NET Framework 4.5, it'll bring in System.Memory 4.5.5.
Hi @AnsisMalins, Thanks for posting your PR. If we understand it correctly, your change avoids the allocation of a managed string to hold the code to be compiled. If that's correct, do you have an estimate of the performance impact of this change relative to the cost of script compilation? It's difficult to believe that it would be significant. BTW, we're currently working on ClearScript 7.5, which we're hoping will greatly reduce managed heap traffic by eliminating heap-allocated lambdas and scopes, among other things.
ClearScript includes many public classes that the client must instantiate. Isn't that a problem for burst-compiled C#? Or were you thinking of bypassing the managed layers completely and interfacing directly with ClearScript's native assemblies? Thanks! |
How far out is ClearScript 7.5? If more than a month, can I get at it early? |
Hi @AnsisMalins,
In addition to extensive changes intended to reduce managed heap pressure, ClearScript 7.5 will include a major new API for exposing host objects to V8 for fast access. Early results indicate a roughly 70% reduction in method call overhead and allocation rate for objects exposed via the new API. Unfortunately, we're still at least a couple of months away.
Once most of the changes are in place and the new API is feature-complete, we could post the pre-release code in a branch, if that would help. We just aren't quite there at the moment. Thanks! |
It would help massively to reduce the overhead we face if you could share the pre-release code for evaluation as soon as it is available! |
Hi @m3taphysics,
Sure, we'll post the code changes to a branch as soon as they're in a release candidate state. If there's sufficient interest, we'll put up pre-release NuGet packages as well. However, please understand that the full benefits will apply only to host resources exposed via a new API. For better or worse, the most straightforward and "natural" .NET code does a lot of allocation, so the new API goes pretty far off the beaten track. Nevertheless, existing ClearScript hosts should see a modest uplift from the reduction or elimination of things like heap-allocated scopes, capturing lambdas, and parameter arrays. Cheers! |
I'm showing you this sketch to give you a change to influence my solution to ensure that it is upstreamable. The goal is to get rid of as many managed allocations as possible when compiling and running JavaScript code. My particular use case involves retrieving a native char array from Unity (the game engine) and piping it straight into ClearScript.
Being able to call ClearScript from bursted C# (no reference types allowed) is a stretch goal.