Padraig O'Sullivan wrote: > On Sat, May 2, 2009 at 3:19 PM, Jay Pipes <[email protected]> wrote: > Further narrowing down has focused on the revision series r982.1.1 > through r982.1.5. Attached is the diff for that, and somewhere in there > is the regression. > > Note that the goal here is not necessarily to undo Padraig's work on > standardizing Bitset, but to look into why/where specifically we're > seeing that regression on concurrency loads >128. Remember, no shame, > no blame, many hands make light work. :) > >> From looking at it myself, one thing that could be an issue (I'm not >> sure) is the assignment operator. For example, there are a few places >> where I do something like this: > >> param->needed_fields= *(table->read_set); >> param->needed_fields|= *(table->write_set); > >> Since the bitset assignment operator will erase all bits in >> param->needed_fields and then insert a copy of each bit in >> table->read_set into param->needed_fields, could there be some >> un-necessary overhead here? > >> Would it be better to do something like: > >> param->needed_fields.reset(); >> param->needed_fields|= *(table->read_set); >> param->needed_fields|= *(table->write_set); > >> and avoid the assignment operator. Are the overheads of the assignment >> operator likely to be an issue?
I don't think so, unless it is implemented poorly... I took a quick look at the libstdc++ implementation, and it uses the default copy assignment operator, which means that it basically does a memcpy() to do the copy. This means we have four memory hits for each element in the bitset (twice in needed_fields, once in read_set, and once in write_set). The latter approach does a memset(), and two times a loop over the read sets to do the |=, it would probably be slower than doing a memcpy() and a loop once, but not certain. It has 5 memory hits for each element in the bitset (three memory hits in needed_fields, one in read_set, and one in write_set). /Matz > >> -Padraig > > Eyeballs please! :) > > Cheers, > > Jay > > Jay Pipes wrote: >>>> So, the regression occurred somewhere in the 971.1.63 subtree: >>>> >>>> +----------------+-------------+-------------+ >>>> | version | concurrency | tps | >>>> +----------------+-------------+-------------+ >>>> | trunk-971.1.62 | 2 | 1049.583333 | >>>> | trunk-971.1.62 | 4 | 1441.090000 | >>>> | trunk-971.1.62 | 8 | 1951.853333 | >>>> | trunk-971.1.62 | 16 | 2594.673333 | >>>> | trunk-971.1.62 | 32 | 3363.063333 | >>>> | trunk-971.1.62 | 64 | 3915.066667 | >>>> | trunk-971.1.62 | 128 | 3908.363333 | >>>> | trunk-971.1.62 | 256 | 2933.943333 | >>>> | trunk-971.1.62 | 512 | 1975.433333 | >>>> | trunk-971.1.62 | 1024 | 1284.683333 | >>>> | trunk-971.1.62 | 2048 | 603.176667 | >>>> | trunk-971.1.63 | 2 | 1041.916667 | >>>> | trunk-971.1.63 | 4 | 1435.300000 | >>>> | trunk-971.1.63 | 8 | 1902.173333 | >>>> | trunk-971.1.63 | 16 | 2583.553333 | >>>> | trunk-971.1.63 | 32 | 3358.056667 | >>>> | trunk-971.1.63 | 64 | 3889.813333 | >>>> | trunk-971.1.63 | 128 | 3853.060000 | >>>> | trunk-971.1.63 | 256 | 2344.860000 | >>>> | trunk-971.1.63 | 512 | 1843.083333 | >>>> | trunk-971.1.63 | 1024 | 1087.163333 | >>>> | trunk-971.1.63 | 2048 | 442.003333 | >>>> | trunk-989 | 2 | 1050.630000 | >>>> | trunk-989 | 4 | 1417.433333 | >>>> | trunk-989 | 8 | 1942.686667 | >>>> | trunk-989 | 16 | 2612.166667 | >>>> | trunk-989 | 32 | 3389.150000 | >>>> | trunk-989 | 64 | 3944.433333 | >>>> | trunk-989 | 128 | 3924.076667 | >>>> | trunk-989 | 256 | 2964.776667 | >>>> | trunk-989 | 512 | 1981.050000 | >>>> | trunk-989 | 1024 | 1273.133333 | >>>> | trunk-989 | 2048 | 563.350000 | >>>> +----------------+-------------+-------------+ >>>> >>>> A benchmark of 971.1.62 did *not* show the regression, so somewhere in >>>> the bitset patch is our regression. I'll be digging further down into >>>> that subtree shortly, but it's good to know where the regression has >>>> been introduced. >>>> >>>> Cheers! >>>> >>>> jay >>>> >>>> Jay Pipes wrote: >>>>> Brian Aker wrote: >>>>>> Hi! >>>>>> A few places to look: >>>>>> 971.1.47 (replication) >>>>> Hi again! OK, so I had to make a few quick changes to the automation >>>>> suite to support non-integer revisions, but those have been made and I >>>>> am happy (and relieved) to report that replication (r971.1.47) is NOT >>>>> the cause of the regression... yeah, Jay! \o/ >>>>> +----------------+-------------+-------------+ >>>>> | version | concurrency | tps | >>>>> +----------------+-------------+-------------+ >>>>> | trunk-989 | 2 | 1050.630000 | >>>>> | trunk-989 | 4 | 1417.433333 | >>>>> | trunk-989 | 8 | 1942.686667 | >>>>> | trunk-989 | 16 | 2612.166667 | >>>>> | trunk-989 | 32 | 3389.150000 | >>>>> | trunk-989 | 64 | 3944.433333 | >>>>> | trunk-989 | 128 | 3924.076667 | >>>>> | trunk-989 | 256 | 2964.776667 | >>>>> | trunk-989 | 512 | 1981.050000 | >>>>> | trunk-989 | 1024 | 1273.133333 | >>>>> | trunk-989 | 2048 | 563.350000 | >>>>> | trunk-971.1.47 | 2 | 1049.383333 | >>>>> | trunk-971.1.47 | 4 | 1407.180000 | >>>>> | trunk-971.1.47 | 8 | 1930.036667 | >>>>> | trunk-971.1.47 | 16 | 2604.316667 | >>>>> | trunk-971.1.47 | 32 | 3379.230000 | >>>>> | trunk-971.1.47 | 64 | 3889.433333 | >>>>> | trunk-971.1.47 | 128 | 3910.143333 | >>>>> | trunk-971.1.47 | 256 | 2933.186667 | >>>>> | trunk-971.1.47 | 512 | 2021.323333 | >>>>> | trunk-971.1.47 | 1024 | 1282.256667 | >>>>> | trunk-971.1.47 | 2048 | 589.543333 | >>>>> +----------------+-------------+-------------+ >>>>>> 971.1.63 (first introduction of Bitmap) >>>>> Running this rev now...it may turn out to be the cause, and if so, I >>>>> will dig down into Padraig and Monty's Bitmap changes to see which >>>>> specific revision contains the culprit :) >>>>> /me loves automated benching... >>>>> Cheers, >>>>> Jay >>>>>> Cheers, >>>>>> -Brian >>>> >>>>>> On May 1, 2009, at 9:11 PM, Jay Pipes wrote: >>>>>> Hi! >>>>>> Somewhere in r990, we have had a significant regression, most evident in >>>>>> concurrency levels greater than 128 concurrent connections: >>>>>> +-----------+-------------+-------------+ >>>>>> | version | concurrency | tps | >>>>>> +-----------+-------------+-------------+ >>>>>> | trunk-989 | 2 | 1050.630000 | >>>>>> | trunk-989 | 4 | 1417.433333 | >>>>>> | trunk-989 | 8 | 1942.686667 | >>>>>> | trunk-989 | 16 | 2612.166667 | >>>>>> | trunk-989 | 32 | 3389.150000 | >>>>>> | trunk-989 | 64 | 3944.433333 | >>>>>> | trunk-989 | 128 | 3924.076667 | >>>>>> | trunk-989 | 256 | 2964.776667 | >>>>>> | trunk-989 | 512 | 1981.050000 | >>>>>> | trunk-989 | 1024 | 1273.133333 | >>>>>> | trunk-989 | 2048 | 563.350000 | >>>>>> | trunk-990 | 2 | 1038.133333 | >>>>>> | trunk-990 | 4 | 1381.933333 | >>>>>> | trunk-990 | 8 | 1897.276667 | >>>>>> | trunk-990 | 16 | 2547.196667 | >>>>>> | trunk-990 | 32 | 3331.570000 | >>>>>> | trunk-990 | 64 | 3896.483333 | >>>>>> | trunk-990 | 128 | 3850.016667 | >>>>>> | trunk-990 | 256 | 2331.543333 | >>>>>> | trunk-990 | 512 | 1862.316667 | >>>>>> | trunk-990 | 1024 | 1103.903333 | >>>>>> | trunk-990 | 2048 | 436.976667 | >>>>>> +-----------+-------------+-------------+ >>>>>> The above TPS column is the average of 3 iterations of a readonly >>>>>> sysbench workload for the revision/concurrency pair. >>>>>> Unfortunately, r990 was a very big revision with a lot of changes: >>>>>> http://bazaar.launchpad.net/~drizzle-developers/drizzle/development/revision/990 >>>>>> This makes it very difficult to tell what precisely has caused the >>>>>> regression. I'd like to suggest that going forward, we really try to >>>>>> keep the size of merges and commits small so that it is easier to tell >>>>>> where regressions occur. >>>>>> Anyone interested in looking through the commit diff above is welcome to >>>>>> suggest ideas where regression may have occurred...but it might be, like >>>>>> Brian says, death by a thousand cuts... >>>>>> Cheers! >>>>>> Jay >>>>>> -- >>>>>> _______________________________________________________ >>>>>> Brian "Krow" Aker, brian at tangent.org >>>>>> Seattle, Washington >>>>>> http://krow.net/ <-- Me >>>>>> http://tangent.org/ <-- Software >>>>>> _______________________________________________________ >>>>>> You can't grep a dead tree. >>>> >>>> >>>>>> _______________________________________________ >>>>>> 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 >>>> >> _______________________________________________ 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 -- Mats Kindahl Senior Software Engineer Database Technology Group Sun Microsystems _______________________________________________ Mailing list: https://launchpad.net/~drizzle-discuss Post to : [email protected] Unsubscribe : https://launchpad.net/~drizzle-discuss More help : https://help.launchpad.net/ListHelp
