Skip to content

Split args and locals #345

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
Closed
wants to merge 2 commits into from
Closed

Split args and locals #345

wants to merge 2 commits into from

Conversation

@chfast
Copy link
Collaborator

@chfast chfast commented May 27, 2020 •
edited
Loading

This is preliminary step for opening two optimization directions:

  1. Optimizing "calling convention" by not copying calls' args around.
  2. Combining allocation for stack and locals + small stack optimization.

This change is a big execution time regression.

fizzy/execute/blake2b/512_bytes_rounds_1_mean                     +0.0803         +0.0802            85            92            85            92                       
fizzy/execute/blake2b/512_bytes_rounds_16_mean                    +0.0979         +0.0979          1263          1387          1264          1387                       
fizzy/execute/ecpairing/onepoint_mean                             +0.0804         +0.0804        484863        523864        484865        523866                       
fizzy/execute/keccak256/512_bytes_rounds_1_mean                   -0.0504         -0.0504           107           102           107           102                       
fizzy/execute/keccak256/512_bytes_rounds_16_mean                  -0.0290         -0.0290          1540          1496          1540          1496                       
fizzy/execute/memset/256_bytes_mean                               +0.1068         +0.1067             8             9             8             9                       
fizzy/execute/memset/60000_bytes_mean                             +0.1202         +0.1202          1587          1778          1587          1778                       
fizzy/execute/mul256_opt0/input0_mean                             +0.1167         +0.1165            28            32            28            32                       
fizzy/execute/mul256_opt0/input1_mean                             +0.1173         +0.1171            28            32            28            32                       
fizzy/execute/sha1/512_bytes_rounds_1_mean                        +0.1152         +0.1151            89            99            89            99                       
fizzy/execute/sha1/512_bytes_rounds_16_mean                       +0.1332         +0.1332          1223          1385          1223          1385                       
fizzy/execute/sha256/512_bytes_rounds_1_mean                      +0.0330         +0.0330            86            88            86            88                       
fizzy/execute/sha256/512_bytes_rounds_16_mean                     +0.0382         +0.0382          1153          1198          1153          1198                       
fizzy/execute/micro/factorial/10_mean                             +0.0075         +0.0069             1             1             1             1                       
fizzy/execute/micro/factorial/20_mean                             +0.0029         +0.0019             2             2             2             2                       
fizzy/execute/micro/fibonacci/24_mean                             -0.0017         -0.0017         14541         14517         14542         14517                       
fizzy/execute/micro/host_adler32/1_mean                           -0.0084         -0.0094             1             1             1             1                       
fizzy/execute/micro/host_adler32/100_mean                         -0.0083         -0.0085             7             7             7             7                       
fizzy/execute/micro/host_adler32/1000_mean                        -0.0083         -0.0084            66            65            66            65                       
fizzy/execute/micro/spinner/1_mean                                -0.0027         -0.0036             0             0             0             0                       
fizzy/execute/micro/spinner/1000_mean                             +0.0595         +0.0598            11            12            11            12

@chfast chfast requested review from axic and gumb0 May 27, 2020 09:37
axic
axic reviewed May 27, 2020
View reviewed changes
lib/fizzy/execute.cpp
{
assert(idx <= locals.size());
return locals[idx];
if (idx < args.size())
Copy link
Member

@axic axic May 27, 2020

Choose a reason for hiding this comment

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

Without making any further optimisations, isn't this making it slower? Previously we copied arguments once.

Copy link
Collaborator Author

@chfast chfast May 27, 2020

Choose a reason for hiding this comment

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

Much slower. See description.

@gumb0
Copy link
Collaborator

gumb0 commented May 27, 2020

Looks fine, but we could merge it together with #347

@axic
Copy link
Member

axic commented May 27, 2020

Even with #347 it has some speed regression.

@chfast
Copy link
Collaborator Author

chfast commented Jun 8, 2020

This is not going to be needed.

@chfast chfast closed this Jun 8, 2020
@axic
Copy link
Member

axic commented Sep 3, 2020

Follow up in #358 and #382.

@axic axic deleted the split_args_and_locals branch September 3, 2020 21:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@axic axic axic left review comments

@gumb0 gumb0 Awaiting requested review from gumb0

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@chfast @gumb0 @axic