Thread
-
[PATCH] dtrace probes for memory manager
Zdenek Kotala <zdenek.kotala@sun.com> — 2009-11-13T20:58:56Z
Attached patch contains new dtrace probes for memory management. Main purpose is to analyze memory footprint - for example how many memory needs transaction, peak memory per context, when memory block is reused or when it is allocate by malloc and so on. There are three groups of probes: 1) general memory context operation: mcxt-alloc mcxt-create mcxt-delete mcxt-free mcxt-realloc mcxt-reset 2) AllocSet operations (called from mcxt) aset-alloc aset-delete aset-free aset-realloc aset-reset 3) AllocSet Block operations. aset-block-free aset-block-new aset-block-realloc aset-block-reset Zdenek
-
Re: [PATCH] dtrace probes for memory manager
Tom Lane <tgl@sss.pgh.pa.us> — 2009-11-13T21:06:16Z
Zdenek Kotala <Zdenek.Kotala@Sun.COM> writes: > Attached patch contains new dtrace probes for memory management. This is a bad idea and I want to reject it outright. No ordinary user is really going to care about those details, and palloc is a sufficiently hot hot-spot that even the allegedly negligible overhead of an inactive dtrace probe is going to cost us. If this goes in, I will disable dtrace support in Red Hat's builds, and I rather imagine that other packagers will react similarly. regards, tom lane
-
Re: [PATCH] dtrace probes for memory manager
Joshua D. Drake <jd@commandprompt.com> — 2009-11-13T21:09:06Z
On Fri, 2009-11-13 at 16:06 -0500, Tom Lane wrote: > Zdenek Kotala <Zdenek.Kotala@Sun.COM> writes: > > Attached patch contains new dtrace probes for memory management. > > This is a bad idea and I want to reject it outright. No ordinary user > is really going to care about those details, and palloc is a > sufficiently hot hot-spot that even the allegedly negligible overhead > of an inactive dtrace probe is going to cost us. No ordinary user is going to use dtrace at all. > > If this goes in, I will disable dtrace support in Red Hat's builds, > and I rather imagine that other packagers will react similarly. Is it possible to have a set of probes that would only be enabled with say, --enable-debug compile time option? I could certainly see the benefit to these probes for profiling but that is such as specific use that it seems to need a specific flag anyway. Sincerely, Joshua D. Drake > > regards, tom lane > -- PostgreSQL.org Major Contributor Command Prompt, Inc: http://www.commandprompt.com/ - 503.667.4564 Consulting, Training, Support, Custom Development, Engineering If the world pushes look it in the eye and GRR. Then push back harder. - Salamander
-
Re: [PATCH] dtrace probes for memory manager
Tom Lane <tgl@sss.pgh.pa.us> — 2009-11-13T21:20:01Z
"Joshua D. Drake" <jd@commandprompt.com> writes: > On Fri, 2009-11-13 at 16:06 -0500, Tom Lane wrote: >> This is a bad idea and I want to reject it outright. No ordinary user >> is really going to care about those details, and palloc is a >> sufficiently hot hot-spot that even the allegedly negligible overhead >> of an inactive dtrace probe is going to cost us. > No ordinary user is going to use dtrace at all. Right, but *those probes are going to cost him performance anyway* if he's using a dtrace-enabled build. Probes associated with I/O calls might be negligible, probes in palloc are not going to be. regards, tom lane
-
Re: [PATCH] dtrace probes for memory manager
Kevin Grittner <kevin.grittner@wicourts.gov> — 2009-11-13T21:22:38Z
"Joshua D. Drake" <jd@commandprompt.com> wrote: > Is it possible to have a set of probes that would only be enabled > with say, --enable-debug compile time option? But we routinely build with that for normal production use so that if we get a core dump or need to backtrace a problem process, we will get meaningful results. Please don't create a performance penalty for compiling with debug info. (We'd probably wind up disabling it....) -Kevin
-
Re: [PATCH] dtrace probes for memory manager
Joshua D. Drake <jd@commandprompt.com> — 2009-11-13T21:25:33Z
On Fri, 2009-11-13 at 15:22 -0600, Kevin Grittner wrote: > "Joshua D. Drake" <jd@commandprompt.com> wrote: > > > Is it possible to have a set of probes that would only be enabled > > with say, --enable-debug compile time option? > > But we routinely build with that for normal production use so that if > we get a core dump or need to backtrace a problem process, we will get > meaningful results. Please don't create a performance penalty for > compiling with debug info. (We'd probably wind up disabling it....) Good point but I still say that the probes are useful. I am not sure we have a good compile time flag for it right now but it seems that would be the way to do it. Joshua D. Drake > > -Kevin > -- PostgreSQL.org Major Contributor Command Prompt, Inc: http://www.commandprompt.com/ - 503.667.4564 Consulting, Training, Support, Custom Development, Engineering If the world pushes look it in the eye and GRR. Then push back harder. - Salamander
-
Re: [PATCH] dtrace probes for memory manager
Alvaro Herrera <alvherre@commandprompt.com> — 2009-11-13T21:34:21Z
Zdenek Kotala wrote: > Attached patch contains new dtrace probes for memory management. Main > purpose is to analyze memory footprint - for example how many memory > needs transaction, peak memory per context, when memory block is reused > or when it is allocate by malloc and so on. Having had to instrument these to figure out some problems, I'd give this patch a +1. However, the performance argument is compelling. As a compromise, maybe we could have a #define that needs to be turned on at compile time to enable these probes; so a regular dtrace-enabled build would not have them, but if you really needed to analyze memory allocations, you could recompile to turn them on. -- Alvaro Herrera http://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support
-
Re: [PATCH] dtrace probes for memory manager
Zdenek Kotala <zdenek.kotala@sun.com> — 2009-11-13T21:38:07Z
Tom Lane píše v pá 13. 11. 2009 v 16:06 -0500: > Zdenek Kotala <Zdenek.Kotala@Sun.COM> writes: > > Attached patch contains new dtrace probes for memory management. > > This is a bad idea and I want to reject it outright. No ordinary user > is really going to care about those details, and palloc is a > sufficiently hot hot-spot that even the allegedly negligible overhead > of an inactive dtrace probe is going to cost us. I don't think that impact is too big here. User space DTrace probes are implemented like function call. When probe is inactive call is replaced by nop. Only what stay in code is arguments preparations. there is asm code 32bit intel from palloc: MemoryContextAlloc+0xae: pushl $0x0 MemoryContextAlloc+0xb0: pushl -0x8(%ebx) MemoryContextAlloc+0xb3: pushl 0xc(%ebp) MemoryContextAlloc+0xb6: movl 0x8(%ebp),%eax MemoryContextAlloc+0xb9: pushl %eax MemoryContextAlloc+0xba: pushl 0x14(%eax) MemoryContextAlloc+0xbd: nop MemoryContextAlloc+0xbe: nop MemoryContextAlloc+0xbf: nop MemoryContextAlloc+0xc0: nop MemoryContextAlloc+0xc1: nop MemoryContextAlloc+0xc2: addl $0x20,%esp You can see is that overhead depends on number of argument. I used five arguments now but two can be enough. Only dtrace script will be complicated in some cases after that. By my opinion if you compare whole palloc code path, Dtrace probes overhead is minimal. Zdenek
-
Re: [PATCH] dtrace probes for memory manager
Zdenek Kotala <zdenek.kotala@sun.com> — 2009-11-13T21:41:38Z
Alvaro Herrera píše v pá 13. 11. 2009 v 18:34 -0300: > Zdenek Kotala wrote: > > Attached patch contains new dtrace probes for memory management. Main > > purpose is to analyze memory footprint - for example how many memory > > needs transaction, peak memory per context, when memory block is reused > > or when it is allocate by malloc and so on. > > Having had to instrument these to figure out some problems, I'd give > this patch a +1. However, the performance argument is compelling. As a > compromise, maybe we could have a #define that needs to be turned on at > compile time to enable these probes; so a regular dtrace-enabled build > would not have them, but if you really needed to analyze memory > allocations, you could recompile to turn them on. But point of dtrace probes is that they are here without recompilation :(. Do we have any test which we could use for performance penalty testing? I don't think that overhead is significant. Zdenek
-
Re: [PATCH] dtrace probes for memory manager
Robert Haas <robertmhaas@gmail.com> — 2009-11-13T22:16:15Z
On Fri, Nov 13, 2009 at 4:41 PM, Zdenek Kotala <Zdenek.Kotala@sun.com> wrote: > Alvaro Herrera píše v pá 13. 11. 2009 v 18:34 -0300: >> Zdenek Kotala wrote: >> > Attached patch contains new dtrace probes for memory management. Main >> > purpose is to analyze memory footprint - for example how many memory >> > needs transaction, peak memory per context, when memory block is reused >> > or when it is allocate by malloc and so on. >> >> Having had to instrument these to figure out some problems, I'd give >> this patch a +1. However, the performance argument is compelling. As a >> compromise, maybe we could have a #define that needs to be turned on at >> compile time to enable these probes; so a regular dtrace-enabled build >> would not have them, but if you really needed to analyze memory >> allocations, you could recompile to turn them on. > > But point of dtrace probes is that they are here without > recompilation :(. Do we have any test which we could use for performance > penalty testing? I don't think that overhead is significant. Don't think. Benchmark. :-) (If you can measure it at all, it's too much, at least IMHO.) ...Robert
-
Re: [PATCH] dtrace probes for memory manager
Zdenek Kotala <zdenek.kotala@sun.com> — 2009-11-13T22:20:48Z
Alvaro Herrera píše v pá 13. 11. 2009 v 18:34 -0300: > Zdenek Kotala wrote: > > Attached patch contains new dtrace probes for memory management. Main > > purpose is to analyze memory footprint - for example how many memory > > needs transaction, peak memory per context, when memory block is reused > > or when it is allocate by malloc and so on. > > Having had to instrument these to figure out some problems, I'd give > this patch a +1. However, the performance argument is compelling. As a > compromise, maybe we could have a #define that needs to be turned on at > compile time to enable these probes; so a regular dtrace-enabled build > would not have them, but if you really needed to analyze memory > allocations, you could recompile to turn them on. The another idea is to have two AllocSet functions set. One without and one with dtrace probes. And switch it by GUC flag for example. From mcxt I create and delete context is important, rest can be taken from alloc set probes. Zdenek
-
Re: [PATCH] dtrace probes for memory manager
Robert Haas <robertmhaas@gmail.com> — 2009-11-13T22:21:34Z
On Fri, Nov 13, 2009 at 5:20 PM, Zdenek Kotala <Zdenek.Kotala@sun.com> wrote: > Alvaro Herrera píše v pá 13. 11. 2009 v 18:34 -0300: >> Zdenek Kotala wrote: >> > Attached patch contains new dtrace probes for memory management. Main >> > purpose is to analyze memory footprint - for example how many memory >> > needs transaction, peak memory per context, when memory block is reused >> > or when it is allocate by malloc and so on. >> >> Having had to instrument these to figure out some problems, I'd give >> this patch a +1. However, the performance argument is compelling. As a >> compromise, maybe we could have a #define that needs to be turned on at >> compile time to enable these probes; so a regular dtrace-enabled build >> would not have them, but if you really needed to analyze memory >> allocations, you could recompile to turn them on. > > The another idea is to have two AllocSet functions set. One without and > one with dtrace probes. And switch it by GUC flag for example. From mcxt > I create and delete context is important, rest can be taken from alloc > set probes. I've always thought it was odd that we had a function-dispatch interface in such a performance-critical path. But if we really want to keep it around this might not be a bad thing to use it for. ...Robert
-
Re: [PATCH] dtrace probes for memory manager
Bernd Helmle <mailings@oopsware.de> — 2009-12-08T20:42:31Z
--On 13. November 2009 17:16:15 -0500 Robert Haas <robertmhaas@gmail.com> wrote: > Don't think. Benchmark. :-) > > (If you can measure it at all, it's too much, at least IMHO.) I've tried to benchmark this now on my (fairly slow compared to server hardware) MacBook and see some negative trend for those memory probes in pgbench. Running dozens of rounds with pgbench (scale 150, 10 clients / 1000 transactions) gives the following numbers (untuned PostgreSQL config): AVG(tps) with dtrace memory probes: 31.62 tps AVG(tps) without dtrace memory probes: 38.24 tps So there seems to be a minor slowdown at least on *my* machine. However, it would be fine if someone can prove these numbers.. What do you guys think, what other tests/parameters can be invoked to test for an impact ? -- Thanks Bernd
-
Re: [PATCH] dtrace probes for memory manager
Greg Smith <greg@2ndquadrant.com> — 2009-12-08T20:51:52Z
Bernd Helmle wrote: > I've tried to benchmark this now on my (fairly slow compared to server > hardware) MacBook and see some negative trend for those memory probes > in pgbench. Running dozens of rounds with pgbench (scale 150, 10 > clients / 1000 transactions) That makes for a 5.5 minute test, which is unfortunately close to the default checkpoint period. You're going to want a pgbench configuration that's doing thousands of operations per second to measure this overhead I think, and let it run a bit longer. The difference you're seeing could easily be just that that the "with probes" result had more checkpoints happen during testing than the other one--if it got even a single checkpoint more, that could be enough to throw results off using the default test and such low TPS results. Try this instead, which will give you a test where checkpoints have a minimal impact, but lots of memory will be thrown around: pgbench -i -s 10 <db> pgbench -S -c 10 -T 600 <db> That will do just SELECT statements against a much smaller database (about 160MB) and will run for 10 minutes each time. -- Greg Smith 2ndQuadrant Baltimore, MD PostgreSQL Training, Services and Support greg@2ndQuadrant.com www.2ndQuadrant.com
-
Re: [PATCH] dtrace probes for memory manager
Bernd Helmle <mailings@oopsware.de> — 2009-12-08T21:06:37Z
--On 8. Dezember 2009 15:51:52 -0500 Greg Smith <greg@2ndquadrant.com> wrote: > Try this instead, which will give you a test where checkpoints have a > minimal impact, but lots of memory will be thrown around: > > pgbench -i -s 10 <db> > pgbench -S -c 10 -T 600 <db> Thanks for the input, will try.... -- Thanks Bernd
-
Re: [PATCH] dtrace probes for memory manager
Zdenek Kotala <zdenek.kotala@sun.com> — 2009-12-09T14:04:29Z
Bernd Helmle píše v út 08. 12. 2009 v 22:06 +0100: > > --On 8. Dezember 2009 15:51:52 -0500 Greg Smith <greg@2ndquadrant.com> > wrote: > > > Try this instead, which will give you a test where checkpoints have a > > minimal impact, but lots of memory will be thrown around: > > > > pgbench -i -s 10 <db> > > pgbench -S -c 10 -T 600 <db> > > Thanks for the input, will try.... > I modified probes to reduce overhead. Prototype patch is attached. Main point is to remove mcxt_alloc probe and keep only aset_alloc. I did also some testing with interesting results. At first I prepare special C store function (attached) which do only allocation and deallocation and I measured how long it takes: On 32bit the memory allocation is slow down 8.4% and on 64bit it is only 4.6%. Good to mention that I call palloc and pfree but in standard behavior pfree is not much used and memory is freed when context is destroyed. It means that we should think about 4.2% and 2.3% instead. But in normal situation database does also other thing and palloc is only one part of code path. It is why I run second test and use sun studio profiling tools (collect/analyzer) to determine how much CPU ticks cost the probes during pg_bench run. And results are much better. AllocSet alloc function takes about 4-5% and probes assembler code takes 0.1-0.2% on 64bit. I did not test 32bit but my expectation is that it should be about 0.3-0.4%. Zdenek
-
Re: [PATCH] dtrace probes for memory manager
Frank Ch. Eigler <fche@redhat.com> — 2009-12-10T19:11:20Z
Zdenek Kotala <Zdenek.Kotala@Sun.COM> writes: > [...] > + header = (StandardChunkHeader *) > + ((char *) ret - STANDARDCHUNKHEADERSIZE); > + > +// TRACE_POSTGRESQL_MCXT_ALLOC(context->name, context, size, header->size, true); > + > [...] If the dormant overhead of these probes is measured or suspected to be excessive, consider using the dtrace-generated per-probe foo_ENABLED() conditional, or a postgres configuration global thusly: if (__builtin_expect(TRACE_POSTGRESQL_MCXT_ALLOC_ENABLED(), 0)) TRACE_POSTGRESQL_MCXT_ALLOC(...); so that the whole instrumentation parameter setup/call can be placed out of the hot line with gcc -freorder-blocks. - FChE -
Re: [PATCH] dtrace probes for memory manager
Zdenek Kotala <zdenek.kotala@sun.com> — 2009-12-10T20:33:28Z
Frank Ch. Eigler píše v čt 10. 12. 2009 v 14:11 -0500: > Zdenek Kotala <Zdenek.Kotala@Sun.COM> writes: > > > [...] > > + header = (StandardChunkHeader *) > > + ((char *) ret - STANDARDCHUNKHEADERSIZE); > > + > > +// TRACE_POSTGRESQL_MCXT_ALLOC(context->name, context, size, header->size, true); > > + > > [...] > > If the dormant overhead of these probes is measured or suspected to be > excessive, consider using the dtrace-generated per-probe foo_ENABLED() > conditional, or a postgres configuration global thusly: TRACE_POSTGRESQL_MCXT_ALLOC and TRACE_POSTGRESQL_ASET_ALLOC are duplicated probes. Have them both make sense but from performance point of view to have one of them is acceptable. foo_enable() is good to use when number of argument and their evaluation cost too much. In this case it does no seem to be much useful. See ASM code: AllocSetAlloc+0x17: xorq %rax,%rax AllocSetAlloc+0x1a: nop AllocSetAlloc+0x1b: nop AllocSetAlloc+0x1c: testl %eax,%eax AllocSetAlloc+0x1e: je +0xb <AllocSetAlloc+0x2b> AllocSetAlloc+0x20: movq %r13,%rdi AllocSetAlloc+0x23: movq %r14,%rsi AllocSetAlloc+0x26: nop AllocSetAlloc+0x27: nop AllocSetAlloc+0x28: nop AllocSetAlloc+0x29: nop AllocSetAlloc+0x2a: nop > if (__builtin_expect(TRACE_POSTGRESQL_MCXT_ALLOC_ENABLED(), 0)) > TRACE_POSTGRESQL_MCXT_ALLOC(...); > > so that the whole instrumentation parameter setup/call can be placed > out of the hot line with gcc -freorder-blocks. compiler specific construct is not good way. Do not forget that also other compiler exists. Zdenek
-
Re: [PATCH] dtrace probes for memory manager
Frank Ch. Eigler <fche@redhat.com> — 2009-12-10T21:34:40Z
Hi - On Thu, Dec 10, 2009 at 09:33:28PM +0100, Zdenek Kotala wrote: > [...] > > If the dormant overhead of these probes is measured or suspected to be > > excessive, consider using the dtrace-generated per-probe foo_ENABLED() > > conditional, or a postgres configuration global thusly: > [...] foo_enable() is good to use when number of argument and their > evaluation cost too much. In this case it does no seem to be much > useful. [...] Right, I just wanted to make the others aware of the option. > > if (__builtin_expect(TRACE_POSTGRESQL_MCXT_ALLOC_ENABLED(), 0)) > > TRACE_POSTGRESQL_MCXT_ALLOC(...); > > > > so that the whole instrumentation parameter setup/call can be placed > > out of the hot line with gcc -freorder-blocks. > > compiler specific construct is not good way. Do not forget that also > other compiler exists. Certainly. Many projects -- but apparently not postgresql -- wrap such branch prediction hints in macros such as likely() and unlikely(), which are easily no-op'd for compilers that don't support this sort of thing. - FChE
-
Re: [PATCH] dtrace probes for memory manager
Robert Haas <robertmhaas@gmail.com> — 2009-12-11T04:55:49Z
On Wed, Dec 9, 2009 at 9:04 AM, Zdenek Kotala <Zdenek.Kotala@sun.com> wrote: > Bernd Helmle píše v út 08. 12. 2009 v 22:06 +0100: >> >> --On 8. Dezember 2009 15:51:52 -0500 Greg Smith <greg@2ndquadrant.com> >> wrote: >> >> > Try this instead, which will give you a test where checkpoints have a >> > minimal impact, but lots of memory will be thrown around: >> > >> > pgbench -i -s 10 <db> >> > pgbench -S -c 10 -T 600 <db> >> >> Thanks for the input, will try.... >> > > I modified probes to reduce overhead. Prototype patch is attached. Main > point is to remove mcxt_alloc probe and keep only aset_alloc. I did also > some testing with interesting results. At first I prepare special C > store function (attached) which do only allocation and deallocation and > I measured how long it takes: > > On 32bit the memory allocation is slow down 8.4% and on 64bit it is > only 4.6%. Good to mention that I call palloc and pfree but in standard > behavior pfree is not much used and memory is freed when context is > destroyed. It means that we should think about 4.2% and 2.3% instead. > > But in normal situation database does also other thing and palloc is > only one part of code path. It is why I run second test and use sun > studio profiling tools (collect/analyzer) to determine how much CPU > ticks cost the probes during pg_bench run. And results are much better. > AllocSet alloc function takes about 4-5% and probes assembler code takes > 0.1-0.2% on 64bit. I did not test 32bit but my expectation is that it > should be about 0.3-0.4%. There's not really enough detail here to determine what you tested and what the results were, and I don't think this patch has any chance at all of getting committed without that. Please clarify. If there's some real-world test where this probe costs 0.3%-0.4%, I think that is sufficient grounds for rejecting this patch. I understand the desire of people to be able to use dtrace, but our performance is too hard-won for me to want to give any measurable of it up for tracing and instrumentation hooks that will only be used by a small number of users in a small number of situations. ...Robert
-
Re: [PATCH] dtrace probes for memory manager
Bernd Helmle <mailings@oopsware.de> — 2009-12-11T14:22:03Z
--On 10. Dezember 2009 23:55:49 -0500 Robert Haas <robertmhaas@gmail.com> wrote: > If there's some real-world test where this probe costs 0.3%-0.4%, I > think that is sufficient grounds for rejecting this patch. I > understand the desire of people to be able to use dtrace, but our > performance is too hard-won for me to want to give any measurable of > it up for tracing and instrumentation hooks that will only be used by > a small number of users in a small number of situations. I repeated the pgbench runs per Greg's advice (see upthread) and it seems there is actually a small slowdown which supports this argument, unfortunately. After repeating the pgbench runs with and without the new probes (note: i've used the new version of the patch, too), the numbers are going to stabilize as follows: without compiled probes: AVG(2531.68) with compiled probes: AVG(2511.97) I can repeat that tests over and over, but this doesn't seem to change the whole picture (so there seems some real argument for a 0.4 - 0.6% cost, at least on *my* machine here with pgbench). -- Thanks Bernd
-
Re: [PATCH] dtrace probes for memory manager
Alvaro Herrera <alvherre@commandprompt.com> — 2009-12-11T14:28:54Z
Bernd Helmle escribió: > I repeated the pgbench runs per Greg's advice (see upthread) and it > seems there is actually a small slowdown which supports this > argument, unfortunately. After repeating the pgbench runs with and > without the new probes (note: i've used the new version of the > patch, too), the numbers are going to stabilize as follows: > > without compiled probes: AVG(2531.68) > with compiled probes: AVG(2511.97) Were the probes enabled? -- Alvaro Herrera http://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc.
-
Re: [PATCH] dtrace probes for memory manager
Zdenek Kotala <zdenek.kotala@sun.com> — 2009-12-11T17:59:38Z
Robert Haas píše v čt 10. 12. 2009 v 23:55 -0500: > On Wed, Dec 9, 2009 at 9:04 AM, Zdenek Kotala <Zdenek.Kotala@sun.com> wrote: > > > > But in normal situation database does also other thing and palloc is > > only one part of code path. It is why I run second test and use sun > > studio profiling tools (collect/analyzer) to determine how much CPU > > ticks cost the probes during pg_bench run. And results are much better. > > AllocSet alloc function takes about 4-5% and probes assembler code takes > > 0.1-0.2% on 64bit. I did not test 32bit but my expectation is that it > > should be about 0.3-0.4%. > > There's not really enough detail here to determine what you tested and > what the results were, and I don't think this patch has any chance at > all of getting committed without that. Please clarify. > > If there's some real-world test where this probe costs 0.3%-0.4%, I > think that is sufficient grounds for rejecting this patch. I > understand the desire of people to be able to use dtrace, but our > performance is too hard-won for me to want to give any measurable of > it up for tracing and instrumentation hooks that will only be used by > a small number of users in a small number of situations. > As I mentioned I run pg_bench -c10 -t1000 and collect data from backends. collect and analyzer is similar tool to gprof. Zdenek