292899 - API for determining the amount of system memory

Status: RESOLVED FIXED

(NSPR :: NSPR, enhancement)

Description

[Brian Ryner (not reading)]

We currently have code in mozilla/netwerk/cache/src/nsCacheService.cpp that
determines the amont of RAM in the system.  This should be moved into NSPR so
that it's easier to use from other places.

Comment 1

[Brian Ryner (not reading)]

Attachment: patch (against NSPRPUB_PRE_4_2_CLIENT_BRANCH)

Comment 2

[Wan-Teh Chang]

Comment on attachment 182616 [details] [diff] [review]
patch (against NSPRPUB_PRE_4_2_CLIENT_BRANCH)

This patch is basically OK.  The indentation level
needs to be changed to 4 to follow NSPR's coding style.
I also suggest we change SystemMemory to PhysicalMemory
because "physical memory" seems to be the more common
term.

I'm wondering if the function should return the signed
PRInt64 type so that it can return -1 on failure.
(By the way, the function needs to have a #else case to call
PR_SetError(PR_NOT_IMPLEMENTED_ERROR, 0).)  I don't think
we will ever have a computer with more than 2^63 bytes of
memory.

Microsoft documentation says we need to use GlobalMemoryStatusEx
instead on computers with more than 4 GB of memory.
http://msdn.microsoft.com/library/default.asp?url=/library/en-us/memory/base/me
morystatus_str.asp

Comment 3

[Brian Ryner (not reading)]

(In reply to comment #2)
> (From update of attachment 182616 [details] [diff] [review] [edit])
> This patch is basically OK.  The indentation level
> needs to be changed to 4 to follow NSPR's coding style.

Hm, why does the emacs modeline have c-basic-offset: 2?

> I also suggest we change SystemMemory to PhysicalMemory
> because "physical memory" seems to be the more common
> term.
> 

I'll do that.

> I'm wondering if the function should return the signed
> PRInt64 type so that it can return -1 on failure.
> (By the way, the function needs to have a #else case to call
> PR_SetError(PR_NOT_IMPLEMENTED_ERROR, 0).)  I don't think
> we will ever have a computer with more than 2^63 bytes of
> memory.

Perhaps not, but 0 seems like a fine error return to me, and that way we're sure
we can return any amount of memory addressable by a 64-bit architecture.

> 
> Microsoft documentation says we need to use GlobalMemoryStatusEx
> instead on computers with more than 4 GB of memory.
> http://msdn.microsoft.com/library/default.asp?url=/library/en-us/memory/base/me
> morystatus_str.asp
> 

Yeah, see the fixme comment in the windows code.  Since this function is only
present on Windows 2000 and newer, I think we'll have to dynamically look up the
function and fall back to GlobalMemoryStatus if it's not present.  I'll go ahead
and code that up.

Comment 4

[Brian Ryner (not reading)]

Attachment: revised patch

I didn't change the return type, but I addressed all of the other comments.  I
changed to use GlobalMemoryStatusEx on win2k+ with a fallback for win95/8.  I
also added this to the prsystem test.

Comment 5

[Wan-Teh Chang]

Comment on attachment 182621 [details] [diff] [review]
revised patch

r=wtc.	Thank you for adding a test for the new function.

>-/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 2 -*- */

This line was imposed on NSPR when the Mozilla code was
open sourced in 1998.  Since none of the NSPR developers
used emacs, it's just a comment to us.

We probably need to add the newly included system headers
to nsprpub/config/system-headers.

>+#if defined(LINUX) || defined(SUN)

We use SOLARIS instead of SUN in NSPR.

Can this patch wait until after Aviary 1.1?  I'm defining
NSPR 4.6 to be the NSPR in Aviary 1.1, so I don't want to
add new functions to NSPR now unless Aviary 1.1 needs them
or you can obtain drivers' aviary1.1 approval for them.

This function also needs to be added to nsprpub/pr/src/nspr.def
and nsprpub/pr/src/nspr_symvec.opt.  See attachment 177691 [details] [diff] [review]
for an example (look at changes to plc.def and plc_symvec.opt).
But we need to know which NSPR version this new function will
go in.

Comment 6

[Brian Ryner (not reading)]

Actually, I was planning to use this for bug 274784 which we are hoping to have
enabled in Firefox 1.1.  So this needs to be something we can use on the trunk
now.  Should I go ahead and label this for version 4.6?

(In reply to comment #5)
> (From update of attachment 182621 [details] [diff] [review] [edit])
> r=wtc.	Thank you for adding a test for the new function.
> 
> >-/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
> 
> This line was imposed on NSPR when the Mozilla code was
> open sourced in 1998.  Since none of the NSPR developers
> used emacs, it's just a comment to us.
> 
> We probably need to add the newly included system headers
> to nsprpub/config/system-headers.
> 
> >+#if defined(LINUX) || defined(SUN)
> 
> We use SOLARIS instead of SUN in NSPR.
> 
> Can this patch wait until after Aviary 1.1?  I'm defining
> NSPR 4.6 to be the NSPR in Aviary 1.1, so I don't want to
> add new functions to NSPR now unless Aviary 1.1 needs them
> or you can obtain drivers' aviary1.1 approval for them.
> 
> This function also needs to be added to nsprpub/pr/src/nspr.def
> and nsprpub/pr/src/nspr_symvec.opt.  See attachment 177691 [details] [diff] [review] [edit]
> for an example (look at changes to plc.def and plc_symvec.opt).
> But we need to know which NSPR version this new function will
> go in.
> 

Comment 7

[Brian Ryner (not reading)]

Attachment: additional changes

These are the changes to system_headers, nspr.def, and nspr_symvec.opt to add
this function to NSPR 4.6.  I wasn't really sure about how nspr_symvec.opt
works, so hopefully this is ok.

Comment 8

[Wan-Teh Chang]

Comment on attachment 182692 [details] [diff] [review]
additional changes

r=wtc.

Comment 9

[Brian Ryner (not reading)]

Comment on attachment 182621 [details] [diff] [review]
revised patch

Requesting approval to land... this only adds a new API so there is no risk to
any current code paths.

Comment 10

[Brendan Eich [:brendan]]

Comment on attachment 182621 [details] [diff] [review]
revised patch

1.1a for front-end firefox code is 1.8b2 for back-end, and using both will get
you a special prize...	a=me.

/be

Comment 11

[Brian Ryner (not reading)]

Checked in on trunk and nspr client branch.

Comment 12

[Wan-Teh Chang]

This will be in NSPR 4.6 and Mozilla 1.8 Beta 2.

Comment 13

[Brian Ryner (not reading)]

Attachment: fix AIX build

The new library functions we call require libodl and libcfg.

Comment 14

[Wan-Teh Chang]

Comment on attachment 182827 [details] [diff] [review]
fix AIX build

It would be a good idea to ask Philip Warren of IBM
whether the linking order between -lodm -lcfg and
-lpthreads is important.  On many platforms, the
pthread library must appear before all other system
libraries in the linking order.

Comment 15

[Julien Pierre]

This change added a newline at the end of nspr.def, and caused my OS/2 build to
fail as follows :

gcc -Zomf -Zdll -Zmap -o nspr4.dll ./nspr4.def ./prvrsion.o io/./prfdcach.o
io/./prmwait.o io/./prmapopt.o io/./priometh.o io/./pripv6.o io/./prlayer.o
io/./prlog.o io/./prmmap.o io/./prpolevt.o io/./prprf.o io/./prscanf.o
io/./prstdio.o threads/./prcmon.o threads/./prrwlock.o threads/./prtpd.o
linking/./prlink.o malloc/./prmem.o md/./prosdep.o memory/./prshm.o
memory/./prshma.o memory/./prseg.o misc/./pralarm.o misc/./pratom.o
misc/./prcountr.o misc/./prdtoa.o misc/./prenv.o misc/./prerr.o misc/./prerror.o
misc/./prerrortable.o misc/./prinit.o misc/./prinrval.o misc/./pripc.o
misc/./prlog2.o misc/./prlong.o misc/./prnetdb.o misc/./prolock.o misc/./prrng.o
misc/./prsystem.o misc/./prthinfo.o misc/./prtpool.o misc/./prtrace.o
misc/./prtime.o malloc/./prmalloc.o io/./prdir.o io/./prfile.o io/./prio.o
io/./prsocket.o misc/./pripcsem.o threads/./prcthr.o threads/./prdump.o
threads/./prmon.o threads/./prsem.o threads/combined/./prucpu.o
threads/combined/./prucv.o threads/combined/./prulock.o
threads/combined/./prustack.o threads/combined/./pruthr.o md/os2/./os2io.o
md/os2/./os2sock.o md/os2/./os2thred.o md/os2/./os2cv.o md/os2/./os2gc.o
md/os2/./os2misc.o md/os2/./os2inrval.o md/os2/./os2sem.o md/os2/./os2_errors.o
md/os2/./os2poll.o md/os2/./os2rng.o md/os2/./os2emx.o md/os2/./os2vaclegacy.o 
-ldl -lsocket
weakld: error: Unresolved symbol (UNDEF EXPORT) '_'.
weakld: info: The symbol is referenced by:
    ./nspr4.def
Ignoring unresolved externals reported from weak prelinker.
ILink : error LNK2022: _ (alias _) : export undefined

There was 1 error detected
make.exe[3]: *** [nspr4.dll] Error 1
make.exe[3]: Leaving directory
`E:/dev/nss/tip/mozilla/nsprpub/OS22.45_gcc_DBG.OBJ/pr/src' 

Please remove the newline.

Comment 16

[Mike Kaply [:mkaply]]

I'd rather fix rules.mk so this doesn't happen...

http://lxr.mozilla.org/seamonkey/source/nsprpub/config/rules.mk#390

Comment 17

[Mike Kaply [:mkaply]]

Attachment: Fix for OS/2

Check for empty lines in rules.mk

I don't like this patch - I'd rather do it in sed - suggestions welcome.

Comment 18

[Wan-Teh Chang]

*** Bug 293400 has been marked as a duplicate of this bug. ***

Comment 19

[Wan-Teh Chang]

Sorry about that.  I've checked in a fix (attachment 182975 [details] [diff] [review]
in bug 293400).

I agree we should enhance our makefile rules to ignore blank
lines, but let's do that as an NSS bug because NSPR copies
NSS's rules for processing *.def files on various platforms.