public inbox for [email protected]help / color / mirror / Atom feed
WaitLatchOrSocket optimization 10+ messages / 6 participants [nested] [flat]
* WaitLatchOrSocket optimization @ 2018-03-15 16:01 Konstantin Knizhnik <[email protected]> 2018-03-15 17:25 ` Re: WaitLatchOrSocket optimization Andres Freund <[email protected]> 0 siblings, 1 reply; 10+ messages in thread From: Konstantin Knizhnik @ 2018-03-15 16:01 UTC (permalink / raw) To: pgsql-hackers Hi hackers, Right now function WaitLatchOrSocket is implemented in very inefficient way: for each invocation it creates epoll instance, registers events and then closes this instance. Certainly it is possible to create wait event set once with CreateWaitEventSet and then use WaitEventSetWait. And it is done in most performance critical places. But there are still lot of places in Postgres where WaitLatchOrSocket or WaitLatch are used. One of them is postgres_fdw. If we run pgbench through postgres_fdw (just redirect pgbench tables using postgres_fdw to the localhost), then at the system with large number (72) of CPU cores, "pgbench -S -M prepared -c 100 -j 32" shows performance about 38k TPS with the following profile: - 73.83% 0.12% postgres postgres [.] PostgresMain ▒ - 73.82% PostgresMain ▒ + 28.18% PortalRun ▒ + 26.48% finish_xact_command ▒ + 13.66% PortalStart ▒ + 1.83% exec_simple_query ▒ + 1.22% pq_getbyte ▒ + 0.89% ReadyForQuery ▒ - 66.04% 0.03% postgres [kernel.kallsyms] [k] entry_SYSCALL_64_fastpath ▒ - 66.01% entry_SYSCALL_64_fastpath ▒ + 61.39% syscall_return_slowpath ▒ + 1.52% sys_epoll_create1 ▒ + 1.30% SYSC_sendto ▒ + 0.94% sys_epoll_pwait ▒ 0.57% SYSC_recvfrom ▒ - 65.61% 0.03% postgres postgres_fdw.so [.] pgfdw_get_result ▒ - 65.58% pgfdw_get_result ▒ - 65.00% WaitLatchOrSocket ▒ + 62.60% __close ▒ + 1.62% CreateWaitEventSet ▒ - 65.09% 0.02% postgres postgres [.] WaitLatchOrSocket ▒ - 65.08% WaitLatchOrSocket ▒ + 62.68% __close ▒ + 1.62% CreateWaitEventSet ▒ + 62.69% 0.02% postgres libpthread-2.26.so [.] __close ▒ So, you can see that more than 60% of CPU is spent in close. If we cache used wait event sets, then performance is increased to 225k TPS: five times! At the systems with smaller number of cores effect of this patch is not so large: at my desktop with 4 cores I get just about 10% improvement at the same test. There are two possible ways of fixing this issue: 1. Patch postgres_fdw to store WaitEventSet in connection. 2. Patch WaitLatchOrSocket to cache created wait event sets. Second approach is more generic and cover all cases of WaitLatch usages. Attached patch implements with approach. The most challenging of this approach is using socket descriptor as part of hash code. Socket can be closed be already and reused, so cached epoll set will not work any more. To solve this issue I always try to add socket to the epoll set, ignoring EEXIST error. Certainly it will add some extra overhead, but looks like it is negligible comparing with overhead of close (if I comment this branch, then pgbench performance is almost the same - 227k TPS). But if there are some other arguments against using cache in WaitLatchOrSocket, we have a patch particularly for postgres_fdw. -- Konstantin Knizhnik Postgres Professional: http://www.postgrespro.com The Russian Postgres Company Attachments: [text/x-patch] latch.patch (3.5K, ../../[email protected]/2-latch.patch) download | inline diff: diff --git a/src/backend/storage/ipc/latch.c b/src/backend/storage/ipc/latch.c index e6706f7..b9cdead 100644 --- a/src/backend/storage/ipc/latch.c +++ b/src/backend/storage/ipc/latch.c @@ -51,6 +51,7 @@ #include "storage/latch.h" #include "storage/pmsignal.h" #include "storage/shmem.h" +#include "utils/memutils.h" /* * Select the fd readiness primitive to use. Normally the "most modern" @@ -340,6 +341,15 @@ WaitLatch(volatile Latch *latch, int wakeEvents, long timeout, wait_event_info); } +#define WAIT_EVENT_HASH_SIZE 113 + +typedef struct WaitEventHashEntry +{ + WaitEventSet* set; + int wakeEvents; + pgsocket sock; +} WaitEventHashEntry; + /* * Like WaitLatch, but with an extra socket argument for WL_SOCKET_* * conditions. @@ -359,29 +369,65 @@ WaitLatchOrSocket(volatile Latch *latch, int wakeEvents, pgsocket sock, int ret = 0; int rc; WaitEvent event; - WaitEventSet *set = CreateWaitEventSet(CurrentMemoryContext, 3); + WaitEventSet *set; + static WaitEventHashEntry wait_event_hash[WAIT_EVENT_HASH_SIZE]; + size_t h = (wakeEvents | ((size_t)sock << 6)) % WAIT_EVENT_HASH_SIZE; + set = wait_event_hash[h].set; + if (set == NULL || wait_event_hash[h].wakeEvents != wakeEvents || wait_event_hash[h].sock != sock) + { + if (set) + FreeWaitEventSet(set); + set = CreateWaitEventSet(TopMemoryContext, 3); + wait_event_hash[h].set = set; + wait_event_hash[h].wakeEvents = wakeEvents; + wait_event_hash[h].sock = sock; + + if (wakeEvents & WL_LATCH_SET) + AddWaitEventToSet(set, WL_LATCH_SET, PGINVALID_SOCKET, + (Latch *) latch, NULL); + + if (wakeEvents & WL_POSTMASTER_DEATH && IsUnderPostmaster) + AddWaitEventToSet(set, WL_POSTMASTER_DEATH, PGINVALID_SOCKET, + NULL, NULL); + + if (wakeEvents & WL_SOCKET_MASK) + { + int ev; + + ev = wakeEvents & WL_SOCKET_MASK; + AddWaitEventToSet(set, ev, sock, NULL, NULL); + } + } +#if defined(WAIT_USE_EPOLL) + /* + * Socket descriptor can be closed and reused. + * Closed descriptor is automatically excluded from epoll set, + * but cached epoll set will not correctly work in this case. + * So we will try to always add socket descriptor and ignore EEXIST error. + */ + else if (wakeEvents & WL_SOCKET_MASK) + { + struct epoll_event epoll_ev; + int rc; + epoll_ev.data.ptr = &set->events[set->nevents-1]; + epoll_ev.events = EPOLLERR | EPOLLHUP; + if (wakeEvents & WL_SOCKET_READABLE) + epoll_ev.events |= EPOLLIN; + if (wakeEvents & WL_SOCKET_WRITEABLE) + epoll_ev.events |= EPOLLOUT; + rc = epoll_ctl(set->epoll_fd, EPOLL_CTL_ADD, sock, &epoll_ev); + if (rc < 0 && errno != EEXIST) + ereport(ERROR, + (errcode_for_socket_access(), + errmsg("epoll_ctl() failed: %m"))); + } +#endif if (wakeEvents & WL_TIMEOUT) Assert(timeout >= 0); else timeout = -1; - if (wakeEvents & WL_LATCH_SET) - AddWaitEventToSet(set, WL_LATCH_SET, PGINVALID_SOCKET, - (Latch *) latch, NULL); - - if (wakeEvents & WL_POSTMASTER_DEATH && IsUnderPostmaster) - AddWaitEventToSet(set, WL_POSTMASTER_DEATH, PGINVALID_SOCKET, - NULL, NULL); - - if (wakeEvents & WL_SOCKET_MASK) - { - int ev; - - ev = wakeEvents & WL_SOCKET_MASK; - AddWaitEventToSet(set, ev, sock, NULL, NULL); - } - rc = WaitEventSetWait(set, timeout, &event, 1, wait_event_info); if (rc == 0) @@ -392,9 +438,6 @@ WaitLatchOrSocket(volatile Latch *latch, int wakeEvents, pgsocket sock, WL_POSTMASTER_DEATH | WL_SOCKET_MASK); } - - FreeWaitEventSet(set); - return ret; } ^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: WaitLatchOrSocket optimization 2018-03-15 16:01 WaitLatchOrSocket optimization Konstantin Knizhnik <[email protected]> @ 2018-03-15 17:25 ` Andres Freund <[email protected]> 2018-03-16 08:03 ` Re: WaitLatchOrSocket optimization Konstantin Knizhnik <[email protected]> 0 siblings, 1 reply; 10+ messages in thread From: Andres Freund @ 2018-03-15 17:25 UTC (permalink / raw) To: Konstantin Knizhnik <[email protected]>; +Cc: pgsql-hackers Hi, On 2018-03-15 19:01:40 +0300, Konstantin Knizhnik wrote: > Right now function WaitLatchOrSocket is implemented in very inefficient way: > for each invocation it creates epoll instance, registers events and then > closes this instance. Right, everything performance critical should be migrated to using a persistent wait even set. > But there are still lot of places in Postgres where WaitLatchOrSocket or > WaitLatch are used. Most don't matter performancewise though. > There are two possible ways of fixing this issue: > 1. Patch postgres_fdw to store WaitEventSet in connection. > 2. Patch WaitLatchOrSocket to cache created wait event sets. I'm strongly opposed to 2). The lifetime of sockets will make this an absolute mess, it'll potentially explode the number of open file handles, and some OSs don't handle a large number of sets at the same time. Greetings, Andres Freund ^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: WaitLatchOrSocket optimization 2018-03-15 16:01 WaitLatchOrSocket optimization Konstantin Knizhnik <[email protected]> 2018-03-15 17:25 ` Re: WaitLatchOrSocket optimization Andres Freund <[email protected]> @ 2018-03-16 08:03 ` Konstantin Knizhnik <[email protected]> 0 siblings, 0 replies; 10+ messages in thread From: Konstantin Knizhnik @ 2018-03-16 08:03 UTC (permalink / raw) To: Andres Freund <[email protected]>; +Cc: pgsql-hackers Hi, On 15.03.2018 20:25, Andres Freund wrote: > Hi, > > On 2018-03-15 19:01:40 +0300, Konstantin Knizhnik wrote: >> Right now function WaitLatchOrSocket is implemented in very inefficient way: >> for each invocation it creates epoll instance, registers events and then >> closes this instance. > Right, everything performance critical should be migrated to using a > persistent wait even set. > > >> But there are still lot of places in Postgres where WaitLatchOrSocket or >> WaitLatch are used. > Most don't matter performancewise though. Yes, I didn't see significant difference in performance on workloads not including postgres_fdw. But greeping for all occurrences of WaitLatch I found some places which are used to be called quite frequently, for example waiting for latch in ProcSleep , waiting for more data in shared memory message queue, waiting for parallel workers to attach, waiting for synchronous replica,... There are about hundred of usages of WaitLatch and I am not sure that all of them do not have impact on performance. >> There are two possible ways of fixing this issue: >> 1. Patch postgres_fdw to store WaitEventSet in connection. >> 2. Patch WaitLatchOrSocket to cache created wait event sets. > I'm strongly opposed to 2). The lifetime of sockets will make this an > absolute mess, it'll potentially explode the number of open file > handles, and some OSs don't handle a large number of sets at the same > time. I understand your concern. But you are not completely right here. First of all life time of socket doesn't actually matter: if socket is closed, then it is automatically removed from event set. Cite from epoll manual: Q6 Will closing a file descriptor cause it to be removed from all epoll sets automatically? A6 Yes, but be aware of the following point. A file descriptor is a reference to an open file description (see open(2)). Whenever a file descriptor is duplicated via dup(2), dup2(2), fcntl(2) F_DUPFD, or fork(2), a new file descriptor referring to the same open file descrip- tion is created. An open file description continues to exist until all file descriptors referring to it have been closed. A file descrip- tor is removed from an epoll set only after all the file descriptors referring to the underlying open file description have been closed (or before if the file descriptor is explicitly removed using epoll_ctl(2) EPOLL_CTL_DEL). This means that even after a file descriptor that is part of an epoll set has been closed, events may be reported for that file descriptor if other file descriptors referring to the same underlying file description remain open. So, only file descriptors created by epoll_create affect number of descriptors opened by a process. But this number is limited by size of the cache. Right now I hardcoded cache size 113, but in most cases even much smaller cache will be enough. I do not see that extra let's say 13 open file descriptors can cause open file descriptor limit exhaustion. From my point of view we should either rewrite WaitLatchOrSocket to not use epoll at all (use poll instead), either cache created event set descriptors. Result of pgbench with poll instead epoll is 140k TPS, which is certainly much better than 38k TPS with current master, but much worser than with cached epoll - 225k TPS. > Greetings, > > Andres Freund -- Konstantin Knizhnik Postgres Professional: http://www.postgrespro.com The Russian Postgres Company ^ permalink raw reply [nested|flat] 10+ messages in thread
* initdb / bootstrap design @ 2022-02-16 02:12 Andres Freund <[email protected]> 2022-02-16 03:14 ` Re: initdb / bootstrap design John Naylor <[email protected]> 2022-02-16 10:47 ` Re: initdb / bootstrap design Peter Eisentraut <[email protected]> 2022-02-16 18:24 ` Re: initdb / bootstrap design Tom Lane <[email protected]> 0 siblings, 3 replies; 10+ messages in thread From: Andres Freund @ 2022-02-16 02:12 UTC (permalink / raw) To: pgsql-hackers Hi, [1] reminded me of a topic that I wanted to bring up at some point: To me the division of labor between initdb and bootstrap doesn't make much sense anymore: initdb reads postgres.bki, replaces a few tokens, starts postgres in bootstrap mode, and then painstakenly feeds bootstrap.bki lines to the server. Given that bootstrap mode parsing is a dedicated parser, only invoked from a single point, what's the point of initdb doing the preprocessing and then incurring pipe overhead? Sure, there's a few tokens that we replace in initdb. As it turns out there's only two rows that are actually variable. The username of the initial superuser in pg_authid and the pg_database row for template 1, where encoding, lc_collate and lc_ctype varies. The rest is all compile time constant replacements we could do as part of genbki.pl. It seems we could save a good number of context switches by opening postgres.bki just before boot_yyparse() in BootstrapModeMain() and having the parser read it. The pg_authid / pg_database rows we could just do via explicit insertions in BootstrapModeMain(), provided by commandline args? Similarly, since the introduction of extensions at the latest, the server knows how to execute SQL from a file. Why don't we just process information_schema.sql, system_views.sql et al that way? If we don't need a dedicated "input" mode feeding boot_yyparse() in bootstrap mode anymore (because bootstrap mode feeds it from postgres.bki directly), we likely could avoid the restart between bootstrap and single user mode. Afaics that only really is needed because we need to send SQL after bootstrap_template1(). That'd likely be a nice speedup, because we don't need to write the bootstrap contents from shared buffers to the OS just to read them back in single user mode. I don't plan to work on this immediately, but I thought it's worth bringing up anyway. Greetings, Andres Freund [1] https://www.postgresql.org/message-id/20220216012953.6d7bzmsblqou3ru4%40alap3.anarazel.de ^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: initdb / bootstrap design 2022-02-16 02:12 initdb / bootstrap design Andres Freund <[email protected]> @ 2022-02-16 03:14 ` John Naylor <[email protected]> 2 siblings, 0 replies; 10+ messages in thread From: John Naylor @ 2022-02-16 03:14 UTC (permalink / raw) To: Andres Freund <[email protected]>; +Cc: pgsql-hackers On Wed, Feb 16, 2022 at 9:12 AM Andres Freund <[email protected]> wrote: > To me the division of labor between initdb and bootstrap doesn't make much > sense anymore: [...] > I don't plan to work on this immediately, but I thought it's worth bringing up > anyway. Added TODO item "Rationalize division of labor between initdb and bootstrap" -- John Naylor EDB: http://www.enterprisedb.com ^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: initdb / bootstrap design 2022-02-16 02:12 initdb / bootstrap design Andres Freund <[email protected]> @ 2022-02-16 10:47 ` Peter Eisentraut <[email protected]> 2022-02-16 17:53 ` Re: initdb / bootstrap design Andres Freund <[email protected]> 2 siblings, 1 reply; 10+ messages in thread From: Peter Eisentraut @ 2022-02-16 10:47 UTC (permalink / raw) To: Andres Freund <[email protected]>; pgsql-hackers On 16.02.22 03:12, Andres Freund wrote: > Sure, there's a few tokens that we replace in initdb. As it turns out there's > only two rows that are actually variable. The username of the initial > superuser in pg_authid and the pg_database row for template 1, where encoding, > lc_collate and lc_ctype varies. The rest is all compile time constant > replacements we could do as part of genbki.pl. > > It seems we could save a good number of context switches by opening > postgres.bki just before boot_yyparse() in BootstrapModeMain() and having the > parser read it. The pg_authid / pg_database rows we could just do via > explicit insertions in BootstrapModeMain(), provided by commandline args? I think we could do the locale setup by updating the pg_database row of template1 after bootstrap, as in the attached patch. (The order of proceedings in the surrounding function might need some refinement in a final patch.) I suspect we could do the treatment of pg_authid similarly. From 10143067fb35191aaa53ce2e5c4a20c4601b7528 Mon Sep 17 00:00:00 2001 From: Peter Eisentraut <[email protected]> Date: Wed, 16 Feb 2022 11:43:10 +0100 Subject: [PATCH] Simplify locale setup of template1 in initdb --- src/bin/initdb/initdb.c | 23 +++++------------------ src/include/catalog/pg_database.dat | 6 +++--- 2 files changed, 8 insertions(+), 21 deletions(-) diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c index 97f15971e2..42e42ca4a4 100644 --- a/src/bin/initdb/initdb.c +++ b/src/bin/initdb/initdb.c @@ -621,15 +621,6 @@ get_id(void) return pg_strdup(username); } -static char * -encodingid_to_string(int enc) -{ - char result[20]; - - sprintf(result, "%d", enc); - return pg_strdup(result); -} - /* * get the encoding id for a given encoding name */ @@ -1396,15 +1387,6 @@ bootstrap_template1(void) bki_lines = replace_token(bki_lines, "POSTGRES", escape_quotes_bki(username)); - bki_lines = replace_token(bki_lines, "ENCODING", - encodingid_to_string(encodingid)); - - bki_lines = replace_token(bki_lines, "LC_COLLATE", - escape_quotes_bki(lc_collate)); - - bki_lines = replace_token(bki_lines, "LC_CTYPE", - escape_quotes_bki(lc_ctype)); - /* Also ensure backend isn't confused by this environment var: */ unsetenv("PGCLIENTENCODING"); @@ -1886,6 +1868,11 @@ make_template0(FILE *cmdfd) NULL }; + PG_CMD_PRINTF("UPDATE pg_database " + " SET encoding = %d, datcollate = '%s', datctype = '%s' " + " WHERE datname = 'template1';\n\n", + encodingid, escape_quotes(lc_collate), escape_quotes(lc_ctype)); + for (line = template0_setup; *line; line++) PG_CMD_PUTS(*line); } diff --git a/src/include/catalog/pg_database.dat b/src/include/catalog/pg_database.dat index e7e42d6023..6bca1ec54b 100644 --- a/src/include/catalog/pg_database.dat +++ b/src/include/catalog/pg_database.dat @@ -14,9 +14,9 @@ { oid => '1', oid_symbol => 'TemplateDbOid', descr => 'default template for new databases', - datname => 'template1', encoding => 'ENCODING', datistemplate => 't', + datname => 'template1', encoding => '0', datistemplate => 't', datallowconn => 't', datconnlimit => '-1', datfrozenxid => '0', - datminmxid => '1', dattablespace => 'pg_default', datcollate => 'LC_COLLATE', - datctype => 'LC_CTYPE', datacl => '_null_' }, + datminmxid => '1', dattablespace => 'pg_default', datcollate => 'C', + datctype => 'C', datacl => '_null_' }, ] -- 2.35.1 Attachments: [text/plain] 0001-Simplify-locale-setup-of-template1-in-initdb.patch (2.4K, ../../[email protected]/2-0001-Simplify-locale-setup-of-template1-in-initdb.patch) download | inline diff: From 10143067fb35191aaa53ce2e5c4a20c4601b7528 Mon Sep 17 00:00:00 2001 From: Peter Eisentraut <[email protected]> Date: Wed, 16 Feb 2022 11:43:10 +0100 Subject: [PATCH] Simplify locale setup of template1 in initdb --- src/bin/initdb/initdb.c | 23 +++++------------------ src/include/catalog/pg_database.dat | 6 +++--- 2 files changed, 8 insertions(+), 21 deletions(-) diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c index 97f15971e2..42e42ca4a4 100644 --- a/src/bin/initdb/initdb.c +++ b/src/bin/initdb/initdb.c @@ -621,15 +621,6 @@ get_id(void) return pg_strdup(username); } -static char * -encodingid_to_string(int enc) -{ - char result[20]; - - sprintf(result, "%d", enc); - return pg_strdup(result); -} - /* * get the encoding id for a given encoding name */ @@ -1396,15 +1387,6 @@ bootstrap_template1(void) bki_lines = replace_token(bki_lines, "POSTGRES", escape_quotes_bki(username)); - bki_lines = replace_token(bki_lines, "ENCODING", - encodingid_to_string(encodingid)); - - bki_lines = replace_token(bki_lines, "LC_COLLATE", - escape_quotes_bki(lc_collate)); - - bki_lines = replace_token(bki_lines, "LC_CTYPE", - escape_quotes_bki(lc_ctype)); - /* Also ensure backend isn't confused by this environment var: */ unsetenv("PGCLIENTENCODING"); @@ -1886,6 +1868,11 @@ make_template0(FILE *cmdfd) NULL }; + PG_CMD_PRINTF("UPDATE pg_database " + " SET encoding = %d, datcollate = '%s', datctype = '%s' " + " WHERE datname = 'template1';\n\n", + encodingid, escape_quotes(lc_collate), escape_quotes(lc_ctype)); + for (line = template0_setup; *line; line++) PG_CMD_PUTS(*line); } diff --git a/src/include/catalog/pg_database.dat b/src/include/catalog/pg_database.dat index e7e42d6023..6bca1ec54b 100644 --- a/src/include/catalog/pg_database.dat +++ b/src/include/catalog/pg_database.dat @@ -14,9 +14,9 @@ { oid => '1', oid_symbol => 'TemplateDbOid', descr => 'default template for new databases', - datname => 'template1', encoding => 'ENCODING', datistemplate => 't', + datname => 'template1', encoding => '0', datistemplate => 't', datallowconn => 't', datconnlimit => '-1', datfrozenxid => '0', - datminmxid => '1', dattablespace => 'pg_default', datcollate => 'LC_COLLATE', - datctype => 'LC_CTYPE', datacl => '_null_' }, + datminmxid => '1', dattablespace => 'pg_default', datcollate => 'C', + datctype => 'C', datacl => '_null_' }, ] -- 2.35.1 ^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: initdb / bootstrap design 2022-02-16 02:12 initdb / bootstrap design Andres Freund <[email protected]> 2022-02-16 10:47 ` Re: initdb / bootstrap design Peter Eisentraut <[email protected]> @ 2022-02-16 17:53 ` Andres Freund <[email protected]> 0 siblings, 0 replies; 10+ messages in thread From: Andres Freund @ 2022-02-16 17:53 UTC (permalink / raw) To: Peter Eisentraut <[email protected]>; +Cc: pgsql-hackers Hi, On 2022-02-16 11:47:31 +0100, Peter Eisentraut wrote: > On 16.02.22 03:12, Andres Freund wrote: > > Sure, there's a few tokens that we replace in initdb. As it turns out there's > > only two rows that are actually variable. The username of the initial > > superuser in pg_authid and the pg_database row for template 1, where encoding, > > lc_collate and lc_ctype varies. The rest is all compile time constant > > replacements we could do as part of genbki.pl. > > > > It seems we could save a good number of context switches by opening > > postgres.bki just before boot_yyparse() in BootstrapModeMain() and having the > > parser read it. The pg_authid / pg_database rows we could just do via > > explicit insertions in BootstrapModeMain(), provided by commandline args? > > I think we could do the locale setup by updating the pg_database row of > template1 after bootstrap, as in the attached patch. Another solution could be to have bootstrap create template0 instead of template1. I think for template0 it'd more accurate to have a hardcoded C collation and ascii encoding (which I don't think we actually have?). > I suspect we could do the treatment of pg_authid similarly. Yea. Greetings, Andres Freund ^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: initdb / bootstrap design 2022-02-16 02:12 initdb / bootstrap design Andres Freund <[email protected]> @ 2022-02-16 18:24 ` Tom Lane <[email protected]> 2 siblings, 0 replies; 10+ messages in thread From: Tom Lane @ 2022-02-16 18:24 UTC (permalink / raw) To: Andres Freund <[email protected]>; +Cc: pgsql-hackers Andres Freund <[email protected]> writes: > Sure, there's a few tokens that we replace in initdb. As it turns out there's > only two rows that are actually variable. The username of the initial > superuser in pg_authid and the pg_database row for template 1, where encoding, > lc_collate and lc_ctype varies. The rest is all compile time constant > replacements we could do as part of genbki.pl. I remembered the reason why it's done that way: if we replaced those values during genbki.pl, the contents of postgres.bki would become architecture-dependent, belying its distribution as a "share" file. While we don't absolutely have to continue treating postgres.bki as architecture-independent, I'm skeptical that there's enough win here to justify a packaging change. initdb is already plenty fast enough for any plausible production usage; it's cases like check-world where we wish it were faster. So I'm thinking what we really ought to pursue is the idea that's been kicked around more than once of capturing the post-initdb state of a cluster's files and just doing "cp -a" to duplicate that later in the test run. regards, tom lane ^ permalink raw reply [nested|flat] 10+ messages in thread
* [PATCH v1 4/8] convert PROC_HDR->startupBufferPinWaitBufId to an atomic @ 2026-07-09 19:26 Nathan Bossart <[email protected]> 0 siblings, 0 replies; 10+ messages in thread From: Nathan Bossart @ 2026-07-09 19:26 UTC (permalink / raw) --- src/backend/storage/lmgr/proc.c | 12 +++--------- src/include/storage/proc.h | 2 +- 2 files changed, 4 insertions(+), 10 deletions(-) diff --git a/src/backend/storage/lmgr/proc.c b/src/backend/storage/lmgr/proc.c index 9d6e69175a5..5f314efb289 100644 --- a/src/backend/storage/lmgr/proc.c +++ b/src/backend/storage/lmgr/proc.c @@ -239,7 +239,7 @@ ProcGlobalShmemInit(void *arg) dlist_init(&ProcGlobal->autovacFreeProcs); dlist_init(&ProcGlobal->bgworkerFreeProcs); dlist_init(&ProcGlobal->walsenderFreeProcs); - ProcGlobal->startupBufferPinWaitBufId = -1; + pg_atomic_init_u32(&ProcGlobal->startupBufferPinWaitBufId, -1); pg_atomic_init_u32(&ProcGlobal->avLauncherProc, INVALID_PROC_NUMBER); pg_atomic_init_u32(&ProcGlobal->walwriterProc, INVALID_PROC_NUMBER); pg_atomic_init_u32(&ProcGlobal->checkpointerProc, INVALID_PROC_NUMBER); @@ -768,10 +768,7 @@ InitAuxiliaryProcess(void) void SetStartupBufferPinWaitBufId(int bufid) { - /* use volatile pointer to prevent code rearrangement */ - volatile PROC_HDR *procglobal = ProcGlobal; - - procglobal->startupBufferPinWaitBufId = bufid; + pg_atomic_write_u32(&ProcGlobal->startupBufferPinWaitBufId, bufid); } /* @@ -780,10 +777,7 @@ SetStartupBufferPinWaitBufId(int bufid) int GetStartupBufferPinWaitBufId(void) { - /* use volatile pointer to prevent code rearrangement */ - volatile PROC_HDR *procglobal = ProcGlobal; - - return procglobal->startupBufferPinWaitBufId; + return pg_atomic_read_u32(&ProcGlobal->startupBufferPinWaitBufId); } /* diff --git a/src/include/storage/proc.h b/src/include/storage/proc.h index 03a1a466fa8..e2034255124 100644 --- a/src/include/storage/proc.h +++ b/src/include/storage/proc.h @@ -499,7 +499,7 @@ typedef struct PROC_HDR /* Current shared estimate of appropriate spins_per_delay value */ int spins_per_delay; /* Buffer id of the buffer that Startup process waits for pin on, or -1 */ - int startupBufferPinWaitBufId; + pg_atomic_uint32 startupBufferPinWaitBufId; } PROC_HDR; extern PGDLLIMPORT PROC_HDR *ProcGlobal; -- 2.50.1 (Apple Git-155) --Ot5x3ffkWEuxilWO Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename=v1-0005-convert-Sharedsort-currentWorker-workersFinished-.patch ^ permalink raw reply [nested|flat] 10+ messages in thread
* [PATCH v1 4/8] convert PROC_HDR->startupBufferPinWaitBufId to an atomic @ 2026-07-09 19:26 Nathan Bossart <[email protected]> 0 siblings, 0 replies; 10+ messages in thread From: Nathan Bossart @ 2026-07-09 19:26 UTC (permalink / raw) --- src/backend/storage/lmgr/proc.c | 12 +++--------- src/include/storage/proc.h | 2 +- 2 files changed, 4 insertions(+), 10 deletions(-) diff --git a/src/backend/storage/lmgr/proc.c b/src/backend/storage/lmgr/proc.c index 9d6e69175a5..5f314efb289 100644 --- a/src/backend/storage/lmgr/proc.c +++ b/src/backend/storage/lmgr/proc.c @@ -239,7 +239,7 @@ ProcGlobalShmemInit(void *arg) dlist_init(&ProcGlobal->autovacFreeProcs); dlist_init(&ProcGlobal->bgworkerFreeProcs); dlist_init(&ProcGlobal->walsenderFreeProcs); - ProcGlobal->startupBufferPinWaitBufId = -1; + pg_atomic_init_u32(&ProcGlobal->startupBufferPinWaitBufId, -1); pg_atomic_init_u32(&ProcGlobal->avLauncherProc, INVALID_PROC_NUMBER); pg_atomic_init_u32(&ProcGlobal->walwriterProc, INVALID_PROC_NUMBER); pg_atomic_init_u32(&ProcGlobal->checkpointerProc, INVALID_PROC_NUMBER); @@ -768,10 +768,7 @@ InitAuxiliaryProcess(void) void SetStartupBufferPinWaitBufId(int bufid) { - /* use volatile pointer to prevent code rearrangement */ - volatile PROC_HDR *procglobal = ProcGlobal; - - procglobal->startupBufferPinWaitBufId = bufid; + pg_atomic_write_u32(&ProcGlobal->startupBufferPinWaitBufId, bufid); } /* @@ -780,10 +777,7 @@ SetStartupBufferPinWaitBufId(int bufid) int GetStartupBufferPinWaitBufId(void) { - /* use volatile pointer to prevent code rearrangement */ - volatile PROC_HDR *procglobal = ProcGlobal; - - return procglobal->startupBufferPinWaitBufId; + return pg_atomic_read_u32(&ProcGlobal->startupBufferPinWaitBufId); } /* diff --git a/src/include/storage/proc.h b/src/include/storage/proc.h index 03a1a466fa8..e2034255124 100644 --- a/src/include/storage/proc.h +++ b/src/include/storage/proc.h @@ -499,7 +499,7 @@ typedef struct PROC_HDR /* Current shared estimate of appropriate spins_per_delay value */ int spins_per_delay; /* Buffer id of the buffer that Startup process waits for pin on, or -1 */ - int startupBufferPinWaitBufId; + pg_atomic_uint32 startupBufferPinWaitBufId; } PROC_HDR; extern PGDLLIMPORT PROC_HDR *ProcGlobal; -- 2.50.1 (Apple Git-155) --Ot5x3ffkWEuxilWO Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename=v1-0005-convert-Sharedsort-currentWorker-workersFinished-.patch ^ permalink raw reply [nested|flat] 10+ messages in thread
end of thread, other threads:[~2026-07-09 19:26 UTC | newest] Thread overview: 10+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2018-03-15 16:01 WaitLatchOrSocket optimization Konstantin Knizhnik <[email protected]> 2018-03-15 17:25 ` Andres Freund <[email protected]> 2018-03-16 08:03 ` Konstantin Knizhnik <[email protected]> 2022-02-16 02:12 initdb / bootstrap design Andres Freund <[email protected]> 2022-02-16 03:14 ` Re: initdb / bootstrap design John Naylor <[email protected]> 2022-02-16 10:47 ` Re: initdb / bootstrap design Peter Eisentraut <[email protected]> 2022-02-16 17:53 ` Re: initdb / bootstrap design Andres Freund <[email protected]> 2022-02-16 18:24 ` Re: initdb / bootstrap design Tom Lane <[email protected]> 2026-07-09 19:26 [PATCH v1 4/8] convert PROC_HDR->startupBufferPinWaitBufId to an atomic Nathan Bossart <[email protected]> 2026-07-09 19:26 [PATCH v1 4/8] convert PROC_HDR->startupBufferPinWaitBufId to an atomic Nathan Bossart <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox