Skip to content

perf: cache native dispatch invokers#4518

Open
Jim8y wants to merge 5 commits intomaster-n3from
fix-native-dispatch-delegates
Open

perf: cache native dispatch invokers#4518
Jim8y wants to merge 5 commits intomaster-n3from
fix-native-dispatch-delegates

Conversation

@Jim8y
Copy link
Copy Markdown
Contributor

@Jim8y Jim8y commented Mar 29, 2026

Description

This change removes the reflection-based hot path from native contract dispatch on master-n3.

InteropDescriptor and ContractMethodMetadata now build cached compiled invokers once, and the runtime dispatch loops in ApplicationEngine.OnSysCall(...) and NativeContract.Invoke(...) reuse pooled object?[] buffers instead of allocating fresh arrays and lists on every invocation.

The optimization keeps the previous behavior intact, including TargetInvocationException wrapping semantics, and adds unit coverage for the new invoker layer.

Change Log

  • Add delegate InteropInvoker
  • Add delegate NativeMethodInvoker
  • Update class InteropDescriptor
  • Update class ContractMethodMetadata
  • Update method ApplicationEngine.OnSysCall(...)
  • Update method NativeContract.Invoke(...)
  • Add file tests/Neo.UnitTests/SmartContract/UT_InteropDescriptor.cs
  • Update file tests/Neo.UnitTests/SmartContract/Native/UT_ContractMethodAttribute.cs

Fixes # (issue) - no GitHub issue is linked for this profiling finding.

Type of change

  • Optimization (the change is only an optimization)
  • Style (the change is only a code style for better maintenance or standard purpose)
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

  • Unit Testing
  • Run Application
  • Local Computer Tests
  • No Testing

Verification commands:

  • dotnet test tests/Neo.UnitTests/Neo.UnitTests.csproj --filter UT_InteropDescriptor --logger "console;verbosity=minimal"
  • dotnet test tests/Neo.UnitTests/Neo.UnitTests.csproj --filter UT_ContractMethodAttribute --logger "console;verbosity=minimal"
  • dotnet test tests/Neo.UnitTests/Neo.UnitTests.csproj --logger "console;verbosity=minimal"

Benchmark results from an external harness that executes GAS.totalSupply() through the real native dispatch path (200 executions x 1000 calls = 200000 native dispatches):

  • base (origin/master-n3): elapsed_ms=2457.30, allocated_bytes=1001359680
  • fixed (fix-native-dispatch-delegates): elapsed_ms=2240.74, allocated_bytes=964599232

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

@github-actions github-actions bot added the N3 label Mar 29, 2026
@codecov
Copy link
Copy Markdown

codecov bot commented Mar 29, 2026

Codecov Report

❌ Patch coverage is 99.02913% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 78.89%. Comparing base (f7bc967) to head (6a31470).

Files with missing lines Patch % Lines
src/Neo/SmartContract/InteropDescriptor.cs 96.15% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@              Coverage Diff              @@
##           master-n3    #4518      +/-   ##
=============================================
+ Coverage      78.76%   78.89%   +0.13%     
=============================================
  Files            236      236              
  Lines          16283    16365      +82     
  Branches        2361     2373      +12     
=============================================
+ Hits           12825    12911      +86     
+ Misses          2708     2700       -8     
- Partials         750      754       +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Jim8y Jim8y marked this pull request as draft March 29, 2026 14:11
@Jim8y Jim8y marked this pull request as ready for review March 29, 2026 14:25
Comment on lines +489 to +492
for (int i = 0; i < parameterCount; i++)
{
context.EvaluationStack.Pop();
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Wouldn't it be better to move this into the finally block so that if a failure occurs the parameters don't remain on the stack?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

but this can not be "fixed" as its the exsiting logic, this pr shall not change any exsiting logic

ajara87
ajara87 previously approved these changes Mar 29, 2026
Copy link
Copy Markdown
Member

@shargon shargon left a comment

Choose a reason for hiding this comment

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

We need benchmarks, maybe the expression is slower

@shargon shargon added the Under Investigation The problem mentioned, or possible improvement, is under investigation label Mar 30, 2026
@Jim8y
Copy link
Copy Markdown
Contributor Author

Jim8y commented Mar 31, 2026

Environment

  • BenchmarkDotNet v0.15.8
  • .NET SDK 10.0.100
  • Runtime: .NET 10.0.0
  • OS: Linux Ubuntu 24.04.4 LTS
  • CPU: Intel Core Ultra 9 285K 3.69GHz
  • Job: ShortRun (IterationCount=3, LaunchCount=1, WarmupCount=3)

Summary

Method master-n3 Mean PR Mean Mean Delta master-n3 Alloc PR Alloc Alloc Delta
Syscall_NoArgs 71.32 ns 65.82 ns -5.50 ns (-7.71%) 118 B 94 B
-24 B (-20.34%)
Syscall_OneArg 111.17 ns 111.92 ns +0.75 ns (+0.67%) 182 B 150 B
-32 B (-17.58%)
Native_NoArgs 130.27 ns 125.26 ns -5.01 ns (-3.85%) 128 B 96 B
-32 B (-25.00%)
Native_OneArg 193.77 ns 184.15 ns -9.62 ns (-4.96%) 272 B 152 B
-120 B (-44.12%)

@shargon
Copy link
Copy Markdown
Member

shargon commented Mar 31, 2026

Syscall_OneArg how can be less percentage if it's possitive?

@Jim8y
Copy link
Copy Markdown
Contributor Author

Jim8y commented Mar 31, 2026

Syscall_OneArg how can be less percentage if it's possitive?

its not, but i think 0.xx% should be considered a reasonable margin of error?

@erikzhang
Copy link
Copy Markdown
Member

Have we started optimizing operations at the nanosecond level?

@Jim8y
Copy link
Copy Markdown
Contributor Author

Jim8y commented Mar 31, 2026

Have we started optimizing operations at the nanosecond level?

I am basically still focusing on optimizing the performance of resync the whole ledger, which will cost hours even days, native contract invoking is one of the hotpath of that. Another hotpath is persistance.

Comment on lines +105 to +111
if (i == 0 && NeedApplicationEngine)
{
callParameters[i] = Expression.Convert(engine, handlerParameters[i].ParameterType);
continue;
}

if (i == 0 && NeedSnapshot)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Merge these two if?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Agree

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

N3 Under Investigation The problem mentioned, or possible improvement, is under investigation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants