ISM shared memory on solaris

Yikes! We thought we were already using ISM on Solaris.

Would you test the attached patch? It uses _solaris_ rather than
SHM_SHARE_MMU in the define test. Does that work too?

---------------------------------------------------------------------------

Josh Wilmes wrote:

I hope this is the right place to send this.. the FAQ in the
distribution mentions http://www.PostgreSQL.org/bugs/bugs.php, which
doesn't work.

We've found that postgresql wasn't using ISM shared memory on solaris,
which theoretically would cost performance. The root cause in our case
was that the "solaris" define is not defined by our compilers or by
postgresql itself.

The patch below simple has it check SHM_SHARE_MMU instead, which should
work fine. I verified (with 'pmap') that the database is now using ISM
on its shared memory, after this patch was applied.

--Josh

--- sysv_shmem.c.orig   2002-09-04 13:31:24.000000000 -0700
+++ sysv_shmem.c        2003-10-23 12:52:26.756765000 -0700
@@ -143,7 +143,7 @@
on_shmem_exit(IpcMemoryDelete, Int32GetDatum(shmid));

/* OK, should be able to attach to the segment */
-#if defined(solaris) && defined(__sparc__)
+#if defined(SHM_SHARE_MMU) && defined(__sparc__)
/* use intimate shared memory on SPARC Solaris */
memAddress = shmat(shmid, 0, SHM_SHARE_MMU);
#else
@@ -323,8 +323,8 @@
shmid = shmget(NextShmemSegID, sizeof(PGShmemHeader), 0);
if (shmid < 0)
continue;                       /* failed: must 
be some other app's */
-
-#if defined(solaris) && defined(__sparc__)
+
+#if defined(SHM_SHARE_MMU) && defined(__sparc__)
/* use intimate shared memory on SPARC Solaris */
memAddress = shmat(shmid, 0, SHM_SHARE_MMU);
#else

---------------------------(end of broadcast)---------------------------
TIP 3: if posting/reading through Usenet, please send an appropriate
subscribe-nomail command to majordomo@postgresql.org so that your
message can get through to the mailing list cleanly

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Nope, __solaris__ is not defined on our system either.

--Josh

Bruce Momjian wrote:

Josh Wilmes wrote:

Nope, __solaris__ is not defined on our system either.

I thought our configure defined __portname__ for every platform, but I
don't see that anywhere, so it seems we rely on the compiler to supply
defines for the cpu and OS.

Does src/tools/ccsym show you your defines? I would like to have
something that identifies Solaris rather than something that checks for
ISM so that if the ISM define isn't found, we throw an error and we hear
about it.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Bruce Momjian wrote:

Josh Wilmes wrote:

Nope, __solaris__ is not defined on our system either.

I thought our configure defined __portname__ for every platform, but I
don't see that anywhere, so it seems we rely on the compiler to supply
defines for the cpu and OS.

Does src/tools/ccsym show you your defines? I would like to have
something that identifies Solaris rather than something that checks for
ISM so that if the ISM define isn't found, we throw an error and we hear
about it.

That would be preferable- i didn't know what was safe to assume would
always be defined.

ccsym is pretty neat. Here's what it shows (gcc)

__GNUC__=2
__GNUC_MINOR__=95
sparc
sun
unix
__svr4__
__SVR4
__sparc__
__sun__
__unix__
__svr4__
__SVR4
__sparc
__sun
__unix
system=unix
system=svr4
__GCC_NEW_VARARGS__
cpu=sparc
machine=sparc

OK, attached patch applied. I tested for 'sun' rather than 'solaris'.

---------------------------------------------------------------------------

Josh Wilmes wrote:

Bruce Momjian wrote:

Josh Wilmes wrote:

Nope, __solaris__ is not defined on our system either.

I thought our configure defined __portname__ for every platform, but I
don't see that anywhere, so it seems we rely on the compiler to supply
defines for the cpu and OS.

Does src/tools/ccsym show you your defines? I would like to have
something that identifies Solaris rather than something that checks for
ISM so that if the ISM define isn't found, we throw an error and we hear
about it.

That would be preferable- i didn't know what was safe to assume would
always be defined.

ccsym is pretty neat. Here's what it shows (gcc)

__GNUC__=2
__GNUC_MINOR__=95
sparc
sun
unix
__svr4__
__SVR4
__sparc__
__sun__
__unix__
__svr4__
__SVR4
__sparc
__sun
__unix
system=unix
system=svr4
__GCC_NEW_VARARGS__
cpu=sparc
machine=sparc

---------------------------(end of broadcast)---------------------------
TIP 7: don't forget to increase your free space map settings

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Please use new patch. This uses __sun__ rather than sun.

---------------------------------------------------------------------------

Josh Wilmes wrote:

Bruce Momjian wrote:

Josh Wilmes wrote:

Nope, __solaris__ is not defined on our system either.

I thought our configure defined __portname__ for every platform, but I
don't see that anywhere, so it seems we rely on the compiler to supply
defines for the cpu and OS.

Does src/tools/ccsym show you your defines? I would like to have
something that identifies Solaris rather than something that checks for
ISM so that if the ISM define isn't found, we throw an error and we hear
about it.

That would be preferable- i didn't know what was safe to assume would
always be defined.

ccsym is pretty neat. Here's what it shows (gcc)

__GNUC__=2
__GNUC_MINOR__=95
sparc
sun
unix
__svr4__
__SVR4
__sparc__
__sun__
__unix__
__svr4__
__SVR4
__sparc
__sun
__unix
system=unix
system=svr4
__GCC_NEW_VARARGS__
cpu=sparc
machine=sparc

---------------------------(end of broadcast)---------------------------
TIP 7: don't forget to increase your free space map settings

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Bruce Momjian <pgman@candle.pha.pa.us> writes:

! #if defined(sun) && defined(__sparc__)
/* use intimate shared memory on SPARC Solaris */
memAddress = shmat(shmid, 0, SHM_SHARE_MMU);
#else

I think this is going in the wrong direction. Why isn't the code just

#if defined(SHM_SHARE_MMU)
/* use intimate shared memory on Solaris */
memAddress = shmat(shmid, 0, SHM_SHARE_MMU);
#else

If the symbol is available I think we probably want to use it. It is an
O/S issue, not a hardware issue, and so the test on __sparc__ seems
quite wrongheaded ...

regards, tom lane

Tom Lane wrote:

Bruce Momjian <pgman@candle.pha.pa.us> writes:

! #if defined(sun) && defined(__sparc__)
/* use intimate shared memory on SPARC Solaris */
memAddress = shmat(shmid, 0, SHM_SHARE_MMU);
#else

I think this is going in the wrong direction. Why isn't the code just

#if defined(SHM_SHARE_MMU)
/* use intimate shared memory on Solaris */
memAddress = shmat(shmid, 0, SHM_SHARE_MMU);
#else

If the symbol is available I think we probably want to use it. It is an
O/S issue, not a hardware issue, and so the test on __sparc__ seems
quite wrongheaded ...

What I was hoping to do with the define test was to throw an error if we
don't find intimate shared memory on Solaris, but the define doesn't
work fir i386/Solaris so we are probably better going with the define as
you suggest --- I just hope we don't fail to include a file and somehow
miss it on some version of Solaris.

Change applied.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Seems like the BEST case would be to have a configure test verify that
it works and define something if it does, but i don't know what such a
test would look like.

--Josh

Bruce Momjian wrote: