Mailing List Archive

On Sun, Sep 30, 2007 at 02:39:17PM -0600, Nathan Kurz wrote:
> On 9/28/07, Marvin Humphrey <marvin@rectangular.com> wrote:
> > No worries, mate. Can you show me how you normally like your
> > DEBUG and ASSERT macros set up?
>
> I'm sorry, but I'm not going to have time to clean up my current
> Debug.h file to your standards before leaving on a trip tomorrow. But
> I've attached it in its current state. I may have broken it by
> starting to make changes to it, but at least it shows what I'm aiming
> for.

I'll work on it. For now, I think it belongs under KinoSearch/Util/Debug.h.
It should probably get included by a new "kino.h" file, which would also
include charmony.h.

The variadic macros are the tricky part -- but at least Charmonizer already
probes for those, and will define HAS_VARIADIC_MACROS if they're available.
If the compiler doesn't support them, ASSERT, DEBUG, and DEBUG_PRINT will have
to be no-op functions.

> The things I like are ASSERT() statements that are run if compiled
> #ifdef DEBUG, as opposed to the system assert() which is #ifndef
> NDEBUG.

Sounds good. Right now I'm using KINO_DEBUG to turn on a bunch of extra
compiler flags (but only under gcc).

> I subscribe to the "Code Complete" theory that one's DEBUG
> version should be so loaded down as to barely run, but the real
> version should be unencumbered by unnecessary checks.

"Barely run" sounds a mite excessive. :) It makes debugging "heavy" tests
all but impossible. t/001-build_invindexes.t takes several minutes under
valgrind on a dual Xeon.

> I find the simple DEBUG defined/undefined dichotomy better for this than the
> 2x2 DEBUG/NDEBUG.

Yar, sounds good.

Here's the section which adds the extra compiler flags...

my $EXTRA_CCFLAGS = '';
if ( $ENV{LUCY_DEBUG} || $ENV{KINO_DEBUG} ) {
require version;
if ( defined $Config{gccversion} ) {
$Config{gccversion} =~ /^([\d\.]+)/ or die "no match";
my $gcc_version = version->new("$1")->numify;
$EXTRA_CCFLAGS = "-DPERL_GCC_PEDANTIC -ansi -pedantic -Wall "
. "-std=c89 -Wno-long-long ";
$EXTRA_CCFLAGS .= "-Wextra " if $gcc_version >= 3.4; # correct
$EXTRA_CCFLAGS .= "-Wno-variadic-macros "
if $gcc_version > 3.2; # at least not on gcc 3.2
}
}

I suppose I'll change that up to use plain old "DEBUG" instead, plus figure
out a way to avoid requiring version.pm.

> I also like DEBUG() statements that can be controlled at runtime to
> print debugging information. I do this based on the DEBUG environment
> variable, and allows control at both the file and function level. It's
> possible that there would be a better fit for KinoSearch.

I think that will be OK.

> Done right, the DEBUG statements in the code serve dual purpose as comments
> and debugging aids.

I'm down with that. You may have noticed that the KS test files have very few
comments, but reasonably verbose test messages. Same principle.

> It's a little tricky to integrate this system with the XS loader and
> builder. I've done it before by creating a Makefile that creates both
> a normal and debug version and changes the XS bootstrap function to
> use libfoo.so or libfoo_debug.so according to the DEBUG environment
> variable. For now it may be simpler to just build one at a time,
> switching by hand.

I think it would be worthwhile to work up a scheme where both get installed
and users can switch between them. There have been a number of times where
people have run up against C code that they can't debug themselves and where
I've struggled to diagnose the problem from a distance. After just a couple
such sessions, the initial investment will be paid back.

Doubling the compilation time is kind of an annoyance during development, so
we'll probably want to define an environment var that disables that
functionality and any associated tests.

Have a good trip,

Marvin Humphrey
Rectangular Research
http://www.rectangular.com/


_______________________________________________
KinoSearch mailing list
KinoSearch@rectangular.com
http://www.rectangular.com/mailman/listinfo/kinosearch

On Sep 30, 2007, at 1:39 PM, Nathan Kurz wrote:

> On 9/28/07, Marvin Humphrey <marvin@rectangular.com> wrote:
>> No worries, mate. Can you show me how you normally like your
>> DEBUG and ASSERT macros set up?

OK, I've finished integrating/implementing this. The guts are
somewhat different, but for the most part the API is the same and you
are going to be able to use it as you are accustomed to. Here are
some the changes that were made which are visible from the outside:

* Output to stderr rather than stdout.
* Enabled via KINO_DEBUG environment variable rather than DEBUG.
* All the macros take their familiar names when short names are in
effect; at other times, they need the KINO_ prefix.
* You don't have to be worry about weird combinations of commas
and whitespace causing strange behavior.

> It's a little tricky to integrate this system with the XS loader and
> builder. I've done it before by creating a Makefile that creates both
> a normal and debug version and changes the XS bootstrap function to
> use libfoo.so or libfoo_debug.so according to the DEBUG environment
> variable. For now it may be simpler to just build one at a time,
> switching by hand.

It'd be pretty killer to get this happening. My initial thought is
that it would be better to load the debug version of the shared
library via a "use" directive than an environment variable, since %
ENV is tainted.

use KinoSearch 'DEBUG';

I don't suppose we could get full C stack traces happenin', could we?

http://www.gnu.org/software/libc/manual/html_node/Backtraces.html

Marvin Humphrey
Rectangular Research
http://www.rectangular.com/



_______________________________________________
KinoSearch mailing list
KinoSearch@rectangular.com
http://www.rectangular.com/mailman/listinfo/kinosearch

On 10/15/07, Marvin Humphrey <marvin@rectangular.com> wrote:
> OK, I've finished integrating/implementing this. The guts are
> somewhat different, but for the most part the API is the same and you
> are going to be able to use it as you are accustomed to. Here are
> some the changes that were made which are visible from the outside:

All looks good to me at a cursory viewing. Well done on making it
conform to the KinoSearch standard while keeping the API. I vaguely
recall that there was some reason I had used multiple static functions
defined in the Debug.h file, but for this project what you did seems
better.

> > It's a little tricky to integrate this system with the XS loader and
> > builder. I've done it before by creating a Makefile that creates both
> > a normal and debug version and changes the XS bootstrap function to
> > use libfoo.so or libfoo_debug.so according to the DEBUG environment
> > variable.
>
> It'd be pretty killer to get this happening. My initial thought is
> that it would be better to load the debug version of the shared
> library via a "use" directive than an environment variable, since %
> ENV is tainted.
>
> use KinoSearch 'DEBUG';

I think the advantage of relying on an environment variable is that
you can use the same executable for debug and live on a server.
Editing the script to comment it in/out is clumsy, making a copy with
one line different seems messy. But what you suggest is a fine
solution for the present.

> I don't suppose we could get full C stack traces happenin', could we?

Not sure if I understand where you want this. For a
die_with_stacktrace() function that could be called from the C
library? Seems possible, if we don't mind that it is gcc specific.

Nathan Kurz
nate@verse.com

_______________________________________________
KinoSearch mailing list
KinoSearch@rectangular.com
http://www.rectangular.com/mailman/listinfo/kinosearch

On Oct 15, 2007, at 7:13 PM, Nathan Kurz wrote:

> All looks good to me at a cursory viewing. Well done on making it
> conform to the KinoSearch standard while keeping the API.

Cool, thankee. :)

> I vaguely recall that there was some reason I had used multiple
> static functions
> defined in the Debug.h file, but for this project what you did seems
> better.

There can certainly be good reasons to do that. For example, in the
current Debug.h, compilers without support for variadic macros (e.g.
MSVC) get statically declared functions no-op functions to hopefully
optimize away:

static void KINO_DEBUG_PRINT(char *_ignore_me, ...) { }

However, I didn't see anything like that in the Debug.h you sent
along. Maybe it was there at one time but it's gone now.

> I think the advantage of relying on an environment variable is that
> you can use the same executable for debug and live on a server.

You can fake that up in your own app like so:

BEGIN {
if ( $ENV{MYAPP_DEBUG} ) {
eval 'use KinoSearch "DEBUG";'
die $@ if $@;
}
}
use KinoSearch::Searcher;
...

>> I don't suppose we could get full C stack traces happenin', could we?
>
> Not sure if I understand where you want this. For a
> die_with_stacktrace() function that could be called from the C
> library? Seems possible, if we don't mind that it is gcc specific.

Right now, the CONFESS macro triggers a call to Carp::confess, which
includes the Perl stack trace... but not the C stack trace. All it
shows from C is the final error message, with file, line, function
when the compiler supports variadic macros. There are often several
layers in the C stack which are not shown. It would sure be handy to
include the C stack trace...

Unfortunately, looking into this more closely, I see that support for
execinfo.h is not widespread.

http://lists.apple.com/archives/Xcode-users/2006/Apr/msg00483.html

Probably best to back-burner the idea.

Marvin Humphrey
Rectangular Research
http://www.rectangular.com/



_______________________________________________
KinoSearch mailing list
KinoSearch@rectangular.com
http://www.rectangular.com/mailman/listinfo/kinosearch