faster rand(::UnitRange{Bool})

[rfourquet]

This becomes about twice as fast on my machine. The motivation is to work-around the regression I created in #25058.

[rfourquet]

@nanosoldier runbenchmarks("random", vs=":master")

[nanosoldier]

Your benchmark job has completed - no performance regressions were detected. A full report can be found here. cc @ararslan

[stevengj]

Is rand(false:true) really important to optimize? Shouldn't we just encourage people to use rand(Bool)?

I ask because this PR seems to add a lot of code to optimize a weird use-case.

[StefanKarpinski]

Agree, this seems like a very odd corner case.

[rfourquet]

I never used this rand(::UnitRange{Bool}) thing, but clearly it should not be used in place of rand(Bool). Rather, it's for the case where the range is a dynamic value with unknown length at compile time. As I said, my motivation was to compensate for the performance regression I introduce somewhere else (because it was annoying to maintain where it was), and I don't want to be the one to make things slower if I can. Feel free to close if you think it should be; just know that, as this PR is improving on master, this implies a combined regression of at least about 2.70 (i.e. almost 3 times as slow) 😉 Otherwise I will merge eventually.

[stevengj]

Yes, but what is the use case for a dynamic value on UnitRange{Bool}? It seems like in any case where you'd need this, you'd be better off passing one of () -> rand(Bool), () -> true, or () -> false.

I don't doubt that this PR makes rand(::UnitRange{Bool}) significantly faster—you can always speed things up by writing specialized code for weird cases. But speed improvements for (apparently) unimportant use cases are not worth significant increases in code complexity.

[rfourquet]

Yes, but what is the use case for a dynamic value on UnitRange{Bool}

I don't know, I never needed that.

It seems like in any case where you'd need this, you'd be better off passing one of () -> rand(Bool), () -> true, or () -> false.

I don't think so, at least in a case where e.g. you get to Bool values aand b as a function parameter, and you need rand(a:b). What you propose would look like a == b ? a : rand(Bool), but rand!(X, a:b) would have to be something like X .= (() -> a == b ? a : rand(Bool)).() or something like this. But hard to say without a real use-case.

[stevengj]

The fact that it is hard to come up with a real use-case for rand(::UnitRange{Bool}) suggests that optimization may be premature.

(I'm not opposed to a bit of defensive pre-optimization in the standard library as long as it doesn't sacrifice much in terms of code size or complexity. But that doesn't seem to be the case here.)

[rfourquet]

Ok, let's re-open this if a convincing use-case pops-up.