is_thd_db_read_only_by_name CPU overhead can be reduced

[mdcallag]

The has_global_grant check (see here) is done early and unconditionally in the function. It is only needed infrequently (see here) for the usage of the binlog_admin_variable.

The function as written accounts for about 2% of CPU time on many of the write-heavy sysbench microbenchmarks that I run. The patch below moves the work from has_global_grant so it is only called when needed and that saves about 2% of CPU (or improves throughput by 2%).

diff --git a/sql/sql_db.cc b/sql/sql_db.cc
index f4e5b4ee3e1..e9b8273f2fb 100644
--- a/sql/sql_db.cc
+++ b/sql/sql_db.cc
@@ -230,9 +230,6 @@ end:
 bool is_thd_db_read_only_by_name(THD *thd, const char *db) {
   DBUG_ENTER("is_thd_db_read_only_by_name");
   bool super = thd->m_main_security_ctx.check_access(SUPER_ACL);
-  bool binlog_admin =
-      thd->m_main_security_ctx.has_global_grant(STRING_WITH_LEN("BINLOG_ADMIN"))
-          .first;
   enum enum_db_read_only flag = DB_READ_ONLY_NULL;

   // Check cached info in THD first.
@@ -254,7 +251,8 @@ bool is_thd_db_read_only_by_name(THD *thd, const char *db) {
   assert(flag >= DB_READ_ONLY_NO && flag <= DB_READ_ONLY_SUPER);

   if (flag == DB_READ_ONLY_SUPER ||
-      (flag == DB_READ_ONLY_YES && !super && !binlog_admin)) {
+      (flag == DB_READ_ONLY_YES && !super &&
+      !thd->m_main_security_ctx.has_global_grant(STRING_WITH_LEN("BINLOG_ADMIN")).first)) {
     DBUG_RETURN(true);
   }

The impact is more obvious on workloads with low concurrency than with high, because there are other perf issues on the high concurrency workloads.

Some example results are below. The numbers in the second column are the throughput for FB MyRocks 8.0.32 with the patch relative to unmodified FB MyRocks 8.0.32 and FB MyRocks was compiled at git hash 49b37df (code current as of 24-05-29) with RocksDB 9.2.1. When the number is 1.0 then FB MyRocks with the patch has the same perf, when > 1.0 then FB MyRocks with the patch is faster.

See here for an overview of how I use sysbench.

From my older small server, sysbench with 1 thread

0.97    1.05    delete_range=100
1.04    1.05    insert_range=100
1.00    1.00    read-write_range=100
0.99    1.01    read-write_range=10
1.02    1.03    update-index_range=100
1.00    1.00    update-inlist_range=100
1.00    1.03    update-nonindex_range=100
1.03    1.04    update-one_range=100
1.02    1.03    update-zipf_range=100
1.01    1.00    write-only_range=10000

From my newer small server, sysbench with 1 thread

1.03    1.02    delete_range=100
1.03    1.03    insert_range=100
1.00    1.00    read-write_range=100
1.01    1.01    read-write_range=10
1.02    1.02    update-index_range=100
1.01    1.01    update-inlist_range=100
1.02    1.02    update-nonindex_range=100
1.03    1.03    update-one_range=100
1.03    1.02    update-zipf_range=100
1.01    1.01    write-only_range=10000

From a 2-socket server, sysbench with 16 threads, the impact is less here

1.03    1.01    delete_range=100
1.03    1.02    insert_range=100
1.00    1.00    read-write_range=100
1.00    0.99    read-write_range=10
1.01    1.01    update-index_range=100
0.97    0.97    update-inlist_range=100
1.01    1.01    update-nonindex_range=100
1.01    1.02    update-one_range=100
1.01    1.01    update-zipf_range=100
1.01    1.00    write-only_range=10000

From a 32-core, 1-socket server, sysbench with 24 threads, the impact is less here

0.98    1.00    delete_range=100
1.00    1.01    insert_range=100
1.00    1.00    read-write_range=100
1.01    1.00    read-write_range=10
1.00    1.00    update-index_range=100
1.01    1.00    update-inlist_range=100
1.00    1.00    update-nonindex_range=100
1.00    1.01    update-one_range=100
1.00    1.00    update-zipf_range=100
1.01    1.01    write-only_range=10000