prb-math icon indicating copy to clipboard operation
prb-math copied to clipboard

Published 20 hours ago • verified publisher icon PaulRBerg

Slightly gas saving in avg function

Open atarpara opened this issue 3 years ago • 4 comments

I remove the unnecessary operation in avg function.

atarpara avatar May 04 '22 16:05 atarpara

Hi @Atarpara! Thanks for your contribution. Unfortunately, I cannot merge this. In fact, I cannot even review it:

  • The code does not compile, it fails with the error "ParserError: Expected ';' but got '}'"
  • The linter job does not pass (see CI logs) (make sure to install the Prettier extension for VSCode)
  • This PR does not include an explanation for why the proposed change brings about a reduction in gas costs while simultaneously keeping the behavior of the avg function the same across all possible inputs and outputs

PaulRBerg avatar May 09 '22 08:05 PaulRBerg

@paulrberg I solved the compiler error.

atarpara avatar May 11 '22 06:05 atarpara

Hi @Atarpara, thanks for that. Would you mind explaining how your gas saving works?

PaulRBerg avatar May 11 '22 08:05 PaulRBerg

Reduce the Number of Operation

Old Way: (x >> 1) + (y >> 1) + (x & y & 1) - Need 6 Operation

New Way : (x & y) + ((x^y) >> 1) - Need 4 Operation

More Info About is Here.

atarpara avatar May 20 '22 05:05 atarpara

Hello @Atarpara, I'm sorry for the delayed response.

I ran a few tests and indeed you are right - your approach is faster than mine.

Happy to accept this PR, but could I kindly ask you to re-apply your changes onto the tip of the staging branch, git push with force to your branch, and then make staging the base of this PR?

Thank you!

PaulRBerg avatar Nov 25 '22 18:11 PaulRBerg

Hi @Atarpara, I'm sorry, I accidentally closed your PR.

But I managed to implement your proposal, and credit you as a git author in commit https://github.com/paulrberg/prb-math/commit/a490c088c5de0564d48064b0c2a0647ab9d4299c.

Thanks again for your contribution.

PaulRBerg avatar Nov 28 '22 10:11 PaulRBerg