GNOME Bugzilla – Bug 756049
vp8dec: few conditions that will rarely be true are missing the compiler hints for likely branch prediction
Last modified: 2016-05-22 21:07:11 UTC
Can be optimized with G_UNLIKELY()/LIKELY() macros. 517 if ===>(!dec->decoder_inited)<==== { 518 ret = open_codec (dec, frame); 519 if (ret == GST_FLOW_CUSTOM_SUCCESS_1) 520 return GST_FLOW_OK; 521 else if (ret != GST_FLOW_OK) 522 return ret; 523 } 534 if ===>(!gst_buffer_map (frame->input_buffer, &minfo, GST_MAP_READ))<=== { 535 GST_ERROR_OBJECT (dec, "Failed to map input buffer"); 536 return GST_FLOW_ERROR; 537 } 544 if ===>(status)<=== { 545 GST_VIDEO_DECODER_ERROR (decoder, 1, LIBRARY, ENCODE, 546 ("Failed to decode frame"), ("%s", gst_vpx_error_name (status)), ret); 547 return ret; 548 }
Created attachment 312637 [details] [review] vp8dec: sprinkle some unlikely() macros
Do you have a benchmark to show the effects of this? I'm not a big fan of the G_LIKELY/UNLIKELY macros, they can easily make performance actually worse and make code less readable. Also if this is going to be merged, same changes need to be done for vp9dec. And probably vp8/9enc.
I can't imagine this being worthwhile in practice. I realise we do use those macros a lot in misc places, but in 99% of those places it's likely not worthwhile, and I have seen cases where gcc actually did the wrong thing with those macros used. So I'd rather not merge this tbh. Do you have any particular reason you want to use them, or is it just something you noticed when you read the code and thought it'd be good to fix up?
Yeah, I stumbled upon this (when playing an erroneous stream). Don't have any benchmark figures yet. Since, in any case, this doesn't hold much significance, I agree, we can skip the merging.
Thanks :)