[ltt-dev] [BABELTRACE RFC PATCH] Open babeltrace-lib API
Mathieu Desnoyers
mathieu.desnoyers at efficios.com
Fri Aug 5 12:54:55 EDT 2011
* Julien Desfossez (julien.desfossez at polymtl.ca) wrote:
> Hi Mathieu,
>
> This is a first attempt at exporting some of babeltrace internals.
> For this first iteration, I just split the convert_trace function in various
> functions that I think will be useful for other readers.
> But now I'm stuck with the babeltrace.h file, there is circular dependency
> between the .h files and I'd like your insight to solve it : if you look at the
> new functions prototypes, you will see that we need to include more .h files in
> babeltrace.h (types, metadata, format) but these require printf_debug and
> printf_verbose.
First off, printf_verbose and printf_debug are macros, not static inline
functions, so there is no dependency problem (the declaration order does
not matter).
But given they are not needed outside of the babeltrace realm, I would
be tempted to move them to e.g. babeltrace-printf.h, which would not be
installed system-side.
More comments below,
>
> I'm not sure how you plan to export this lib, so before getting into deep
> modifications, I'd like your opinion on the function I exported.
>
> Thanks,
>
> Julien
>
> Signed-off-by: Julien Desfossez <julien.desfossez at polymtl.ca>
> ---
> converter/babeltrace-lib.c | 90 +++++++++++++++++++++++++++++++++-----------
> 1 files changed, 68 insertions(+), 22 deletions(-)
>
> diff --git a/converter/babeltrace-lib.c b/converter/babeltrace-lib.c
> index 6cc2b7b..d557c30 100644
> --- a/converter/babeltrace-lib.c
> +++ b/converter/babeltrace-lib.c
> @@ -29,7 +29,7 @@
> #include <babeltrace/ctf-text/types.h>
> #include <babeltrace/prio_heap.h>
>
> -static int read_event(struct ctf_file_stream *sin)
> +int read_event(struct ctf_file_stream *sin)
> {
> int ret;
>
> @@ -43,6 +43,17 @@ static int read_event(struct ctf_file_stream *sin)
> return 0;
> }
>
> +int write_event(struct ctf_text_stream_pos *sout,
> + struct ctf_file_stream *file_stream)
> +{
> + int ret;
> + ret = sout->parent.event_cb(&sout->parent, &file_stream->parent);
> + if (ret) {
> + fprintf(stdout, "[error] Writing event failed.\n");
> + }
> + return ret;
> +}
> +
> /*
> * returns true if a < b, false otherwise.
> */
> @@ -56,27 +67,25 @@ int stream_compare(void *a, void *b)
> return 0;
> }
>
> -int convert_trace(struct trace_descriptor *td_write,
> - struct trace_descriptor *td_read)
> +int init_heap(struct ctf_trace *tin, int gt(void *a, void *b))
> {
> - struct ctf_trace *tin = container_of(td_read, struct ctf_trace, parent);
> - struct ctf_text_stream_pos *sout =
> - container_of(td_write, struct ctf_text_stream_pos, trace_descriptor);
> int stream_id;
> int ret = 0;
> + struct ctf_stream_class *stream;
> + struct ctf_file_stream *file_stream;
> + int filenr;
>
> tin->stream_heap = g_new(struct ptr_heap, 1);
> - heap_init(tin->stream_heap, 0, stream_compare);
> + heap_init(tin->stream_heap, 0, gt);
>
> /* Populate heap with each stream */
> for (stream_id = 0; stream_id < tin->streams->len; stream_id++) {
> - struct ctf_stream_class *stream = g_ptr_array_index(tin->streams, stream_id);
> - int filenr;
> + stream = g_ptr_array_index(tin->streams, stream_id);
>
> if (!stream)
> continue;
> for (filenr = 0; filenr < stream->streams->len; filenr++) {
> - struct ctf_file_stream *file_stream = g_ptr_array_index(stream->streams, filenr);
> + file_stream = g_ptr_array_index(stream->streams, filenr);
> ret = read_event(file_stream);
> if (ret == EOF) {
> ret = 0;
> @@ -92,36 +101,73 @@ int convert_trace(struct trace_descriptor *td_write,
> }
> }
>
> +end:
> + return ret;
> +}
> +
> +struct ctf_file_stream *get_next(struct ptr_heap *stream_heap)
> +{
> + return heap_maximum(stream_heap);
> +}
> +
Hrm, the "get_next" should take care of calling all of
remove_file_stream/rebalance_heap for you. I think you don't need to
expose the heap per se to the lib user.
This could be wrapped into the concept of a trace iterator. So we can
have:
/*
* struct babeltrace_iter: data structure representing an iterator on an
* input trace (and eventually on a collection of traces too).
*/
/*
* Initialization/teardown.
*/
struct babeltrace_iter *babeltrace_iter_create(struct trace_descriptor *td);
void babeltrace_iter_destroy(struct babeltrace_iter *iter);
/*
* Move within the trace.
*/
/*
* babeltrace_iter_next: Move stream position to the next event.
*
* Does *not* read the event.
* Return EOF if reached end of trace. (EOF value is usually -1)
* Any other negative return value: error.
* 0: success, event is ready.
*/
int babeltrace_iter_next(struct babeltrace_iter *iter);
/* Get the current position for each stream of the trace */
struct babeltrace_iter_pos *
babeltrace_iter_get_pos(struct babeltrace_iter *iter);
/* The position needs to be freed after use */
void babeltrace_iter_free_pos(struct babeltrace_iter_pos *pos);
/* Seek the trace to the position */
int babeltrace_iter_seek_pos(struct babeltrace_iter *iter,
struct babeltrace_iter_pos *pos);
/*
* babeltrace_iter_seek_time: Seek the trace to the given timestamp.
*
* Return EOF if timestamp is after the last event of the trace.
* Return other negative value for other errors.
* Return 0 for success.
*/
int babeltrace_iter_seek_time(struct babeltrace_iter *iter,
uint64_t timestamp);
/*
* babeltrace_iter_read_event: Read the current event data.
*
* @iter: trace iterator (input)
* @stream: stream containing event at current position (output)
* @event: current event (output)
* Return 0 on success, negative error value on error.
*/
int babeltrace_iter_read_event(struct babeltrace_iter *iter,
struct ctf_stream **stream,
struct ctf_stream_event **event);
And yes, to use the information returned by
babeltrace_iter_read_event(), you'll need:
babeltrace/ctf-ir/metadata.h
which needs:
babeltrace/types.h
babeltrace/format.h
babeltrace/ctf/types.h
But the rest (struct babeltrace_iter and struct babeltrace_iter_pos)
should keep their layout internal to babeltrace.
Thoughts ?
Thanks,
Mathieu
> +void remove_file_stream(struct ctf_trace *tin, struct ctf_file_stream *file_stream)
> +{
> + struct ctf_file_stream *removed;
> + removed = heap_remove(tin->stream_heap);
> + assert(removed == file_stream);
> +}
> +
> +void rebalance_heap(struct ctf_trace *tin, struct ctf_file_stream *file_stream)
> +{
> + struct ctf_file_stream *removed;
> +
> + /* Reinsert the file stream into the heap, and rebalance. */
> + removed = heap_replace_max(tin->stream_heap, file_stream);
> + assert(removed == file_stream);
> +}
> +
> +void cleanup_heap(struct ctf_trace *tin)
> +{
> + heap_free(tin->stream_heap);
> + g_free(tin->stream_heap);
> +}
> +
> +int convert_trace(struct trace_descriptor *td_write,
> + struct trace_descriptor *td_read)
> +{
> + struct ctf_file_stream *file_stream;
> + struct ctf_trace *tin = container_of(td_read, struct ctf_trace, parent);
> + struct ctf_text_stream_pos *sout =
> + container_of(td_write, struct ctf_text_stream_pos, trace_descriptor);
> + int ret = 0;
> +
> + ret = init_heap(tin, stream_compare);
> + if (ret)
> + goto end;
> +
> /* Replace heap entries until EOF for each stream (heap empty) */
> for (;;) {
> - struct ctf_file_stream *file_stream, *removed;
> -
> - file_stream = heap_maximum(tin->stream_heap);
> + file_stream = get_next(tin->stream_heap);
> if (!file_stream) {
> /* end of file for all streams */
> ret = 0;
> break;
> }
> - ret = sout->parent.event_cb(&sout->parent, &file_stream->parent);
> + ret = write_event(sout, file_stream);
> if (ret) {
> - fprintf(stdout, "[error] Writing event failed.\n");
> goto end;
> }
> ret = read_event(file_stream);
> if (ret == EOF) {
> - removed = heap_remove(tin->stream_heap);
> - assert(removed == file_stream);
> + remove_file_stream(tin, file_stream);
> ret = 0;
> continue;
> } else if (ret)
> goto end;
> - /* Reinsert the file stream into the heap, and rebalance. */
> - removed = heap_replace_max(tin->stream_heap, file_stream);
> - assert(removed == file_stream);
> + rebalance_heap(tin, file_stream);
> }
>
> end:
> - heap_free(tin->stream_heap);
> - g_free(tin->stream_heap);
> + cleanup_heap(tin);
> return ret;
> }
> --
> 1.7.4.1
>
--
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com
More information about the lttng-dev
mailing list