maria-developers team mailing list archive

[Sergey Petrunia <sergey@xxxxxxxxxxx>]

Hi Varun,

So I was thinking of the impact of the optimizer trace, and decided to look at
how the most common check "if optimizer trace is enabled" is done.

The code at SQL layer does this kind of check 

  if (thd->trace_started())

(it is wrapped somewhere with "unlikely" but this is not consistent)


THD::trace_started is defined as:


bool THD::trace_started()
{
  return opt_trace.is_started();
}

which opt_trace here is Opt_trace_context. looking there:

class Opt_trace_context 
{
  ...
  bool is_started()
  {
    return current_trace && is_enabled();
  }

following:

bool Opt_trace_context::is_enabled()
{
  if (current_trace)
    return current_trace->is_enabled();
  return false;
}

current_trace is Opt_trace_stmt, looking there:

bool Opt_trace_stmt::is_enabled()
{
  return I_S_disabled == 0;
}

This is quite convoluted, and could probably made shorter.

Disassembling sql_select.cc, I can see that the calls to thd->trace_started()
are not inlined:

   if (unlikely(thd->trace_started()))
   10ac9:       4c 89 ff                mov    rdi,r15
   10acc:       e8 00 00 00 00          call   10ad1 <_ZL32best_extension_by_limited_searchP4JOINyjddjjj+0x1a1>
   10ad1:       84 c0                   test   al,al
   10ad3:       0f 85 fc 02 00 00       jne    10dd5 <_ZL32best_extension_by_limited_searchP4JOINyjddjjj+0x4a5>


    if (unlikely(thd->trace_started()))
   1197b:       4c 89 f7                mov    rdi,r14
   1197e:       e8 00 00 00 00          call   11983 <_Z11choose_planP4JOINy+0x8a3>
   11983:       84 c0                   test   al,al
   11985:       0f 85 2f 01 00 00       jne    11aba <_Z11choose_planP4JOINy+0x9da>

  if (thd->trace_started())
   2d6f7:       48 8b bd b0 fc ff ff    mov    rdi,QWORD PTR [rbp-0x350]
   2d6fe:       e8 00 00 00 00          call   2d703 <_ZN4JOIN14optimize_innerEv+0x1963>
   2d703:       84 c0                   test   al,al
   2d705:       0f 85 6c 09 00 00       jne    2e077 <_ZN4JOIN14optimize_innerEv+0x22d7>

    if (thd->trace_started())
   2d7da:       48 8b bd b0 fc ff ff    mov    rdi,QWORD PTR [rbp-0x350]
   2d7e1:       e8 00 00 00 00          call   2d7e6 <_ZN4JOIN14optimize_innerEv+0x1a46>
   2d7e6:       84 c0                   test   al,al
   2d7e8:       0f 85 ff 1a 00 00       jne    2f2ed <_ZN4JOIN14optimize_innerEv+0x354d>

But does it all matter? 

I think the action plan should be:

1. Reproduce the slowdown as it was reported in MDEV-18822. 
2. If #1 succeeds in several re-runs, declare THD::trace_started as:

class THD {
public:
  bool trace_started; // initialize this to false in constructor and never touch it 
  inline bool trace_started() const { return trace_started; }
};
 
and then use search-replace to change "thd->trace_started()"  to 
"unlikely(thd->trace_started)" 

and then re-run the benchmark.

Any thoughts?

BR
 Sergei
-- 
Sergei Petrunia, Software Developer
MariaDB Corporation | Skype: sergefp | Blog: http://s.petrunia.net/blog