Further speed improvements #8
Conversation
by specializing to the amount of bits rotated, the called function is a one-argument function, so elm does not have to apply the usual F and A wrappers that enable currying
| State (Tuple5 (initial.a + a) (initial.b + b) (initial.c + c) (initial.d + d) (initial.e + e)) | ||
| State | ||
| { a = initial.a + a | ||
| , b = initial.b + b | ||
| , c = initial.c + c | ||
| , d = initial.d + d | ||
| , e = initial.e + e | ||
| } |
There was a problem hiding this comment.
A plain record is much faster than using the alias (which is a function internally, the record literal is really an object literal in JS)
| @@ -373,24 +374,84 @@ So in the recursion, `b16` is dropped, all the others shift one position to the | |||
| Then the `deltaState` is also updated with the `value`. | |||
|
|
|||
| -} | |||
| reduceWords i deltaState b16 b15 b14 b13 b12 b11 b10 b9 b8 b7 b6 b5 b4 b3 b2 b1 = | |||
| if (i - blockSize) < 0 then | |||
| reduceWords i ((DeltaState { a, b, c, d, e }) as deltaState) b16 b15 b14 b13 b12 b11 b10 b9 b8 b7 b6 b5 b4 b3 b2 b1 = | |||
There was a problem hiding this comment.
This function now performs 8 calculations per iteration.That means fewer function calls, but diminishing returns mean that it's not worth it to go up to 16.
| (shiftedA + f + e + int) | ||
| |> Bitwise.shiftRightZfBy 0 |
There was a problem hiding this comment.
This shiftRightZfBy 0 fixes the bug on the large file reported in #5
| rotateLeftBy1 : Int -> Int | ||
| rotateLeftBy1 i = | ||
| -- because of how `rotateLeftBy1` is used, the `Bitwise.shiftRightZfBy 0` is not required | ||
| Bitwise.or (Bitwise.shiftRightZfBy 31 i) (Bitwise.shiftLeftBy 1 i) |
There was a problem hiding this comment.
specializing to shift by 1 is important. This function takes only 1 argument now, an so elm does not need to apply wrappers that enable currying of this function. Much faster.
|
Brilliant, thanks. Love learning about elm perf from you, thanks for all the explanations |
|
Now published as 2.1.1 (cc @BKSpurgeon) |
and also a bug fix, see comments