[linux-dvb] dvb-apps lib, structs declared with bit fields

[Oivind]

On 7/6/05, Manu Abraham <manu at kromtek.com> wrote:
> Oivind wrote:
> > Apologizes for the direct reply.
> >
> > On 7/5/05, DUBOST Brice <dubost at crans.ens-cachan.fr> wrote:
> >
> >>Oivind wrote:
> >>
> >>>I notice that bit fields are used in many of the structs, as for
> >>>example from en50221_hlci.h;
> >>>
> >>>struct en50221_pmt_object {
> >>>        unsigned ca_pmt_tag: 24;
> >>>        uint8_t *asn_1_length;
> >>>        unsigned ca_pmt_list_mgmt: 8;
> >>>        unsigned program_number: 16;
> >>>        unsigned reserved_1: 2;
> >>>
> >>>        [...]
> >>>
> >>>};
> >>>
> >>>
> >>>What exactly is the benefit of this?
> >>>
> >>
> >>Because the object come like this
> >>Dvb objct tries to use less data as possible.
> >>
> >>For example
> >>http://www.bc.groteck.ru/pdf-standard-specifications/multiplexing/dvb-vbi/en301775.v1.2.1.pdf
> >
> >
> > Well, yes, an en50221 pmt object looks like that.
> >
> > But no, that is not the reason, because that object is again decoded
> > and reassembled as a binary stream by the driver before it is sent to
> 
> The reason is the same, but for the driver the issue is minimal. But
> whereas for the library the same issue exists..
> Why it is reassmebled and all those is done is just to maintain
> compatibility.
> 
> Moreover it makes sense to maintain the same structure as the protocol
> specifications.
> EN50221, ISO13818, EN 300 468.
> 
> Manu


You have told me earlier that the essence of this project is *speed
and portability*, otherwise it would be "just a drop in the ocean", as
were your exact words.

Ok, so let me do the homework on bit fields.

First, let's read about bit fields. For example, here;
http://publications.gbdirect.co.uk/c_book/chapter6/bitfields.html
Especially read the following statement carefully:
"The main use of bitfields is either to allow tight packing of data or
to be able to specify the fields within some externally produced data
files. C gives no guarantee of the ordering of fields within machine
words, so if you do use them for the latter reason, you program will
not only be non-portable, it will be compiler-dependent too."

If this weren't bad enough for portability, let's look at performance
issues too - for real. Consider the little code that I wrote to get an
idea of bit field performance on my Pentium M platform.

---------------------------------------------------------------------------
#include <unistd.h>
#include <sys/time.h>
#include <stdio.h>
#include <sys/resource.h>

// run a million rounds for each
#define ROUNDS 1000000


// small struct with bit fields (snippet from libdvbsi/pmt.h)
struct tbfpmt {
        unsigned short header_length;
        unsigned table_id: 8;
        unsigned section_syntax : 1;
        unsigned reserved_1: 2;
        unsigned section_length: 12;
        unsigned program_number: 16;
        unsigned reserved_2: 2;
        unsigned version_number: 5;
};


// small struct without bit fields
struct tpmt {
        unsigned int header_length;
        unsigned int table_id;
        unsigned int section_syntax;
        unsigned int reserved_1;
        unsigned int section_length;
        unsigned int program_number;
        unsigned int reserved_2;
        unsigned int version_number;
};


// returns elapsed process time in microseconds
// i.e don't include context switches and other processes
long get_process_usecs()
{
	struct rusage ru;
	getrusage(RUSAGE_SELF,&ru);
	return ru.ru_utime.tv_sec * 1000000 + ru.ru_utime.tv_usec;
}


int main(int argc, char *argv[])
{
	long t1,t2,i;
	struct tbfpmt bfpmt;
	struct tpmt pmt;

	// ---------------------
	// time bit field struct
	// ---------------------

	// start timing
	t1 = get_process_usecs();
	for (i=0;i<ROUNDS;i++)
	{
		bfpmt.header_length += i;
		bfpmt.table_id += i;
		bfpmt.section_syntax += i;
		bfpmt.reserved_1 += i;
		bfpmt.section_length += i;
		bfpmt.program_number += i;
		bfpmt.reserved_2 += i;
		bfpmt.version_number += i;
	} 
	// end timing
	t2 = get_process_usecs();
	// the garbage value below is just to display something from the
	// struct to prevent the compiler from neglecting the for-loop
	// in an optimization attempt.
	printf("Bit field process time   : %d ms.\t\t(garbage:%x)\n",
		(t2-t1)/1000,bfpmt.version_number);


	// -------------------
	// time regular struct
	// -------------------
	
	// start timing
	t1 = get_process_usecs();
	for (i=0;i<ROUNDS;i++)
	{
		pmt.header_length += i;
		pmt.table_id += i;
		pmt.section_syntax += i;
		pmt.reserved_1 += i;
		pmt.section_length += i;
		pmt.program_number += i;
		pmt.reserved_2 += i;
		pmt.version_number += i;
	}
	// end timing 
	t2 = get_process_usecs();
	printf("No bit field process time: %d ms.\t\t(garbage:%x)\n",
		(t2-t1)/1000,pmt.version_number);
	
	return 0;
}
---------------------------------------------------------------------------

So let's compile and see the results.....

[oivind at laptop test]$ gcc -O3 bitfieldtest.c -o bitfieldtest
[oivind at laptop test]$ ./bitfieldtest
Bit field process time   : 96 ms.               (garbage:18)
No bit field process time: 24 ms.               (garbage:724f6995)
[oivind at laptop test]$ ./bitfieldtest
Bit field process time   : 97 ms.               (garbage:18)
No bit field process time: 24 ms.               (garbage:724f6995)
[oivind at laptop test]$ ./bitfieldtest
Bit field process time   : 97 ms.               (garbage:18)
No bit field process time: 25 ms.               (garbage:724f6995)
[oivind at laptop test]$               

Ouch! The compiler will, with maximum optimizations, roughly generate
code that is close to 4 times slower on my platform when accessing
structs with bit fields. This will of course differ on other systems.
Anyway, all that overhead is *completely useless* because you don't
depend on anything external for it and could therefore easily avoid
it.

So, having the project goals in mind, I condole the choice of
implementation - unless a much better reason for keeping bit fields
are presented.



Oivind