After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 594225 - [API] [bytereader] add _unchecked() variants and inline most common functions
[API] [bytereader] add _unchecked() variants and inline most common functions
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
git master
Other All
: Normal enhancement
: 0.10.25
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2009-09-05 12:24 UTC by Tim-Philipp Müller
Modified: 2009-09-06 18:45 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
bytereader: add inlined _unchecked() variants for some functions (36.02 KB, patch)
2009-09-05 12:25 UTC, Tim-Philipp Müller
committed Details | Review
bytereader: add inline versions of the most common getters and setter (12.04 KB, patch)
2009-09-05 12:26 UTC, Tim-Philipp Müller
committed Details | Review
bytereader: add unchecked and inline versions of the float getters/peekers (12.52 KB, patch)
2009-09-05 12:26 UTC, Tim-Philipp Müller
committed Details | Review

Description Tim-Philipp Müller 2009-09-05 12:24:37 UTC
I propose (inlined) _unchecked() variants for the most common get and peek functions. This is useful e.g. for reading data in inner loops where we've already checked that sufficient data is available. Of course GST_READ_*() could be used there, but I believe that using GstByteReader + _unchecked() functions is generally the better alternative because the code isn't cluttered with statements that move the read pointer forward, and also the _unchecked() suffix makes it clear that one should check whether there's enough data available beforehand.

Secondly, I propose to inline the most common getters and setters. The attached patch does this by adding private _gst_byter_reader_*inline() variants and then mapping the normal functions to the inline functions with an additional G_LIKELY where it makes sense. Besides the presumed performance increase through inlining, I hope the compiler will be able to generate better/tighter code for the most common use case where the byte reader is allocated on the stack and not touched by any external functions.

The change also cleans up the code a bit by removing the non-macro generated special cases for *int8, *int24 and float* (hopefully correctly).
Comment 1 Tim-Philipp Müller 2009-09-05 12:25:34 UTC
Created attachment 142528 [details] [review]
bytereader: add inlined _unchecked() variants for some functions
Comment 2 Tim-Philipp Müller 2009-09-05 12:26:03 UTC
Created attachment 142529 [details] [review]
bytereader: add inline versions of the most common getters and setter
Comment 3 Tim-Philipp Müller 2009-09-05 12:26:25 UTC
Created attachment 142530 [details] [review]
bytereader: add unchecked and inline versions of the float getters/peekers
Comment 4 Sebastian Dröge (slomo) 2009-09-05 13:07:42 UTC
Looks good, please commit :)

But please add a FIXME 0.11 so we can remove the functions for 0.11 (and the same for the bitreader too I guess).
Comment 5 Tim-Philipp Müller 2009-09-06 18:45:31 UTC
commit 96a565bdca71ca8d9ac27b75e25a767cd8e6ad45
Author: Tim-Philipp Müller <tim.muller@collabora.co.uk>
Date:   Sun Sep 6 19:43:47 2009 +0100

    bitreader, bytereader: add some FIXME 0.11 comments and fix indenting

commit 4c103b00b5f3accad40759170138076e278c36b2
Author: Tim-Philipp Müller <tim.muller@collabora.co.uk>
Date:   Fri Sep 4 17:15:35 2009 +0100

    bytereader: add unchecked and inline versions of the float getters/peekers
    
    API: gst_byte_reader_get_float*_unchecked()

commit 31ab1244871e1ef5cce3c571ffb3c20805b3ba8b
Author: Tim-Philipp Müller <tim.muller@collabora.co.uk>
Date:   Fri Sep 4 16:52:06 2009 +0100

    bytereader: add inline versions of the most common getters and setters

commit 080b2e4fd547f371b727a5d20e05beecef748ece
Author: Tim-Philipp Müller <tim.muller@collabora.co.uk>
Date:   Wed Sep 2 11:20:04 2009 +0100

    bytereader: add inlined _unchecked() variants for some functions
    
    API: gst_byte_reader_skip_unchecked()
    API: gst_byte_reader_peek_*_unchecked()
    API: gst_byte_reader_get_*_unchecked()
    API: gst_byte_reader_{peek,get,dup}_data_unchecked()