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

Replace function exceptwith and unionwith with faster functions #1174

Merged
Merged
Show file tree
Hide file tree
Changes from 26 commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
36c1d16
Merge pull request #1 from neo-project/master
Qiao-Jin Oct 22, 2019
b7f9c02
Replace ExceptWith & UnionWith with equal but faster functionality
Oct 22, 2019
318c99f
Optimization
shargon Oct 22, 2019
0d6b842
Merge branch 'master' into replace-func-exceptwith-unionwith
shargon Oct 22, 2019
af23059
Optimization
shargon Oct 22, 2019
82930da
Optimize remove
shargon Oct 22, 2019
54d2af8
Update neo/Network/P2P/TaskManager.cs
Qiao-Jin Oct 23, 2019
a745880
Code optimization
Oct 23, 2019
e0f9145
Merge branch 'replace-func-exceptwith-unionwith' into review
Qiao-Jin Oct 23, 2019
a84eff9
Update Helper.cs
Qiao-Jin Oct 23, 2019
dd36f20
Merge pull request #2 from shargon/review
Qiao-Jin Oct 23, 2019
089b935
Small change
shargon Oct 23, 2019
a571c22
Optimization
shargon Oct 23, 2019
4381b4c
Update Helper.cs
shargon Oct 23, 2019
dd4a435
Revert
shargon Oct 23, 2019
95136ed
Optimization
shargon Oct 23, 2019
edc0333
Optimize FIFOSet
shargon Oct 23, 2019
4ddc299
Rename
shargon Oct 23, 2019
6da2fc2
Inline
shargon Oct 23, 2019
bcf1c62
Merge pull request #4 from shargon/optimization
Qiao-Jin Oct 23, 2019
88753a6
Update UT_FIFOSet.cs
shargon Oct 23, 2019
59ee120
Fix
shargon Oct 23, 2019
921a343
Update UT_FIFOSet.cs
shargon Oct 23, 2019
621c4e3
Update FIFOSet.cs
shargon Oct 23, 2019
051b7c1
Update FIFOSet.cs
shargon Oct 23, 2019
96418c4
Merge branch 'master' into replace-func-exceptwith-unionwith
vncoelho Oct 23, 2019
f325f96
Merge branch 'master' into replace-func-exceptwith-unionwith
Qiao-Jin Oct 28, 2019
b1f5914
Merge branch 'master' into replace-func-exceptwith-unionwith
erikzhang Oct 30, 2019
f1cef96
Revert FIFOSet
erikzhang Oct 30, 2019
2e704e6
Update Helper.cs
erikzhang Oct 30, 2019
12aa262
Optimize
erikzhang Oct 30, 2019
9b71d5f
Merge branch 'master' into replace-func-exceptwith-unionwith
vncoelho Oct 30, 2019
b23244f
Merge branch 'master' into replace-func-exceptwith-unionwith
vncoelho Oct 30, 2019
360f20a
Reverting independet byte checks to SequenceEqual
vncoelho Oct 31, 2019
ec2bfd7
Merge branch 'master' into replace-func-exceptwith-unionwith
vncoelho Oct 31, 2019
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
18 changes: 18 additions & 0 deletions neo.UnitTests/IO/Caching/UT_FIFOSet.cs
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,24 @@ public void TestExceptWith()
};
set.ExceptWith(new UInt256[] { b, c });
CollectionAssert.AreEqual(set.ToArray(), new UInt256[] { a });

set = new FIFOSet<UInt256>(10)
{
a,
b,
c
};
set.ExceptWith(new UInt256[] { a });
CollectionAssert.AreEqual(set.ToArray(), new UInt256[] { b, c });

set = new FIFOSet<UInt256>(10)
{
a,
b,
c
};
set.ExceptWith(new UInt256[] { c });
CollectionAssert.AreEqual(set.ToArray(), new UInt256[] { a, b });
}
}
}
38 changes: 38 additions & 0 deletions neo/Helper.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using Microsoft.Extensions.Configuration;
using Neo.IO.Caching;
using Neo.Plugins;
using System;
using System.Collections.Generic;
Expand Down Expand Up @@ -54,6 +55,43 @@ internal static int GetLowestSetBit(this BigInteger i)
throw new Exception();
}

internal static void Remove<T>(this HashSet<T> set, HashSet<T> remove)
{
if (set.Count > 600_000)
{
set.ExceptWith(remove);
}
else
{
set.RemoveWhere(u => remove.Contains(u));
shargon marked this conversation as resolved.
Show resolved Hide resolved
}
}

internal static void Remove<T>(this HashSet<T> set, FIFOSet<T> remove)
where T : IEquatable<T>
{
if (set.Count > 600_000)
{
set.ExceptWith(remove);
}
else
{
set.RemoveWhere(u => remove.Contains(u));
}
}

internal static void Remove<T, V>(this HashSet<T> set, IDictionary<T, V> remove)
{
if (set.Count > 600_000)
{
set.ExceptWith(remove.Keys);
}
else
{
set.RemoveWhere(u => remove.ContainsKey(u));
}
}

internal static string GetVersion(this Assembly assembly)
{
CustomAttributeData attribute = assembly.CustomAttributes.FirstOrDefault(p => p.AttributeType == typeof(AssemblyInformationalVersionAttribute));
Expand Down
112 changes: 97 additions & 15 deletions neo/IO/Caching/FIFOSet.cs
Original file line number Diff line number Diff line change
@@ -1,16 +1,30 @@
using System;
using System.Collections;
using System.Collections.Generic;
using System.Collections.Specialized;
using System.Linq;

namespace Neo.IO.Caching
{
internal class FIFOSet<T> : IEnumerable<T> where T : IEquatable<T>
{
class Entry
Copy link
Member

Choose a reason for hiding this comment

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

Why change this class?

Copy link
Member

Choose a reason for hiding this comment

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

Is faster than the OrderedDictionary

Copy link
Member

Choose a reason for hiding this comment

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

Is this a linked list?

Copy link
Member

Choose a reason for hiding this comment

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

Yes

Copy link
Member

Choose a reason for hiding this comment

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

Then the performance of Contains() is low. This is very important. Otherwise we can use System.Collections.Generic.LinkedList<T> directly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

According to my tests, this Contains is faster than the previous version.

Have you tested occasions when the FIFOSet be very large? In this occasion, when knownHashes trys to add payload hashes into itself, it will firstly check whether the specified hash already exists. So if knowhashes is very large and transactions keep pouring in, time consumed by this line can be rather long.

Copy link
Member

Choose a reason for hiding this comment

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

This check was done in the previous version also

Copy link
Member

Choose a reason for hiding this comment

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

if (dictionary.Contains(item)) return false;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This check was done in the previous version also

But I think in previous version the computation complexity of func contains is O(1)

Copy link
Member

Choose a reason for hiding this comment

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

Agree with @Qiao-Jin. I can't imagine that linked lists will be faster than hash tables.

{
public readonly T Value;
public Entry Next;

public Entry(T current)
{
Value = current;
}
}

private readonly int maxCapacity;
private readonly int removeCount;
private readonly OrderedDictionary dictionary;

private int _count = 0;
private Entry _firstEntry = null;
private Entry _lastEntry = null;

public int Count => _count;

public FIFOSet(int maxCapacity, decimal batchSize = 0.1m)
{
Expand All @@ -19,45 +33,113 @@ public FIFOSet(int maxCapacity, decimal batchSize = 0.1m)

this.maxCapacity = maxCapacity;
this.removeCount = Math.Max((int)(maxCapacity * batchSize), 1);
this.dictionary = new OrderedDictionary(maxCapacity);
}

public bool Add(T item)
{
if (dictionary.Contains(item)) return false;
if (dictionary.Count >= maxCapacity)
if (Contains(item)) return false;
if (Count >= maxCapacity)
{
if (removeCount == maxCapacity)
{
dictionary.Clear();
_lastEntry = _firstEntry = null;
_count = 0;
}
else
{
for (int i = 0; i < removeCount; i++)
dictionary.RemoveAt(0);
{
RemoveFirst();
}
}
}
dictionary.Add(item, null);

if (_lastEntry == null)
{
_firstEntry = _lastEntry = new Entry(item);
}
else
{
_lastEntry = _lastEntry.Next = new Entry(item);
}

_count++;
return true;
}

private void RemoveFirst()
{
if (_firstEntry != null)
{
_firstEntry = _firstEntry.Next;
_count--;
}
}

public bool Contains(T item)
{
return dictionary.Contains(item);
var current = _firstEntry;

while (current != null)
{
if (current.Value.Equals(item)) return true;
current = current.Next;
}

return false;
}

public void ExceptWith(IEnumerable<UInt256> hashes)
erikzhang marked this conversation as resolved.
Show resolved Hide resolved
private void Remove(T item)
{
foreach (var hash in hashes)
var prev = _firstEntry;
var current = _firstEntry;

while (current != null)
{
dictionary.Remove(hash);
if (current.Value.Equals(item))
{
if (prev == null)
{
// First entry
_firstEntry = current.Next;
}
else
{
if (current == _firstEntry)
{
_firstEntry = prev.Next;
}
else
{
prev.Next = current.Next;
}
}
_count--;
return;
}

prev = current;
current = current.Next;
}
}

public void ExceptWith(IEnumerable<T> entries)
{
foreach (var entry in entries)
{
Remove(entry);
}
}

public IEnumerator<T> GetEnumerator()
{
var entries = dictionary.Keys.Cast<T>().ToArray();
foreach (var entry in entries) yield return entry;
var current = _firstEntry;

while (current != null)
{
yield return current.Value;
current = current.Next;
}
}

IEnumerator IEnumerable.GetEnumerator() => GetEnumerator();
Expand Down
8 changes: 4 additions & 4 deletions neo/Network/P2P/TaskManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -58,11 +58,11 @@ private void OnNewTasks(InvPayload payload)
return;
}
HashSet<UInt256> hashes = new HashSet<UInt256>(payload.Hashes);
hashes.ExceptWith(knownHashes);
hashes.Remove(knownHashes);
if (payload.Type == InventoryType.Block)
session.AvailableTasks.UnionWith(hashes.Where(p => globalTasks.ContainsKey(p)));

hashes.ExceptWith(globalTasks.Keys);
hashes.Remove(globalTasks);
if (hashes.Count == 0)
{
RequestTasks(session);
Expand Down Expand Up @@ -203,7 +203,7 @@ private void RequestTasks(TaskSession session)
if (session.HasTask) return;
if (session.AvailableTasks.Count > 0)
{
session.AvailableTasks.ExceptWith(knownHashes);
session.AvailableTasks.Remove(knownHashes);
session.AvailableTasks.RemoveWhere(p => Blockchain.Singleton.ContainsBlock(p));
HashSet<UInt256> hashes = new HashSet<UInt256>(session.AvailableTasks);
if (hashes.Count > 0)
Expand All @@ -213,7 +213,7 @@ private void RequestTasks(TaskSession session)
if (!IncrementGlobalTask(hash))
hashes.Remove(hash);
}
session.AvailableTasks.ExceptWith(hashes);
session.AvailableTasks.Remove(hashes);
foreach (UInt256 hash in hashes)
session.Tasks[hash] = DateTime.UtcNow;
foreach (InvPayload group in InvPayload.CreateGroup(InventoryType.Block, hashes.ToArray()))
Expand Down
9 changes: 8 additions & 1 deletion neo/UIntBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
using System;
using System.IO;
using System.Linq;
using System.Runtime.CompilerServices;

namespace Neo
{
Expand Down Expand Up @@ -61,7 +62,12 @@ public bool Equals(UIntBase other)
return true;
if (data_bytes.Length != other.data_bytes.Length)
return false;
return data_bytes.SequenceEqual(other.data_bytes);

for (int x = data_bytes.Length - 1; x >= 0; x--)
{
if (data_bytes[x] != other.data_bytes[x]) return false;
vncoelho marked this conversation as resolved.
Show resolved Hide resolved
}
return true;
}

/// <summary>
Expand Down Expand Up @@ -110,6 +116,7 @@ void ISerializable.Serialize(BinaryWriter writer)
/// <summary>
/// Method ToArray() returns the byte array data_bytes, which stores the little-endian unsigned int
/// </summary>
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public byte[] ToArray()
{
return data_bytes;
Expand Down