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.

Sign up for GitHub

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

Jump to bottom

Fixes #946 #950

Merged
merged 15 commits into from
Jul 30, 2019
Merged

Fixes #946 #950

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
99059aa
Fixes #946
erikzhang Jul 24, 2019
ad67223
Copy ut from https://github.com/neo-project/neo/pull/947 (#952)
shargon Jul 24, 2019
2dfac18
Added entries with wrong prefix
shargon Jul 24, 2019
8f689e1
Fix
shargon Jul 24, 2019
8ac83c6
Complete fix
shargon Jul 24, 2019
06349e3
Update DataCache.cs
shargon Jul 24, 2019
f2606e1
Revert DataCache.cs
erikzhang Jul 24, 2019
ac85174
real fix
erikzhang Jul 24, 2019
50c51da
Update DataCache.cs
shargon Jul 25, 2019
41ce918
Sort unrelated grouping
erikzhang Jul 26, 2019
12570f9
Merge branch 'master' into fixes/find
shargon Jul 26, 2019
d2ccbaa
Merge branch 'master' into fixes/find
shargon Jul 28, 2019
875407d
Merge branch 'master' into fixes/find
shargon Jul 29, 2019
68ab63a
Fix unit tests
shargon Jul 29, 2019
ca51799
Merge branch 'master' into fixes/find
erikzhang Jul 30, 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
12 changes: 6 additions & 6 deletions neo.UnitTests/IO/UT_IOHelper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,9 +234,9 @@ public void TestReadBytesWithGrouping()
else
{
byte[] caseArray = new byte[] { 0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA,
0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA,0x00,
0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA,0x10,
0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA,
0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA,0x00,
0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA,0x10,
0xAA, 0xAA, 0xAA, 0xAA, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,0x11};
MemoryStream stream = new MemoryStream();
Expand Down Expand Up @@ -415,11 +415,11 @@ public void TestWriteBytesWithGrouping()
byte[] byteArray = new byte[stream.Length];
stream.Read(byteArray, 0, (int)stream.Length);
Assert.AreEqual(Encoding.Default.GetString(new byte[] { 0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA,
0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA,0x00,
0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA,0x10,
0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA,
0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA,0x00,
0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA,0x10,
0xAA, 0xAA, 0xAA, 0xAA, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,0x0C}), Encoding.Default.GetString(byteArray));
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,0x04}), Encoding.Default.GetString(byteArray));
}

[TestMethod]
Expand Down Expand Up @@ -546,4 +546,4 @@ public void TestWriteVarString()
Assert.AreEqual(0x61, byteArray[1]);
}
}
}
}
126 changes: 126 additions & 0 deletions neo.UnitTests/UT_DataCache.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,126 @@
using Microsoft.VisualStudio.TestTools.UnitTesting;
using Neo.IO.Caching;
using Neo.Ledger;
using System.Linq;

namespace Neo.UnitTests
{
[TestClass]
public class UT_DataCache
{
[TestInitialize]
public void TestSetup()
{
TestBlockchain.InitializeMockNeoSystem();
}

[TestMethod]
public void TestCachedFind_Between()
{
var snapshot = TestBlockchain.GetStore().GetSnapshot();
var storages = snapshot.Storages;
var cache = new CloneCache<StorageKey, StorageItem>(storages);

storages.DeleteWhere((k, v) => k.ScriptHash == UInt160.Zero);

storages.Add
(
new StorageKey() { Key = new byte[] { 0x01, 0x01 }, ScriptHash = UInt160.Zero },
new StorageItem() { IsConstant = false, Value = new byte[] { } }
);
storages.Add
(
new StorageKey() { Key = new byte[] { 0x00, 0x01 }, ScriptHash = UInt160.Zero },
new StorageItem() { IsConstant = false, Value = new byte[] { } }
);
storages.Add
(
new StorageKey() { Key = new byte[] { 0x00, 0x03 }, ScriptHash = UInt160.Zero },
new StorageItem() { IsConstant = false, Value = new byte[] { } }
);
cache.Add
(
new StorageKey() { Key = new byte[] { 0x01, 0x02 }, ScriptHash = UInt160.Zero },
new StorageItem() { IsConstant = false, Value = new byte[] { } }
);
cache.Add
(
new StorageKey() { Key = new byte[] { 0x00, 0x02 }, ScriptHash = UInt160.Zero },
new StorageItem() { IsConstant = false, Value = new byte[] { } }
);

CollectionAssert.AreEqual(
cache.Find(new byte[21]).Select(u => u.Key.Key[1]).ToArray(),
new byte[] { 0x01, 0x02, 0x03 }
);

storages.DeleteWhere((k, v) => k.ScriptHash == UInt160.Zero);
}

[TestMethod]
public void TestCachedFind_Last()
{
var snapshot = TestBlockchain.GetStore().GetSnapshot();
var storages = snapshot.Storages;
var cache = new CloneCache<StorageKey, StorageItem>(storages);

storages.DeleteWhere((k, v) => k.ScriptHash == UInt160.Zero);

storages.Add
(
new StorageKey() { Key = new byte[] { 0x00, 0x01 }, ScriptHash = UInt160.Zero },
new StorageItem() { IsConstant = false, Value = new byte[] { } }
);
storages.Add
(
new StorageKey() { Key = new byte[] { 0x01, 0x01 }, ScriptHash = UInt160.Zero },
new StorageItem() { IsConstant = false, Value = new byte[] { } }
);
cache.Add
(
new StorageKey() { Key = new byte[] { 0x00, 0x02 }, ScriptHash = UInt160.Zero },
new StorageItem() { IsConstant = false, Value = new byte[] { } }
);
cache.Add
(
new StorageKey() { Key = new byte[] { 0x01, 0x02 }, ScriptHash = UInt160.Zero },
new StorageItem() { IsConstant = false, Value = new byte[] { } }
);

CollectionAssert.AreEqual(
cache.Find(new byte[21]).Select(u => u.Key.Key[1]).ToArray(),
new byte[] { 0x01, 0x02 }
);

storages.DeleteWhere((k, v) => k.ScriptHash == UInt160.Zero);
}

[TestMethod]
public void TestCachedFind_Empty()
{
var snapshot = TestBlockchain.GetStore().GetSnapshot();
var storages = snapshot.Storages;
var cache = new CloneCache<StorageKey, StorageItem>(storages);

storages.DeleteWhere((k, v) => k.ScriptHash == UInt160.Zero);

cache.Add
(
new StorageKey() { Key = new byte[] { 0x00, 0x02 }, ScriptHash = UInt160.Zero },
new StorageItem() { IsConstant = false, Value = new byte[] { } }
);
cache.Add
(
new StorageKey() { Key = new byte[] { 0x01, 0x02 }, ScriptHash = UInt160.Zero },
new StorageItem() { IsConstant = false, Value = new byte[] { } }
);

CollectionAssert.AreEqual(
cache.Find(new byte[21]).Select(u => u.Key.Key[1]).ToArray(),
new byte[] { 0x02 }
);

storages.DeleteWhere((k, v) => k.ScriptHash == UInt160.Zero);
}
}
}
21 changes: 21 additions & 0 deletions neo/IO/ByteArrayComparer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
using System;
using System.Collections.Generic;

namespace Neo.IO
{
internal class ByteArrayComparer : IComparer<byte[]>
{
public static readonly ByteArrayComparer Default = new ByteArrayComparer();

public int Compare(byte[] x, byte[] y)
{
int length = Math.Min(x.Length, y.Length);
for (int i = 0; i < length; i++)
{
int r = x[i].CompareTo(y[i]);
if (r != 0) return r;
}
return x.Length.CompareTo(y.Length);
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

@rodoufu can you review this?

54 changes: 48 additions & 6 deletions neo/IO/Caching/DataCache.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,16 +118,58 @@ public void DeleteWhere(Func<TKey, TValue, bool> predicate)
}
}

/// <summary>
/// Find the entries that start with the `key_prefix`
/// </summary>
/// <param name="key_prefix">Must maintain the deserialized format of TKey</param>
/// <returns>Entries found with the desired prefix</returns>
public IEnumerable<KeyValuePair<TKey, TValue>> Find(byte[] key_prefix = null)
{
IEnumerable<(byte[], TKey, TValue)> cached;
lock (dictionary)
{
foreach (var pair in FindInternal(key_prefix ?? new byte[0]))
if (!dictionary.ContainsKey(pair.Key))
yield return pair;
foreach (var pair in dictionary)
if (pair.Value.State != TrackState.Deleted && (key_prefix == null || pair.Key.ToArray().Take(key_prefix.Length).SequenceEqual(key_prefix)))
yield return new KeyValuePair<TKey, TValue>(pair.Key, pair.Value.Item);
cached = dictionary
.Where(p => p.Value.State != TrackState.Deleted && (key_prefix == null || p.Key.ToArray().Take(key_prefix.Length).SequenceEqual(key_prefix)))
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks good to me, must do it in a deserialized format, to avoid different storage formats affecting Find.

.Select(p =>
(
KeyBytes: p.Key.ToArray(),
p.Key,
p.Value.Item
))
.OrderBy(p => p.KeyBytes, ByteArrayComparer.Default)
.ToArray();
}
var uncached = FindInternal(key_prefix ?? new byte[0])
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this FindInternal implementation-specific? This is a good moment to avoid specific decisions affecting the outcome of Find.

.Where(p => !dictionary.ContainsKey(p.Key))
.Select(p =>
(
KeyBytes: p.Key.ToArray(),
p.Key,
p.Value
));
using (var e1 = cached.GetEnumerator())
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this "intercalation" strategy should be done in a separate function, so it's clear and also testable (perhaps useful in other situations too).

using (var e2 = uncached.GetEnumerator())
{
(byte[] KeyBytes, TKey Key, TValue Item) i1, i2;
bool c1 = e1.MoveNext();
bool c2 = e2.MoveNext();
i1 = c1 ? e1.Current : default;
i2 = c2 ? e2.Current : default;
while (c1 || c2)
{
if (!c2 || (c1 && ByteArrayComparer.Default.Compare(i1.KeyBytes, i2.KeyBytes) < 0))
shargon marked this conversation as resolved.
Show resolved Hide resolved
{
yield return new KeyValuePair<TKey, TValue>(i1.Key, i1.Item);
c1 = e1.MoveNext();
i1 = c1 ? e1.Current : default;
}
else
{
yield return new KeyValuePair<TKey, TValue>(i2.Key, i2.Item);
c2 = e2.MoveNext();
i2 = c2 ? e2.Current : default;
}
}
}
}

Expand Down
13 changes: 6 additions & 7 deletions neo/IO/Helper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,17 +92,16 @@ public static byte[] ReadBytesWithGrouping(this BinaryReader reader)
{
using (MemoryStream ms = new MemoryStream())
{
int padding = 0;
int count;
do
{
byte[] group = reader.ReadBytes(GroupingSizeInBytes);
Copy link
Member

Choose a reason for hiding this comment

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

We should reuse this buffer

byte[] group=new byte[GroupSizeInBytes];
do{
read.Read(group,0,GroupSizeInBytes);

Copy link
Member Author

Choose a reason for hiding this comment

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

read.Read(group,0,GroupSizeInBytes);

This may read data less than GroupSizeInBytes and use an extra loop, which is very troublesome.

padding = reader.ReadByte();
if (padding > GroupingSizeInBytes)
count = reader.ReadByte();
if (count > GroupingSizeInBytes)
throw new FormatException();
int count = GroupingSizeInBytes - padding;
if (count > 0)
ms.Write(group, 0, count);
} while (padding == 0);
} while (count == GroupingSizeInBytes);
return ms.ToArray();
}
}
Expand Down Expand Up @@ -200,7 +199,7 @@ public static void WriteBytesWithGrouping(this BinaryWriter writer, byte[] value
while (remain >= GroupingSizeInBytes)
{
writer.Write(value, index, GroupingSizeInBytes);
writer.Write((byte)0);
writer.Write((byte)GroupingSizeInBytes);
index += GroupingSizeInBytes;
remain -= GroupingSizeInBytes;
}
Expand All @@ -209,7 +208,7 @@ public static void WriteBytesWithGrouping(this BinaryWriter writer, byte[] value
int padding = GroupingSizeInBytes - remain;
for (int i = 0; i < padding; i++)
writer.Write((byte)0);
writer.Write((byte)padding);
writer.Write((byte)remain);
}

public static void WriteFixedString(this BinaryWriter writer, string value, int length)
Expand Down
2 changes: 1 addition & 1 deletion neo/SmartContract/InteropService.NEO.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -353,7 +353,7 @@ private static bool Storage_Find(ApplicationEngine engine)
while (remain >= 16)
{
ms.Write(prefix, index, 16);
ms.WriteByte(0);
ms.WriteByte(16);
index += 16;
remain -= 16;
}
Expand Down