GNOME Bugzilla – Bug 690688
Miscellaneuous fixes to gi/object.c
Last modified: 2015-10-27 23:04:38 UTC
Mostly micro-optimizations caught while working on bug 690450
Created attachment 232178 [details] [review] object: mark a number of debug and error path as LIKELY/UNLIKELY as appropriate Marking conditional expression with branch probabilities should help the compiler optimize for the common case, even without actual profile feedback.
Created attachment 232179 [details] [review] object: stop checking for a number of impossible conditions We were checking and trying to resume from a few impossible conditions, that are indicative from a gjs bug. It's better to just assert here, so the error is caught early and the check is removed in optimized builds.
Created attachment 232180 [details] [review] object: use g_signal_connect_closure_by_id We parse the signal name few lines earlier, we might just as well use the parsed id.
Created attachment 232181 [details] [review] object: typecheck the object passed to _init While unlikely and generally a programmer error, calling GObject.Object.prototype._init with a this argument that is not a GObject should not assert but only throw.
Created attachment 232182 [details] [review] object: remove support for multiple JSRuntimes That support will never happen, and removing that code allows us to replace g_object_get_data with a faster g_object_get_qdata.
Review of attachment 232178 [details] [review]: I'd be cautious about this. It's *really* bad if we have a miss with one of these, and branch prediction should work in most cases. ::: gi/object.c @@ -596,3 @@ - GType *interfaces; - guint n_interfaces; - guint i; this large deletion needs to be a separate patch, if it was intentional. it's breaking interface lookup, so i assume the comment needs fixing.
Review of attachment 232179 [details] [review]: OK.
Review of attachment 232180 [details] [review]: OK.
Review of attachment 232181 [details] [review]: OK.
Review of attachment 232182 [details] [review]: Sure.
Created attachment 232290 [details] [review] object: mark a number of debug and error path as LIKELY/UNLIKELY as appropriate Marking conditional expression with branch probabilities should help the compiler optimize for the common case, even without actual profile feedback.
Created attachment 232291 [details] [review] object: consolidate code to resolve methods without introspection The code that attempted to find a method using GType interfaces and the code that resolved methods for classes without all introspection info is the same. Consolidate it and reduce instruction cache pressure.
Attachment 232179 [details] pushed as 0a0a6d4 - object: stop checking for a number of impossible conditions Attachment 232180 [details] pushed as 412cb8d - object: use g_signal_connect_closure_by_id Attachment 232181 [details] pushed as 22cbe54 - object: typecheck the object passed to _init Attachment 232182 [details] pushed as c1bcc19 - object: remove support for multiple JSRuntimes
Review of attachment 232291 [details] [review]: OK.
Comment on attachment 232291 [details] [review] object: consolidate code to resolve methods without introspection Attachment 232291 [details] pushed as ef9a05a - object: consolidate code to resolve methods without introspection
Not going to push the LIKELY/UNLIKELY patch at this point. It's also dubious it would really help. Closing this one as FIXED.