optimize stack size requirements of reed_solomon_decode() #3

Merged

benpicco merged 2 commits from stack_opt into master

2022-02-25 19:43:13 -06:00

benpicco commented

2022-02-25 04:14:13 -06:00

(Migrated from github.com)

I noticed that reed_solomon_decode() would consume quite a lot of stack space due to the 3 DATA_SHARDS_MAX sized int buffers (3060 bytes).

It looks like the range of uint8_t is sufficient for these, which already quarters the stack size required.
Then if the data shard number is known at compile time, we can also select a lower value for DATA_SHARDS_MAX - make that configurable via CFLAGS.

I noticed that `reed_solomon_decode()` would consume quite a lot of stack space due to the 3 `DATA_SHARDS_MAX` sized `int` buffers (3060 bytes). It looks like the range of `uint8_t` is sufficient for these, which already quarters the stack size required. Then if the data shard number is known at compile time, we can also select a lower value for `DATA_SHARDS_MAX` - make that configurable via `CFLAGS`.

sleepybishop (Migrated from github.com) reviewed

2022-02-25 12:30:00 -06:00

rs.h

I would suggest leaving this as is as it's intention is to inform the maximum shards that can be used in GF2^8, to further reduce the size of the permutation vectors in reed_solomon_decode() maybe a new define needs to introduced or maybe use vla's there, though that's problematic for embedded targets as well

#define DATA_SHARDS_MAX 255

I would suggest leaving this as is as it's intention is to inform the maximum shards that can be used in `GF2^8`, to further reduce the size of the permutation vectors in `reed_solomon_decode()` maybe a new define needs to introduced or maybe use vla's there, though that's problematic for embedded targets as well ```suggestion #define DATA_SHARDS_MAX 255 ```

benpicco (Migrated from github.com) reviewed

2022-02-25 13:14:38 -06:00

rs.h

Another approach would be allocating the array in reed_solomon_new() but that would inflict unnececary cost on the encode-only path.

Ok but I would prefer the VLA version then. Shard number is not so variable/would have to be sanitized if it comes from the network anyway and the code is only called once per shard ensemble, so the VLA cost is negligible (but I am too not a fan). Another approach would be allocating the array in `reed_solomon_new()` but that would inflict unnececary cost on the encode-only path.

sleepybishop (Migrated from github.com) reviewed

2022-02-25 19:42:37 -06:00

rs.h

Another approach would be allocating the array in reed_solomon_new() but that would inflict unnececary cost on the encode-only path.

Agree, that was the reasoning behind _decode being the way it is. Let's go with VLA.

> Another approach would be allocating the array in `reed_solomon_new()` but that would inflict unnececary cost on the encode-only path. Agree, that was the reasoning behind `_decode` being the way it is. Let's go with VLA.