public inbox for [email protected]
help / color / mirror / Atom feedFrom: Chao Li <[email protected]>
To: Fujii Masao <[email protected]>
Cc: wang.xiao.peng <[email protected]>
Cc: [email protected]
Cc: PostgreSQL Hackers <[email protected]>
Cc: Tom Lane <[email protected]>
Cc: Hannu Krosing <[email protected]>
Subject: Re: pg_test_timing: fix unit typo and widen diff type
Date: Wed, 22 Apr 2026 16:57:32 +0800
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAHGQGwEM=s5vXPpXoismwRMNCnAmwwMJPnQEQYbPFTJGouV0rQ@mail.gmail.com>
References: <[email protected]>
<CAP53PkwEZ+UMmwA8cbUSUdE6phXi+xWmn5t-YAnvKf0qoBnOvg@mail.gmail.com>
<[email protected]>
<[email protected]>
<[email protected]>
<CAHGQGwEM=s5vXPpXoismwRMNCnAmwwMJPnQEQYbPFTJGouV0rQ@mail.gmail.com>
> On Apr 22, 2026, at 15:04, Fujii Masao <[email protected]> wrote:
>
> On Thu, Apr 9, 2026 at 3:13 PM wang.xiao.peng <[email protected]> wrote:
>> Just finished reviewing the v3 patches - looks good to me. The pg_leftmost_one_pos64 fix is spot on, and using INT64_FORMAT for the error message is the right approach.
>
> Since pg_leftmost_one_pos64() can return up to 63, should the size of
> histogram[] be changed from 32 to 64? If we want to display the full
> histogram[] in the output, max_bit in output() would also need to be set to 63.
> Alternatively, it may be fine to keep max_bit = 31 and show only
> the first 32 histogram entries.
I revisited this patch and think it makes sense to extend histogram[] to 64 entries.
In practice, the higher buckets will almost always remain zero, and output() already omits trailing histogram entries with zero counts, so this change is unlikely to affect the actual runtime output in normal cases.
Accordingly, I also adjusted the minimum width of the first column, since the maximum int64 value has 19 digits, and updated the printf format accordingly.
>
> Patch 0001 looks good to me.
Cool.
PFA v4:
* 0001 unchanged from v3.
* 0002 changed size of histogram[] 64.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
Attachments:
[application/octet-stream] v4-0001-pg_test_timing-fix-unit-in-backward-clock-warning.patch (1.3K, 2-v4-0001-pg_test_timing-fix-unit-in-backward-clock-warning.patch)
download | inline diff:
From f977c49219a708f7bec0f15d0250dc8554584639 Mon Sep 17 00:00:00 2001
From: "Chao Li (Evan)" <[email protected]>
Date: Thu, 2 Apr 2026 10:21:40 +0800
Subject: [PATCH v4 1/2] pg_test_timing: fix unit in backward-clock warning
pg_test_timing measures timing differences in nanoseconds via
INSTR_TIME_GET_NANOSEC(), but the backward-clock warning incorrectly
reported the value as milliseconds. Fix the message to use "ns" so
that it matches the actual unit of diff.
Author: Chao Li <[email protected]>
Reviewed-by: Lukas Fittl <[email protected]>
Reviewed-by: Fujii Masao <[email protected]>
Discussion: https://postgr.es/m/[email protected]
---
src/bin/pg_test_timing/pg_test_timing.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/bin/pg_test_timing/pg_test_timing.c b/src/bin/pg_test_timing/pg_test_timing.c
index 2afb0e6a410..944b25df1f2 100644
--- a/src/bin/pg_test_timing/pg_test_timing.c
+++ b/src/bin/pg_test_timing/pg_test_timing.c
@@ -281,7 +281,7 @@ test_timing(unsigned int duration, TimingClockSourceType source, bool fast_timin
if (unlikely(diff < 0))
{
fprintf(stderr, _("Detected clock going backwards in time.\n"));
- fprintf(stderr, _("Time warp: %d ms\n"), diff);
+ fprintf(stderr, _("Time warp: %d ns\n"), diff);
exit(1);
}
--
2.50.1 (Apple Git-155)
[application/octet-stream] v4-0002-pg_test_timing-use-int64-for-largest-observed-tim.patch (3.6K, 3-v4-0002-pg_test_timing-use-int64-for-largest-observed-tim.patch)
download | inline diff:
From d8f053cba2221b365317d3dbf8642cf28cefac72 Mon Sep 17 00:00:00 2001
From: "Chao Li (Evan)" <[email protected]>
Date: Thu, 2 Apr 2026 10:29:06 +0800
Subject: [PATCH v4 2/2] pg_test_timing: use int64 for largest observed time
delta
pg_test_timing obtains timing deltas with INSTR_TIME_GET_NANOSEC(),
which returns a nanosecond value that should be stored in int64. Adjust
diff and largest_diff to use int64 accordingly.
Also fix the corresponding printf format specifiers so these values are
printed correctly.
Author: Chao Li <[email protected]>
Reviewed-by: Lukas Fittl <[email protected]>
Reviewed-by: Fujii Masao <[email protected]>
Discussion: https://postgr.es/m/[email protected]
---
src/bin/pg_test_timing/pg_test_timing.c | 28 ++++++++++++++-----------
1 file changed, 16 insertions(+), 12 deletions(-)
diff --git a/src/bin/pg_test_timing/pg_test_timing.c b/src/bin/pg_test_timing/pg_test_timing.c
index 944b25df1f2..530347a9aa5 100644
--- a/src/bin/pg_test_timing/pg_test_timing.c
+++ b/src/bin/pg_test_timing/pg_test_timing.c
@@ -18,14 +18,14 @@ static unsigned int test_duration = 3;
static double max_rprct = 99.99;
/* record duration in powers of 2 nanoseconds */
-static long long int histogram[32];
+static long long int histogram[64];
/* record counts of first 10K durations directly */
#define NUM_DIRECT 10000
static long long int direct_histogram[NUM_DIRECT];
/* separately record highest observed duration */
-static int32 largest_diff;
+static int64 largest_diff;
static long long int largest_diff_count;
@@ -262,8 +262,8 @@ test_timing(unsigned int duration, TimingClockSourceType source, bool fast_timin
while (INSTR_TIME_GT(end_time, cur))
{
- int32 diff,
- bits;
+ int64 diff;
+ int32 bits;
instr_time diff_time;
prev = cur;
@@ -281,13 +281,17 @@ test_timing(unsigned int duration, TimingClockSourceType source, bool fast_timin
if (unlikely(diff < 0))
{
fprintf(stderr, _("Detected clock going backwards in time.\n"));
- fprintf(stderr, _("Time warp: %d ns\n"), diff);
+ fprintf(stderr, _("Time warp: " INT64_FORMAT " ns\n"), diff);
exit(1);
}
/* What is the highest bit in the time diff? */
if (diff > 0)
- bits = pg_leftmost_one_pos32(diff) + 1;
+ {
+ bits = pg_leftmost_one_pos64(diff) + 1;
+ /* histogram should be define large enough */
+ Assert(bits < lengthof(histogram));
+ }
else
bits = 0;
@@ -324,7 +328,7 @@ test_timing(unsigned int duration, TimingClockSourceType source, bool fast_timin
static void
output(uint64 loop_count)
{
- int max_bit = 31;
+ int max_bit = lengthof(histogram) - 1;
const char *header1 = _("<= ns");
const char *header1b = _("ns");
const char *header2 = /* xgettext:no-c-format */ _("% of total");
@@ -342,7 +346,7 @@ output(uint64 loop_count)
max_bit--;
/* set minimum column widths */
- len1 = Max(8, len1);
+ len1 = Max(19, len1);
len2 = Max(10, len2);
len3 = Max(10, len3);
len4 = Max(10, len4);
@@ -360,8 +364,8 @@ output(uint64 loop_count)
double prct = (double) histogram[i] * 100 / loop_count;
rprct += prct;
- printf("%*ld %*.4f %*.4f %*lld\n",
- len1, (1L << i) - 1,
+ printf("%*llu %*.4f %*.4f %*lld\n",
+ len1, (1ULL << i) - 1,
len2, prct,
len3, rprct,
len4, histogram[i]);
@@ -409,8 +413,8 @@ output(uint64 loop_count)
double prct = (double) largest_diff_count * 100 / loop_count;
printf("...\n");
- printf("%*d %*.4f %*.4f %*lld\n",
- len1, largest_diff,
+ printf("%*lld %*.4f %*.4f %*lld\n",
+ len1, (long long) largest_diff,
len2, prct,
len3, 100.0,
len4, largest_diff_count);
--
2.50.1 (Apple Git-155)
view thread (11+ messages) latest in thread
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: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: pg_test_timing: fix unit typo and widen diff type
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox