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

bn: introduce in-place pow, .ipow() [WIP] #121

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

bn: introduce in-place pow, .ipow() [WIP] #121

wants to merge 1 commit into from

Conversation

axic
Copy link
Contributor

@axic axic commented Feb 4, 2016

This changes pow() to be in-place and adds the usual wrapper.

Also added a test case for the special 0 power case.

@indutny
Copy link
Owner

indutny commented Feb 4, 2016

Benchmarks? Last time I checked it - in-place multiplication was a bit slower than regular one. This is why I haven't used it in elliptic.

@axic
Copy link
Contributor Author

axic commented Feb 4, 2016

Perhaps the benchmarks aren't the best. The code is:

var x = new bn(13);

add('pow', {
  'bn.js pow': function (fixture) {
    fixture.a1.clone().pow(x);
  },
  'bn.js ipow': function (fixture) {
    fixture.a1.clone().ipow(x);
  },

The results are:

new:
bn.js#pow x 7,371 ops/sec ±1.31% (9 runs sampled)
bn.js ipow#pow x 7,072 ops/sec ±4.80% (7 runs sampled)
bn.js#pow x 6,836 ops/sec ±7.52% (7 runs sampled)
bn.js ipow#pow x 6,955 ops/sec ±4.15% (7 runs sampled)
bn.js#pow x 7,504 ops/sec ±3.62% (9 runs sampled)
bn.js ipow#pow x 7,451 ops/sec ±2.85% (9 runs sampled)

old:
bn.js#pow x 7,475 ops/sec ±3.68% (9 runs sampled)
bn.js#pow x 6,838 ops/sec ±5.94% (9 runs sampled)
bn.js#pow x 6,669 ops/sec ±4.90% (9 runs sampled)
bn.js#pow x 7,335 ops/sec ±3.60% (9 runs sampled)

I do see a bigger difference between mul & imul on its own. That should be documented though if it cannot be improved. One would assume the in-place versions are faster.

@axic axic changed the title bn: introduce in-place pow, .ipow() bn: introduce in-place pow, .ipow() [WIP] Feb 4, 2016
@indutny
Copy link
Owner

indutny commented Feb 4, 2016

Yeah, I was even thinking about removing them at all. What's the point of having in-place functions if there are not faster? They are definitely not easier, and not safer to use.

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

Successfully merging this pull request may close these issues.

3 participants