Thread: [HACKERS] memory fields from getrusage()
I'm interested to expose output of the remaining (memory) fields from getrusage(). postgres=# SET log_parser_stats='on'; postgres=# SELECT c.oid::regclass, usagecount FROM pg_buffercache b JOIN pg_class c USING (relfilenode) WHERE usagecount=1; LOG: PARSER STATISTICS DETAIL: ! system usage stats: ! 0.000197 elapsed 0.000119 user 0.000079 system sec ! [0.011572 user0.007714 sys total] ! 0/0 [0/264] filesystem blocks in/out ! 0/39 [0/989] page faults/reclaims,0 [0] swaps ! 0 [0] signals rcvd, 0/0 [0/0] messages rcvd/sent ! 0/0 [2/2] voluntary/involuntarycontext switches ! 3920 MAX RESIDENT, 0 SHARED, 0 UNSHARED DATA, 0 UNSHARED STACK (Kb) Before that can work for some platforms, it looks like rusagestub.h needs to have any desired fields added, and ./src/port/getrusage.c should memset(0) in the non-windows case before adding any reliance on the rest of the structure. This comment from ~1996 says: https://doxygen.postgresql.org/postgres_8c_source.html4421 * the only stats we don't show here are for memory usage-- i can't4422 * figure out how to interpret the relevant fields in the rusage struct,4423 * and they changenames across o/s platforms, anyway. if you can figure4424 * out what the entries mean, you can somehow extractresident set size,4425 * shared text size, and unshared data and stack sizes. .. is that really (still) the case for supported platforms? I'm hoping that in 2017 one can just call getrusage() if autoconf says it's okay ?? Justin
Justin Pryzby <pryzby@telsasoft.com> writes: > This comment from ~1996 says: > https://doxygen.postgresql.org/postgres_8c_source.html > 4421 * the only stats we don't show here are for memory usage -- i can't > 4422 * figure out how to interpret the relevant fields in the rusage struct, > 4423 * and they change names across o/s platforms, anyway. if you can figure > 4424 * out what the entries mean, you can somehow extract resident set size, > 4425 * shared text size, and unshared data and stack sizes. > .. is that really (still) the case for supported platforms? I'm hoping that in > 2017 one can just call getrusage() if autoconf says it's okay ?? We already do call getrusage(). The point of that comment is that the contents of the resulting struct rusage are not very well standardized. POSIX says only The <sys/resource.h> header defines the rusage structure that includes at least the following members: struct timeval ru_utime user time usedstruct timeval ru_stime system time used (seems the same in 1997 and 2008 text). So the existing code has already got its neck stuck way out in terms of portability. Maybe you could push it further, but I bet you'll find that the situation isn't any better than it was at the time that comment was written. It's entirely possible that we could simplify the code some, because I suspect that Windows is the only remaining platform that doesn't HAVE_GETRUSAGE. But that in itself doesn't mean we can use any more fields than we do now. regards, tom lane
On Sat, Jun 10, 2017 at 9:31 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > We already do call getrusage(). The point of that comment is that the > contents of the resulting struct rusage are not very well standardized. > POSIX says only > > The <sys/resource.h> header defines the rusage structure that includes > at least the following members: > > struct timeval ru_utime user time used > struct timeval ru_stime system time used > > (seems the same in 1997 and 2008 text). So the existing code has already > got its neck stuck way out in terms of portability. Maybe you could push > it further, but I bet you'll find that the situation isn't any better than > it was at the time that comment was written. > > It's entirely possible that we could simplify the code some, because > I suspect that Windows is the only remaining platform that doesn't > HAVE_GETRUSAGE. But that in itself doesn't mean we can use any more > fields than we do now. It might be worth adding platform-specific code for common platforms. I checked macOS X 10.10.5 (Yosemite) and a Linux system (hydra, old Fedora release) and they seem to list identical fields for struct rusage, which is good as far as it goes, but Linux documents a bunch of the interesting fields (ru_ixrss, ru_idrss, ru_isrss, ru_nswap, ru_msgsnd, ru_msgrcv, ru_nsignals) as unused, and apparently returns ru_maxrss in kilobytes where macOS reports it in bytes. It seems like it would be a good idea to install code specific to Linux that displays all and only those values that are meaningful on Linux, and (less importantly) similarly for macOS. Linux is such a common platform that reporting bogus zero values and omitting other fields that are actually meaningful does not seem like a very good plan. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On Tue, Jun 13, 2017 at 12:16:00PM -0400, Robert Haas wrote:
> It might be worth adding platform-specific code for common platforms.
All I care (which linux happily/happens to support) is maxrss; I was probably
originally interested in this while digging into an issue with hash agg.
I think it's fine to show zeros for unsupported fields; that's what getusage(2)
and time(1) do after all.
pryzbyj@pryzbyj:~$ sh -c 'command time -v find /dev' 2>&1 >/dev/null |grep -Fw 0 User time (seconds): 0.00
Systemtime (seconds): 0.00 Elapsed (wall clock) time (h:mm:ss or m:ss): 0:00.00 Average shared text size
(kbytes):0 Average unshared data size (kbytes): 0 Average stack size (kbytes): 0 Average total size
(kbytes):0 Average resident set size (kbytes): 0 Major (requiring I/O) page faults: 0 Swaps: 0
Filesystem inputs: 0 File system outputs: 0 Socket messages sent: 0 Socket messages received: 0
Signalsdelivered: 0
> it would be a good idea to install code specific to Linux that
> displays all and only those values that are meaningful on Linux, and
> (less importantly) similarly for macOS. Linux is such a common
> platform that reporting bogus zero values and omitting other fields
> that are actually meaningful does not seem like a very good plan.
That has the issue that it varies not just by OS but also by OS version. For
example PG already shows context switches and FS in/out puts, but they're
nonzero only since linux 2.6 (yes, 2.4 is ancient and unsupported but still).
ru_nvcsw (since Linux 2.6) ru_inblock (since Linux 2.6.22)
..and other fields are "currently unused", but maybe supported in the past or
future(?) ru_ixrss (unmaintained) This field is currently unused on Linux.
Are you thinking of something like this, maybe hidden away in a separate file
somewhere?
#if defined(__linux__) || defined(BSD)appendStringInfo(&str, "!\t%ld max resident, %ld shared, %ld unshared data, %ld
unsharedstack (kB)\n", r.ru_maxrss, r.ru_ixrss, r.ru_idrss, r.ru_isrss);
#elif defined(__darwin__)appendStringInfo(&str, "!\t%ld max resident, %ld shared, %ld unshared data, %ld unshared stack
(kB)\n",r.ru_maxrss/1024, r.ru_ixrss/1024, r.ru_idrss/1024, r.ru_isrss/1024);
#endif /* __linux__ */
Justin
On Wed, Jun 14, 2017 at 6:28 PM, Justin Pryzby <pryzby@telsasoft.com> wrote: > On Tue, Jun 13, 2017 at 12:16:00PM -0400, Robert Haas wrote: >> It might be worth adding platform-specific code for common platforms. > > All I care (which linux happily/happens to support) is maxrss; I was probably > originally interested in this while digging into an issue with hash agg. > > I think it's fine to show zeros for unsupported fields; that's what getusage(2) > and time(1) do after all. Meh. I think if we're going to go to the trouble of customizing this code by platform, we ought to get rid of values that are meaningless on that platform. >> it would be a good idea to install code specific to Linux that >> displays all and only those values that are meaningful on Linux, and >> (less importantly) similarly for macOS. Linux is such a common >> platform that reporting bogus zero values and omitting other fields >> that are actually meaningful does not seem like a very good plan. > > That has the issue that it varies not just by OS but also by OS version. For > example PG already shows context switches and FS in/out puts, but they're > nonzero only since linux 2.6 (yes, 2.4 is ancient and unsupported but still). > > ru_nvcsw (since Linux 2.6) > ru_inblock (since Linux 2.6.22) If people who are still running 10 or 15 year old versions of Linux see some zeroes for fields that might've been populated on a newer release, I'm OK with that. Maybe it will encourage them to upgrade. We've completely removed platform support for systems newer than that. > ..and other fields are "currently unused", but maybe supported in the past or > future(?) > ru_ixrss (unmaintained) > This field is currently unused on Linux. Could be worth a bit of research here. > Are you thinking of something like this, maybe hidden away in a separate file > somewhere? > > #if defined(__linux__) || defined(BSD) > appendStringInfo(&str, "!\t%ld max resident, %ld shared, %ld unshared data, %ld unshared stack (kB)\n", r.ru_maxrss,r.ru_ixrss, r.ru_idrss, r.ru_isrss); > #elif defined(__darwin__) > appendStringInfo(&str, "!\t%ld max resident, %ld shared, %ld unshared data, %ld unshared stack (kB)\n", r.ru_maxrss/1024,r.ru_ixrss/1024, r.ru_idrss/1024, r.ru_isrss/1024); > #endif /* __linux__ */ I don't think it needs to go in a separate file. I'd just patch ShowUsage(). -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On Thu, Jun 15, 2017 at 10:29:21AM -0400, Robert Haas wrote:
> On Wed, Jun 14, 2017 at 6:28 PM, Justin Pryzby <pryzby@telsasoft.com> wrote:
> > On Tue, Jun 13, 2017 at 12:16:00PM -0400, Robert Haas wrote:
> >> It might be worth adding platform-specific code for common platforms.
> >
> > All I care (which linux happily/happens to support) is maxrss; I was probably
> > originally interested in this while digging into an issue with hash agg.
>
> I don't think it needs to go in a separate file. I'd just patch ShowUsage().
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index f99dd0a..7f57a84 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -4467,6 +4467,21 @@ ShowUsage(const char *title)
r.ru_nvcsw - Save_r.ru_nvcsw,
r.ru_nivcsw - Save_r.ru_nivcsw,
r.ru_nvcsw, r.ru_nivcsw);
+
+#if defined(__linux__)
+ appendStringInfo(&str,
+ "!\t%ld max resident (kB)\n",
+ r.ru_maxrss);
+#elif defined(BSD)
+ appendStringInfo(&str,
+ "!\t%ld max resident, %ld shared, %ld unshared data, %ld unshared stack (kB)\n",
+ r.ru_maxrss, r.ru_ixrss, r.ru_idrss, r.ru_isrss);
+#elif defined(__darwin__)
+ appendStringInfo(&str,
+ "!\t%ld max resident, %ld shared, %ld unshared data, %ld unshared stack (kB)\n",
+ r.ru_maxrss/1024, r.ru_ixrss/1024, r.ru_idrss/1024, r.ru_isrss/1024);
+#endif /* __linux__ */
+
#endif /* HAVE_GETRUSAGE */
/* remove trailing newline */
Comments ?
Testing or suggestions on !linux would be useful.
Justin
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Attachment
Justin Pryzby <pryzby@telsasoft.com> writes:
> Comments ?
I was wondering whether it'd be better to drive this off of configure
probes for the existence of the struct fields. However, in view of
the same fields having different contents on some platforms, such
a probe wouldn't really help much --- you'd still need platform-aware
logic.
Please add to next commitfest so we remember about this when the time
is ripe.
regards, tom lane
On 6/15/17 10:58, Justin Pryzby wrote:
> On Thu, Jun 15, 2017 at 10:29:21AM -0400, Robert Haas wrote:
>> On Wed, Jun 14, 2017 at 6:28 PM, Justin Pryzby <pryzby@telsasoft.com> wrote:
>>> On Tue, Jun 13, 2017 at 12:16:00PM -0400, Robert Haas wrote:
>>>> It might be worth adding platform-specific code for common platforms.
>>>
>>> All I care (which linux happily/happens to support) is maxrss; I was probably
>>> originally interested in this while digging into an issue with hash agg.
>>
>> I don't think it needs to go in a separate file. I'd just patch ShowUsage().
I have committed a patch that shows maxrss, with /1024 adjustment for
macOS. That should cover all platforms that I could find.(*)
I omitted the i{x,d,s}rss stuff. My understanding is that to show them
you should divide the numbers by the elapsed time in some specific way.
Seeing that neither Linux or macOS fill these numbers in, I lost
interest. Someone could produce a separate patch to address this.
Since you were only interested in maxrss to begin with, I think this
thread is concluded.(*)
(*) modulo buildfarm wrath
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Sat, Sep 2, 2017 at 7:46 AM, Peter Eisentraut <peter.eisentraut@2ndquadrant.com> wrote: > On 6/15/17 10:58, Justin Pryzby wrote: >> On Thu, Jun 15, 2017 at 10:29:21AM -0400, Robert Haas wrote: >>> On Wed, Jun 14, 2017 at 6:28 PM, Justin Pryzby <pryzby@telsasoft.com> wrote: >>>> On Tue, Jun 13, 2017 at 12:16:00PM -0400, Robert Haas wrote: >>>>> It might be worth adding platform-specific code for common platforms. >>>> >>>> All I care (which linux happily/happens to support) is maxrss; I was probably >>>> originally interested in this while digging into an issue with hash agg. >>> >>> I don't think it needs to go in a separate file. I'd just patch ShowUsage(). > > I have committed a patch that shows maxrss, with /1024 adjustment for > macOS. That should cover all platforms that I could find.(*) Apparently ru_maxrss is in *pages* on Solaris-derived systems: https://illumos.org/man/3c/getrusage AIX seems to be like Linux and FreeBSD (kilobytes): https://www.ibm.com/support/knowledgecenter/en/ssw_aix_61/com.ibm.aix.basetrf1/getrusage_64.htm -- Thomas Munro http://www.enterprisedb.com
On Sat, Sep 02, 2017 at 02:00:44PM +1200, Thomas Munro wrote: > On Sat, Sep 2, 2017 at 7:46 AM, Peter Eisentraut > <peter.eisentraut@2ndquadrant.com> wrote: > > On 6/15/17 10:58, Justin Pryzby wrote: > >> On Thu, Jun 15, 2017 at 10:29:21AM -0400, Robert Haas wrote: > >>> On Wed, Jun 14, 2017 at 6:28 PM, Justin Pryzby <pryzby@telsasoft.com> wrote: > >>>> On Tue, Jun 13, 2017 at 12:16:00PM -0400, Robert Haas wrote: > >>>>> It might be worth adding platform-specific code for common platforms. > >>>> > >>>> All I care (which linux happily/happens to support) is maxrss; I was probably > >>>> originally interested in this while digging into an issue with hash agg. > >>> > >>> I don't think it needs to go in a separate file. I'd just patch ShowUsage(). > > > > I have committed a patch that shows maxrss, with /1024 adjustment for > > macOS. That should cover all platforms that I could find.(*) > > Apparently ru_maxrss is in *pages* on Solaris-derived systems: > > https://illumos.org/man/3c/getrusage ..but note that that: "The ru_maxrss, ru_ixrss, ru_idrss, and ru_isrss members of the rusage structure are set to 0 in this implementation." Same here: https://docs.oracle.com/cd/E23823_01/html/816-5168/getrusage-3c.html ..and earlier solaris docs don't seem to mention getrusage at all (?) posix docs say: http://pubs.opengroup.org/onlinepubs/009695399/functions/getrusage.html |CHANGE HISTORY | First released in Issue 4, Version 2. ..which I gather is SUSv1 c. 1995 Curiously, GNU time reported maxrss too high by a factor of 4 (and that was present in early centos6.X) https://bugzilla.redhat.com/show_bug.cgi?id=702826 Justin