List:Commits« Previous MessageNext Message »
From:Guilhem Bichot Date:March 31 2011 7:49pm
Subject:Re: bzr commit into mysql-trunk branch (jorgen.loland:3323) Bug#11766327
View as plain text  
Hello,

Roy Lyseng a écrit, Le 31.03.2011 20:30:
> On 31.03.11 19.06, Guilhem Bichot wrote:
> 
> 
>>> @@ -11072,10 +11056,12 @@ bool setup_sj_materialization(JOIN_TAB *
>>> */
>>> for (i= 0; i < sjm->table_count; i++)
>>> {
>>> - remove_sj_conds(&tab[i].select_cond);
>>> + Item *cond= tab[i].get_cond();
>>
>> If this intermediate variable is not needed I suggest to delete it.
>> But maybe it is needed, I haven't thought about return-by-value, 
>> lvalue etc.
>>
>>> + remove_sj_conds(&cond);
> 
> This does not seem correct to me. remove_sj_conds() is supposed to 
> modify tab[i].select_cond, but after the fix it modifies the local copy.

Yes, that was my fear too.

> I also have a personal preference issue: get_ as a prefix is redundant, 
> I would replace get_cond() with condition() (but keep set_condition() 
> for the setter).

I let you sort this out with Jorgen; I don't care.

> 
>>> if (tab[i].select)
>>> remove_sj_conds(&tab[i].select->cond);
>>> }
>>> +
>>> if (!(sjm->in_equality= create_subquery_equalities(thd, emb_sj_nest)))
>>> DBUG_RETURN(TRUE); /* purecov: inspected */
>>> }
>>> @@ -11464,6 +11450,58 @@ uint JOIN_TAB::get_sj_strategy() const
>>> return s;
>>> }
>>>
>>> +/**
>>> + Extend JOIN_TAB->cond and JOIN_TAB->select->cond by AND'ing
>>> + add_cond to them
>>> +
>>> + @param add_cond The condition to AND with the existing conditions
>>> + @param line Code line this method was called from
>>> +
>>> + @retval true if there was a memory allocation error, in which + 
>>> case the
>>> conditions remain unchanged
>>> + @retval false otherwise
>>> +
>>> +*/
>>> +bool JOIN_TAB::extend_and_fix_jt_and_sel_cond(Item *add_cond, uint 
>>> line)
>>> +{
>>> + if (extend_and_fix_cond(add_cond, line))
>>
>> could add unlikely()
> 
> I'm a bit afraid of the "code contamination" of these unlikely() macros.

Well... we have those macros... I find it's good if it can help lower 
the performance impact of all those rarely executed "if (out of memory)".
What bad do they do?

> There is also an option to gather branch prediction statistics using 
> two-stage compilations. (The first stage gathers statistics, the second 
> uses statistics to apply branch prediction hints). The benefit of this 
> is that code need no changes, the drawback is the need for two-stage 
> compilation, which makes it hard to do in developer sandboxes.

I'm not aware of two-stage compilation. What compiler does that?
And, isn't it dangerous? I mean, OOM happens rarely on the Lab machine 
where the release is built, as rarely as on the customer's, so if(OOM) 
will be optimized the same way, but for the other if()s, they may be 
optimized in some way on the Lab machine, in a way which isn't 
representative of a real load, and thus would perform poorly at the 
customer's??
Thread
bzr commit into mysql-trunk branch (jorgen.loland:3323) Bug#11766327Jorgen Loland25 Mar
  • Re: bzr commit into mysql-trunk branch (jorgen.loland:3323) Bug#11766327Guilhem Bichot31 Mar
    • Re: bzr commit into mysql-trunk branch (jorgen.loland:3323) Bug#11766327Roy Lyseng31 Mar
      • Re: bzr commit into mysql-trunk branch (jorgen.loland:3323) Bug#11766327Guilhem Bichot31 Mar
        • Re: bzr commit into mysql-trunk branch (jorgen.loland:3323) Bug#11766327Roy Lyseng31 Mar