Welcome! Log In Create A New Profile

Advanced

Re: [PATCH] Replaced loop with __builtin_ctzl for detecting first 0 in bitnamp

Maxim Dounin
November 27, 2020 03:18PM
Hello!

On Fri, Nov 27, 2020 at 01:22:46PM -0500, goldstein.w.n@gmail.com wrote:

> # HG changeset patch
> # User Noah Goldstein <goldstein.w.n@gmail.com>
> # Date 1606497081 18000
> # Fri Nov 27 12:11:21 2020 -0500
> # Node ID 7ec2fc7b29d6614df28152dd4a895e6139138890
> # Parent 90cc7194e993f8d722347e9f46a00f65dffc3935
> Replaced loop with __builtin_ctzl for detecting first 0 in bitnamp
> No particular pressing reason for this change other than the performance benefit it yields. Converts a loop that could take 63 iterations (and had a branch) to 5 instructions
>
> diff -r 90cc7194e993 -r 7ec2fc7b29d6 src/core/ngx_slab.c
> --- a/src/core/ngx_slab.c Fri Nov 27 00:01:20 2020 +0300
> +++ b/src/core/ngx_slab.c Fri Nov 27 12:11:21 2020 -0500
> @@ -235,36 +235,33 @@
>
> if (bitmap[n] != NGX_SLAB_BUSY) {
>
> - for (m = 1, i = 0; m; m <<= 1, i++) {
> - if (bitmap[n] & m) {
> - continue;
> + m = ~bitmap[n];
> + i = __builtin_ctzl(m);

Thank you for the patch.

Unfortunately, __builtin_ctzl() is not portable and hence the
patch cannot be committed for obvious reasons. Further, even if
__builtin_ctzl() is available, there no guarantees that it can be
used on an uintptr_t variable, as uintptr_t can be larger than
long (notably, 64bit Windows uses LLP64 data model).

Also note that there are other similar loops in the various places
of the code, and changing just one would certainly confuse
readers.

Note well that slab allocations are expected to be rare, and this
loop is not expected to be performance-critical. Most performance
impact on slab allocations are expected to be from shared mutex
locking.

If you have practical reasons to assume this code needs to be
optimized, please share the details. If it indeed needs to, we
can consider making a portable solution.

--
Maxim Dounin
http://mdounin.ru/
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel
Subject Author Views Posted

[PATCH] Replaced loop with __builtin_ctzl for detecting first 0 in bitnamp

Anonymous User 112 November 27, 2020 01:24PM

Re: [PATCH] Replaced loop with __builtin_ctzl for detecting first 0 in bitnamp

Maxim Dounin 35 November 27, 2020 03:18PM



Sorry, you do not have permission to post/reply in this forum.

Online Users

Guests: 62
Record Number of Users: 6 on February 13, 2018
Record Number of Guests: 421 on December 02, 2018
Powered by nginx      Powered by FreeBSD      PHP Powered      Powered by MariaDB      ipv6 ready