Paul McCullagh wrote:
> And we should watch out for things like this:
>
> The MySQL version:
>
> bitmap_is_subset(x, y)
>
> The C++ "way":
>
> (x & y) == x
>
> It looks cute, but how is it implemented!?
Well... from reading the code above, I don't know any more about how
bitmap_is_subset is implemented than I do about how operator& is
implemented.
For me, which ever of the methods we choose to use above (I know stewart
hates the operator overloading), is that we actually have an API and
stick to it. (and that the API isn't defined as "this object is
implemented as an array of bytes, so just grab the internal array
pointer and modify it" :) )
> The thing is, there are simply great ways to optimize this operation if
> you write custom code to do it.
>
> Another problem I see is that the bitmap implementation basically
> encourages sub-optimal code. Take a look at the Drizzle implementation
> of ha_get_max_bit() (a function I use in PBXT) below. The Drizzle
> version is maybe 100 times more understandable, but I would guess the
> MySQL version is at least 10 times faster!
Most of the time that I've heard this assertion be made about things in
the MySQL code being 10 times faster, they haven't actually held up to
benchmarking scrutiny. Benchmarking is good...
> In general the thing that makes me suspicious of the C++ standard lib is
> that it is not necessarily written to be "database server grade"
> (especially if they put the emphasis on readability instead of speed).
I hear what you are saying, but it reminds me of the continual
temptation to get that extra little bit of juice out by breaking the API
and directly messing with the internals of the object that you are
dealing with, which is what your MySQL code below looks like it's doing.
(although I'll admit I haven't actually looked in to what exactly a
my_bitmap_map is or what its last_word_ptr and last_word_mask do - it
does seem to me of a "hey, here are pointers to my internal impl...")
The main problem here wasn't a sub-optimal STL bitset implementation, it
was a semantic/usage problem in that the original bitmap used here was
runtime sized and was replaced with a compile-time size object.
> By "database server grade" I mean _every_ instruction counts.
Totally agree... but before we claim instructions in either direction,
we should make sure to actually look at instructions produced rather
than guessing that the C++ one makes more instructions. (and I get sort
of touchy about that as you can see, because people are quick to jump on
the C++ is slow bandwagon, and statements like "I'm sure it's slower"
only fuel that fire)
The point isn't to argue that C++ is better or worse. (as fun as
language flame wars can be) Rather, I just want to reitterate:
1) Use consistent API (allows for impl to be improved in future)
2) Directly test any claims of speed one way or the other
Monty
> ---------------------------------------------------------------
>
> static u_int ha_get_max_bit(MX_BITMAP *map)
> {
> #ifdef DRIZZLED
> size_t i;
>
> if (!map->any())
> return 0;
> for (i=0; i<map->size(); i++) {
> if (map->test(i))
> break;
> }
> return i+1;
> #else
> my_bitmap_map *data_ptr = map->bitmap;
> my_bitmap_map *end_ptr = map->last_word_ptr;
> my_bitmap_map b;
> u_int cnt = map->n_bits;
>
> for (; end_ptr >= data_ptr; end_ptr--) {
> if ((b = *end_ptr)) {
> my_bitmap_map mask;
>
> if (end_ptr == map->last_word_ptr && map->last_word_mask)
> mask = map->last_word_mask >> 1;
> else
> mask = 0x80000000;
> while (!(b & mask)) {
> b = b << 1;
> /* Should not happen, but if it does, we hang! */
> if (!b)
> return map->n_bits;
> cnt--;
> }
> return cnt;
> }
> if (end_ptr == map->last_word_ptr)
> cnt = ((cnt-1) / 32) * 32;
> else
> cnt -= 32;
> }
> return 0;
> #endif
> }
>
>
>
>
> On May 6, 2009, at 9:03 AM, Stewart Smith wrote:
>
>> So, there was a perf regression noticed as a result of replacing
>> my_bitmap with std::bitset.
>>
>> This is why:
>>
>> These code changes:
>>
>> - MY_BITMAP *save_read_set, *save_write_set;
>> + bitset<MAX_FIELDS> *save_read_set, *save_write_set;
>>
>> means we stop doing things like this:
>>
>> - bitmap_init(&column_bitmap, bitmap, head->s->fields, false);
>>
>> and instead statically say "this bitset has MAX_FIELDS bits".
>>
>> For the bitmap_init() call, the my_bitmap code only allocates enough
>> buffer space for the number of fields (in this case, grabbing the field
>> count from the table share).
>>
>> For the std::bitset, this allocates room for MAX_FIELDS, which is 4096.
>> i.e. a full 512 byte bitmap.
>>
>> So for tables with around 4000 columns, both implementations are likely
>> close in performance :)
>>
>> However, for the common case (e.g. sysbench), it isn't. I prove with
>> benchmarks (same code, just changing MAX_FIELDS down to 64) and not
>> handwaving:
>>
>>
>> $ uname -a
>> Linux willster 2.6.30-020630rc2-generic #020630rc2 SMP Wed Apr 15
>> 13:20:18 UTC 2009 x86_64 GNU/Linux
>>
>> $ grep 'model name' /proc/cpuinfo
>> model name : Intel(R) Core(TM)2 Duo CPU T7250 @ 2.00GHz
>> model name : Intel(R) Core(TM)2 Duo CPU T7250 @ 2.00GHz
>>
>> $ sysbench --test=oltp --drizzle-host=127.0.0.1 --drizzle-port=9306
>> --drizzle-db=test --drizzle-mysql=off --drizzle-user=root
>> --db-ps-mode=disable --db-driver=drizzle --drizzle-table-engine=myisam
>> --max-time=60 --max-requests=2000 --oltp-read-only=on
>> --oltp-table-size=20000 --num-threads=8 run
>>
>> MAX_FIELDS=4096 with std::bitset
>> read/write requests: 28000 (3016.51 per sec.)
>> read/write requests: 28000 (2804.91 per sec.)
>> read/write requests: 28000 (2824.13 per sec.)
>> read/write requests: 28000 (2830.43 per sec.)
>>
>> MAX_FIELDS=64 with std::bitset
>> read/write requests: 28000 (3274.91 per sec.)
>> read/write requests: 28000 (2981.82 per sec.)
>> read/write requests: 28000 (2921.28 per sec.)
>> read/write requests: 28000 (2934.67 per sec.)
>> read/write requests: 28000 (3295.95 per sec.)
>>
>>
>> http://gcc.gnu.org/onlinedocs/libstdc++/manual/bitset.html
>>
>> Is a good read - especially the completely useless suggestions for
>> workarounds. "just burn megabytes of memory", or "limit what your
>> program can accept" or (my favourite) "just use the compiler and linker
>> at runtime". Some redeption with theorising that the last solution is
>> that of a "raving lunatic".
>>
>>> The problem is that vector<bool> doesn't behave like a normal vector
>>> anymore. There have been recent journal articles which discuss the
>>> problems (the ones by Herb Sutter in the May and July/August 1999 issues
>>> of C++ Report cover it well). Future revisions of the ISO C++ Standard
>>> will change the requirement for vector<bool> specialization. In the
>>> meantime, deque<bool> is recommended (although its behavior is sane,
>>> you probably will not get the space savings, but the allocation scheme
>>> is different than that of vector).
>>
>> So it's going to be vector<bool> to the rescue.
>>
>> I then want to bench it against Mats Kindal's magic code.
>> --
>> Stewart Smith
>>
>> _______________________________________________
>> Mailing list: https://launchpad.net/~drizzle-discuss
>> Post to : [email protected]
>> Unsubscribe : https://launchpad.net/~drizzle-discuss
>> More help : https://help.launchpad.net/ListHelp
>
>
>
> --
> Paul McCullagh
> PrimeBase Technologies
> www.primebase.org
> www.blobstreaming.org
> pbxt.blogspot.com
>
>
>
>
> _______________________________________________
> Mailing list: https://launchpad.net/~drizzle-discuss
> Post to : [email protected]
> Unsubscribe : https://launchpad.net/~drizzle-discuss
> More help : https://help.launchpad.net/ListHelp
>
_______________________________________________
Mailing list: https://launchpad.net/~drizzle-discuss
Post to : [email protected]
Unsubscribe : https://launchpad.net/~drizzle-discuss
More help : https://help.launchpad.net/ListHelp
