Use Slice literals in Crypto::Bcrypt#15781
Use Slice literals in Crypto::Bcrypt#15781straight-shoota merged 2 commits intocrystal-lang:masterfrom
Slice literals in Crypto::Bcrypt#15781Conversation
| @s = uninitialized UInt32[1024] | ||
|
|
||
| @p.to_slice.copy_from(P.to_slice) | ||
| @s.to_slice.copy_from(S.to_slice) |
There was a problem hiding this comment.
Suggestion: I guess we should do the same for CYPHER_TEXT in Crypto::Bcrypt#hash_password, to avoid allocating a Slice?
There was a problem hiding this comment.
It's just 24 bytes, I don't think that would have any observable impact on build or run times
There was a problem hiding this comment.
It's still a pointless call to the GC at runtime.
There was a problem hiding this comment.
I'd consider this a regression.
Previously, CIPHER_TEXT was a static array, so dup didn't have any effect. It would already be copied to the stack anyway.
Now that we change CIPHER_TEXT to a slice, dup would introduce a new (and unnecessary) heap allocation.
There was a problem hiding this comment.
We could probably leave this one as a StaticArray then. We could even construct the array in the method body itself to remove the constant initializer overhead.
There was a problem hiding this comment.
Indeed, we don't need the constant.
There was a problem hiding this comment.
We don't need it. But it's still nice to put explicit names on things. And to extract the data definition out of the algorithm.
We technically don't need P and S either. But I don't think we would like to inline them.
The constant initializer overhead should go away with true slice literals.
straight-shoota
changed the title
Crypto::BcryptSlice literals in Crypto::Bcrypt
3 participants
These constants are all private and thus are allowed to go from
StaticArraytoSlicetransparently.