Feature to use unbounded weights #1081
Conversation
|
@mbudiu-vmw, you must sign every commit in this pull request acknowledging our Developer Certificate of Origin before your changes are merged. This can be done by adding
|
|
Fixes #878 |
|
Overheads seem to be on the order of 40%-50%, more than I expected. |
|
@mbudiu-vmw, you must sign every commit in this pull request acknowledging our Developer Certificate of Origin before your changes are merged. This can be done by adding
|
The value of the weight doesn't have a lot to do with it here, if it does indeed spike into heap integers that will be a significant performance penalty, but there's also a massive baseline penalty because the unbounded weight type is at minimum 24 bytes of stack space (probably 25 though, meaning that with alignment it's realistically more like 32 bytes), meaning that every arrangement, stream, collection and vector is now significantly larger than it was, plus the possibility of having a heap integer completely disables any chance the compiler has of auto-vectorizing code, meaning that we can't take advantage of any instruction-level parallelism with anything that previously was |
|
If this is the best we can do in Rust maybe we should just implement this in C++ and wrap it in Rust. |
|
There won't be any difference in C++ |
|
@mbudiu-vmw, you must sign every commit in this pull request acknowledging our Developer Certificate of Origin before your changes are merged. This can be done by adding
|
|
|
||
| ### Supported features | ||
|
|
||
| The benchmarks can be run using checked_weights, which will cause a | ||
| Rust panic on weight overflow. This can be done by running: | ||
|
|
||
| ```sh | ||
| cargo make benchmarks --features checked_weights | ||
| ``` | ||
|
|
||
| The benchmarks can be run using unbounded_weights, which will always | ||
| produce correct results, but may run slower. This can be done by running: | ||
|
|
||
| ```sh | ||
| cargo make benchmarks --features unbounded_weights | ||
| ``` |
There was a problem hiding this comment.
Put this chunk above the links
|
Please mark comments as resolved once you've addressed them |
|
Often github resolves the comments by itself |
This is a feature for the generated rust code.
When enabling this feature the code will always produce correct results.
Unlike the
checked_weightsfeature, this feature will get slower when weights are larger instead of panicking.Panics are still possible if the weights are read and expected to fit into 64 bits and they don't. This is currently done by ThreadUpdateHandler.
I will post additional information about the slowdown this is causing on benchmarks.
However, I believe this (or a similar feature) should be merged. Correctness is more important than performance.