[ltt-dev] [BABELTRACE PATCH] Set the position when creating the iterator

[Mathieu Desnoyers]

* Julien Desfossez (julien.desfossez at polymtl.ca) wrote:
> Allow the user to seek in the file stream when the iterator is created.
> For now we only support to stay on the current position

Staying at the current position when creating an iterator is a bit odd
with the current implementation, because it leaves the caller thinking
it can destroy an iterator on a stream, then re-create one, and that
they will be at the exact same position (when in fact we lost all the
event that were in the prior iterator's heap, because stream_read_event
moves the position forward when populating the heap on creation). We
have to modify the iter_create code if we want to support this kind of
position seek.

> or seek at the
> beginning.
> 
> Signed-off-by: Julien Desfossez <julien.desfossez at polymtl.ca>
> ---
>  converter/babeltrace-lib.c      |   18 ++++++++++++++++--
>  include/babeltrace/babeltrace.h |    3 ++-
>  2 files changed, 18 insertions(+), 3 deletions(-)
> 
> diff --git a/converter/babeltrace-lib.c b/converter/babeltrace-lib.c
> index ee34d28..bd8033f 100644
> --- a/converter/babeltrace-lib.c
> +++ b/converter/babeltrace-lib.c
> @@ -79,7 +79,8 @@ int stream_compare(void *a, void *b)
>  		return 0;
>  }
>  
> -struct babeltrace_iter *babeltrace_iter_create(struct trace_collection *tc)
> +struct babeltrace_iter *babeltrace_iter_create(struct trace_collection *tc,
> +		int whence, size_t offset)

The "whence + offset" arguments make sense for seeking a single stream,
but not for a trace collection.

I recommend something like this:

struct babeltrace_iter *babeltrace_iter_create(struct trace_collection *tc,
                                struct babeltrace_iter_seek *seek)
{


with:

struct babeltrace_iter_seek {
        enum {
                BT_SEEK_TIME,
                BT_SEEK_POS,
                BT_SEEK_CUR,
                BT_SEEK_BEGIN,
                BT_SEEK_END,
        } type;
        union {
                uint64_t seek_time;
                struct babeltrace_iter_pos *pos;
        } u;
};

The seek time and seek pos are quite straightforward considering the
earlier message I sent detailing the babeltrace API. Seek begin seeks
all files to the beginning. Seek end seeks all files to the end (which
can be useful to figure out the timestamp of the last event available in
the trace collection). Seek "cur" is a bit more tricky:

When we create an iterator with BT_SEEK_CUR, we want to call heap_insert
on each of the streams, _but_ we don't want to call stream_read_event(),
so we keep the current event for each stream. So, basically, we require
that any babeltrace_iter_create called by BT_SEEK_CUR should follow a
prior babeltrace_iter_destroy (so the events are already populated with
data). If we detect that BT_SEEK_CUR is called on a trace collection
that has never had its events populated by reads, we can fallback on a
seek begin.

If "seek" pointer is NULL, we can default to a BT_SEEK_CUR, which seems
like a sane default.

Thoughts ?

Thanks,

Mathieu


>  {
>  	int i, stream_id;
>  	int ret = 0;
> @@ -118,6 +119,19 @@ struct babeltrace_iter *babeltrace_iter_create(struct trace_collection *tc)
>  				file_stream = g_ptr_array_index(stream->streams,
>  						filenr);
>  
> +				switch (whence) {
> +				case SEEK_CUR:
> +					break;
> +				case SEEK_SET:
> +					assert(offset == 0); /* only seek supported for now */
> +					file_stream->pos.cur_index = 0;
> +					file_stream->pos.offset = 0;
> +					file_stream->pos.content_size = 0;
> +					break;
> +				default:
> +					assert(0);
> +				}
> +
>  				ret = stream_read_event(file_stream);
>  				if (ret == EOF) {
>  					ret = 0;
> @@ -211,7 +225,7 @@ int convert_trace(struct trace_descriptor *td_write,
>  	sout = container_of(td_write, struct ctf_text_stream_pos,
>  			trace_descriptor);
>  
> -	iter = babeltrace_iter_create(trace_collection_read);
> +	iter = babeltrace_iter_create(trace_collection_read, SEEK_CUR, 0);
>  	while (babeltrace_iter_read_event(iter, &stream, &event) == 0) {
>  		ret = sout->parent.event_cb(&sout->parent, stream);
>  		if (ret) {
> diff --git a/include/babeltrace/babeltrace.h b/include/babeltrace/babeltrace.h
> index c8b0581..9a69fd0 100644
> --- a/include/babeltrace/babeltrace.h
> +++ b/include/babeltrace/babeltrace.h
> @@ -28,7 +28,8 @@ struct ctf_stream;
>  /*
>   * babeltrace_iter_create - Allocate a trace collection iterator.
>   */
> -struct babeltrace_iter *babeltrace_iter_create(struct trace_collection *tc);
> +struct babeltrace_iter *babeltrace_iter_create(struct trace_collection *tc,
> +		int whence, size_t offset);
>  
>  /*
>   * babeltrace_iter_destroy - Free a trace collection iterator.
> -- 
> 1.7.4.1
> 

-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com