Memory and peformance optimizations in interop.#2382
Merged
Conversation
Collaborator
|
Nice, thanks for bringing the PR. In these cases would love to see benchmarks added to benchmark project that simulate the software you are tweaking - would show the actual benefits. |
Contributor
Author
|
Benchmark code: public class MemoryManager
{
static byte[] _buffer = new byte[1000];
public byte[] get_bytes(int start, int length) => _buffer.AsSpan(start, length).ToArray();
public void fill(int start, byte[] data) => data.AsSpan().CopyTo(_buffer.AsSpan(start, data.Length));
}
public class Uint8Converter : IObjectConverter
{
public bool TryConvert(Engine engine, object value, [NotNullWhen(true)] out JsValue? result)
{
if (value is byte[] byteArray)
{
result = engine.Intrinsics.Uint8Array.Construct((ReadOnlySpan<byte>)byteArray);
return true;
}
result = null;
return false;
}
}
[MemoryDiagnoser]
public class ByteArrayInteropBenchmark
{
private Engine _stockEngine = new Engine(new Options()).SetValue("__memory", new MemoryManager());
private Engine _uint8Engine;
public ByteArrayInteropBenchmark()
{
var uint8Options = new Options();
uint8Options.Interop.ObjectConverters.Add(new Uint8Converter());
_uint8Engine = new Engine(uint8Options).SetValue("__memory", new MemoryManager());
}
[Benchmark]
public void Stock() =>
_stockEngine.Execute("""
for(let i=0; i<10; i++) {
var bytes = __memory.get_bytes(100*i, 10);
__memory.fill(100*i, bytes);
}
""");
[Benchmark]
public void UInt8() =>
_uint8Engine.Execute("""
for(let i=0; i<10; i++) {
var bytes = __memory.get_bytes(100*i, 10);
__memory.fill(100*i, bytes);
}
""");
}Results:
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR aims to reduce GC pressure and improve interop performance in Jint by minimizing allocations during CLR invocation and by optimizing Uint8Array creation/conversion paths.
Changes:
- Cache
JsValue.ToObject()results inMethodInfoFunction.Callto avoid repeated conversions during overload resolution. - Optimize
Uint8ArrayConstructor.Construct(ReadOnlySpan<byte>)by writing span data directly to the underlying buffer. - Add a fast-path in
JsTypedArray.ToNativeArray<T>()forUint8Array→byte[]conversions.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| Jint/Runtime/Interop/MethodInfoFunction.cs | Attempts to reduce repeated ToObject() conversions during interop method argument processing. |
| Jint/Native/TypedArray/TypedArrayConstructor.Uint8Array.cs | Changes how Uint8Array is constructed from ReadOnlySpan<byte> to reduce per-element work. |
| Jint/Native/JsTypedArray.cs | Adds a fast-path intended to avoid allocating/copying when converting Uint8Array to byte[]. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
StringEpsilon
marked this pull request as ready for review
April 6, 2026 11:44
I'll rebase this commit later.
StringEpsilon
force-pushed
the
Optimizations
branch
from
April 6, 2026 12:21
1f3074f to
d0e6f5f
Compare
lahma
enabled auto-merge (squash)
April 6, 2026 12:41
This was referenced Apr 13, 2026
This was referenced Jun 8, 2026
This was referenced Jun 29, 2026
This was referenced Jul 7, 2026
This was referenced Jul 13, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Disclaimer: I don't know enough about jint to be sure that these optimizations don't cause any side effects, hence this is a draft. But all the tests pass on my machine so I think it's working.
Background: I was investigating the performance of an application I help maintain and noticed that Jint allocates a LOT of
doubles andbytes, which greatly increases the GC pressure we experience as we execute JS very frequently on the main loop.The change in
MethodInfoFunction.cs(commit "MethodInfoFunction.Call: Cache argument object") might be a performance regression for some specific cases, but just about cuts in half the allocations fordoubleI see with dotMemory.After that, I experimented with using a custom type converter to use a uint8array for
byte[]instead of a regular array ofdoubles. That turned out to be slower because of how theToNativeArray<T>()is implemented. Adding a fast path for uint8 arrays that simply returns the internal buffer array greatly reduces allocations and negates the performance penalty I saw for using the custom converter.Related is the change to Uint8ArrayConstructor: The previous implementation seems to be a lot slower (and with more allocations) than simply creating a copy of the ROS.
The latter 2 changes may cause some breakage, as I'm not sure that they work for all uint8 arrays and the respective construction cases. But I hope that my hasty changes can lead to some proper optimizations in these areas.