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

Pass arguments to host function as const Value* #574

Merged
merged 2 commits into from
Oct 14, 2020
Merged

Conversation

chfast
Copy link
Collaborator

@chfast chfast commented Oct 5, 2020

No description provided.

@codecov
Copy link

codecov bot commented Oct 5, 2020

Codecov Report

Merging #574 into master will increase coverage by 0.00%.
The diff coverage is 80.64%.

@@           Coverage Diff           @@
##           master     #574   +/-   ##
=======================================
  Coverage   98.26%   98.26%           
=======================================
  Files          63       63           
  Lines        9254     9238   -16     
=======================================
- Hits         9093     9078   -15     
+ Misses        161      160    -1     

Base automatically changed from fix_invoke to master October 5, 2020 12:05
@chfast chfast mentioned this pull request Oct 5, 2020
@axic axic added the optimization Performance optimization label Oct 9, 2020
@chfast
Copy link
Collaborator Author

chfast commented Oct 13, 2020

fizzy/execute/blake2b/512_bytes_rounds_1_mean                     -0.0466         -0.0466            87            83            87            83
fizzy/execute/blake2b/512_bytes_rounds_16_mean                    -0.0469         -0.0469          1316          1254          1316          1254
fizzy/execute/ecpairing/onepoint_mean                             -0.0572         -0.0572        410735        387251        410738        387254
fizzy/execute/keccak256/512_bytes_rounds_1_mean                   -0.0288         -0.0288           101            98           101            98
fizzy/execute/keccak256/512_bytes_rounds_16_mean                  -0.0370         -0.0370          1503          1447          1503          1447
fizzy/execute/memset/256_bytes_mean                               -0.1201         -0.1201             7             6             7             6
fizzy/execute/memset/60000_bytes_mean                             -0.1262         -0.1262          1599          1397          1599          1397
fizzy/execute/mul256_opt0/input1_mean                             -0.1268         -0.1268            29            25            29            25
fizzy/execute/ramanujan_pi/33_runs_mean                           -0.0470         -0.0470           131           125           131           125
fizzy/execute/sha1/512_bytes_rounds_1_mean                        -0.1235         -0.1235            94            83            94            83
fizzy/execute/sha1/512_bytes_rounds_16_mean                       -0.1257         -0.1257          1314          1149          1314          1149
fizzy/execute/sha256/512_bytes_rounds_1_mean                      -0.1704         -0.1704           100            83           100            83
fizzy/execute/sha256/512_bytes_rounds_16_mean                     -0.1655         -0.1655          1367          1141          1367          1141
fizzy/execute/taylor_pi/pi_1000000_runs_mean                      -0.0126         -0.0126         40540         40028         40541         40028
fizzy/execute/micro/eli_interpreter/exec105_mean                  -0.0957         -0.0957             5             4             5             4
fizzy/execute/micro/factorial/20_mean                             -0.0808         -0.0808             1             1             1             1
fizzy/execute/micro/fibonacci/24_mean                             -0.0652         -0.0652          5230          4889          5230          4889
fizzy/execute/micro/host_adler32/1_mean                           -0.0139         -0.0139             0             0             0             0
fizzy/execute/micro/host_adler32/1000_mean                        -0.0088         -0.0088            29            29            29            29
fizzy/execute/micro/spinner/1_mean                                -0.0751         -0.0751             0             0             0             0
fizzy/execute/micro/spinner/1000_mean                             -0.1015         -0.1015            10             9            10             9

@chfast chfast requested review from axic and gumb0 October 13, 2020 19:48
@chfast chfast marked this pull request as ready for review October 13, 2020 19:48
@axic
Copy link
Member

axic commented Oct 13, 2020

The ecpairing change which should not be affected just shows we're still experiencing the same measurement/optimisation problem.

@@ -486,7 +486,7 @@ inline bool invoke_function(
{
const auto num_args = func_type.inputs.size();
assert(stack.size() >= num_args);
span<const Value> call_args{stack.rend() - num_args, num_args};
const auto call_args = stack.rend() - num_args;
Copy link
Member

Choose a reason for hiding this comment

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

Didn't I do the same change in some PR already? Perhaps one of your old refactorings.

Copy link
Member

@axic axic left a comment

Choose a reason for hiding this comment

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

I'd wait for @gumb0's review too.

int depth) noexcept -> fizzy::ExecutionResult {
const auto result = func(context, wrap(&instance), wrap(args.data()), args.size(), depth);
const auto result = func(context, wrap(&instance), wrap(args), 0, depth);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This should be fixed, now or later.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Pushed the fix.

@chfast chfast merged commit 4c2c504 into master Oct 14, 2020
@chfast chfast deleted the host_funcs_args branch October 14, 2020 16:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
optimization Performance optimization
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants