pgsql: Add assert checking to pg_leftmost_one_pos32() and friends

Started by John Naylorover 3 years ago4 messagescomitters
Jump to latest
#1John Naylor
john.naylor@enterprisedb.com

Add assert checking to pg_leftmost_one_pos32() and friends

Discussion: /messages/by-id/CAFBsxsEPc+FnX_0vmmQ5DHv60sk4rL_RZJ+MD6ei=76L0kFMvA@mail.gmail.com

Branch
------
master

Details
-------
https://git.postgresql.org/pg/commitdiff/204b0cbecb82ab3fde2e12998a89e7227cd64094

Modified Files
--------------
src/include/port/pg_bitutils.h | 86 +++++++++++++++++++++++++++++-------------
1 file changed, 60 insertions(+), 26 deletions(-)

#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: John Naylor (#1)
Re: pgsql: Add assert checking to pg_leftmost_one_pos32() and friends

John Naylor <john.naylor@postgresql.org> writes:

Add assert checking to pg_leftmost_one_pos32() and friends

I can see that this was worth writing for testing purposes, but
is it really worth carrying permanently? Even in a debug build,
the ratio of cycles expended to chances of finding a problem seems
mighty poor, and you've done a lot of damage to the readability
of these functions too.

Maybe we could condition the duplicate computation on some
additional not-commonly-defined macro? That'd do little for the
readability issue though.

regards, tom lane

#3John Naylor
john.naylor@enterprisedb.com
In reply to: Tom Lane (#2)
Re: pgsql: Add assert checking to pg_leftmost_one_pos32() and friends

On Mon, Feb 20, 2023 at 10:17 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:

John Naylor <john.naylor@postgresql.org> writes:

Add assert checking to pg_leftmost_one_pos32() and friends

I can see that this was worth writing for testing purposes, but
is it really worth carrying permanently? Even in a debug build,
the ratio of cycles expended to chances of finding a problem seems
mighty poor, and you've done a lot of damage to the readability
of these functions too.

That's a fair point, and it's doubtful we'll need to add another platform
anytime soon. I'll work on removing the asserts.

--
John Naylor
EDB: http://www.enterprisedb.com

#4John Naylor
john.naylor@enterprisedb.com
In reply to: John Naylor (#3)
Re: pgsql: Add assert checking to pg_leftmost_one_pos32() and friends

On Tue, Feb 21, 2023 at 11:59 AM John Naylor <john.naylor@enterprisedb.com>
wrote:

On Mon, Feb 20, 2023 at 10:17 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:

John Naylor <john.naylor@postgresql.org> writes:

Add assert checking to pg_leftmost_one_pos32() and friends

I can see that this was worth writing for testing purposes, but
is it really worth carrying permanently? Even in a debug build,
the ratio of cycles expended to chances of finding a problem seems
mighty poor, and you've done a lot of damage to the readability
of these functions too.

That's a fair point, and it's doubtful we'll need to add another platform

anytime soon. I'll work on removing the asserts.

The attached is closer to the previous coding and passes CI. I'll indent
and push this tomorrow after giving it another look, unless there is
further review.

--
John Naylor
EDB: http://www.enterprisedb.com

Attachments:

remove-asserts-bitscan.patchtext/x-patch; charset=US-ASCII; name=remove-asserts-bitscan.patchDownload+56-93