Optimize OnPersist

[shargon]

Developing the rpc storage in test engine (https://github.com/neo-project/neo-devpack-dotnet/pull/904) I have realized how "expensive" it is to calculate the gas per block.

Currently, we store a history of gas changes, which is necessary for calculating the reward of accounts when they call claim, but it is not necessary for the OnPersist of the block. However, we iterate backwards to calculate it in the same way during the block's persist, something completely unnecessary.

It can be optimized by avoiding iterating over the storage, we have 2 options.

• Do it with the fork introduced at https://github.com/neo-project/neo/pull/2942
• Modify the current Prefix_GasPerBlock, I think it has never been done in any of the networks and can only be done by consensus

Opinions?

[shargon]

I vote for replace the Prefix_GasPerBlock

[roman-khimov]

There is no need to change the DB scheme and store duplicating data, just cache it. https://github.com/nspcc-dev/neo-go/blob/71fb759c0d009218437d0048f7ba085a2a74c66f/pkg/core/native/native_neo.go#L45-L47

[shargon]

There is no need to change the DB scheme and store duplicating data, just cache it. https://github.com/nspcc-dev/neo-go/blob/71fb759c0d009218437d0048f7ba085a2a74c66f/pkg/core/native/native_neo.go#L45-L47

That's solve the node issue, but it won't solve external readers like rpc storage.

[shargon]

There is no need to change the DB scheme and store duplicating data, just cache it. https://github.com/nspcc-dev/neo-go/blob/71fb759c0d009218437d0048f7ba085a2a74c66f/pkg/core/native/native_neo.go#L45-L47

@roman-khimov Or we can support seek backwards in rpc, and cache it in node :)

https://github.com/neo-project/neo-devpack-dotnet/pull/904/files#r1491847096 https://github.com/neo-project/neo-devpack-dotnet/pull/904/files#diff-e3fcac199233399d9d0cb9e8eb1ff1cbba7a97f2d1da2e73923ede29715f2225R20

[cschuchardt88]

There is no need to change the DB scheme and store duplicating data, just cache it. https://github.com/nspcc-dev/neo-go/blob/71fb759c0d009218437d0048f7ba085a2a74c66f/pkg/core/native/native_neo.go#L45-L47

@roman-khimov Or we can support seek backwards in rpc, and cache it in node :)

https://github.com/neo-project/neo-devpack-dotnet/pull/904/files#r1491847096 https://github.com/neo-project/neo-devpack-dotnet/pull/904/files#diff-e3fcac199233399d9d0cb9e8eb1ff1cbba7a97f2d1da2e73923ede29715f2225R20

It does support it, for iterators in contracts.

https://github.com/neo-project/neo-modules/blob/7faf5356ce86e49e10a75fcffa0aba654f16cd9d/src/RpcServer/RpcServer.SmartContract.cs#L243

[cschuchardt88]

@shargon what is the Key parts for Prefix_GasPerBlock? KeyBuilder(id, Prefix_GasPerBlock).add(.....)? Also MemoryStore is really slow. That is where the problem, I'm pretty sure.

[shargon]

Currently, we store a history of gas changes, which is necessary for calculating the reward of accounts when they call claim, but it is not necessary for the OnPersist of the block. However, we iterate backwards to calculate it in the same way during the block's persist, something completely unnecessary.

[shargon]

@neo-project/core How to test? debug the node, and press F11 (step into) in GasPerBlock... you will see all the Find and Seek logic, this should be a direct read.

[shargon]

We can also optimize with the same technique GetDesignatedByRole

[shargon]

@neo-project/core @roman-khimov if we will need to resync the chain for 3.7, this is something to think about it, GasPerBlock and GetDesignatedByRole are used in all blocks.

[roman-khimov]

This problem is still not worth changing the DB scheme, caching solves it completely and it'd be faster than additional data in the DB anyway.

[shargon]

This problem is still not worth changing the DB scheme, caching solves it completely and it'd be faster than additional data in the DB anyway.

And remove the nefFile from contractState? We only need the Script and tokens, and all the nefFile is deserialized in all contract calls, at least we can avoid the checksum check

[roman-khimov]

This would remove the symmetry of deploy/getcontract. We deploy NEF/manifest, we can expect to get them back the same way. We have a number of external interfaces that return NEFs as well, this change can break them.

Deserialization optimization is OK. We actually have a contract cache in NeoGo, so we don't deserialize often.

[AnnaShaleva]

Vote up for cache usage instead of storage scheme change.

That's solve the node issue, but it won't solve external readers like rpc storage.

Could you explain, why? Because in our implementation a similar cache is available for external users via exported method (e.g. external consensus service uses https://github.com/nspcc-dev/neo-go/blob/71fb759c0d009218437d0048f7ba085a2a74c66f/pkg/core/native/native_neo.go#L1120 without any issues). We initialize this cache when needed (on node resume or test historic VM creation basically, ref. https://github.com/nspcc-dev/neo-go/blob/71fb759c0d009218437d0048f7ba085a2a74c66f/pkg/core/native/native_neo.go#L329), and it doesn't cost a lot. The only caveat is that we should keep it up-to-date and relevant, but it's possible.