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

ReferenceCounterV2 #3526

Open
wants to merge 23 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
22c2107
ReferenceCounterV2
shargon Oct 11, 2024
51ce4ba
update more interfaces
shargon Oct 11, 2024
a4b936b
Test DUP (count)
shargon Oct 11, 2024
01445c2
Merge branch 'master' into remove-tarjan-v2
shargon Oct 14, 2024
bc2e52e
Merge branch 'master' into remove-tarjan-v2
Jim8y Oct 24, 2024
93c42fc
Merge branch 'master' into remove-tarjan-v2
Jim8y Nov 1, 2024
cab91ca
Merge branch 'master' into remove-tarjan-v2
Jim8y Nov 3, 2024
71cc724
Merge branch 'master' into remove-tarjan-v2
Jim8y Nov 4, 2024
e158c5f
Check subitems
shargon Nov 4, 2024
37c1c00
Merge branch 'master' into remove-tarjan-v2
Jim8y Nov 7, 2024
c0efc7c
New version, not compatible with circular references
shargon Nov 7, 2024
6352c5b
Update tests/Neo.VM.Tests/UT_ReferenceCounter.cs
shargon Nov 7, 2024
8276180
Merge branch 'master' into remove-tarjan-v2
Jim8y Nov 8, 2024
21e5434
Merge branch 'master' into remove-tarjan-v2
shargon Nov 8, 2024
066cc8b
Some fixes, still with issues
shargon Nov 8, 2024
9347754
Merge branch 'remove-tarjan-v2' of https://github.com/neo-project/neo…
shargon Nov 8, 2024
57f122e
| Method | Mean | Error | StdDev |
shargon Nov 8, 2024
75ccbae
fix comment
shargon Nov 8, 2024
e2a10e5
Merge branch 'master' into remove-tarjan-v2
shargon Nov 8, 2024
0a85aad
update UT and engine
Jim8y Nov 9, 2024
e38f73e
Merge branch 'master' into remove-tarjan-v2
Jim8y Nov 9, 2024
a79dd96
Merge branch 'master' into remove-tarjan-v2
Jim8y Nov 9, 2024
4986c54
Merge branch 'master' into remove-tarjan-v2
cschuchardt88 Nov 12, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 55 additions & 0 deletions benchmarks/Neo.VM.Benchmarks/Benchmarks.ReferenceCounter.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
// Copyright (C) 2015-2024 The Neo Project.
//
// Benchmarks.ReferenceCounter.cs file belongs to the neo project and is free
// software distributed under the MIT software license, see the
// accompanying file LICENSE in the main directory of the
// repository or http://www.opensource.org/licenses/mit-license.php
// for more details.
//
// Redistribution and use in source and binary forms with or without
// modifications are permitted.

using BenchmarkDotNet.Attributes;
using Neo.Test;
using Neo.Test.Extensions;
using Neo.Test.Types;
using System.Text;

namespace Neo.VM.Benchmark
{
public class Benchmarks_ReferenceCounter : VMJsonTestBase
{
[Benchmark]
public void V2()
{
Run<ReferenceCounterV2>();
}

[Benchmark]
public void V1()
{
Run<ReferenceCounter>();
}

private void Run<T>() where T : IReferenceCounter, new()
{
var path = Path.GetFullPath("../../../../../../../../../tests/Neo.VM.Tests/Tests");

foreach (var file in Directory.GetFiles(path, "*.json", SearchOption.AllDirectories))
{
var realFile = Path.GetFullPath(file);
var json = File.ReadAllText(realFile, Encoding.UTF8);
var ut = json.DeserializeJson<VMUT>();

try
{
ExecuteTest<T>(ut);
}
catch (Exception ex)
{
throw new AggregateException("Error in file: " + realFile, ex);
}
}
}
}
}
2 changes: 1 addition & 1 deletion benchmarks/Neo.VM.Benchmarks/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
var runBenchmark = true;

// Define the benchmark or execute class
var benchmarkType = typeof(Benchmarks_PoCs);
var benchmarkType = typeof(Benchmarks_ReferenceCounter);

/*
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
Expand Down
4 changes: 2 additions & 2 deletions src/Neo.VM/ExecutionEngine.cs
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ protected internal set
/// Initializes a new instance of the <see cref="ExecutionEngine"/> class.
/// </summary>
/// <param name="jumpTable">The jump table to be used.</param>
public ExecutionEngine(JumpTable? jumpTable = null) : this(jumpTable, new ReferenceCounter(), ExecutionEngineLimits.Default)
public ExecutionEngine(JumpTable? jumpTable = null) : this(jumpTable, new ReferenceCounterV2(), ExecutionEngineLimits.Default)
{
}

Expand All @@ -94,7 +94,7 @@ protected internal set
/// <param name="jumpTable">The jump table to be used.</param>
/// <param name="referenceCounter">The reference counter to be used.</param>
/// <param name="limits">Restrictions on the VM.</param>
protected ExecutionEngine(JumpTable? jumpTable, IReferenceCounter referenceCounter, ExecutionEngineLimits limits)
public ExecutionEngine(JumpTable? jumpTable, IReferenceCounter referenceCounter, ExecutionEngineLimits limits)
{
JumpTable = jumpTable ?? JumpTable.Default;
Limits = limits;
Expand Down
128 changes: 128 additions & 0 deletions src/Neo.VM/ReferenceCounterV2.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,128 @@
// Copyright (C) 2015-2024 The Neo Project.
//
// ReferenceCounterV2.cs file belongs to the neo project and is free
// software distributed under the MIT software license, see the
// accompanying file LICENSE in the main directory of the
// repository or http://www.opensource.org/licenses/mit-license.php
// for more details.
//
// Redistribution and use in source and binary forms with or without
// modifications are permitted.

using Neo.VM.Types;
using System;
using System.Collections.Generic;

namespace Neo.VM
{
/// <summary>
/// Used for reference counting of objects in the VM.
/// </summary>
public sealed class ReferenceCounterV2 : IReferenceCounter
{
private readonly List<StackItem> _stack = [];

/// <summary>
/// Gets the count of references.
/// </summary>
public int Count => _stack.Count;

/// <summary>
/// Adds item to Reference Counter
/// </summary>
/// <param name="item">The item to add.</param>
/// <param name="count">Number of similar entries</param>
public void AddStackReference(StackItem item, int count = 1)
{
for (var x = 0; x < count; x++)
Copy link
Contributor

Choose a reason for hiding this comment

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

this actually is not necessary, currently we process his when we create the compound type. For instance, new array will pass the rc and add all its subitems.

Copy link
Member Author

Choose a reason for hiding this comment

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

I know it, but it's hard to explain, there are cases where the items are removed without exists.

  • create a map with items without push the map to stack (unit test)
  • Push the map to stack
  • Add the map to static variables
  • RET

maybe is not this case that i remember, but when the stack was clean, I received "item not found" error

{
if (item is CompoundType compound && ReferenceEqualsIndexOf(item) == -1)
{
// Add sub items only if it was not present

foreach (var subItem in compound.SubItems)
{
AddStackReference(subItem);
}
}

_stack.Add(item);
}
}

/// <summary>
/// Removes item from Reference Counter
/// </summary>
/// <param name="item">The item to remove.</param>
public void RemoveStackReference(StackItem item)
{
var indexOf = ReferenceEqualsIndexOf(item);

if (indexOf == -1)
{
throw new InvalidOperationException("Reference was not added before");
Copy link
Member Author

Choose a reason for hiding this comment

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

This version is not compatible with circular references, but at the same it works like a protection

}

_stack.RemoveAt(indexOf);

if (item is CompoundType compound && ReferenceEqualsIndexOf(item, indexOf) == -1)
{
// Remove all the childrens only if the compound is not present

foreach (var subItem in compound.SubItems)
{
RemoveStackReference(subItem);
}
}
}

private int ReferenceEqualsIndexOf(StackItem item, int index = 0)
Copy link
Contributor

Choose a reason for hiding this comment

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

It's complexity is O(n), may not fast in some cases.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, this version is slower than I expected, I tried with different ways, and still slower than expected

{
// Note: List use Equals, and Struct don't allow to use it, so we iterate over the list

for (; index < _stack.Count; index++)
{
if (ReferenceEquals(_stack[index], item))
{
return index;
}
}

return -1;
}

public void AddReference(StackItem item, CompoundType parent)
{
if (ReferenceEqualsIndexOf(parent) != -1)
{
// Add only if the parent is present

AddStackReference(item);
}
}

public void RemoveReference(StackItem item, CompoundType parent)
{
if (ReferenceEqualsIndexOf(parent) != -1)
{
// Remove only if the parent is present

RemoveStackReference(item);
}
}

public void AddZeroReferred(StackItem item)
{
// This version don't use this method
}

/// <summary>
/// Checks and processes items that have zero references.
/// </summary>
/// <returns>The current reference count.</returns>
public int CheckZeroReferred()
{
return Count;
}
}
}
4 changes: 4 additions & 0 deletions tests/Neo.VM.Tests/Types/TestEngine.cs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,10 @@ public class TestEngine : ExecutionEngine

public TestEngine() : base(ComposeJumpTable()) { }

public TestEngine(IReferenceCounter referenceCounter) : base(ComposeJumpTable(), referenceCounter, ExecutionEngineLimits.Default)
{
}

private static JumpTable ComposeJumpTable()
{
JumpTable jumpTable = new JumpTable();
Expand Down
21 changes: 20 additions & 1 deletion tests/Neo.VM.Tests/UT_ReferenceCounter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ public void TestRemoveReferrer()
Assert.AreEqual(3, engine.ReferenceCounter.Count);
Assert.AreEqual(VMState.BREAK, debugger.StepInto());
Assert.AreEqual(2, engine.ReferenceCounter.Count);
Assert.AreEqual(VMState.HALT, debugger.Execute());
Assert.AreEqual(VMState.HALT, debugger.StepInto());
Assert.AreEqual(1, engine.ReferenceCounter.Count);
}

Expand Down Expand Up @@ -259,5 +259,24 @@ public void TestInvalidReferenceStackItem()
arr.Add(arr2);
Assert.AreEqual(11, reference.Count);
}

[TestMethod]
public void TestDup()
{
using ScriptBuilder sb = new();
sb.Emit(OpCode.PUSH1);
sb.Emit(OpCode.DUP);

using ExecutionEngine engine = new();
engine.LoadScript(sb.ToArray());
Assert.AreEqual(0, engine.ReferenceCounter.Count);
engine.ExecuteNext();
Assert.AreEqual(1, engine.ReferenceCounter.Count);
engine.ExecuteNext();
Assert.AreEqual(2, engine.ReferenceCounter.Count);
engine.ExecuteNext();
Assert.AreEqual(2, engine.ReferenceCounter.Count);
Assert.AreEqual(VMState.HALT, engine.Execute());
}
}
}
Loading
Loading