#ifndef before #define BN_ARRAY_SIZE

[umnikos]

I want to use this as a submodule but I need numbers larger than 1024 bits

[umnikos]

As no feedback has been given and I needed these features I will use my own fork.

[kokke]

Hi @umnikos and thanks for your interest in this project :)

I'm sorry for keeping you waiting :( Like everyone else I also have other obligations, which is why I'm sometimes super slow to react to these issues and PRs. You are of course free to use your own fork if you do not have the patience to wait for me :)

I see two main changes in your PR:

1. ifndef before define BN_ARRAY_SIZE
2. Karatsuba multiplication

---

1) ifndef before define BN_ARRAY_SIZE

I don't think your suggestion works well here, if what you want is parametric integer size.

The problem with using an ifndef-guard, is that unless you explicitly #define the array-size everywhere you include the header file / bn.h (which also mean in bn.c, which you will have to edit as well) you will have a default array-size that you can override, but which will silently return to the default. No errors will be thrown if you inadvertently #define two different array-sizes.

I.e. it makes it possible to have the c-file compiled with one array size (i.e. one integer max-size), and the header-file indicating otherwise.

I think it would be better to refactor the code such that the array-sizes are parametric. E.g. converting

void bignum_add(struct bn* a, struct bn* b, struct bn* c); /* c = a + b */

to

void bignum_add(struct bn* a, struct bn* b, struct bn* c, size_t array_size);

... or something similar.

If you only need fixed-size integers all the time, it should be possible to use macros

#define    bn_add(a, b)    bignum_add(a, b, FIXED_ARRAY_SIZE)

How do you like this approach instead?

---

2) Karatsuba multiplication

I've dabbled with karatsuba multiplication in the past, but did not get anywhere near theoretical speed-ups in practice. Have you by chance benchmarked the difference in run-time between naiive multiplication and the karatsuba-method?

[umnikos]

_1) ifndef before define BN_ARRAYSIZE Your concern is very valid and it turns out I didn't actually test if this part works (and it doesn't). Whoops. The alternatives you suggested are worse in my opinion. As the user I can easily make all of my files import from my own file bn_prelude.h which just imports bn.h but defines the array size first and it's just 3 lines of code in the end.

2) Karatsuba multiplication I don't know if this particular implementation is the theoretical maximum speed (recursion is fundamentally slow) but it's noticeably faster (just looking at it it's at least 2x faster) than the regular multiplication you had before so I consider it a big success. For non-big numbers it also performs as fast as normal multiplication since there's a check for that particular case (it serves as a base case for the recursion)

I can open a pull request with only the karatsuba multiplication if you want :)

[kokke]

I can open a pull request with only the karatsuba multiplication if you want :)

Please do. I consider keeping the naiive implementation, and letting a compile-time switch (#define-macro) decide. In some environments recursion is verboten, so it'd be nice to have the choice.

While meddling with multiplication, have you seen #11 ? I never got around to taking proper care of that PR either ;(

[umnikos]

I took a quick look at #11 and it's still a regular O(n²) algorithm. I also took a look at your exponentiation algorithm and it's very bad, so expect another pull request from me :)

[kokke]

so expect another pull request from me :)

:laughing: exactly what I was hoping :+1: :+1: :+1:

Please keep in mind that this code is (also) aimed at resource-constrained resources. Many optimizations make use of plenty temporary variables, each allocating potentially hundreds of bytes - many small devices have 2-4kb stack at most.

If possible, I would prefer to have two functions to choose from at compile time, as mentioned above :)

[umnikos]

The pull request is open if you haven't seen it. Also I did some digging and discovered -Dname=value is a way to pass preprocessor definitions while compiling as a parameter and it seems to work with my ifndef thing.

[umnikos]

Today I learned about .patch files and how they would be way better suited to do this than my #ifndef and forked repo hack. Oh well...