945046 - Report private and vsize-max-contiguous in about:memory on FreeBSD

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Comment 1

[Jan Beich]

Attachment: move out classes from XP_WIN

Comment 2

[Jan Beich]

Attachment: gather new stats from kinfo_vmentry, v0

In the shell it'd look like this

$ procstat -v $(pgrep firefox) |
  awk '{ rss+=$5; prss+=$6 } 
       END { printf "private %d Kb\nresident %d Kb\n",
                    prss*4096/1024, rss*4096/1024 }'
private 52532 Kb
resident 719780 Kb

$ procstat -v $(pgrep firefox) | 
  awk '{ oldreg=reg; reg=$3-$2; vsize+=reg;
         if(reg < oldreg) reg=oldreg } 
       END { printf "vsize %d Kb\nvsize-max-contiguous %d Kb\n",
                    vsize/1024, reg/1024 } '
vsize 1083576 Kb
vsize-max-contiguous 76800 Kb

For some reason "resident" via procstat(8) has slightly lower value than "rss" in top(1) and ps(1). Not sure if this affects "private" ...

Comment 3

[Nicholas Nethercote [inactive]]

Comment on attachment 8340816 [details] [diff] [review]
gather new stats from kinfo_vmentry, v0

Review of attachment 8340816 [details] [diff] [review]:
-----------------------------------------------------------------

r=me for the nsMemoryReporterManager.cpp changes.  (BTW, I don't know anything about kinfo_getvmmap(), but the way you've used it looks entirely reasonable.)

::: xpcom/base/nsMemoryReporterManager.cpp
@@ +207,5 @@
> +#ifdef __FreeBSD__
> +#include <libutil.h>
> +
> +static nsresult
> +GetKinfoVmentrySelf(int64_t* prss, uint64_t* maxreg)

It's error-prone that the caller needs to zero the values.  I'd do that within this function instead.

@@ +234,5 @@
> +{
> +    int64_t priv = 0;
> +    nsresult rv = GetKinfoVmentrySelf(&priv, NULL);
> +    if (NS_SUCCEEDED(rv))
> +        *aN = priv * getpagesize();

Until now, "private" has been Windows-only, and the term has a specific meaning there.  I don't know if the meaning on FreeBSD is quite the same.  But this doesn't really worry me, because FreeBSD is a tier-3 platform.

Comment 4

[Mike Hommey [:glandium]]

Comment on attachment 8340816 [details] [diff] [review]
gather new stats from kinfo_vmentry, v0

Review of attachment 8340816 [details] [diff] [review]:
-----------------------------------------------------------------

r+ on the build parts

Comment 5

[Jan Beich]

Attachment: gather new stats from kinfo_vmentry, v0

(In reply to Nicholas Nethercote [:njn] from comment #3)
> It's error-prone that the caller needs to zero the values.  I'd do that
> within this function instead.

done

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/3d407295139e
https://hg.mozilla.org/integration/mozilla-inbound/rev/cb5d23080c70

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/3d407295139e
https://hg.mozilla.org/mozilla-central/rev/cb5d23080c70