Re: patch for geqo tweaks

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Nathan Wagner <nw+pg(at)hydaspes(dot)if(dot)org>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: patch for geqo tweaks
Date: 2015-11-06 16:45:38
Message-ID: 20851.1446828338@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Nathan Wagner <nw+pg(at)hydaspes(dot)if(dot)org> writes:
> On Wed, Nov 04, 2015 at 12:51:52PM -0500, Tom Lane wrote:
>> I'm not very impressed with the first patch: it might save a few
>> geqo_randint() calls, but it seems to do so at the price of making the
>> swap choices less random --- for instance it sure looks to me like the
>> last array element is now less likely to participate in swaps than
>> other elements. Unless you can prove that actually the swapping is
>> still unbiased, I'm inclined to reject this part.

> If I have understood the original code correctly, we need to select two
> different random integers between 0 and num_gene-1, inclusive. That
> happens to be num_gene possible results.

> Having chosen the first one, which I will call "swap1", we now only have
> num_gene-1 possible results, which need to range from either 0 to
> swap1-1 or from swap1+1 to num_gene-1, which is num_gene-1 possible
> results. I treat this as a single range from 0 to num_gene-2 and
> generate a number within that range, which I will call "swap2".

> If swap2 is between 0 and swap1-1, it is in the first range, and no
> adjustment is necessary. If it is greater than or equal to swap1, then
> it is in the second range. However the generated swap2 in the second
> range will be between swap1 and num_gene-2, whereas we need it to be
> between swap1+1 and num_gene-1, so I add one to swap2, adjusting the
> range to the needed range.

Ah, after thinking some more, I see how that works. I tend to think
that your other proposal of

swap1 = geqo_randint(root, num_gene - 1, 0);
swap2 = geqo_randint(root, num_gene - 2, 0);
if (swap2 === swap1)
swap2 = num_gene - 1;

would be clearer, since only the forbidden case gets remapped.

However, really the whole argument is moot, because I notice that
geqo_mutation() is only called in the "#ifdef CX" code path, which
we don't use. So there's little point in improving it.

(There's a fair amount of dead code in /geqo/, which I've never had
the energy to clean up, but maybe we should do that sometime. It
seems unlikely that anyone will ever be interested in experimenting
with the ifdef'ed-out code paths.)

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andres Freund 2015-11-06 16:52:35 Re: Re: BUG #13685: Archiving while idle every archive_timeout with wal_level hot_standby
Previous Message Robert Haas 2015-11-06 16:42:56 Re: Re: BUG #13685: Archiving while idle every archive_timeout with wal_level hot_standby