Created attachment 157667 [details] [review]
GiggleRevision: Set properties directly

Going through g_object_set just adds extra overhead for nothing

Created attachment 157668 [details] [review]
libgiggle: Avoid type checking in heavily used methods.

Those properties are accessed way way way too often to allow ourselves
to do heavy type checking.

It's risky... but there's a substantial speedup

Review of attachment 157667 [details] [review]:

Thank you for the patch!

Only a little comment:

::: libgiggle/giggle-revision.c
@@ +301,3 @@
 	g_return_if_fail (GIGGLE_IS_REVISION (revision));
+	revision->priv->date = date;
+	g_free (revision->priv->date);

Are you sure about this?

date is a const struct tm

I only copied the logic from revision_set_property.

Review of attachment 157668 [details] [review]:

Yeah, I already made some test removing the type checking, but I was not sure because this is supossed to be public api. Anyway I think nobody uses libgiggle, so I think the change is worth.

Only a comment:

::: libgiggle/giggle-ref.c
@@ +180,3 @@
 {
 giggle_ref_get_name (GiggleRef *ref)
+	if (G_LIKELY (ref))

Could this be changed for something like:

if (G_LIKELY (ref->priv->name))

Maybe is better to avoid erros

Review of attachment 157668 [details] [review]:

::: libgiggle/giggle-ref.c
@@ +180,3 @@
 giggle_ref_get_name (GiggleRef *ref)
 {
+	if (G_LIKELY (ref))

I could have avoided the check altogether to be honest. The only weird thing that could happen would be passing a NULL pointer to that method.

If you *Really* want to check for ref->priv->name, you'd also have to check for (ref) and (ref->priv) ... which seems a bit absurd. Also, you don't need to check for ref->priv->name, since a NULL value is valid, no ?

Anyway, a lot of bikeshedding in the end :( Those values should actually be public in the instance structure tbh (yay for me bringing the wrath of GSeal-advocates on me)

(In reply to comment #4)
> I only copied the logic from revision_set_property.

I said that because I get this warning when apply your patch:

giggle-revision.c:304: warning: assignment discards qualifiers from pointer target type

Also, maybe is better to use the public functions in _set_properties() so we can have only one code path?

Comment on attachment 157668 [details] [review]
libgiggle: Avoid type checking in heavily used methods.

commit 0a9349dbcf6dbb365cf647a6f6338c4408c40973

Wee! ~6 seconds less! ;)

ahahaha rock on :)

The latest *big* slowdown is all this string matching/regexp work being done regarding authors/committers.
A simple way to speed that up might be to keep the original unstripped author/committer line in a hash table as the key and the value could be the author/committer object. Most of the time the committer/author line is always the same ("Firstname Lastname <email@doesnt.change>".

Anyway... for another bug report I guess...

Comment on attachment 157667 [details] [review]
GiggleRevision: Set properties directly

commit dc7e0027d308ab3a0d4dfeff6bdbb5bf6d8909c1

Thank you for the patches Edward,

I think we can close this bug now.
Could you fill another bug for your latest comments? ;)