-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Paul McCullagh wrote:
> As has been mentioned elsewhere the most important thing is that Drizzle
> has automated regression tests in place.
Yep, that plus we are discussing these regressions in a public forum
where many excellent folks like yourself can comment on them. :) The
suggestions and input we get from everyone is invaluable, and we follow
up on all suggestions.
> And this has already proved its worth, in finding this problem so
> quickly. As you say, there is no real substitute for performance testing!
Yeah. Today I will shortly begin the craziness of running the automated
benchmarks against revisions before r950...wish me luck. ;)
> <rant>
> However there are some things that get my old C-head cooking, and that
> is when inefficiencies are obvious ("how is it implemented" was kind of
> rhetorical).
>
> From the 2 examples below:
>
> 1) "(x & y) == x" makes 2 passes over the complete data set and creates
> a temporary bitmap on the stack for the result of the & operation.
>
> bitmap_is_subset(x, y) on the other hand makes one pass of the data and
> returns early in the FALSE case.
>
> 2) My estimate below (10 times faster) was not just a wild guess. It is
> based on the fact that the C++ implementation works bit-for-bit, and the
> C implementation works on 32 bits at a time.
>
> The point is to simply watch out for this sort of thing because
> sometimes C++ encourages it:
>
> 1) The C++ bitmap has no subsetof() method. The official recommended way
> of doing this is to use "(x & y) == x".
>
> 2) The bit-wise C++ implementation ha_get_max_bit() is the only way to
> do it without fiddling with the internal implementation.
> </rant>
>
> On the other hand, I totally agree with you that C can encourage other
> bad habits like just hacking the internal implementation without regard
> for API!
>
> So this is not a matter of which is better C or C++. It is a matter of
> being aware of the pitfalls of both languages and consciously working
> against them.
Couldn't agree more! :)
- -jay
> On May 6, 2009, at 7:08 PM, Monty Taylor wrote:
>
>> 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
>>>
>>
>
>
>
> --
> 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
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
iEYEARECAAYFAkoCzyEACgkQ2upbWsB4UtFpJwCeKrbT37SBe+iP9BCrwxG5UdPi
GxgAnRQQwCfm/a3474o41oNMzDn6mNIh
=IDZW
-----END PGP SIGNATURE-----
_______________________________________________
Mailing list: https://launchpad.net/~drizzle-discuss
Post to : [email protected]
Unsubscribe : https://launchpad.net/~drizzle-discuss
More help : https://help.launchpad.net/ListHelp
