Some of VM benchmark scripts are improperly constructed

[AnnaShaleva]

Describe the bug Some of benchmarks added in https://github.com/neo-project/neo/pull/3512 result in FAULTed VM. It happens because these benchmarks are old, they use NEWBUFFER with too large argument:

// PUSHINT32 1048575
// NEWBUFFER

Whereas since https://github.com/neo-project/neo-vm/pull/514 VM limit for stackitem size is 2*maxUint16. The results corresponding with faulty benchmarks posted in https://github.com/neo-project/neo/pull/3512#issuecomment-2399359076 are invalid.

The following benchmarks must be rewritten and fixed:

• NeoIssue2723
• PoC_NewBuffer
• PoC_Cat
• PoC_Left
• PoC_Right
• PoC_Substr
• PoC_MemCpy

To Reproduce

1. Take the following code that runs benchmarks: https://github.com/neo-project/neo/blob/5d2d8daa73f2609abe489456e22ba375ebb11cf9/benchmarks/Neo.VM.Benchmarks/Benchmarks.POC.cs#L387-L395

2. Replace Debug.Assert(engine.State == VMState.HALT); statement with the following:

           if (engine.State != VMState.HALT)
           {
               throw new Exception(engine.State.ToString());
           }

3. See failing benchmarks, for example:

   
   // Found 1 benchmarks:
   //   Benchmarks_Correct.PoC_Cat: DefaultJob

// ** // Benchmark: Benchmarks_Correct.PoC_Cat: DefaultJob // Execute // Launch: 1 / 1 // Execute: dotnet 7443c395-9aef-4a0e-806f-9e725e575501.dll --anonymousPipes 109 110 --benchmarkName Neo.VM.Benchmark.Benchmarks_Correct.PoC_Cat --job Default --benchmarkId 0 in /home/anna/Documents/GitProjects/neo-project/neo/benchmarks/Neo.VM.Benchmarks/bin/Release/net8.0/7443c395-9aef-4a0e-806f-9e725e575501/bin/Release/net8.0 // Failed to set up high priority (Permission denied). In order to run benchmarks with high priority, make sure you have the right permissions. // BeforeAnythingElse

// Benchmark Process Environment Information: // BenchmarkDotNet v0.13.12 // Runtime=.NET 8.0.3 (8.0.324.11423), X64 RyuJIT AVX2 // GC=Concurrent Workstation // HardwareIntrinsics=AVX2,AES,BMI1,BMI2,FMA,LZCNT,PCLMUL,POPCNT VectorSize=256 // Job: DefaultJob

OverheadJitting 1: 1 op, 255355.00 ns, 255.3550 us/op

System.Reflection.TargetInvocationException: Exception has been thrown by the target of an invocation. ---> System.Exception: FAULT at Neo.VM.Benchmark.Benchmarks_Correct.Run(String poc) in /home/anna/Documents/GitProjects/neo-project/neo/benchmarks/Neo.VM.Benchmarks/Benchmarks.Correct.cs:line 52 at Neo.VM.Benchmark.Benchmarks_Correct.PoC_Cat() in /home/anna/Documents/GitProjects/neo-project/neo/benchmarks/Neo.VM.Benchmarks/Benchmarks.Correct.cs:line 40 at BenchmarkDotNet.Autogenerated.Runnable_0.WorkloadActionNoUnroll(Int64 invokeCount) in /home/anna/Documents/GitProjects/neo-project/neo/benchmarks/Neo.VM.Benchmarks/bin/Release/net8.0/7443c395-9aef-4a0e-806f-9e725e575501/7443c395-9aef-4a0e-806f-9e725e575501.notcs:line 311 at BenchmarkDotNet.Engines.Engine.RunIteration(IterationData data) at BenchmarkDotNet.Engines.EngineFactory.Jit(Engine engine, Int32 jitIndex, Int32 invokeCount, Int32 unrollFactor) at BenchmarkDotNet.Engines.EngineFactory.CreateReadyToRun(EngineParameters engineParameters) at BenchmarkDotNet.Autogenerated.Runnable_0.Run(IHost host, String benchmarkName) in /home/anna/Documents/GitProjects/neo-project/neo/benchmarks/Neo.VM.Benchmarks/bin/Release/net8.0/7443c395-9aef-4a0e-806f-9e725e575501/7443c395-9aef-4a0e-806f-9e725e575501.notcs:line 176 at System.RuntimeMethodHandle.InvokeMethod(Object target, Void** arguments, Signature sig, Boolean isConstructor) at System.Reflection.MethodBaseInvoker.InvokeDirectByRefWithFewArgs(Object obj, Span1 copyOfArgs, BindingFlags invokeAttr) --- End of inner exception stack trace --- at System.Reflection.MethodBaseInvoker.InvokeDirectByRefWithFewArgs(Object obj, Span1 copyOfArgs, BindingFlags invokeAttr) at System.Reflection.MethodBaseInvoker.InvokeWithFewArgs(Object obj, BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture) at BenchmarkDotNet.Autogenerated.UniqueProgramName.AfterAssemblyLoadingAttached(String[] args) in /home/anna/Documents/GitProjects/neo-project/neo/benchmarks/Neo.VM.Benchmarks/bin/Release/net8.0/7443c395-9aef-4a0e-806f-9e725e575501/7443c395-9aef-4a0e-806f-9e725e575501.notcs:line 57

**Expected behavior**
All benchmarks must not use `NEWBUFFER` with invalid (too large) argument. All benchmarks should end in HALT VM state except those benchmarks that use endless cycles. Benchmarks with endless cycles must be checked against resulting execution error: the error must be caused by the end of GAS.

**Platform:**
 - Version: 52393be239bce163c14ff09b15dbabd4a6bc3240

[Jim8y]

All benchmarks should end in HALT VM state

is not necessary, its benchmark, we just need to know how long it could run, even if it faults, actually worse case fault is normal cause it runs until it exhaust the gas, instead of reaching a halt states.

[AnnaShaleva]

is not necessary

It is necessary for those benchmarks that are expected to end in HALT state.

it runs until it exhaust the gas,

As I said in the issue, for these benchmarks we need to check the exact exception message, i.e. to check that the failure reason is exactly run out of GAS. Otherwise, this benchmark is invalid.

instead of reaching a halt states.

Well, half of benchmarks from #3512 fail on something like third instruction execution, e.g.:

        [Benchmark]
        public void PoC_Cat()
        {
            // INITSLOT 0100
            // PUSHINT32 1048575
            // NEWBUFFER            <--- FAULTs VM execution here.

[Jim8y]

we added some limits to the vm.since those pocs. any way, you do it if you think its the best, these pocs are no.longer that importsnd to me, they helped me to rise the attention of this issue already, we can focus on benchmarking opcodes.

[Jim8y]

BTW, those are not benchmark for opcodes, but PoCs, opcode benchmarks are different.

[roman-khimov]

Yeah. And they're broken. They are not proving anything. If you like them this way --- well, ok.

[Jim8y]

Yeah. And they're broken. They are not proving anything. If you like them this way --- well, ok.

Its not about i like it or not, its that all of those PoCs are meaningless for me now, what is important for me is the precise benchmark for opcodes, so, you can say those PoC as a bug, or you can also ignore them, i am ok with either.

[dusmart]

Great Idea. We'll add some new PoCs that fit to the new version of NEO. But it's OK to keep the original ones IMO.