Thread: [PATCH] Reduce noise from tsort

[PATCH] Reduce noise from tsort

From
Martijn van Oosterhout
Date:
Tiny patch to Makefile.shlib to reduce noise created by tsearch. All it
does is remove duplicates. The behaviour itself is not changed.

Have a nice day,
--
Martijn van Oosterhout   <kleptog@svana.org>   http://svana.org/kleptog/
> Patent. n. Genius is 5% inspiration and 95% perspiration. A patent is a
> tool for doing 5% of the work and then sitting around waiting for someone
> else to do the other 95% so you can sue them.

Attachment

Re: [PATCH] Reduce noise from tsort

From
Tom Lane
Date:
Martijn van Oosterhout <kleptog@svana.org> writes:
> Tiny patch to Makefile.shlib to reduce noise created by tsearch. All it
> does is remove duplicates. The behaviour itself is not changed.

What cases exactly does that fix ... and are you sure it doesn't break
anything?

Personally I've been wondering for some time why we use lorder/tsort
at all.  Are there any platforms we support where this is still needed?
(Given the existence of circular references within libpq.a, one would
think that tsort wouldn't help such a platform anyway.)

            regards, tom lane

Re: [PATCH] Reduce noise from tsort

From
Martijn van Oosterhout
Date:
On Sat, Apr 15, 2006 at 12:12:53PM -0400, Tom Lane wrote:
> Martijn van Oosterhout <kleptog@svana.org> writes:
> > Tiny patch to Makefile.shlib to reduce noise created by tsearch. All it
> > does is remove duplicates. The behaviour itself is not changed.
>
> What cases exactly does that fix ... and are you sure it doesn't break
> anything?

Oh, right. Like you say, when creating libpq.a it spits out several
screenfuls of (completely useless) info and it's the same thing over
and over again. With the sort it only emits each loop once, which is
less than one screen. I don't see how it could break anything because
the output is semantically equivalent to the input.

> Personally I've been wondering for some time why we use lorder/tsort
> at all.  Are there any platforms we support where this is still needed?
> (Given the existence of circular references within libpq.a, one would
> think that tsort wouldn't help such a platform anyway.)

I've never worked with a system that cared about the order within
libraries so I've never really experienced the problem. But I leave it
in because I figure it must fix something for someone somewhere...

Have a nice day,
--
Martijn van Oosterhout   <kleptog@svana.org>   http://svana.org/kleptog/
> Patent. n. Genius is 5% inspiration and 95% perspiration. A patent is a
> tool for doing 5% of the work and then sitting around waiting for someone
> else to do the other 95% so you can sue them.

Attachment

Re: [PATCH] Reduce noise from tsort

From
Tom Lane
Date:
Martijn van Oosterhout <kleptog@svana.org> writes:
> On Sat, Apr 15, 2006 at 12:12:53PM -0400, Tom Lane wrote:
>> Personally I've been wondering for some time why we use lorder/tsort
>> at all.  Are there any platforms we support where this is still needed?
>> (Given the existence of circular references within libpq.a, one would
>> think that tsort wouldn't help such a platform anyway.)

> I've never worked with a system that cared about the order within
> libraries so I've never really experienced the problem. But I leave it
> in because I figure it must fix something for someone somewhere...

Well, I vote we take it out, which would eliminate these warnings
instead of just shorten them.  On a platform where tsorting a non-shared
library's contents is actually essential, libpq.a would be useless
anyway because of the circular internal references.  Presumably,
anyone who's using Postgres on such a platform only cares about the .so
library.  So I don't see any point in including the tsort step.

(AFAIK we inherited the tsort stuff from Berkeley; it may have been
useful once upon a time, but that was a long time ago.)

            regards, tom lane

Re: [PATCH] Reduce noise from tsort

From
Tom Lane
Date:
I wrote:
> Well, I vote we take it out, which would eliminate these warnings
> instead of just shorten them.  On a platform where tsorting a non-shared
> library's contents is actually essential, libpq.a would be useless
> anyway because of the circular internal references.  Presumably,
> anyone who's using Postgres on such a platform only cares about the .so
> library.  So I don't see any point in including the tsort step.
> (AFAIK we inherited the tsort stuff from Berkeley; it may have been
> useful once upon a time, but that was a long time ago.)

Attached is a proposed patch that removes all trace of lorder and tsort
from our source tree.  The one part of it that I'm not too sure about is
the change to the non-ELF build rule in Makefile.*bsd.  Anyone have a
non-ELF BSD system sitting around that they can test on?

            regards, tom lane

Index: configure.in
===================================================================
RCS file: /cvsroot/pgsql/configure.in,v
retrieving revision 1.456
diff -c -r1.456 configure.in
*** configure.in    7 Apr 2006 17:50:03 -0000    1.456
--- configure.in    15 Apr 2006 19:13:57 -0000
***************
*** 559,565 ****
    AC_SUBST(ld_R_works)
  esac
  AC_PROG_RANLIB
- AC_CHECK_PROGS(LORDER, lorder)
  AC_PATH_PROG(TAR, tar)
  PGAC_CHECK_STRIP

--- 559,564 ----
Index: src/Makefile.global.in
===================================================================
RCS file: /cvsroot/pgsql/src/Makefile.global.in,v
retrieving revision 1.221
diff -c -r1.221 Makefile.global.in
*** src/Makefile.global.in    12 Feb 2006 07:29:36 -0000    1.221
--- src/Makefile.global.in    15 Apr 2006 19:13:58 -0000
***************
*** 222,228 ****
  LDREL = -r
  LDOUT = -o
  RANLIB = @RANLIB@
- LORDER = @LORDER@
  X = @EXEEXT@

  # Perl
--- 222,227 ----
Index: src/Makefile.shlib
===================================================================
RCS file: /cvsroot/pgsql/src/Makefile.shlib,v
retrieving revision 1.102
diff -c -r1.102 Makefile.shlib
*** src/Makefile.shlib    5 Jan 2006 03:01:34 -0000    1.102
--- src/Makefile.shlib    15 Apr 2006 19:13:59 -0000
***************
*** 266,282 ****

  all-shared-lib: $(shlib)

- ifndef LORDER
- MK_NO_LORDER := true
- endif
-
  ifndef haslibarule
  lib$(NAME).a: $(OBJS)
- ifdef MK_NO_LORDER
      $(LINK.static) $@ $^
- else
-     $(LINK.static) $@ `$(LORDER) $^ | tsort`
- endif
      $(RANLIB) $@
  endif #haslibarule

--- 266,274 ----
Index: src/makefiles/Makefile.aix
===================================================================
RCS file: /cvsroot/pgsql/src/makefiles/Makefile.aix,v
retrieving revision 1.24
diff -c -r1.24 Makefile.aix
*** src/makefiles/Makefile.aix    19 Jan 2006 21:19:12 -0000    1.24
--- src/makefiles/Makefile.aix    15 Apr 2006 19:13:59 -0000
***************
*** 3,9 ****
  MAKE_EXPORTS= true

  RANLIB= touch
- MK_NO_LORDER= true
  AROPT = crs

  DLSUFFIX = .so
--- 3,8 ----
Index: src/makefiles/Makefile.freebsd
===================================================================
RCS file: /cvsroot/pgsql/src/makefiles/Makefile.freebsd,v
retrieving revision 1.22
diff -c -r1.22 Makefile.freebsd
*** src/makefiles/Makefile.freebsd    9 Dec 2005 21:19:36 -0000    1.22
--- src/makefiles/Makefile.freebsd    15 Apr 2006 19:13:59 -0000
***************
*** 21,27 ****
      $(LD) $(LDREL) $(LDOUT) $<.obj -x $<
      @echo building shared object $@
      @rm -f $@.pic
!     @${AR} cq $@.pic `lorder $<.obj | tsort`
      ${RANLIB} $@.pic
      @rm -f $@
      $(LD) -x -Bshareable -Bforcearchive -o $@ $@.pic
--- 21,27 ----
      $(LD) $(LDREL) $(LDOUT) $<.obj -x $<
      @echo building shared object $@
      @rm -f $@.pic
!     @${AR} cq $@.pic $<.obj
      ${RANLIB} $@.pic
      @rm -f $@
      $(LD) -x -Bshareable -Bforcearchive -o $@ $@.pic
Index: src/makefiles/Makefile.irix
===================================================================
RCS file: /cvsroot/pgsql/src/makefiles/Makefile.irix,v
retrieving revision 1.4
diff -c -r1.4 Makefile.irix
*** src/makefiles/Makefile.irix    9 Dec 2005 21:19:36 -0000    1.4
--- src/makefiles/Makefile.irix    15 Apr 2006 19:13:59 -0000
***************
*** 1,4 ****
- MK_NO_LORDER= true
  AROPT = crs
  rpath = -Wl,-rpath,'$(rpathdir)'

--- 1,3 ----
Index: src/makefiles/Makefile.netbsd
===================================================================
RCS file: /cvsroot/pgsql/src/makefiles/Makefile.netbsd,v
retrieving revision 1.12
diff -c -r1.12 Makefile.netbsd
*** src/makefiles/Makefile.netbsd    9 Dec 2005 21:19:36 -0000    1.12
--- src/makefiles/Makefile.netbsd    15 Apr 2006 19:13:59 -0000
***************
*** 23,29 ****
      $(LD) $(LDREL) $(LDOUT) $<.obj -x $<
      @echo building shared object $@
      @rm -f $@.pic
!     @${AR} cq $@.pic `lorder $<.obj | tsort`
      ${RANLIB} $@.pic
      @rm -f $@
      $(LD) -x -Bshareable -Bforcearchive -o $@ $@.pic
--- 23,29 ----
      $(LD) $(LDREL) $(LDOUT) $<.obj -x $<
      @echo building shared object $@
      @rm -f $@.pic
!     @${AR} cq $@.pic $<.obj
      ${RANLIB} $@.pic
      @rm -f $@
      $(LD) -x -Bshareable -Bforcearchive -o $@ $@.pic
Index: src/makefiles/Makefile.openbsd
===================================================================
RCS file: /cvsroot/pgsql/src/makefiles/Makefile.openbsd,v
retrieving revision 1.12
diff -c -r1.12 Makefile.openbsd
*** src/makefiles/Makefile.openbsd    9 Dec 2005 21:19:36 -0000    1.12
--- src/makefiles/Makefile.openbsd    15 Apr 2006 19:13:59 -0000
***************
*** 21,27 ****
      $(LD) $(LDREL) $(LDOUT) $<.obj -x $<
      @echo building shared object $@
      @rm -f $@.pic
!     @${AR} cq $@.pic `lorder $<.obj | tsort`
      ${RANLIB} $@.pic
      @rm -f $@
      $(LD) -x -Bshareable -Bforcearchive -o $@ $@.pic
--- 21,27 ----
      $(LD) $(LDREL) $(LDOUT) $<.obj -x $<
      @echo building shared object $@
      @rm -f $@.pic
!     @${AR} cq $@.pic $<.obj
      ${RANLIB} $@.pic
      @rm -f $@
      $(LD) -x -Bshareable -Bforcearchive -o $@ $@.pic

Re: [PATCH] Reduce noise from tsort

From
Tom Lane
Date:
I wrote:
> Attached is a proposed patch that removes all trace of lorder and tsort
> from our source tree.

For lack of any objection, I've applied this --- we can modify or
reverse it if any platforms turn out to have a problem with it.

            regards, tom lane

Re: [PATCH] Reduce noise from tsort

From
Peter Eisentraut
Date:
Tom Lane wrote:
> Well, I vote we take it out, which would eliminate these warnings
> instead of just shorten them.  On a platform where tsorting a
> non-shared library's contents is actually essential, libpq.a would be
> useless anyway

I don't think that the primary purpose of tsort/lorder is to produce a
functioning library.  The idea is to reduce the link time when the
library is later used by reducing the number of passes that the link
editor has to make over the input libraries.  I doubt this matters much
anymore, and it's certainly not going to help with the sort of
libraries that PostgreSQL produces with circular references all over.

--
Peter Eisentraut
http://developer.postgresql.org/~petere/

Re: [PATCH] Reduce noise from tsort

From
Tom Lane
Date:
Peter Eisentraut <peter_e@gmx.net> writes:
> Tom Lane wrote:
>> Well, I vote we take it out, which would eliminate these warnings
>> instead of just shorten them.  On a platform where tsorting a
>> non-shared library's contents is actually essential, libpq.a would be
>> useless anyway

> I don't think that the primary purpose of tsort/lorder is to produce a
> functioning library.  The idea is to reduce the link time when the
> library is later used by reducing the number of passes that the link
> editor has to make over the input libraries.

That is a revisionist view of history.  The GNU coreutils people,
for instance, remember it the same way I do:
http://www.delorie.com/gnu/docs/textutils/coreutils_32.html

(In practice, I'm not sure anyone still uses libpq.a at all, rather
than libpq.so ...)

            regards, tom lane

Re: [PATCH] Reduce noise from tsort

From
"Dave Page"
Date:

> -----Original Message-----
> From: pgsql-patches-owner@postgresql.org
> [mailto:pgsql-patches-owner@postgresql.org] On Behalf Of Tom Lane
> Sent: 20 April 2006 00:19
> To: Peter Eisentraut
> Cc: pgsql-patches@postgresql.org; Martijn van Oosterhout
> Subject: Re: [PATCHES] [PATCH] Reduce noise from tsort
>
> (In practice, I'm not sure anyone still uses libpq.a at all,
> rather than libpq.so ...)

FWIW, I do for some builds of pgAdmin.

Regards, Dave.