[MDEV-33181] Performance schema introduces many conditional branches even when disabled at runtime Created: 2024-01-04  Updated: 2024-01-04

Status: Open
Project: MariaDB Server
Component/s: Performance Schema
Affects Version/s: 5.5
Fix Version/s: 11.5

Type: Bug Priority: Major
Reporter: Marko Mäkelä Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: performance

Issue Links:
Relates
relates to MDEV-24973 Performance schema duplicates rarely ... Closed

 Description   

Unless MariaDB is built with cmake -DPLUGIN_PERFSCHEMA=NO, there will be some measurable overhead related to some frequently invoked operations, such as mysql_mutex_lock() and mysql_mutex_unlock(). Take the latter for example. It would resolve to the following inline function:

{
  int result;
 
#ifdef HAVE_PSI_MUTEX_INTERFACE
  if (psi_likely(that->m_psi != NULL))
    PSI_MUTEX_CALL(unlock_mutex)(that->m_psi);
#endif
 
#ifdef SAFE_MUTEX
  result= safe_mutex_unlock(&that->m_mutex, src_file, src_line);
#else
  result= pthread_mutex_unlock(&that->m_mutex);
#endif
 
  return result;
}

The call to pfs_unlock_mutex_v1() via the function pointer PSI_MUTEX_CALL(unlock_mutex), as well as the condition preceding that call are executed within the critical section of the mutex.

It would be better to make use of function pointers, say,

return PSI_MUTEX_CALL(unlock_mutex)(that);

and make the function pointer PSI_server->unlock_mutex point to either pthread_mutex_unlock or pfs_unlock_mutex(). If the server is compiled with PLUGIN_PERFSCHEMA=NO, the macro can resolve bypass the function pointer, something like this:

#ifdef HAVE_PSI_MUTEX_INTERFACE
# define mysql_mutex_unlock(M) PSI_MUTEX_CALL(unlock_mutex)(M)
#elif defined SAFE_MUTEX
# define mysql_mutex_unlock(M) \
  safe_mutex_unlock(M, __FILE__, __LINE__)
#else
# define mysql_mutex_unlock(M) pthread_mutex_unlock(M)
#endif

The function pointer would be set to pthread_mutex_unlock by static initialization, and assigned to the instrumented function at the end of pfs_init_func(). In this way, if the performance schema is disabled at runtime, mysql_mutex_unlock() would translate to a simple call to pthread_mutex_unlock() via a function pointer.

While we are at it, I would also consider removing memory bloat like this:

struct st_mysql_mutex
{
  /** The real mutex. */
#ifdef SAFE_MUTEX
  safe_mutex_t m_mutex;
#else
  pthread_mutex_t m_mutex;
#endif
  /**
    The instrumentation hook.
    Note that this hook is not conditionally defined,
    for binary compatibility of the @c mysql_mutex_t interface.
  */
  struct PSI_mutex *m_psi;
};

Which binary compatibility is this about? To my knowledge, all plugins are typically included in the MariaDB Server repository and compiled from source code.



 Comments   
Comment by Marko Mäkelä [ 2024-01-04 ]

With regard to mutexes, this would be a follow-up to MDEV-24973.

Generated at Thu Feb 08 10:36:59 UTC 2024 using Jira 8.20.16#820016-sha1:9d11dbea5f4be3d4cc21f03a88dd11d8c8687422.