Re: [PATCH 1/6] maintenance: add get_random_minute()

From: Taylor Blau <me@ttaylorr.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: Derrick Stolee via GitGitGadget <gitgitgadget@gmail.com>,
	git@vger.kernel.org, sandals@crustytoothpaste.net,
	lenaic@lhuard.fr, Derrick Stolee <derrickstolee@github.com>
Subject: Re: [PATCH 1/6] maintenance: add get_random_minute()
Date: Tue, 8 Aug 2023 16:01:46 -0400	[thread overview]
Message-ID: <ZNKfKs1mLQhnybvF@nand.local> (raw)
In-Reply-To: <xmqqsf8t8oo5.fsf@gitster.g>

On Tue, Aug 08, 2023 at 09:49:30AM -0700, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
> > Provided if code simplification is a good enough rationale, the
> > patch looks sensible, but I find its use of u64 a bit questionable
> > (though not wrong).  I would have expected that the type of "rand"
> > would be the same as backoff_ms and wait_ms, two variables involved
> > in the same expression.
>
> Ah, not so fast.  The use of modulo means we need to be careful
> about about the fuzzing factor going negative, and use of unsigned
> type allows us to forget about it.
>
> (fuzz % 250), when fuzz is of a signed random integral type, ranges
> between -250 and +250 and because we want the center of our
> distribution at 1000, so I think the following is equivalent.

[...]

> -		wait_ms = (750 + rand % 500) * backoff_ms / 1000;
> +		wait_ms = (1000 + fuzz % 250) * backoff_ms / 1000;

I was going to say that having rand be an unsigned type was the right
behavior, but with this change it can (and should) be signed. Thanks for
the simplification :-).

Thanks,
Taylor