Re: LWLock optimization for multicore Power machines

From: Alexander Korotkov <a(dot)korotkov(at)postgrespro(dot)ru>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Bernd Helmle <mailings(at)oopsware(dot)de>, Andres Freund <andres(at)anarazel(dot)de>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: LWLock optimization for multicore Power machines
Date: 2017-03-31 10:38:31
Message-ID: CAPpHfdv7PrXat6Uokg6LMxf6e8bEmwhBD2VRWnK-z4ANKDMyrA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox
Thread:
Lists: pgsql-hackers

On Sun, Mar 26, 2017 at 12:29 AM, Alexander Korotkov <
a(dot)korotkov(at)postgrespro(dot)ru> wrote:

> On Sat, Mar 25, 2017 at 11:32 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
>> Alexander Korotkov <a(dot)korotkov(at)postgrespro(dot)ru> writes:
>> > I moved PPC implementation of pg_atomic_fetch_mask_add_u32() into
>> > port/atomics/arch-ppc.h. I also had to declare pg_atomic_uint32 there
>> to
>> > satisfy usage of this type as argument
>> > of pg_atomic_fetch_mask_add_u32_impl().
>>
>> Hm, you did something wrong there, because now I get a bunch of failures:
>>
>> ccache gcc -Wall -Wmissing-prototypes -Wpointer-arith
>> -Wdeclaration-after-statement -Wendif-labels -Wmissing-format-attribute
>> -Wformat-security -fno-strict-aliasing -fwrapv -g -O2
>> -I../../../../src/include -c -o brin.o brin.c
>> In file included from ../../../../src/include/port/atomics.h:123,
>> from ../../../../src/include/utils/dsa.h:17,
>> from ../../../../src/include/nodes/tidbitmap.h:26,
>> from ../../../../src/include/access/genam.h:19,
>> from ../../../../src/include/nodes/execnodes.h:17,
>> from ../../../../src/include/access/brin.h:14,
>> from brin.c:18:
>> ../../../../src/include/port/atomics/generic.h:154:3: error: #error "No
>> pg_atomic_test_and_set provided"
>> ../../../../src/include/port/atomics.h: In function
>> 'pg_atomic_init_flag':
>> ../../../../src/include/port/atomics.h:178: warning: implicit
>> declaration of function 'pg_atomic_init_flag_impl'
>> ../../../../src/include/port/atomics.h: In function
>> 'pg_atomic_test_set_flag':
>> ../../../../src/include/port/atomics.h:193: warning: implicit
>> declaration of function 'pg_atomic_test_set_flag_impl'
>> ../../../../src/include/port/atomics.h: In function
>> 'pg_atomic_unlocked_test_flag':
>> ../../../../src/include/port/atomics.h:208: warning: implicit
>> declaration of function 'pg_atomic_unlocked_test_flag_impl'
>> ... and so on.
>>
>> I'm not entirely sure what the intended structure of these header files
>> is. Maybe Andres can comment.
>>
>
> It seems that on this platform definition of atomics should be provided by
> fallback.h. But it doesn't because I already defined PG_HAVE_ATOMIC_U32_SUPPORT
> in arch-ppc.h. I think in this case we shouldn't provide ppc-specific
> implementation of pg_atomic_fetch_mask_add_u32(). However, I don't know
> how to do this assuming arch-ppc.h is included before compiler-specific
> headers. Thus, in arch-ppc.h we don't know yet if we would find
> implementation of atomics for this platform. One possible solution is to
> provide assembly implementation for all atomics in arch-ppc.h.
>

BTW, implementation for all atomics in arch-ppc.h would be too invasive and
shouldn't be considered for v10.
However, I made following workaround: declare pg_atomic_uint32 and
pg_atomic_fetch_mask_add_u32_impl() only when we know that generic-gcc.h
would declare gcc-based atomics.
Could you, please, check it on Apple PPC?

------
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

Attachment Content-Type Size
lwlock-power-5.patch application/octet-stream 7.9 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2017-03-31 10:48:07 Re: Parallel query execution with SPI
Previous Message Pierre Ducroquet 2017-03-31 10:01:01 Re: Other formats in pset like markdown, rst, mediawiki