search

kazu-yamamoto / crypton

lowlevel set of cryptographic primitives for haskell
Other
25 stars 16 forks source link

Consider marking suitlble FFI calls as `unsfe` #14

Closed TheKK closed 1 year ago

TheKK commented 1 year ago

Please correct me if I'm wrong or this is not a real issue 🙇

According to this section, we know that

which implies that we only pass pinned object to safe FFI currently. And about safe

which is not the use cases in Crypton, since we only use FFI to run CPU instructions and never pass callback to or run Haskell calls in cbits (at least for Cipher & BlockCipher as far as I know).

Does that mean we're safe to mark most of FFI as unsafe and hope it'll gain more performance?

One potential downside and discussion was documented here:

when you call an unsafe function, and a global garbage collection is triggered in another thread (...), the garbage collector cannot proceed

a foreign call to a function that is guaranteed to take a short time, and does not call back into Haskell can be marked unsafe. This works both for the single-threaded and the multi-threaded runtime.

It would be great to hear your opinion or suggestions :)

kazu-yamamoto commented 1 year ago

Is this about crypton or a general discussion?

crypton uses unsafe calls only, I believe.

unsafe can be used if the foreign code does not block. If it blocks, the OS(native) thread running a Haskell thread blocks.

If you use safe, another OS thread is spawned to run the foreign code. Thus, the original OS thread does not block.

TheKK commented 1 year ago

This is crypton specific discussion.

Take https://github.com/kazu-yamamoto/crypton/blob/master/Crypto/Cipher/AES/Primitive.hs#L596 for instance, it doesn't specify its safeness so would default to safe. And I did observe performance difference of this single call between safe and unsafe version using Criterion, and found that unsafe version is about 200% faster on my environment.

kazu-yamamoto commented 1 year ago

Ah, safe is omitted. That's why I cannot find them. I guess we can add unsafe without problems. Would you kindly send a PR?

TheKK commented 1 year ago

Sure, I'm glad to help!

I also plan to run (and add some if they do not exist) benchmark to make sure I don't break things. It would be good to know if there's anything I could do to make this change safer, I'm happy to apply them.

kazu-yamamoto commented 1 year ago

I have no idea on safety in this issue.

arybczak commented 1 year ago

FYI this needs some care. Unsafe calls should only be made with functions that take a bounded, short time to run, as they prevent the capability they're run at from from context switching to a different Haskell thread while they run.

TheKK commented 1 year ago

Sorry for late update.

After some experiments, I observed that marking all FFI unsafe do slow some operations down, and only few of them have slightly improvement. I also agree with @arybczak 's kind reminder, most hash/crypto functions take buffer then operate on it, which the execution time heavily depends on the length of buffer thus not suitable for unsafe FFI.

So we could close issue now. Thanks for you guy's sharing, I learn a lot from them :)