On Sat, 20 Jan 2024 at 09:03, vignesh C wrote:
>
> On Mon, 20 Feb 2023 at 16:06, David Geier wrote:
> >
> > Hi!
> >
> > On 2/14/23 13:48, David Geier wrote:
> > >
> > > It still fails.
> > >
> > > I'll get Cirrus-CI working on my own Github fork so I can make sure it
> > > really compiles on all
On Mon, 20 Feb 2023 at 16:06, David Geier wrote:
>
> Hi!
>
> On 2/14/23 13:48, David Geier wrote:
> >
> > It still fails.
> >
> > I'll get Cirrus-CI working on my own Github fork so I can make sure it
> > really compiles on all platforms before I submit a new version.
>
> It took some time until
Hi!
On 2/14/23 13:48, David Geier wrote:
It still fails.
I'll get Cirrus-CI working on my own Github fork so I can make sure it
really compiles on all platforms before I submit a new version.
It took some time until Cirrus CI allowed me to run tests against my new
GitHub account (there's
Hi!
On 2/14/23 12:11, David Geier wrote:
Hi,
I think I fixed the compilation errors. It was due to a few variables
being declared under
#if defined(__x86_64__) && defined(__linux__)
while being used also under non x86 Linux.
I also removed again the code to obtain the TSC frequency under
Hi,
On 2/7/23 19:12, Andres Freund wrote:
This fails to build on several platforms:
https://cirrus-ci.com/github/postgresql-cfbot/postgresql/commitfest%2F42%2F3751
I think I fixed the compilation errors. It was due to a few variables
being declared under
#if defined(__x86_64__) &&
Hi,
On 2023-01-24 14:30:34 +0100, David Geier wrote:
> Attached is v7 of the patch:
>
> - Rebased on latest master (most importantly on top of the int64 instr_time
> commits). - Includes two commits from Andres which introduce
> INSTR_TIME_SET_SECONDS(), INSTR_TIME_IS_LT() and WIP to report
>
Hi,
On 1/23/23 21:30, Andres Freund wrote:
That's been the case since my first post in the thread :). Mainly, it seems
easier to detect underflow cases during subtraction that way. And the factor
of 2 in range doesn't change a whole lot.
I just realized it the other day :).
If you have time
Hi
I think at least some should be converted to just accumulate in an
instr_time...
I think that's for a later patch though?
Yep, at least quite similar.
OK. I coded it up in the latest version of the patch.
Depending on how low we want to keep the error, I don't think we can:
If I set the
Hi,
On 1/23/23 18:41, Andres Freund wrote:
If we add it, it probably shouldn't depend on TIMING, but on
SUMMARY. Regression test queries showing EXPLAIN ANALYZE output all do
something like
EXPLAIN (ANALYZE, COSTS OFF, SUMMARY OFF, TIMING OFF)
the SUMMARY OFF gets rid of the "top-level"
Hi,
On 2023-01-23 18:52:44 +0100, David Geier wrote:
> One thing I was wondering about: why did you chose to use a signed instead
> of an unsigned 64-bit integer for the ticks?
That's been the case since my first post in the thread :). Mainly, it seems
easier to detect underflow cases during
Hi,
On 2023-01-23 18:49:37 +0100, David Geier wrote:
> On 1/21/23 05:12, Andres Freund wrote:
> > We do currently do the conversion quite frequently. Admittedly I was
> > partially motivated by trying to get the per-loop overhead in pg_test_timing
> > down ;)
> >
> > But I think it's a real
Hi,
On 1/21/23 06:31, Andres Freund wrote:
I pushed the int64-ification commits.
Great. I started rebasing.
One thing I was wondering about: why did you chose to use a signed
instead of an unsigned 64-bit integer for the ticks?
If you have time to look at the pg_test_timing part, it'd be
Hi,
On 1/21/23 05:12, Andres Freund wrote:
We do currently do the conversion quite frequently. Admittedly I was
partially motivated by trying to get the per-loop overhead in pg_test_timing
down ;)
But I think it's a real issue. Places where we do, but shouldn't, convert:
- ExecReScan() -
Hi,
On 2023-01-23 18:23:17 +0100, David Geier wrote:
> On 1/21/23 05:14, Andres Freund wrote:
> > The elapsed time is already inherently unstable, so we shouldn't have any
> > test
> > output showing the time.
> >
> > But I doubt showing it in every explain is a good idea - we use instr_time
>
Hi,
On 1/21/23 05:14, Andres Freund wrote:
The elapsed time is already inherently unstable, so we shouldn't have any test
output showing the time.
But I doubt showing it in every explain is a good idea - we use instr_time in
plenty of other places. Why show it in explain, but not in all those
Hi,
On 2023-01-20 21:31:57 -0800, Andres Freund wrote:
> On 2023-01-20 20:16:13 -0800, Andres Freund wrote:
> > I'm planning to push most of my changes soon, had hoped to get to it a bit
> > sooner, but ...
>
> I pushed the int64-ification commits.
There's an odd compilation failure on AIX.
Hi,
On 2023-01-20 20:16:13 -0800, Andres Freund wrote:
> On 2023-01-18 14:05:35 +0100, David Geier wrote:
> > @Andres: will you take care of these changes and provide me with an updated
> > patch set so I can rebase the RDTSC changes?
> > Otherwise, I can also apply Tom suggestions to your patch
On 2023-01-20 22:50:37 -0600, Justin Pryzby wrote:
> On Fri, Jan 20, 2023 at 04:40:32PM -0800, Andres Freund wrote:
> > From 5a458d4584961dedd3f80a07d8faea66e57c5d94 Mon Sep 17 00:00:00 2001
> > From: Andres Freund
> > Date: Mon, 16 Jan 2023 11:19:11 -0800
> > Subject: [PATCH v8 4/5] wip: report
On Fri, Jan 20, 2023 at 04:40:32PM -0800, Andres Freund wrote:
> From 5a458d4584961dedd3f80a07d8faea66e57c5d94 Mon Sep 17 00:00:00 2001
> From: Andres Freund
> Date: Mon, 16 Jan 2023 11:19:11 -0800
> Subject: [PATCH v8 4/5] wip: report nanoseconds in pg_test_timing
>
> - The i7-860 system
Hi,
On 2023-01-18 14:02:48 +0100, David Geier wrote:
> On 1/16/23 18:37, Andres Freund wrote:
> > I'm doubtful this is worth the complexity it incurs. By the time we convert
> > out of the instr_time format, the times shouldn't be small enough that the
> > accuracy is affected much.
>
> I don't
Hi,
On 2023-01-18 14:05:35 +0100, David Geier wrote:
> @Andres: will you take care of these changes and provide me with an updated
> patch set so I can rebase the RDTSC changes?
> Otherwise, I can also apply Tom suggestions to your patch set and send out
> the complete patch set.
I'm planning to
Hi,
On 2023-01-20 07:43:00 +0100, David Geier wrote:
> On 1/18/23 13:52, David Geier wrote:
> > On 1/16/23 21:39, Pavel Stehule wrote:
> > >
> > > po 16. 1. 2023 v 21:34 odesílatel Tomas Vondra
> > > napsal:
> > >
> > > Hi,
> > >
> > > there's minor bitrot in the Mkvcbuild.pm change,
Hi,
On 2023-01-19 11:47:49 +0100, David Geier wrote:
> > I also couldn't help and hacked a bit on the rdtsc pieces. I did figure out
> > how to do the cycles->nanosecond conversion with integer shift and multiply
> > in
> > the common case, which does show a noticable speedup. But that's for
Hi,
On 2023-01-20 22:27:07 -0500, Tom Lane wrote:
> Andres Freund writes:
> >> Perhaps an INSTR_TIME_ZERO() that could be assigned in variable definitions
> >> could give us the best of both worlds?
>
> > I tried that in the attached 0005. I found that it reads better if I also
> > add
> >
Andres Freund writes:
>> Perhaps an INSTR_TIME_ZERO() that could be assigned in variable definitions
>> could give us the best of both worlds?
> I tried that in the attached 0005. I found that it reads better if I also add
> INSTR_TIME_CURRENT(). If we decide to go for this, I'd roll it into
Hi,
On 2023-01-17 10:50:53 -0800, Andres Freund wrote:
> On 2023-01-17 12:26:57 -0500, Tom Lane wrote:
> > > 0001) Add INSTR_TIME_SET_ZERO() calls where otherwise 0002 causes gcc to
> > > warn
> > > Alternatively we can decide to deprecate INSTR_TIME_SET_ZERO() and
> > > just
On 1/20/23 07:43, David Geier wrote:
> On 1/18/23 13:52, David Geier wrote:
>> On 1/16/23 21:39, Pavel Stehule wrote:
>>>
>>> po 16. 1. 2023 v 21:34 odesílatel Tomas Vondra
>>> napsal:
>>>
>>> Hi,
>>>
>>> there's minor bitrot in the Mkvcbuild.pm change, making cfbot
>>> unhappy.
>>>
On 1/18/23 13:52, David Geier wrote:
On 1/16/23 21:39, Pavel Stehule wrote:
po 16. 1. 2023 v 21:34 odesílatel Tomas Vondra
napsal:
Hi,
there's minor bitrot in the Mkvcbuild.pm change, making cfbot
unhappy.
As for the patch, I don't have much comments. I'm wondering if
it'd
Hi Andres,
I also couldn't help and hacked a bit on the rdtsc pieces. I did figure out
how to do the cycles->nanosecond conversion with integer shift and multiply in
the common case, which does show a noticable speedup. But that's for another
day.
I also have code for that here. I decided
Hi,
@Andres: will you take care of these changes and provide me with an
updated patch set so I can rebase the RDTSC changes?
Otherwise, I can also apply Tom suggestions to your patch set and send
out the complete patch set.
--
David Geier
(ServiceNow)
On 1/16/23 18:37, Andres Freund wrote:
Hi,
On 2023-01-02 14:28:20 +0100, David Geier wrote:
I'm doubtful this is worth the complexity it incurs. By the time we convert
out of the instr_time format, the times shouldn't be small enough that the
accuracy is affected much.
I don't feel strong
On 1/16/23 21:39, Pavel Stehule wrote:
po 16. 1. 2023 v 21:34 odesílatel Tomas Vondra
napsal:
Hi,
there's minor bitrot in the Mkvcbuild.pm change, making cfbot unhappy.
As for the patch, I don't have much comments. I'm wondering if it'd be
useful to indicate which timing
Hi,
On 2023-01-17 12:26:57 -0500, Tom Lane wrote:
> Andres Freund writes:
> > Here's an updated version of the move to representing instr_time as
> > nanoseconds. It's now split into a few patches:
>
> I took a quick look through this.
Thanks!
> > 0001) Add INSTR_TIME_SET_ZERO() calls where
Andres Freund writes:
> Here's an updated version of the move to representing instr_time as
> nanoseconds. It's now split into a few patches:
I took a quick look through this.
> 0001) Add INSTR_TIME_SET_ZERO() calls where otherwise 0002 causes gcc to
> warn
> Alternatively we can
Hi,
On 2023-01-17 08:46:12 -0500, Robert Haas wrote:
> On Fri, Jan 13, 2023 at 2:56 PM Andres Freund wrote:
> > Does anybody see a reason to not move forward with this aspect? We do a fair
> > amount of INSTR_TIME_ACCUM_DIFF() etc, and that gets a good bit cheaper by
> > just using nanoseconds.
On Fri, Jan 13, 2023 at 2:56 PM Andres Freund wrote:
> Does anybody see a reason to not move forward with this aspect? We do a fair
> amount of INSTR_TIME_ACCUM_DIFF() etc, and that gets a good bit cheaper by
> just using nanoseconds. We'd also save memory in BufferUsage (144-122 bytes),
>
po 16. 1. 2023 v 21:34 odesílatel Tomas Vondra <
tomas.von...@enterprisedb.com> napsal:
> Hi,
>
> there's minor bitrot in the Mkvcbuild.pm change, making cfbot unhappy.
>
> As for the patch, I don't have much comments. I'm wondering if it'd be
> useful to indicate which timing source was actually
Hi,
there's minor bitrot in the Mkvcbuild.pm change, making cfbot unhappy.
As for the patch, I don't have much comments. I'm wondering if it'd be
useful to indicate which timing source was actually used for EXPLAIN
ANALYZE, say something like:
Planning time: 0.197 ms
Execution time: 0.225 ms
Hi,
On 2023-01-02 14:28:20 +0100, David Geier wrote:
> I also somewhat improved the accuracy of the cycles to milli- and
> microseconds conversion functions by having two more multipliers with higher
> precision. For microseconds we could also keep the computation integer-only.
> I'm wondering
Hi,
On 2023-01-13 11:55:47 -0800, Andres Freund wrote:
> Does anybody see a reason to not move forward with this aspect? We do a fair
> amount of INSTR_TIME_ACCUM_DIFF() etc, and that gets a good bit cheaper by
> just using nanoseconds. We'd also save memory in BufferUsage (144-122 bytes),
>
On Wed, 4 Jan 2023 at 17:32, David Geier wrote:
>
> I fixed the compilation error on CFBot.
> I missed adding instr_time.c to the Meson makefile.
> New patch set attached.
The patch does not apply on top of HEAD as in [1], please post a rebased patch:
=== Applying patches on top of PostgreSQL
Hi,
On 2023-01-13 15:25:16 -0500, Tom Lane wrote:
> Andres Freund writes:
> > Does anybody see a reason to not move forward with this aspect? We do a fair
> > amount of INSTR_TIME_ACCUM_DIFF() etc, and that gets a good bit cheaper by
> > just using nanoseconds.
>
> Cheaper, and perhaps more
Andres Freund writes:
> On 2023-01-04 13:02:05 +0100, David Geier wrote:
>> Subject: [PATCH 1/3] Change instr_time to just store nanoseconds, that's
>> cheaper.
> Does anybody see a reason to not move forward with this aspect? We do a fair
> amount of INSTR_TIME_ACCUM_DIFF() etc, and that gets a
Hi,
On 2023-01-04 13:02:05 +0100, David Geier wrote:
> From be18633d4735f680c7910fcb4e8ac90c4eada131 Mon Sep 17 00:00:00 2001
> From: David Geier
> Date: Thu, 17 Nov 2022 10:22:01 +0100
> Subject: [PATCH 1/3] Change instr_time to just store nanoseconds, that's
> cheaper.
Does anybody see a
Hi,
CFBot shows some compilation errors as in [1], please post an updated
version for the same:
09:08:12.525] /usr/bin/ld:
src/bin/pg_test_timing/pg_test_timing.p/pg_test_timing.c.o: warning:
relocation against `cycles_to_sec' in read-only section `.text'
[09:08:12.525] /usr/bin/ld:
On Tue, 3 Jan 2023 at 14:08, David Geier wrote:
>
> Hi Lukas,
>
> On 1/2/23 20:50, Lukas Fittl wrote:
> > Thanks for continuing to work on this patch, and my apologies for
> > silence on the patch.
>
> It would be great if you could review it.
> Please also share your thoughts around exposing the
Hi Lukas,
On 1/2/23 20:50, Lukas Fittl wrote:
Thanks for continuing to work on this patch, and my apologies for
silence on the patch.
It would be great if you could review it.
Please also share your thoughts around exposing the used clock source as
GUC and renaming INSTR_TIME_GET_DOUBLE() to
On Fri, Jul 15, 2022 at 11:21 AM Maciek Sakrejda wrote:
> On Fri, Jul 1, 2022 at 10:26 AM Andres Freund wrote:
> > On 2022-07-01 01:23:01 -0700, Lukas Fittl wrote:
> >...
> > > Known WIP problems with this patch version:
> > >
> > > * There appears to be a timing discrepancy I haven't yet worked
Hi David,
Thanks for continuing to work on this patch, and my apologies for silence
on the patch.
Its been hard to make time, and especially so because I typically develop
on an ARM-based macOS system where I can't test this directly - hence my
tests with virtualized EC2 instances, where I ran
Hi,
I re-based again on master and applied the following changes:
I removed the fallback for obtaining the TSC frequency from /proc/cpu as
suggested by Andres. Worst-case we fall back to clock_gettime().
I added code to obtain the TSC frequency via CPUID when under a
hypervisor. I had to
I missed attaching the patches.
--
David Geier
(ServiceNow)
From f4e962729ca605498d0c8bfc97d0f42d68a0df06 Mon Sep 17 00:00:00 2001
From: David Geier
Date: Thu, 17 Nov 2022 10:22:01 +0100
Subject: [PATCH 1/2] WIP: Change instr_time to just store nanoseconds, that's
cheaper.
---
I think it would be great to get this patch committed. Beyond the
reasons already mentioned, the significant overhead also tends to skew
the reported runtimes in ways that makes it difficult to compare them.
For example, if two nodes are executed equally often but one needs twice
the time to
On Tue, Sep 06, 2022 at 11:32:18AM +0500, Ibrar Ahmed wrote:
> Hunk #5 succeeded at 147 with fuzz 2 (offset -3 lines).
> Hunk #6 FAILED at 170.
> Hunk #7 succeeded at 165 (offset -69 lines).
> 2 out of 7 hunks FAILED -- saving rejects to file
> src/include/portability/instr_time.h.rej
> patching
On Fri, Jul 15, 2022 at 11:22 PM Maciek Sakrejda
wrote:
> I ran that original test case with and without the patch. Here are the
> numbers I'm seeing:
>
> master (best of three):
>
> postgres=# SELECT count(*) FROM lotsarows;
> Time: 582.423 ms
>
> postgres=# EXPLAIN (ANALYZE, TIMING OFF) SELECT
I ran that original test case with and without the patch. Here are the
numbers I'm seeing:
master (best of three):
postgres=# SELECT count(*) FROM lotsarows;
Time: 582.423 ms
postgres=# EXPLAIN (ANALYZE, TIMING OFF) SELECT count(*) FROM lotsarows;
Time: 616.102 ms
postgres=# EXPLAIN (ANALYZE,
Hi,
On 2022-07-01 01:23:01 -0700, Lukas Fittl wrote:
> On Fri, Jun 12, 2020 at 4:28 PM Andres Freund wrote:
>
> > The attached patches are really just a prototype. I'm also not really
> > planning to work on getting this into a "production ready" patchset
> > anytime soon. I developed it
On Fri, Jun 12, 2020 at 4:28 PM Andres Freund wrote:
> The attached patches are really just a prototype. I'm also not really
> planning to work on getting this into a "production ready" patchset
> anytime soon. I developed it primarily because I found it the overhead
> made it too hard to nail
On Sat, Jun 13, 2020 at 11:28 AM Andres Freund wrote:
> [PATCH v1 1/2] WIP: Change instr_time to just store nanoseconds, that's
> cheaper.
Makes a lot of sense. If we do this, I'll need to update pgbench,
which just did something similar locally. If I'd been paying
attention to this thread I
so 13. 6. 2020 v 1:28 odesílatel Andres Freund napsal:
> Hi,
>
> Currently using EXPLAIN (ANALYZE) without TIMING OFF regularly changes
> the resulting timing enough that the times aren't meaningful. E.g.
>
> CREATE TABLE lotsarows(key int not null);
> INSERT INTO lotsarows SELECT
Hi,
Currently using EXPLAIN (ANALYZE) without TIMING OFF regularly changes
the resulting timing enough that the times aren't meaningful. E.g.
CREATE TABLE lotsarows(key int not null);
INSERT INTO lotsarows SELECT generate_series(1, 5000);
VACUUM FREEZE lotsarows;
-- best of three:
SELECT
60 matches
Mail list logo
