Hi Sergei, On Fri, Nov 3, 2023 at 5:24 PM Sergei Golubchik <s...@mariadb.org> wrote:
> Hi, Aleksey, > > On Oct 24, Aleksey Midenkov wrote: > > > > diff --git a/storage/myisam/ha_myisam.cc > b/storage/myisam/ha_myisam.cc > > > > index ddab8bcf741..27151548704 100644 > > > > --- a/storage/myisam/ha_myisam.cc > > > > +++ b/storage/myisam/ha_myisam.cc > > > > @@ -714,6 +714,8 @@ static int compute_vcols(MI_INFO *info, uchar > *record, int keynum) > > > > /* This mutex is needed for parallel repair */ > > > > mysql_mutex_lock(&info->s->intern_lock); > > > > TABLE *table= (TABLE*)(info->external_ref); > > > > + if (!current_thd) > > > > + set_current_thd(table->in_use); > > > > > > why not to do it in thr_find_all_keys(), unconditionally? > > > you'd avoid an if() executed per row. > > > > It is done only for repair, isn't it, so what are we supposed to > > optimize by removing this if? > > it's done for repair, for *every row* of the table, there can be many > millions of rows. It's the condition that will be true for the first row > and then it'll be always false. There is simply no reason to reevaluate > it for every row. > I understand this perfectly. There is just no good place for this set_current_thd(), so this was the choice between the two evils. I chosen code cleanliness instead of performance from the 3 facts: 1. This is only for repair operation that should happen less than once a week; 2. This is only for a table that contains virtual columns; 3. Specific weight of this condition is small against the execution of compute_vcols(). > > > > Also, I'd suggest a comment, like > > > > > > /* > > > To evaluate vcols we must have current_thd set. > > > This will set current_thd in all threads to the same THD, but it's > > > safe, because vcols are always evaluated under > info->s->intern_lock. > > > */ > > > > This is self-explanatory for the code in compute_vcols(). Another > > argument to keep that in compute_vcols(). > > If it would've been self-explanatory, I wouldn't have asked. Meaning it > wasn't self-explanatory to me. > Ok. > > Regards, > Sergei > Chief Architect, MariaDB Server > and secur...@mariadb.org > -- @midenok
_______________________________________________ developers mailing list -- developers@lists.mariadb.org To unsubscribe send an email to developers-le...@lists.mariadb.org
