Skip to content
This repository has been archived by the owner on Nov 22, 2023. It is now read-only.

Add class Instruction to simplify ExecutionEngine #102

Merged
merged 5 commits into from
Mar 28, 2019
Merged

Conversation

erikzhang
Copy link
Member

No description provided.

@erikzhang erikzhang requested a review from shargon March 27, 2019 14:41
@erikzhang
Copy link
Member Author

erikzhang commented Mar 27, 2019

@shargon After you approve this PR, I will create a new PR which uses ReadOnlySpan to optimize byte array replication.

* Change project to: dotnet core

Currently we have this framework:

 `<TargetFrameworks>netstandard1.6;net461</TargetFrameworks>`

In order to unify the projects i think that we should define it as:

`<TargetFramework>netcoreapp2.2</TargetFramework>`

* Update neo-vm.csproj

* fix .csproj
@shargon
Copy link
Member

shargon commented Mar 27, 2019

I think that we can initialize RandomAccessStack for start with a optimal capacity and prevent the first redimension of the internal list

@erikzhang
Copy link
Member Author

I think that we can initialize RandomAccessStack for start with a optimal capacity and prevent the first redimension of the internal list

What do you mean?

@erikzhang
Copy link
Member Author

@shargon I reverted the change of upgrading .netstandard to 2.0. If you need to use reflection, you can add it back. Now I will start to work on ReadOnlySpan.

@shargon
Copy link
Member

shargon commented Mar 28, 2019

What do you mean?

RandomAccessStack use a list, and list could be initialized with the desired initial capacity.

image

If we look inside the list we have an array, and is redimensionated according to the length.

image

image

By default this array started with 0 length

image

But we can define a better one, like 32, or just 4, and prevent this first redimension

image

@erikzhang
Copy link
Member Author

But you don't know how much space will the contract use.

@shargon
Copy link
Member

shargon commented Mar 28, 2019

Agree, but at least 4, is the minimum for one item

@erikzhang
Copy link
Member Author

I think there is almost no difference between 0 and 4. The capacity is for large List.

{
this.OpCode = (OpCode)script[ip++];
int operandSize = 0;
switch (OperandSizePrefixTable[(int)OpCode])
Copy link
Member

Choose a reason for hiding this comment

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

This way is better :)

@shargon
Copy link
Member

shargon commented Mar 28, 2019

I think that with your last changes, we don't need reflection :)

@erikzhang
Copy link
Member Author

OK. Then I will merge it first.

@erikzhang erikzhang merged commit bfb68c8 into master Mar 28, 2019
@erikzhang erikzhang deleted the instruction branch March 28, 2019 08:43
@shargon
Copy link
Member

shargon commented Mar 28, 2019

I really like the change that this PR gives, good job!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants