pg.ddx.io pgsql-hackers@postgresql.org mailing list archive help / color / mirror / Atom feed From: Jeff Davis <pgsql@j-davis.com>
To: Masahiko Sawada <sawada.mshk@gmail.com>
Cc: Amit Kapila <amit.kapila16@gmail.com>
Cc: vignesh C <vignesh21@gmail.com>
Cc: Peter Smith <smithpb2250@gmail.com>
Cc: Tomas Vondra <tomas.vondra@enterprisedb.com>
Cc: Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com>
Cc: Shubham Khanna <khannashubham1197@gmail.com>
Cc: Dilip Kumar <dilipbalaut@gmail.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: Improve eviction algorithm in ReorderBuffer
Date: Tue, 09 Apr 2024 11:04:49 -0700
Message-ID: <791d98f474e518387d09eb390b8a12f265d130cc.camel@j-davis.com> (raw )
In-Reply-To: <CAD21AoAtR2A4f1-JFUE9KRkiPqMRvxS+3W=ZR7FpuXm8VSVASQ@mail.gmail.com >
References: <CAD21AoAfKTgrBrLq96GcTv9d6k97zaQcDM-rxfKEt4GSe0qnaQ@mail.gmail.com >
<CAD21AoAkzQkEV8JVNonWbJNqCegpC5h8XsRfNbNjzDgAqj8uEA@mail.gmail.com >
<CAA4eK1+m=hfCSB0MWGTODM5Dj=P6Mzns3vQPKrTxp6ccVWgL2g@mail.gmail.com >
<CAD21AoBP5NcD3R6fJiX8QurH+Dhh2j_Ee1Ohc+b1XvSfvj6mnA@mail.gmail.com >
<CAA4eK1JqbW57NxFcp2weapDRMhN9+_HjLXq7W12UYUWcWfm_sg@mail.gmail.com >
<CAD21AoDffo37RC-eUuyHJKVEr017V2YYDLyn1xF_00ofptWbkg@mail.gmail.com >
<CAHv8RjJbd-EVs=YJLL6=XAA0ETbK-r8Zm5subP0CAuafOMbj1A@mail.gmail.com >
<CAD21AoBTgU=ZmJSoysZ+pokuGUEtFMU_4QvOaZCo2Gs7UKdORg@mail.gmail.com >
<CAD21AoA7L2Vwn3=3wMQxGrkisHfB7XK8wjh9x-rD7Q7Vms16LA@mail.gmail.com >
<TYCPR01MB120770098C895866564E29C24F5442@TYCPR01MB12077.jpnprd01.prod.outlook.com >
<CAD21AoDhuybyryVkmVkgPY8uVrjGLYchL8EY8-rBm1hbZJpwLw@mail.gmail.com >
<CALDaNm3NbbMpg_zo3wniJTFMsC4a6aCzOy6KsEhnhQgcsbaukA@mail.gmail.com >
<23e03725-8c77-431d-9f53-51709260ffeb@enterprisedb.com >
<CAD21AoDKMMRMEWAyL_nBpzJwktKyWLSmao_2uOA+_CrrB77bnw@mail.gmail.com >
<787cd90b-48ff-4c4a-a63e-a2b3f494385d@enterprisedb.com >
<CAD21AoAPkLQ9C5SA8ngPzegyq1C36+hGX78FwduvM6kOyfVU9A@mail.gmail.com >
<CAA4eK1+2zRU4+NAoUAiYYW1peGrE3JjSx9SrqeTGM=8r=0-UoA@mail.gmail.com >
<CALDaNm0-fSZdo=fVpSBfO+4_c+8HGuzWxZFZhpRPhjpPc5pifA@mail.gmail.com >
<CAD21AoBYjJmz7q_=Z+eXJgm0FScyu3_iGFshPAvnq78B2KL3qQ@mail.gmail.com >
<CAHut+PvZ12UDr1v7PX2CAEq1jo2rqWgB3gT9PRpQdboydAGTNQ@mail.gmail.com >
<CAD21AoAmj_gQt3LadRO5YrUr109AHBnbMh4Qt4cx7Wv3UwYJTA@mail.gmail.com >
<CAD21AoCY=x_XaNTOKae9bT-V5Fr2kY=6zUSSug3=Np_Ybue9VA@mail.gmail.com >
<CALDaNm14V8KHrxvqoOXd786Dkae-DFR4TDq1sj18SXCYUdDGzw@mail.gmail.com >
<CAD21AoDomWEkepxa8uE6s2eSoiB+gjtt51zx3Cnz6oLV_51U2Q@mail.gmail.com >
<CAA4eK1JTbEX-PcGwSni=W3J=viq4-BeRp1cmQAscJw0xxMAskQ@mail.gmail.com >
<CAD21AoA6=+tL=btB_s9N+cZK7tKz1W=PQyNq72nzjUcdyE+wZw@mail.gmail.com >
<CAD21AoC51TL6k+PFXyWEie-MVo7dqCHmPzTqfUaUueyx9OfLyw@mail.gmail.com >
<12747c15811d94efcc5cda72d6b35c80d7bf3443.camel@j-davis.com >
<fa89b6963899a05931f905dc7b1597e55b8204f5.camel@j-davis.com >
<CAD21AoAd7ZUz1SVKc+WNqNcTJdBZOBUeV_iExyzqk8WLyoNy1A@mail.gmail.com >
<f13d88915e7e197ccb78968a825d1c021f62c8ce.camel@j-davis.com >
<CAD21AoCfP++VF+12Za7SV_vQpRpFbe+Kf_LQsRPAw_pJj0YS3Q@mail.gmail.com >
<cecac19e0fb5739d4fd955000d19d88d060924d4.camel@j-davis.com >
<CAD21AoAtR2A4f1-JFUE9KRkiPqMRvxS+3W=ZR7FpuXm8VSVASQ@mail.gmail.com >
On Fri, 2024-04-05 at 16:58 +0900, Masahiko Sawada wrote:
> > I have some further comments and I believe changes are required for
> > v17.
I also noticed that the simplehash is declared in binaryheap.h with
"SH_SCOPE extern", which seems wrong. Attached is a rough patch to
bring the declarations into binaryheap.c.
Note that the attached patch uses "SH_SCOPE static", which makes sense
to me in this case, but causes a bunch of warnings in gcc. I will post
separately about silencing that warning, but for now you can either
use:
SH_SCOPE static inline
which is probably fine, but will encourage the compiler to inline more
code, when not all callers even use the hash table. Alternatively, you
can do:
SH_SCOPE static pg_attribute_unused()
which looks a bit wrong to me, but seems to silence the warnings, and
lets the compiler decide whether or not to inline.
Also probably needs comment updates, etc.
Regards,
Jeff Davis
Attachments:
v1-0001-binaryheap-move-hash-table-out-of-header-into-bin.patch (text/x-patch, 2.9K) download
From 08dcf21646e4ded22b10fd0ed536d3bbf6fc1328 Mon Sep 17 00:00:00 2001
From: Jeff Davis <jeff@j-davis.com>
Date: Tue, 9 Apr 2024 10:45:00 -0700
Subject: [PATCH v1] binaryheap: move hash table out of header into
binaryheap.c.
Commit b840508644 declared the internal hash table in the header with
scope "extern", which was unnecessary.
---
src/common/binaryheap.c | 15 ++++++++++++++-
src/include/lib/binaryheap.h | 25 +------------------------
2 files changed, 15 insertions(+), 25 deletions(-)
diff --git a/src/common/binaryheap.c b/src/common/binaryheap.c
index c20ed50acc..2501dad6f2 100644
--- a/src/common/binaryheap.c
+++ b/src/common/binaryheap.c
@@ -25,6 +25,18 @@
#include "common/hashfn.h"
#include "lib/binaryheap.h"
+/*
+ * Struct for a hash table element to store the node's index in the bh_nodes
+ * array.
+ */
+typedef struct bh_nodeidx_entry
+{
+ bh_node_type key;
+ int index; /* entry's index within the node array */
+ char status; /* hash status */
+ uint32 hash; /* hash values (cached) */
+} bh_nodeidx_entry;
+
/*
* Define parameters for hash table code generation. The interface is *also*
* declared in binaryheap.h (to generate the types, which are externally
@@ -37,12 +49,13 @@
#define SH_HASH_KEY(tb, key) \
hash_bytes((const unsigned char *) &key, sizeof(bh_node_type))
#define SH_EQUAL(tb, a, b) (memcmp(&a, &b, sizeof(bh_node_type)) == 0)
-#define SH_SCOPE extern
+#define SH_SCOPE static
#ifdef FRONTEND
#define SH_RAW_ALLOCATOR pg_malloc0
#endif
#define SH_STORE_HASH
#define SH_GET_HASH(tb, a) a->hash
+#define SH_DECLARE
#define SH_DEFINE
#include "lib/simplehash.h"
diff --git a/src/include/lib/binaryheap.h b/src/include/lib/binaryheap.h
index 4c1a1bb274..8b47132fc3 100644
--- a/src/include/lib/binaryheap.h
+++ b/src/include/lib/binaryheap.h
@@ -29,29 +29,6 @@ typedef Datum bh_node_type;
*/
typedef int (*binaryheap_comparator) (bh_node_type a, bh_node_type b, void *arg);
-/*
- * Struct for a hash table element to store the node's index in the bh_nodes
- * array.
- */
-typedef struct bh_nodeidx_entry
-{
- bh_node_type key;
- int index; /* entry's index within the node array */
- char status; /* hash status */
- uint32 hash; /* hash values (cached) */
-} bh_nodeidx_entry;
-
-/* Define parameters necessary to generate the hash table interface. */
-#define SH_PREFIX bh_nodeidx
-#define SH_ELEMENT_TYPE bh_nodeidx_entry
-#define SH_KEY_TYPE bh_node_type
-#define SH_SCOPE extern
-#ifdef FRONTEND
-#define SH_RAW_ALLOCATOR pg_malloc0
-#endif
-#define SH_DECLARE
-#include "lib/simplehash.h"
-
/*
* binaryheap
*
@@ -76,7 +53,7 @@ typedef struct binaryheap
* node's index in bh_nodes. This enables the caller to perform
* binaryheap_remove_node_ptr(), binaryheap_update_up/down in O(log n).
*/
- bh_nodeidx_hash *bh_nodeidx;
+ struct bh_nodeidx_hash *bh_nodeidx;
} binaryheap;
extern binaryheap *binaryheap_allocate(int num_nodes,
--
2.34.1
view thread (93+ messages) latest in thread
Message-ID: <791d98f474e518387d09eb390b8a12f265d130cc.camel@j-davis.com>
Permalink: ../791d98f474e518387d09eb390b8a12f265d130cc.camel@j-davis.com/
Also on: postgresql.org/message-id/791d98f474e518387d09eb390b8a12f265d130cc.camel@j-davis.com
copy link · copy postgr.es · copy git am
reply Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: pgsql@j-davis.com, sawada.mshk@gmail.com, amit.kapila16@gmail.com, vignesh21@gmail.com, smithpb2250@gmail.com, tomas.vondra@enterprisedb.com, kuroda.hayato@fujitsu.com, khannashubham1197@gmail.com, dilipbalaut@gmail.com, pgsql-hackers@lists.postgresql.org
Subject: Re: Improve eviction algorithm in ReorderBuffer
In-Reply-To: <791d98f474e518387d09eb390b8a12f265d130cc.camel@j-davis.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox