RE: Minor boehm-gc fixes

["Boehm, Hans" <hans dot boehm at hp dot com>]

It does look to me like there is something genuinely wrong with the 32-bit version.  Somehow the collector is failing to collect a lot of the heap.  Perhaps the root set includes some of the heap?  Or includes a huge memory region full of junk?  I would look at GC_dump() output from near the end of the run and see what the root set looks like.  You may have to set the GC_DUMP_REGULARLY environment variable.

I would guess that this problem will not get in the way of short-lived code, but you may get unbounded heap growth for longer-running applications.

Presumably the final heap size is much smaller if you run this by itself?

I'm not an expert on the build stuff.  The rest looks obvious to me.

Hans

> -----Original Message-----
> From: Rainer Orth [mailto:ro@CeBiTec.Uni-Bielefeld.DE]
> Sent: Friday, October 01, 2010 4:20 AM
> To: java-patches@gcc.gnu.org
> Cc: Boehm, Hans; Tom Tromey
> Subject: Re: Minor boehm-gc fixes
> 
> Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE> writes:
> 
> > [Please keep me on the Cc:, I'm not subscribed to java-patches.]
> >
> > While investigating the boehm-gc gctest crash on Solaris 11/x86 with
> CVS
> > GNU ld that led to this patch
> >
> > 	More Solaris 2 ABI fixes: emit _DYNAMIC etc. into .dynsym
> >         http://sourceware.org/ml/binutils/2010-09/msg00482.html
> >
> > I've found two minor issues where the fixes are obvious.  They are no
> > longer applicable to CVS boehm-gc, though.
> >
> > I'm not sure what to do about the third one: for unknown reasons,
> > configure.ac has
> >
> > dnl We need to override the top-level CFLAGS.  This is how we do it.
> > MY_CFLAGS="$CFLAGS"
> > AC_SUBST(MY_CFLAGS)
> >
> > yet MY_CFLAGS isn't set anywhere else.  This causes problems because
> it
> > isn't passed to subdirs (missing from AM_MAKEFLAGS) or multilibs
> > (missing from multi-do in the toplevel config-ml.in), as I noticed
> when
> > trying to rebuild with make CFLAGS='-g3 -O0'.  I'd suggest to just
> scrap
> > it to avoid those problems, but perhaps Tom (who introduced this 12
> > years ago) can shed some light on this?
> 
> Unfortunately, I haven't received any response to the question above.
> Further investigation shows that boehm-gc/Makefile.am can be simplified
> considerably: I can not only remove MY_CFLAGS completely (which is just
> an alias for CFLAGS), but also the private definitions of LTCOMPILE,
> LTLDFLAGS, and LINK: after replacing MY_CLAGS by CFLAGS in the current
> definitions, LTLDFLAGS is identical to the default, LINK as well (with
> the exception of using $(CC) instead of $(CCLD)), and the only
> difference between the LTCOMPILE default and the local one is the
> additional passing of GC_CFLAGS.  But GC_CFLAGS is already assigned to
> AM_CFLAGS, so those flags are simply passed twice.  If you remove the
> whole mess, everything works as before.  If building with
> CFLAGS='<something>', they are properly passed down to non-default
> multilibs.
> 
> Tested by configuring and building on i386-pc-solaris2.10 with i386 and
> amd64 multilibs.
> 
> Ok for mainline now?
> 
> > With the gld patch above (not yet approved), boehm-gc and libjava
> test
> > results on i386-pc-solaris2.11 are just as good as with Sun ld, with
> one
> > exception: while gctest works fine if run manually, it fails if the
> > 32-bit version is run from make check:
> >
> > % ./gctest
> > Completed 3 tests
> > Allocated 5680907 collectable objects
> > Allocated 306 uncollectable objects
> > Allocated 3743702 atomic objects
> > Allocated 34440 stubborn objects
> > Finalized 6615/6615 objects - finalization is probably ok
> > Total number of bytes allocated is 190636656
> > Final heap size is 11628544 bytes
> > Collector appears to work
> > Completed 66 collections
> > % make check
> > [...]
> > Completed 3 tests
> > Allocated 5682423 collectable objects
> > Allocated 306 uncollectable objects
> > Allocated 3749384 atomic objects
> > Allocated 34421 stubborn objects
> > Finalized 6583/6583 objects - finalization is probably ok
> > Total number of bytes allocated is 189829352
> > Final heap size is 77918208 bytes
> > Unexpected heap growth - collector may be broken
> > Test failed
> >
> > On the other hand, for 64-bit gctest, both manual invocation and make
> > check work fine.
> >
> > Maybe Hans has a suggesting how to debug this?  Unfortunately, due to
> PR
> > boehm-gc/11412 (boehm-gc testing problems), such failures easily go
> > unnoticed right now.  I plan to convert the boehm-gc testing to
> DejaGnu
> > at some point to remedy this.
> >
> > I'm going to check in the patch below as obvious unless someone
> objects.
> 
> I'll wait for approval of the MY_CFLAGS etc. removal and check the
> whole
> bunch in as a unit.
> 
> Thanks.
> 	Rainer
> 
> 
> 2010-09-24  Rainer Orth  <ro@CeBiTec.Uni-Bielefeld.DE>
> 
> 	boehm-gc:
> 	* dyn_load.c: Fix typo.
> 	* pthread_support.c (GC_get_thread_stack_base) [DEBUG_THREADS]:
> 	Use GC_printf0.
> 
> 	* configure.ac (MY_CFLAGS): Remove.
> 	* configure: Regenerate.
> 	* Makefile.am (LTCOMPILE): Remove.
> 	(LTLDFLAGS): Remove.
> 	(LINK): Remove.
> 	(AM_MAKEFLAGS): Don't pass MY_CLAGS.
> 	* Makefile.in: Regenerate.
> 	* include/Makefile.in: Regenerate.
> 
> diff -r c16225ebe332 boehm-gc/Makefile.am
> --- a/boehm-gc/Makefile.am	Wed Sep 29 12:45:38 2010 +0200
> +++ b/boehm-gc/Makefile.am	Fri Oct 01 13:05:51 2010 +0200
> @@ -94,15 +94,6 @@
>  .S.lo:
>  	$(LTCOMPILE) -Wp,-P -x assembler-with-cpp -c $<
> 
> -## We have our own definition of LTCOMPILE because we want to use our
> -## CFLAGS, not those passed in from the top level make.
> -LTCOMPILE = $(LIBTOOL) $(AM_LIBTOOLFLAGS) $(LIBTOOLFLAGS) --
> mode=compile \
> -	$(CC) $(DEFS) $(AM_CPPFLAGS) $(CPPFLAGS) \
> -	$(AM_CFLAGS) $(MY_CFLAGS) $(GC_CFLAGS)
> -LTLDFLAGS = $(shell $(top_srcdir)/../libtool-ldflags $(LDFLAGS))
> -LINK = $(LIBTOOL) $(AM_LIBTOOLFLAGS) $(LIBTOOLFLAGS) --mode=link \
> -	$(CC) $(AM_CFLAGS) $(MY_CFLAGS) $(LTLDFLAGS) -o $@
> -
>  # Work around what appears to be a GNU make bug handling MAKEFLAGS
>  # values defined in terms of make variables, as is the case for CC and
>  # friends when we are called from the top level Makefile.
> @@ -122,7 +113,6 @@
>  	"LIBCFLAGS_FOR_TARGET=$(LIBCFLAGS_FOR_TARGET)" \
>  	"MAKE=$(MAKE)" \
>  	"MAKEINFO=$(MAKEINFO) $(MAKEINFOFLAGS)" \
> -	"MY_CFLAGS=$(MY_CFLAGS)" \
>  	"PICFLAG=$(PICFLAG)" \
>  	"PICFLAG_FOR_TARGET=$(PICFLAG_FOR_TARGET)" \
>  	"SHELL=$(SHELL)" \
> diff -r c16225ebe332 boehm-gc/configure.ac
> --- a/boehm-gc/configure.ac	Wed Sep 29 12:45:38 2010 +0200
> +++ b/boehm-gc/configure.ac	Fri Oct 01 13:05:51 2010 +0200
> @@ -498,10 +498,6 @@
>   *) AC_MSG_RESULT(no) ;;
>  esac
> 
> -dnl We need to override the top-level CFLAGS.  This is how we do it.
> -MY_CFLAGS="$CFLAGS"
> -AC_SUBST(MY_CFLAGS)
> -
>  dnl Include defines that have become de facto standard.
>  dnl ALL_INTERIOR_POINTERS can be overridden in startup code.
>  AC_DEFINE(SILENT,1,[disables statistics printing])
> diff -r c16225ebe332 boehm-gc/dyn_load.c
> --- a/boehm-gc/dyn_load.c	Wed Sep 29 12:45:38 2010 +0200
> +++ b/boehm-gc/dyn_load.c	Fri Oct 01 13:05:51 2010 +0200
> @@ -122,7 +122,7 @@
>  #    endif
>  #  endif
> 
> -/* An user-supplied routine that is called to dtermine if a DSO must
> +/* An user-supplied routine that is called to determine if a DSO must
>     be scanned by the gc.  */
>  static int (*GC_has_static_roots)(const char *, void *, size_t);
>  /* Register the routine.  */
> diff -r c16225ebe332 boehm-gc/pthread_support.c
> --- a/boehm-gc/pthread_support.c	Wed Sep 29 12:45:38 2010 +0200
> +++ b/boehm-gc/pthread_support.c	Fri Oct 01 13:05:51 2010 +0200
> @@ -1139,7 +1139,7 @@
>    if (pthread_getattr_np (my_pthread, &attr) != 0)
>      {
>  #   ifdef DEBUG_THREADS
> -      GC_printf1("Can not determine stack base for attached thread");
> +      GC_printf0("Can not determine stack base for attached thread");
>  #   endif
>        return 0;
>      }
> 
> 
> --
> -----------------------------------------------------------------------
> ------
> Rainer Orth, Center for Biotechnology, Bielefeld University