[Fixes/Optimizations/Styles] KeyBuilder, StorageKey, StorageItem, MemorySnapshot, MemoryStore, ByteArrayComparer & ByteArrayEqualityComparer

[cschuchardt88]

NEW

Method	Mean	Error	StdDev	Median	Min	Max
TestDataCacheAdd	0.0042 ns	0.0087 ns	0.0081 ns	0.0000 ns	0.0000 ns	0.0283 ns
TestDataCacheUpdate	0.0062 ns	0.0080 ns	0.0075 ns	0.0016 ns	0.0000 ns	0.0217 ns
TestDataCacheDelete	36.0960 ns	0.2631 ns	0.2461 ns	36.1001 ns	35.6243 ns	36.5847 ns
TestDataCacheGet	0.0000 ns	0.0000 ns	0.0000 ns	0.0000 ns	0.0000 ns	0.0000 ns

OLD

Method	Mean	Error	StdDev	Median	Min	Max
TestDataCacheAdd	0.0105 ns	0.0162 ns	0.0152 ns	0.0062 ns	0.0000 ns	0.0533 ns
TestDataCacheUpdate	0.0011 ns	0.0044 ns	0.0041 ns	0.0000 ns	0.0000 ns	0.0159 ns
TestDataCacheDelete	36.5685 ns	0.5138 ns	0.4807 ns	36.4161 ns	35.9531 ns	37.9400 ns
TestDataCacheGet	0.0099 ns	0.0187 ns	0.0175 ns	0.0009 ns	0.0000 ns	0.0582 ns

Change Log

KeyBuilder

• Fixed issue with ToArray disposing memory. Leads to not being able to call ToArray again.
• Fixed naming conventions

StorageKey

• Replaced byte[] to use Memory<byte> for faster caching.
• Fixed naming conventions
• Added many optimizations.

StorageItem

• Fixed issues with pointers in ReadOnlyMemory<byte>.
• Fixed with moving byte[] around to new instances of the class.
• Fixed naming conventions

MemorySnapshot

• Removed redundant processing of byte[]
• Fixed seek method to be inline with MemoryStore, LevelDbStore and RocksDbStore; to allow the same results.

MemoryStore

• Removed redundant processing of byte[]

ByteArrayComparer

• Added inherit to IComparer interface. To allow objects to be passed. (NOTE: Can be used with other dotnet classes that ONLY require IComparer.)
• Added many optimizations

ByteArrayEqualityComparer

• Added inherit to IEqualityComparer interface. To allow objects to be passed. (NOTE: Can be used with other dotnet classes that ONLY require IEqualityComparer.)
• Added DisallowNull attribute on GetHashCode parameters.
• Changed the name of Default instance to Instance, to match dotnet naming conventions.
• Added many optimizations

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?

• Local Environment

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

@superboyiii
Please test when you can.

[superboyiii]

@cschuchardt88 Missing using Neo.Extensions; in KeyBuilder.cs

[cschuchardt88]

@cschuchardt88 Missing using Neo.Extensions; in KeyBuilder.cs

fixed now.

[superboyiii]

@cschuchardt88 UT isn't passed.

[cschuchardt88]

@cschuchardt88 UT isn't passed.

OK fixed. Typo with merge.

[superboyiii]

Tested full data on mainnet, it's compatible.

[shargon]

Please avoid remove the clones, this change must come alone, it could affect the security

[cschuchardt88]

Please avoid remove the clones, this change must come alone, it could affect the security

Could you be more specific what you mean by clones?

[shargon]

Please avoid remove the clones, this change must come alone, it could affect the security

Could you be more specific what you mean by clones?

?[..] and .ToArray()[..]

[cschuchardt88]

Please avoid remove the clones, this change must come alone, it could affect the security

Could you be more specific what you mean by clones?

?[..] and .ToArray()[..]

When using ToArray() on Memory<> or ReadOnlyMemory<> It creates a new buffer. It even says that on the comments.

[shargon]

@Jim8y and @nan01ab please take a look, it seems ok to me now

[shargon]

My unique concern now is this https://github.com/neo-project/neo/pull/3705/files#r1944601500

[shargon]

Every time that is invoked a new buffer is created, so the "cache" is not complete, and slower

[cschuchardt88]

How is it slower? No memory is moved. Its like creating a Span on byte[] nothing moves.

[cschuchardt88]

So my way is faster

NEW

Method	Mean	Error	StdDev	Median	Min	Max
TestDataCacheAdd	0.0042 ns	0.0087 ns	0.0081 ns	0.0000 ns	0.0000 ns	0.0283 ns
TestDataCacheUpdate	0.0062 ns	0.0080 ns	0.0075 ns	0.0016 ns	0.0000 ns	0.0217 ns
TestDataCacheDelete	36.0960 ns	0.2631 ns	0.2461 ns	36.1001 ns	35.6243 ns	36.5847 ns
TestDataCacheGet	0.0000 ns	0.0000 ns	0.0000 ns	0.0000 ns	0.0000 ns	0.0000 ns

OLD

Method	Mean	Error	StdDev	Median	Min	Max
TestDataCacheAdd	0.0105 ns	0.0162 ns	0.0152 ns	0.0062 ns	0.0000 ns	0.0533 ns
TestDataCacheUpdate	0.0011 ns	0.0044 ns	0.0041 ns	0.0000 ns	0.0000 ns	0.0159 ns
TestDataCacheDelete	36.5685 ns	0.5138 ns	0.4807 ns	36.4161 ns	35.9531 ns	37.9400 ns
TestDataCacheGet	0.0099 ns	0.0187 ns	0.0175 ns	0.0009 ns	0.0000 ns	0.0582 ns

[cschuchardt88]

@shargon I even had @superboyiii already test before all your changes and check the data. Everything is good. After this we can remove CloneCache its unneeded now.

@nan01ab I reverted back to what i had, You can review now.

[shargon]

@shargon I even had @superboyiii already test before all your changes and check the data. Everything is good. After this we can remove CloneCache its unneeded now.

@nan01ab I reverted back to what i had, You can review now.

It can works, but also can be slower, and unnecessary

[cschuchardt88]

@shargon I even had @superboyiii already test before all your changes and check the data. Everything is good. After this we can remove CloneCache its unneeded now.
@nan01ab I reverted back to what i had, You can review now.

It can works, but also can be slower, and unnecessary

So my way is faster

NEW

Method	Mean	Error	StdDev	Median	Min	Max
TestDataCacheAdd	0.0042 ns	0.0087 ns	0.0081 ns	0.0000 ns	0.0000 ns	0.0283 ns
TestDataCacheUpdate	0.0062 ns	0.0080 ns	0.0075 ns	0.0016 ns	0.0000 ns	0.0217 ns
TestDataCacheDelete	36.0960 ns	0.2631 ns	0.2461 ns	36.1001 ns	35.6243 ns	36.5847 ns
TestDataCacheGet	0.0000 ns	0.0000 ns	0.0000 ns	0.0000 ns	0.0000 ns	0.0000 ns

OLD

Method	Mean	Error	StdDev	Median	Min	Max
TestDataCacheAdd	0.0105 ns	0.0162 ns	0.0152 ns	0.0062 ns	0.0000 ns	0.0533 ns
TestDataCacheUpdate	0.0011 ns	0.0044 ns	0.0041 ns	0.0000 ns	0.0000 ns	0.0159 ns
TestDataCacheDelete	36.5685 ns	0.5138 ns	0.4807 ns	36.4161 ns	35.9531 ns	37.9400 ns
TestDataCacheGet	0.0099 ns	0.0187 ns	0.0175 ns	0.0009 ns	0.0000 ns	0.0582 ns

[shargon]

@shargon I even had @superboyiii already test before all your changes and check the data. Everything is good. After this we can remove CloneCache its unneeded now.
@nan01ab I reverted back to what i had, You can review now.

It can works, but also can be slower, and unnecessary

So my way is faster

NEW

Method	Mean	Error	StdDev	Median	Min	Max
TestDataCacheAdd	0.0042 ns	0.0087 ns	0.0081 ns	0.0000 ns	0.0000 ns	0.0283 ns
TestDataCacheUpdate	0.0062 ns	0.0080 ns	0.0075 ns	0.0016 ns	0.0000 ns	0.0217 ns
TestDataCacheDelete	36.0960 ns	0.2631 ns	0.2461 ns	36.1001 ns	35.6243 ns	36.5847 ns
TestDataCacheGet	0.0000 ns	0.0000 ns	0.0000 ns	0.0000 ns	0.0000 ns	0.0000 ns

OLD

Method	Mean	Error	StdDev	Median	Min	Max
TestDataCacheAdd	0.0105 ns	0.0162 ns	0.0152 ns	0.0062 ns	0.0000 ns	0.0533 ns
TestDataCacheUpdate	0.0011 ns	0.0044 ns	0.0041 ns	0.0000 ns	0.0000 ns	0.0159 ns
TestDataCacheDelete	36.5685 ns	0.5138 ns	0.4807 ns	36.4161 ns	35.9531 ns	37.9400 ns
TestDataCacheGet	0.0099 ns	0.0187 ns	0.0175 ns	0.0009 ns	0.0000 ns	0.0582 ns

Try with my changes

[cschuchardt88]

Try with my changes

Too late I forced push. Plus the only different that you ending up doing in the end, was having everything the same as before. Only change was byte[] to Memory<byte>