Skip to contentSkip to conversation

tracing/function-graph-tracer: improve duration output

PATCH Applied Frederic Weisbecker 4 participants 16 messages Dec 3, 2008 Tracing

lore
Share & tools
lore

Frederic Weisbecker

author
#0 lore raw

PATCH 2/2 tracing/function-graph-tracer: improve duration output

Impact: better output of duration for long calls

The old duration output didn't exceeded 9999.999 us to fit the column and the nanosecs were always 3 numbers. As Ingo suggested, it's better to have the whole microseconds elapsed time and shift the nanosecs precision if needed to fit the maximum 7 numbers. And usec need more number, the case should be rare and important enough to break a bit the column alignment to show it.

So, depending of the duration value, we now have these patterns:

u.nnn
uu.nnn
uuu.nnn
uuuu.nnn
uuuuu.nn
uuuuuu.n
uuuuuuuu.....

Signed-off-by Frederic Weisbecker <[email protected]>

---

kernel/trace/trace_functions_graph.c +43 -12
index 23e19d2..c66578f 100644
@@ -232,11 +232,50 @@ trace_branch_is_leaf(struct trace_iterator *iter,
 }
 
 
-static inline int
+static enum print_line_t
 print_graph_duration(unsigned long long duration, struct trace_seq *s)
 {
 	unsigned long nsecs_rem = do_div(duration, 1000);
-	return trace_seq_printf(s, "%4llu.%3lu us |  ", duration, nsecs_rem);
+	/* log10(ULONG_MAX) + '\0' */
+	char msecs_str[21];
+	char nsecs_str[5];
+	int ret, len;
+	int i;
+
+	sprintf(msecs_str, "%lu", (unsigned long) duration);
+
+	/* Print msecs */
+	ret = trace_seq_printf(s, msecs_str);
+	if (!ret)
+		return TRACE_TYPE_PARTIAL_LINE;
+
+	len = strlen(msecs_str);
+
+	/* Print nsecs (we don't want to exceed 7 numbers) */
+	if (len < 7) {
+		snprintf(nsecs_str, 8 - len, "%03lu", nsecs_rem);
+		ret = trace_seq_printf(s, ".%s", nsecs_str);
+		if (!ret)
+			return TRACE_TYPE_PARTIAL_LINE;
+		len += strlen(nsecs_str);
+	}
+
+	ret = trace_seq_printf(s, " us ");
+	if (!ret)
+		return TRACE_TYPE_PARTIAL_LINE;
+
+	/* Print remaining spaces to fit the row's width */
+	for (i = len; i < 7; i++) {
+		ret = trace_seq_printf(s, " ");
+		if (!ret)
+			return TRACE_TYPE_PARTIAL_LINE;
+	}
+
+	ret = trace_seq_printf(s, "|  ");
+	if (!ret)
+		return TRACE_TYPE_PARTIAL_LINE;
+	return TRACE_TYPE_HANDLED;
+
 }
 
 /* Signal a overhead of time execution to the output */

@@ -273,10 +312,6 @@ print_graph_entry_leaf(struct trace_iterator *iter,
 	call = &entry->graph_ent;
 	duration = graph_ret->rettime - graph_ret->calltime;
 
-	/* Must not exceed 8 characters: 9999.999 us */
-	if (duration > 10000000ULL)
-		duration = 9999999ULL;
-
 	/* Overhead */
 	if (tracer_flags.val & TRACE_GRAPH_PRINT_OVERHEAD) {
 		ret = print_graph_overhead(duration, s);

@@ -286,7 +321,7 @@ print_graph_entry_leaf(struct trace_iterator *iter,
 
 	/* Duration */
 	ret = print_graph_duration(duration, s);
-	if (!ret)
+	if (ret == TRACE_TYPE_PARTIAL_LINE)
 		return TRACE_TYPE_PARTIAL_LINE;
 
 	/* Function */

@@ -387,10 +422,6 @@ print_graph_return(struct ftrace_graph_ret *trace, struct trace_seq *s,
 	int ret;
 	unsigned long long duration = trace->rettime - trace->calltime;
 
-	/* Must not exceed 8 characters: xxxx.yyy us */
-	if (duration > 10000000ULL)
-		duration = 9999999ULL;
-
 	/* Pid */
 	if (verif_pid(s, ent->pid, cpu) == TRACE_TYPE_PARTIAL_LINE)
 		return TRACE_TYPE_PARTIAL_LINE;

@@ -422,7 +453,7 @@ print_graph_return(struct ftrace_graph_ret *trace, struct trace_seq *s,
 
 	/* Duration */
 	ret = print_graph_duration(duration, s);
-	if (!ret)
+	if (ret == TRACE_TYPE_PARTIAL_LINE)
 		return TRACE_TYPE_PARTIAL_LINE;
 
 	/* Closing brace */
Signature · 1 line
1.5.6.3
author
#1 lore raw
Quoted 11 lines · Frederic Weisbecker

Impact: better output of duration for long calls

The old duration output didn't exceeded 9999.999 us to fit the column and the nanosecs were always 3 numbers. As Ingo suggested, it's better to have the whole microseconds elapsed time and shift the nanosecs precision if needed to fit the maximum 7 numbers. And usec need more number, the case should be rare and important enough to break a bit the column alignment to show it.

So, depending of the duration value, we now have these patterns:

Quoted 7 lines

u.nnn
uu.nnn
uuu.nnn
uuuu.nnn
uuuuu.nn
uuuuuu.n
uuuuuuuu.....

An example of a trace after these two patches (with funcgraph-proc enabled):

 0)    cat-2796    |               |            put_prev_task_fair() {
 0)    cat-2796    |   0.609 us    |              update_curr();
 0)    cat-2796    |   1.842 us    |            }
 0)    cat-2796    |               |            pick_next_task_fair() {
 0)    cat-2796    |   0.526 us    |              wakeup_preempt_entity();
 0)    cat-2796    |   0.797 us    |              set_next_entity();
 0)    cat-2796    |   0.541 us    |              hrtick_start_fair();
 0)    cat-2796    |   4.195 us    |            }
 0)    cat-2796    |   1.263 us    |            _spin_trylock();
 0)    cat-2796    |   0.624 us    |            _spin_unlock();
 0)    cat-2796    |   0.789 us    |            _spin_trylock();
 0)    cat-2796    |   0.617 us    |            _spin_unlock();
 0)    cat-2796    |   0.579 us    |            native_load_sp0();
 0)    cat-2796    |   0.722 us    |            native_load_tls();

------------------------------------------
| 0) cat-2796 => events/-9
------------------------------------------

 0)   events/-9    |               |    finish_task_switch() {
 0)   events/-9    |   0.789 us    |      _spin_unlock_irq();
 0)   events/-9    |   2.309 us    |    }
 0)   events/-9    | ! 269848.5 us |  }
 0)   events/-9    |   0.601 us    |  finish_wait();
 0)   events/-9    |   0.662 us    |  kthread_should_stop();
 0)   events/-9    |               |  run_workqueue() {
 0)   events/-9    |   1.136 us    |    _spin_lock_irq();
 0)   events/-9    |   0.918 us    |    _spin_unlock_irq();
 0)   events/-9    |               |    console_callback() {
 0)   events/-9    |               |      acquire_console_sem() {
 0)   events/-9    |               |        down() {
 0)   events/-9    |   1.534 us    |          _spin_lock_irqsave();
 0)   events/-9    |   0.812 us    |          _spin_unlock_irqrestore();
 0)   events/-9    |   4.369 us    |        }
 0)   events/-9    |   5.903 us    |      }
Applied
#2 lore raw
Quoted 12 lines · Frederic Weisbecker
Quoted 11 lines · Frederic Weisbecker

Impact: better output of duration for long calls

The old duration output didn't exceeded 9999.999 us to fit the column and the nanosecs were always 3 numbers. As Ingo suggested, it's better to have the whole microseconds elapsed time and shift the nanosecs precision if needed to fit the maximum 7 numbers. And usec need more number, the case should be rare and important enough to break a bit the column alignment to show it.

So, depending of the duration value, we now have these patterns:

Quoted 7 lines
Quoted 7 lines

u.nnn
uu.nnn
uuu.nnn
uuuu.nnn
uuuuu.nn
uuuuuu.n
uuuuuuuu.....

applied, thanks!

Quoted 17 lines

An example of a trace after these two patches (with funcgraph-proc enabled):

 0)    cat-2796    |               |            put_prev_task_fair() {
 0)    cat-2796    |   0.609 us    |              update_curr();
 0)    cat-2796    |   1.842 us    |            }
 0)    cat-2796    |               |            pick_next_task_fair() {
 0)    cat-2796    |   0.526 us    |              wakeup_preempt_entity();
 0)    cat-2796    |   0.797 us    |              set_next_entity();
 0)    cat-2796    |   0.541 us    |              hrtick_start_fair();
 0)    cat-2796    |   4.195 us    |            }
 0)    cat-2796    |   1.263 us    |            _spin_trylock();
 0)    cat-2796    |   0.624 us    |            _spin_unlock();
 0)    cat-2796    |   0.789 us    |            _spin_trylock();
 0)    cat-2796    |   0.617 us    |            _spin_unlock();
 0)    cat-2796    |   0.579 us    |            native_load_sp0();
 0)    cat-2796    |   0.722 us    |            native_load_tls();
Quoted 3 lines

------------------------------------------
| 0) cat-2796 => events/-9
------------------------------------------

small detail: i'd suggest to remove the newline before the context-switch box. That makes it appear visually to attach logically with the switch-out place, not the switch-in place:

Quoted 10 lines
 0)    cat-2796    |   0.579 us    |            native_load_sp0();
 0)    cat-2796    |   0.722 us    |            native_load_tls();

------------------------------------------
| 0) cat-2796 => events/-9
------------------------------------------

 0)   events/-9    |               |    finish_task_switch() {
 0)   events/-9    |   0.789 us    |      _spin_unlock_irq();
 0)   events/-9    |   2.309 us    |    }
 0)   events/-9    | ! 269848.5 us |  }

which is what we want to express.

Ingo

#3 lore raw

btw., i tested the graph tracer on a 16-way 64-bit box as well, and the multi-digit CPU field printout magic works fine there too:

  9)               |    pick_next_task() {
  9)   0.283 us    |      pick_next_task_fair();
  9)   0.276 us    |      pick_next_task_rt();
  9)   0.273 us    |      pick_next_task_fair();
  9)   0.279 us    |      pick_next_task_idle();
  9)   2.596 us    |    }
  9)   0.566 us    |    __lock_text_start();
 10)   0.299 us    |        }
 10)               |        mm_init() {
 10)               |          pgd_alloc() {
 10)               |            __get_free_pages() {
 10)               |              alloc_pages_current() {
 10)   0.330 us    |                policy_zonelist();
 10)               |                __alloc_pages_internal() {

Ingo

author
#4 lore raw

2008/12/3 Ingo Molnar <[email protected]>:

Quoted 10 lines

small detail: i'd suggest to remove the newline before the context-switch box. That makes it appear visually to attach logically with the switch-out place, not the switch-in place:

Quoted 6 lines
 0)    cat-2796    |   0.579 us    |            native_load_sp0();
 0)    cat-2796    |   0.722 us    |            native_load_tls();

------------------------------------------
| 0) cat-2796 => events/-9
------------------------------------------

Quoted 7 lines
Quoted 4 lines
 0)   events/-9    |               |    finish_task_switch() {
 0)   events/-9    |   0.789 us    |      _spin_unlock_irq();
 0)   events/-9    |   2.309 us    |    }
 0)   events/-9    | ! 269848.5 us |  }

which is what we want to express.

Ok, I'll fix it.

Quoted 19 lines

btw., i tested the graph tracer on a 16-way 64-bit box as well, and the multi-digit CPU field printout magic works fine there too:

 9)               |    pick_next_task() {
 9)   0.283 us    |      pick_next_task_fair();
 9)   0.276 us    |      pick_next_task_rt();
 9)   0.273 us    |      pick_next_task_fair();
 9)   0.279 us    |      pick_next_task_idle();
 9)   2.596 us    |    }
 9)   0.566 us    |    __lock_text_start();
 10)   0.299 us    |        }
 10)               |        mm_init() {
 10)               |          pgd_alloc() {
 10)               |            __get_free_pages() {
 10)               |              alloc_pages_current() {
 10)   0.330 us    |                policy_zonelist();
 10)               |                __alloc_pages_internal() {
Quoted 2 lines

Ingo

Hmm not really, there should be one space more before the 9 to fit the log10(MAX_CPU) here and not break
the column.

Thanks!

#5 lore raw
Quoted 22 lines · Frédéric Weisbecker
Quoted 4 lines

btw., i tested the graph tracer on a 16-way 64-bit box as well, and

the

Quoted 17 lines

multi-digit CPU field printout magic works fine there too:

 9)               |    pick_next_task() {
 9)   0.283 us    |      pick_next_task_fair();
 9)   0.276 us    |      pick_next_task_rt();
 9)   0.273 us    |      pick_next_task_fair();
 9)   0.279 us    |      pick_next_task_idle();
 9)   2.596 us    |    }
 9)   0.566 us    |    __lock_text_start();
 10)   0.299 us    |        }
 10)               |        mm_init() {
 10)               |          pgd_alloc() {
 10)               |            __get_free_pages() {
 10)               |              alloc_pages_current() {
 10)   0.330 us    |                policy_zonelist();
 10)               |                __alloc_pages_internal() {
Quoted 6 lines
Quoted 2 lines

Ingo

Hmm not really, there should be one space more before the 9 to fit the log10(MAX_CPU) here and not break
the column.

It did in my copy of Ingo's email...

author
#6 lore raw

2008/12/3 Peter Zijlstra <[email protected]>:

Quoted 23 lines · Frédéric Weisbecker
Quoted 22 lines
Quoted 4 lines

btw., i tested the graph tracer on a 16-way 64-bit box as well, and

the

Quoted 17 lines

multi-digit CPU field printout magic works fine there too:

 9)               |    pick_next_task() {
 9)   0.283 us    |      pick_next_task_fair();
 9)   0.276 us    |      pick_next_task_rt();
 9)   0.273 us    |      pick_next_task_fair();
 9)   0.279 us    |      pick_next_task_idle();
 9)   2.596 us    |    }
 9)   0.566 us    |    __lock_text_start();
 10)   0.299 us    |        }
 10)               |        mm_init() {
 10)               |          pgd_alloc() {
 10)               |            __get_free_pages() {
 10)               |              alloc_pages_current() {
 10)   0.330 us    |                policy_zonelist();
 10)               |                __alloc_pages_internal() {
Quoted 8 lines
Quoted 6 lines
Quoted 2 lines

Ingo

Hmm not really, there should be one space more before the 9 to fit the log10(MAX_CPU) here and not break
the column.

It did in my copy of Ingo's email...

So perhaps I should drop gmail...... :-(

#7 lore raw
Quoted 27 lines · Frédéric Weisbecker

2008/12/3 Peter Zijlstra <[email protected]>:

Quoted 26 lines · Frédéric Weisbecker
Quoted 25 lines
Quoted 4 lines

btw., i tested the graph tracer on a 16-way 64-bit box as well, and

the

Quoted 19 lines

multi-digit CPU field printout magic works fine there too:

 9)               |    pick_next_task() {
 9)   0.283 us    |      pick_next_task_fair();
 9)   0.276 us    |      pick_next_task_rt();
 9)   0.273 us    |      pick_next_task_fair();
 9)   0.279 us    |      pick_next_task_idle();
 9)   2.596 us    |    }
 9)   0.566 us    |    __lock_text_start();
 10)   0.299 us    |        }
 10)               |        mm_init() {
 10)               |          pgd_alloc() {
 10)               |            __get_free_pages() {
 10)               |              alloc_pages_current() {
 10)   0.330 us    |                policy_zonelist();
 10)               |                __alloc_pages_internal() {

Ingo

Quoted 8 lines
Quoted 5 lines
Quoted 3 lines

Hmm not really, there should be one space more before the 9 to fit the log10(MAX_CPU) here and not break
the column.

It did in my copy of Ingo's email...

So perhaps I should drop gmail...... :-(

I've been told that gmail has ways of extracting email through POP3. Perhaps that manages to preserve layout.. if not, I'll file a bug with google and get a real mail account pronto :-)

author
#8 lore raw

2008/12/3 Peter Zijlstra <[email protected]>:

Quoted 3 lines

I've been told that gmail has ways of extracting email through POP3. Perhaps that manages to preserve layout.. if not, I'll file a bug with google and get a real mail account pronto :-)

Yes of course, when I am at home I can use pop3 and imap to extract them. But when I'm at work, I only have access to the Web....

maintainer
#9 lore raw
In reply to Ingo Molnar
Quoted 18 lines · Ingo Molnar
Quoted 18 lines

An example of a trace after these two patches (with funcgraph-proc enabled):

 0)    cat-2796    |               |            put_prev_task_fair() {
 0)    cat-2796    |   0.609 us    |              update_curr();
 0)    cat-2796    |   1.842 us    |            }
 0)    cat-2796    |               |            pick_next_task_fair() {
 0)    cat-2796    |   0.526 us    |              wakeup_preempt_entity();
 0)    cat-2796    |   0.797 us    |              set_next_entity();
 0)    cat-2796    |   0.541 us    |              hrtick_start_fair();
 0)    cat-2796    |   4.195 us    |            }
 0)    cat-2796    |   1.263 us    |            _spin_trylock();
 0)    cat-2796    |   0.624 us    |            _spin_unlock();
 0)    cat-2796    |   0.789 us    |            _spin_trylock();
 0)    cat-2796    |   0.617 us    |            _spin_unlock();
 0)    cat-2796    |   0.579 us    |            native_load_sp0();
 0)    cat-2796    |   0.722 us    |            native_load_tls();
Quoted 7 lines
Quoted 3 lines

------------------------------------------
| 0) cat-2796 => events/-9
------------------------------------------

small detail: i'd suggest to remove the newline before the context-switch box. That makes it appear visually to attach logically with the switch-out place, not the switch-in place:

Actually, I was also thinking of keeping the CPU number in front:

------------------------------------------
0) cat-2796 => events/-9
------------------------------------------

This way we can easily grep for a common CPU and still get the context switches:

grep '^0)' trace > /tmp/trace0

Of course it will probably also work without the '^' now, but then it might grab ftrace_printks with a '0)' if we add those.

-- Steve

Quoted 17 lines
Quoted 11 lines
 0)    cat-2796    |   0.579 us    |            native_load_sp0();
 0)    cat-2796    |   0.722 us    |            native_load_tls();

------------------------------------------
| 0) cat-2796 => events/-9
------------------------------------------

 0)   events/-9    |               |    finish_task_switch() {
 0)   events/-9    |   0.789 us    |      _spin_unlock_irq();
 0)   events/-9    |   2.309 us    |    }
 0)   events/-9    | ! 269848.5 us |  }

which is what we want to express.

Ingo

author
#10 lore raw

2008/12/3 Steven Rostedt <[email protected]>:

Quoted 10 lines

Actually, I was also thinking of keeping the CPU number in front:

------------------------------------------
0) cat-2796 => events/-9
------------------------------------------

This way we can easily grep for a common CPU and still get the context switches:

grep '^0)' trace > /tmp/trace0

I agree. More easy to grep. Ingo, Peter, others, what do you think?

Quoted 2 lines

Of course it will probably also work without the '^' now, but then it might grab ftrace_printks with a '0)' if we add those.

I proposed a solution for ftrace_printk (which I planned to implement this evening).
Since we are in a C-style output, that would be good to put the ftrace_printk as /* C comments */ inside just below the function that called it. Like this:

func_that_called_ftrace_printk {
    /* Message from ftrace_printk */
}

This way we keep the idea of comment which comes along the role of ftrace_printk and we know
which function called it, at which depth.....

Do you like it this way?

#11 lore raw
Quoted 7 lines · Frédéric Weisbecker

2008/12/3 Steven Rostedt <[email protected]>:

Quoted 6 lines

Actually, I was also thinking of keeping the CPU number in front:

------------------------------------------
0) cat-2796 => events/-9
------------------------------------------

Quoted 6 lines
Quoted 4 lines

This way we can easily grep for a common CPU and still get the context switches:

grep '^0)' trace > /tmp/trace0

I agree. More easy to grep. Ingo, Peter, others, what do you think?

Everything that aids parsing is good :-)

Quoted 14 lines
Quoted 2 lines

Of course it will probably also work without the '^' now, but then it might grab ftrace_printks with a '0)' if we add those.

I proposed a solution for ftrace_printk (which I planned to implement this evening).
Since we are in a C-style output, that would be good to put the ftrace_printk as /* C comments */ inside just below the function that called it. Like this:

func_that_called_ftrace_printk {
    /* Message from ftrace_printk */
}
Quoted 5 lines

This way we keep the idea of comment which comes along the role of ftrace_printk and we know
which function called it, at which depth.....

Do you like it this way?

Yep

maintainer
#12 lore raw
In reply to Frédéric Weisbecker
Quoted 12 lines · Fr?d?ric Weisbecker

I proposed a solution for ftrace_printk (which I planned to implement this evening).
Since we are in a C-style output, that would be good to put the ftrace_printk as /* C comments */ inside just below the function that called it. Like this:

func_that_called_ftrace_printk {
    /* Message from ftrace_printk */
}
Quoted 5 lines

This way we keep the idea of comment which comes along the role of ftrace_printk and we know
which function called it, at which depth.....

Do you like it this way?

Yes definitely. I thought we already decided to do it that way ;-)

-- Steve

#15 lore raw
In reply to Frédéric Weisbecker
Quoted 7 lines · Frédéric Weisbecker

2008/12/3 Steven Rostedt <[email protected]>:

Quoted 6 lines

Actually, I was also thinking of keeping the CPU number in front:

------------------------------------------
0) cat-2796 => events/-9
------------------------------------------

Quoted 6 lines
Quoted 4 lines

This way we can easily grep for a common CPU and still get the context switches:

grep '^0)' trace > /tmp/trace0

I agree. More easy to grep. Ingo, Peter, others, what do you think?

sure.

Quoted 13 lines
Quoted 2 lines

Of course it will probably also work without the '^' now, but then it might grab ftrace_printks with a '0)' if we add those.

I proposed a solution for ftrace_printk (which I planned to implement this evening). Since we are in a C-style output, that would be good to put the ftrace_printk as /* C comments */ inside just below the function that called it. Like this:

func_that_called_ftrace_printk {
    /* Message from ftrace_printk */
}
Quoted 5 lines

This way we keep the idea of comment which comes along the role of ftrace_printk and we know
which function called it, at which depth.....

Do you like it this way?

yeah, that would be lovely to have too.

Ingo

author
#13 lore raw
In reply to Steven Rostedt

2008/12/3 Steven Rostedt <[email protected]>:

Quoted 14 lines · Fr?d?ric Weisbecker
Quoted 12 lines

I proposed a solution for ftrace_printk (which I planned to implement this evening).
Since we are in a C-style output, that would be good to put the ftrace_printk as /* C comments */ inside just below the function that called it. Like this:

func_that_called_ftrace_printk {
    /* Message from ftrace_printk */
}
Quoted 7 lines
Quoted 5 lines

This way we keep the idea of comment which comes along the role of ftrace_printk and we know
which function called it, at which depth.....

Do you like it this way?

Yes definitely. I thought we already decided to do it that way ;-)

A proposition about this. I want to use __ftrace_printk, make it generic for both function tracers and have a way to store the current depth.
One solution: changing the print_entry to have the ip available for function-tracer and depth available for return-function_tracer.

struct print_entry {
	struct trace_entry	ent;
        union {
	    unsigned long	ip;
            unsigned long       depth;
        } from;
	char			buf[];
};

We could make __ftrace_printk verify which tracer is running.

If normal tracer then

     trace_vprintk(__builtin_return_address(0), .....)
else if graph-tracer
     trace_vprintk(current->curr_ret_stack, ......)

This way we can even forget the ip argument of __trace_printk. I prefer to ask before changing an exported symbol API....

maintainer
#14 lore raw
Quoted 18 lines · Fr?d?ric Weisbecker

A proposition about this. I want to use __ftrace_printk, make it generic for both function tracers and have a way to store the current depth.
One solution: changing the print_entry to have the ip available for function-tracer and depth available for return-function_tracer.

struct print_entry {
	struct trace_entry	ent;
        union {
	    unsigned long	ip;
            unsigned long       depth;
        } from;
	char			buf[];
};

We could make __ftrace_printk verify which tracer is running.

Quoted 7 lines

If normal tracer then

     trace_vprintk(__builtin_return_address(0), .....)
else if graph-tracer
     trace_vprintk(current->curr_ret_stack, ......)

This way we can even forget the ip argument of __trace_printk. I prefer to ask before changing an exported symbol API....

Actually, I would prefer to keep both.

	struct print_entry {
		struct trace_entry	ent;
		unsigned long		ip;
		int			depth;
		char			buf[];
	};

And pass in both. Have the readers of the functions figure out what to do with either, and keep the messiness away from the API.

-- Steve