Re: [PATCH] GTY documentation update

[Ralf Wildenhues <Ralf dot Wildenhues at gmx dot de>]

Hello Laurynas,

* Laurynas Biveinis wrote on Mon, Nov 08, 2010 at 12:35:47PM CET:
> Comments welcome, both in content and style/grammar ("a"/"the" are
> particularly troublesome for me).

You can make reviews even easier by ensuring that your patches are
either inline (and not mangled) or attached but not declared as
octet-stream MIME type, so that replying to them inline is
straight-forward.  Thanks.

> OK for trunk?

I cannot approve nor reject, and this is a purely syntactical review,
not a semantic one.

> 2010-11-08  Laurynas Biveinis  <laurynas.biveinis@gmail.com>
> 
>         * gty.texi (GTY Options): Clarify that variable_size produces
> allocators taking
>         size in bytes, compare with length option.  Add size
> calculation example.
>         (Invoking the garbage collector): Ensure that sentences are
> followed by two spaces.
>         Describe that pointer fields must be initialized at ggc_collect call.
>         (Troubleshooting): New section.

Although not specified by the GNU Coding Standards or GCC's Coding
Conventions, it is a good idea to keep the line length for ChangeLog
entries below 80 or 72, even if only to make them look neat and un-
wrapped in email.  ;-)

> --- gcc/doc/gty.texi	(revision 166430)
> +++ gcc/doc/gty.texi	(working copy)
> @@ -70,6 +70,7 @@
>  * GGC Roots::           Making global variables GGC roots.
>  * Files::               How the generated files work.
>  * Invoking the garbage collector::   How to invoke the garbage collector.
> +* Troubleshooting::     When something does not work as expected.
>  @end menu
>  
>  @node GTY Options
> @@ -358,7 +359,9 @@
>  the type machinery cannot tell how many bytes need to be allocated at 
>  each allocation.  The @code{variable_size} is used to mark such types.
>  The type machinery then provides allocators that take a parameter 
> -indicating an exact size of object being allocated.  
> +indicating an exact size of object being allocated.  Note that the size 
> +must be provided in bytes whereas the @code{length} option works with 

Several lines in your patch introduce (more) trailing white space in the
text.  (git diff can makes this really easy to spot for only changed
lines, I'm not sure how to otherwise avoid seeing them in the unchanged
rest of the file, so as to not mix in unrelated whitespace changes.)

> +array lengths in number of elements.

>  For example,
>  @smallexample
> @@ -374,6 +377,12 @@
>    field_vec = ggc_alloc_sorted_fields_type (size);
>  @end smallexample
>  
> +If @code{field_vec->elts} stores @code{n} elements, then @code{size}

I'd use @var{n} ...

> +could be calculated as follows:
> +@smallexample
> +  size_t size = sizeof (struct sorted_fields_type) + n * sizeof (tree);

and here as well; but it's a bit matter of taste, I guess.

> +@end smallexample
> +
>  @findex special
>  @item special ("@var{name}")
>  
> @@ -487,12 +496,44 @@
>  with many other garbage collectors, it is not implicitly invoked by
>  allocation routines when a lot of memory has been consumed. So the
>  only way to have GGC reclaim storage it to call the @code{ggc_collect}
> -function explicitly. This call is an expensive operation, as it may
> -have to scan the entire heap. Beware that local variables (on the GCC
> +function explicitly.  This call is an expensive operation, as it may
> +have to scan the entire heap.  Beware that local variables (on the GCC
>  call stack) are not followed by such an invocation (as many other
>  garbage collectors do): you should reference all your data from static
>  or external @code{GTY}-ed variables, and it is advised to call
> -@code{ggc_collect} with a shallow call stack. The GGC is an exact mark
> +@code{ggc_collect} with a shallow call stack.  The GGC is an exact mark
>  and sweep garbage collector (so it does not scan the call stack for
> -pointers). In practice GCC passes don't often call @code{ggc_collect}
> +pointers).  In practice GCC passes don't often call @code{ggc_collect}
>  themselves, because it is called by the pass manager between passes.
> +
> +At the time of @code{ggc_collect} call all pointers in the GC-marked 

s/of/& the/

> +structures must be valid or @code{NULL}.  In practice this means that 
> +there should not be uninitialized pointer fields in the structures 
> +even if your code never reads or writes those fields at a particular
> +instance.  One way to ensure this is to use cleared versions of 
> +allocators unless all the fields are initialized manually immediatelly

immediately

> +after the allocation.

s/the //

> +@node Troubleshooting
> +@section Troubleshooting the garbage collector
> +@cindex garbage collector, troubleshooting
> +
> +With the current garbage collector implementation most issues should
> +show up as GCC compilation errors.  Some of the most commonly 
> +encountered issues are described in the following.
> +
> +@itemize @bullet
> +@item Gengtype does not produce allocators for a GTY-marked type.
> +Gengtype checks if there is at least one possible path from GC roots 
> +to at least one instance of each type before outputing allocators.  If

outputting

> +there is no such path, the GTY markers will be ignored and no allocators
> +will be output.  Solve this by making sure that there exists at least 
> +one such path.  If creating it is unfeasible or raises a ``code smell'', 
> +consider if you really must use GC for allocating such type.
> +
> +@item Link-time errors about undefined @code{gt_ggc_r_foo_bar} and 
> +similarly-named symbols.
> +Check if your @code{foo_bar} source file has @code{#include "gt-foo_bar"} 
> +as its very last line.
> +
> +@end itemize

Thanks,
Ralf