From what I see, the PR contains two changes: One, changing int out for a long, on the basis that it’s more consistent with the functions being used, which is true enough, but it’s simultaneously less consistent with CDDA’s codebase, which uses ints all over the place. I suppose this sates Muffin’s personal sense of order, but doesn’t fix anything and as jbytheway points out, may cause problems on some platforms. I don’t see a pressing need for that change, but it’s also a minor detail.
The big change is to swap out the RNG to a mersenne twister, on what I can only see as the blind ass-pull assumption that the RNG is causing slowness. I gotta say, Kevin’s absolutely right on this, the right way to fix slowness is to first profile your code and figure out where the problem actually is. (In my experience, it’s often in a place you never would have guessed.) Then having identified it, you can work on the actual source of the problem without wasting time and potentially introducing bugs by working on things that won’t help.
Just throwing popular algorithms into your codebase willy-nilly because somebody told you they were fast, and hoping for the best, is absolutely the wrong way to go about optimization.
Asking for benchmarks to show that this would help isn’t exactly a high bar to hurdle, either. Any n00b coder with an afternoon free can run a benchmark. It’s not rocket science or even a big ask. Maybe the mersenne twister would help, maybe not, but we don’t know without some numbers to show what it will do.
Besides all of that, if you think that post of yours wasn’t insulting or likely to result in censure, you’ve got way bigger problems than your buddy’s PR getting denied.
