jlouis
commented
7 years ago the CPU_BOUND entry is necessary in order to use dirty schedulers for this. In less than a month, Erlang/OTP 20.0 lands and it enables dirty schedulers by default. Thus the macro is probably not needed.
Indentation is a bit in shambles. I'd just pull the stuff and then reindent everything to match though.
One particular design decision is when to push things to dirty schedulers. This library evaluates the size of its arguments and runs small jobs on the main scheduler while it outsources larger jobs to a dirty sched in order to avoid stalling the main scheduler for too long. We'd need to measure the added routines for this.
This project has a QuickCheck test model which verifies the different parts of the code is operating correctly. We should probably add these functions to the verification suite.
I have yet to read the details of the C code, but it looks like you also got rid of some of the warnings produced by e.g., clang, which is awfully nice.
All in all, nice set of changes, though I'd like to avoid pulling the CPU_BOUND macro part as it is going to be standard anyway. If you run a library such as this without enabling dirty schedulers, you risk hosing your Erlang installation in some pretty nasty ways since a NIF call can easily take more than the allowed 1ms.
I have attempted to add NIF's for Argon2 password API and generic hashing API. I use these api's in one of my projects.
The other change I have made is wrt ERL_NIF_DIRTY_JOB_CPU_BOUND constant. I added a macro to selectively include this constant in nif_funcs[] definitions only if the feature is enabled in erlang vm.
I would appreciate any feedback on the changes that I have made.