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

Improve the performance of NeoVM #95

Merged
merged 8 commits into from Mar 27, 2019

Conversation

2 participants
@erikzhang
Copy link
Member

erikzhang commented Mar 26, 2019

No description provided.

erikzhang added some commits Mar 26, 2019

@erikzhang erikzhang requested a review from shargon Mar 26, 2019

@shargon
Copy link
Member

shargon left a comment

I would like to add some Inline Attributes for methods like ToByteArray

Show resolved Hide resolved src/neo-vm/Unsafe.cs
Show resolved Hide resolved src/neo-vm/Unsafe.cs
Show resolved Hide resolved src/neo-vm/Unsafe.cs

shargon added some commits Mar 26, 2019

Add comment
Add a comment in order to prevent the use in the future without check the length previously
@shargon

This comment has been minimized.

Copy link
Member

shargon commented Mar 26, 2019

I will add some UT, and i will approve it, good move :)

@erikzhang

This comment has been minimized.

Copy link
Member Author

erikzhang commented Mar 27, 2019

@shargon Can I merge this?

@shargon

This comment has been minimized.

Copy link
Member

shargon commented Mar 27, 2019

Give me 2 hours for try to do some UT

@shargon

This comment has been minimized.

Copy link
Member

shargon commented Mar 27, 2019

We can add

[MethodImpl(MethodImplOptions.AggressiveInlining)]

In all ToByteArray methods?

@erikzhang

This comment has been minimized.

Copy link
Member Author

erikzhang commented Mar 27, 2019

AggressiveInlining has no effect on virtual methods.

Some ut for performance PR (#100)
* SHL and SHR

* SIZE

* Change category

* Typo
@shargon

This comment has been minimized.

Copy link
Member

shargon commented Mar 27, 2019

I just want to add SUBSTR test and it's done :)

@shargon shargon merged commit c45330e into master Mar 27, 2019

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@shargon shargon deleted the performance branch Mar 27, 2019

@ixje ixje referenced this pull request Apr 17, 2019

Merged

Update VM #932

3 of 5 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.