List:Commits« Previous MessageNext Message »
From:Marc Alff Date:April 1 2010 2:11pm
Subject:Re: bzr commit into mysql-next-mr-bugfixing branch (marc.alff:3144)
Bug#52502
View as plain text  
Hi Guilhem,

On 4/1/10 7:31 AM, Guilhem Bichot wrote:
> Hello,
>
> Marc Alff a écrit, Le 01.04.2010 10:08:
>> #At file:///Users/malff/BZR_TREE/mysql-next-mr-bugfixing-52502/ based
>> on revid:alik@stripped
>>
>> 3144 Marc Alff 2010-04-01
>> Bug#52502 Performance schema does not start with large mutex_instance
>> buffers
>
>> === modified file 'storage/perfschema/pfs_instr.cc'
>> --- a/storage/perfschema/pfs_instr.cc 2010-03-22 12:48:18 +0000
>> +++ b/storage/perfschema/pfs_instr.cc 2010-04-01 08:08:12 +0000
>> @@ -445,6 +445,77 @@ void cleanup_file_hash(void)
>> }
>> }
>>
>> +void PFS_scan::init(uint random, uint max_size)
>> +{
>
> I'd do a single unconditional m_pass=0 here: m_pass is a counter of the
> number of passes already done, and when the object is not yet used this
> counter should always be 0 (kind of, by definition), so it makes sense
> to set it here (and it saves a bit of code too, by eliminating m_pass=0
> in branches below).

Yes, good point.
I will fix that when pushing.

>
>> + if (max_size == 0)
>> + {
>> + /* Degenerated case, no buffer */
>> + m_pass= 0;
>> + m_pass_max= 0;
>> + return;
>> + }
>
>> === modified file 'storage/perfschema/pfs_instr.h'
>> --- a/storage/perfschema/pfs_instr.h 2010-01-12 01:47:27 +0000
>> +++ b/storage/perfschema/pfs_instr.h 2010-04-01 08:08:12 +0000
>> @@ -136,6 +136,48 @@ struct PFS_table : public PFS_instr
>> */
>> #define LOCKER_STACK_SIZE 3
>>
>> +/**
>> + @def PFS_MAX_ALLOC_RETRY
>> + Maximum number of times the code attempts to allocate an item
>> + from internal buffers, before giving up.
>> +*/
>> +#define PFS_MAX_ALLOC_RETRY 1000
>> +
>> +#define PFS_MAX_SCAN_PASS 2
>> +
>> +/**
>> + Helper to scan circular buffers.
>> + Given a buffer of size [0, max_size - 1],
>> + and a random starting point in the buffer,
>> + this helper returns up to two [first, last -1] intervals that:
>> + - fit into the [0, max_size - 1] range,
>> + - have a maximum combined length of at most PFS_MAX_ALLOC_RETRY.
>> +*/
>> +struct PFS_scan
>> +{
>> +public:
>> + void init(uint random, uint max_size);
>
> At your option, PFS_scan could also be a class and init() would be its
> constructor.

What I have in mind for later is something like:

PFS_scan scan;

if (complicated use case)
   scan.init_one_way(...)
else
   scan.init_another_way(...)

for ( ; scan.has_pass(); scan_next_pass())

where the init logic can even get moved to a dedicated function if needed.

For this, the variable declaration must be separated from the various 
inits, which can not be done when using C++ constructors.

This is for much later, but to give you an idea, for example, the scan 
algorithm may be different depending of whether a "very static" or "very 
dynamic" mutex is allocated, if we decide to put static/dynamic objects 
into different regions in the [0, max_size -1] area, to improve the 
overall success rate when looking for an empty slot ...

>
>> + bool has_pass() const
>> + { return (m_pass < m_pass_max); }
>> +
>> + void next_pass()
>> + { m_pass++; }
>> + + uint first()
>
> can be made "const"

Yes, will do.

>
>> + { return m_first[m_pass]; }
>> +
>> + uint last()
>
> same
>
>> + { return m_last[m_pass]; }
>> +
>> +private:
>> + uint m_pass;
>> + uint m_pass_max;
>> + uint m_first[PFS_MAX_SCAN_PASS];
>> + uint m_last[PFS_MAX_SCAN_PASS];
>> +};
>> +
>
> A quite clean patch, thanks. Ok to push.

Thanks for the review.

Regards,
-- Marc
Thread
bzr commit into mysql-next-mr-bugfixing branch (marc.alff:3144)Bug#52502Marc Alff1 Apr
  • Re: bzr commit into mysql-next-mr-bugfixing branch (marc.alff:3144)Bug#52502Guilhem Bichot1 Apr
    • Re: bzr commit into mysql-next-mr-bugfixing branch (marc.alff:3144)Bug#52502Marc Alff1 Apr