[Bug 35882] Different event type should be logged in different files

[bugzilla-daemon at freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=35882

--- Comment #5 from Nicolas Dufresne <nicolas.dufresne at collabora.co.uk> 2011-05-05 11:06:47 PDT ---
(In reply to comment #2)
> http://cgit.collabora.co.uk/git/user/nicolas/telepathy-logger.git/commit/?h=split-event&id=14314bd21d31b8502e85111a6dabc6fa09a2f56a
> -log_store_xml_get_timestamp_filename (void)
> +log_store_xml_get_timestamp_filename (GType type)
> 
> would you object to 
> +log_store_xml_get_timestamp_filename (TplEvent *event)
> (which looks at the timestamp of the event as well as its type) or 
> +log_store_xml_get_timestamp_filename (GType type, gint64 timestamp) ?
> I think I might need this for logging message-edits.
> 

Yes, you probably need that as we want timestamp to come from event for text
edit (guessing you may want edits to be stored in the same file as the original
message), I just didn't want to change implementation here as it's unrelated to
this work.

> 
> http://cgit.collabora.co.uk/git/user/nicolas/telepathy-logger.git/commit/?h=split-event&id=c22ca3933c5ef47a19682e7809c03469b8018a1d
> 
> Have we released a logger which puts call events in with text events? If so,
> get_dates(CALL) will miss some days for some users. If we haven't made any such
> release then we're safe.
> (similarly with
> http://cgit.collabora.co.uk/git/user/nicolas/telepathy-logger.git/commit/?h=split-event&id=1ce4c111999aa5dfae42f4b3a326fca5c2678ee7
> )

The answer is yes, but only as "Experimental Call Support". No call logs reader
have been released yet and no distro I know of enable this experimental option.
But if they do, the effect is that old logs will just not show, no big deal
imho.

> 
> 
> http://cgit.collabora.co.uk/git/user/nicolas/telepathy-logger.git/commit/?h=split-event&id=429771478868a128f9a8b1be7d77c872b5fb8021
> kudos for exposing GQueues in the api (they're much nicer to work with) but:

Could be an option for 0.3 ...

> 
> - g_queue_push_tail (&events, event);
> + {
> + while (index != NULL &&
> + tpl_event_get_timestamp (event) <
> + tpl_event_get_timestamp (TPL_EVENT (index->data)))
> + index = g_list_next (index);
> +
> + if (index != NULL)
> + {
> + g_queue_insert_after (events, index, event);
> + index = g_list_next (index);
> + }
> + else
> + {
> + g_queue_push_tail (events, event);
> + index = events->head;
> + }
> +
> + num_events++;
> + }
> Do you mean g_queue_insert_sorted() or something else?
> Alternatively, could you just throw away most of this code and just
> g_queue_sort() at the end?

g_queue_insert_sorted() would be much slower, since it would linearly lookup
from start at every insert. Also, we could not control the way it is inserted,
which may cause logs of same timestamp to not follow log order.

In this implementation, I iterate only once on the queue I'm merging in,
skipping the element I just added, to preserve log order.

> 
> + else if (type_mask & TPL_EVENT_MASK_CALL)
> do you mean:
> +
> + if (type_mask & TPL_EVENT_MASK_CALL)

Oops, good catch ! I'll try and cover that in the unit tests.

> 
> The poet in me is pining for g_queue_concat (grand_list, list_to_swallow
> (transfer full)) (=== g_list_concat (grand_list->tail, queue_to_swallow->head); 
> grand_list->tail = list_to_swallow->tail; 
> grand_list->length += queue_to_swallow->length; 
> *queue_to_swallow = G_QUEUE_INIT; 
> g_queue_free(queue_to_swallow);) and/or g_queue_merge_sorted (grand_list,
> list_to_swallow) so that we don't have to pass in a mutable queue.

The list would not be ordered correctly, we need to merge it, not concatenate
it.

> 
> I can't quite see how these changes will make opening text chats any faster,
> but I can see how they would make extracting a call log faster (assuming that
> we haven't made any releases with the mixed log format, in which case it would
> break extracting a call log)

log_store_xml_get_dates() has a big performance gain as it no longer have to
look inside the file to figure-out if a date contains texts (or calls) events.
The implementation of log_store_xml_get_filtered_events() (use to get inital 5
messages in Empathy) uses first call _get_dates() and iterates over those dates
until the amount of required events is reached. We can clearly optimize more
this case instead of doing all this split, but it would not allow implementing
_exists() correctly (currently broken for performance reason) and would not
optimize _get_dates() at the same time.

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.