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();


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 
   10ad1:       84 c0                   test   al,al
   10ad3:       0f 85 fc 02 00 00       jne    10dd5 

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

  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 
   2d703:       84 c0                   test   al,al
   2d705:       0f 85 6c 09 00 00       jne    2e077 

    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 
   2d7e6:       84 c0                   test   al,al
   2d7e8:       0f 85 ff 1a 00 00       jne    2f2ed 

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 {
  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 

and then re-run the benchmark.

Any thoughts?

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

Mailing list: https://launchpad.net/~maria-developers
Post to     : maria-developers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~maria-developers
More help   : https://help.launchpad.net/ListHelp

Reply via email to