On 7/6/05, Manu Abraham <manu@xxxxxxxxxxx> wrote:
> Oivind wrote:
> > Apologizes for the direct reply.
> >
> > On 7/5/05, DUBOST Brice <dubost@xxxxxxxxxxxxxxxxxxx> 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@laptop test]$ gcc -O3 bitfieldtest.c -o bitfieldtest
[oivind@laptop test]$ ./bitfieldtest
Bit field process time : 96 ms. (garbage:18)
No bit field process time: 24 ms. (garbage:724f6995)
[oivind@laptop test]$ ./bitfieldtest
Bit field process time : 97 ms. (garbage:18)
No bit field process time: 24 ms. (garbage:724f6995)
[oivind@laptop test]$ ./bitfieldtest
Bit field process time : 97 ms. (garbage:18)
No bit field process time: 25 ms. (garbage:724f6995)
[oivind@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
