From d47996975a9b9f30a8397ecca018d5ccb53ee114 Mon Sep 17 00:00:00 2001 From: Watson Date: Sat, 18 May 2019 02:00:57 +0900 Subject: [PATCH 1/2] Use rb_ivar_get/rb_ivar_set to improve performance MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit By convert C-string to Ruby string ID each time, it will take a slightly time to call method each time. This patch will catch the Ruby string ID to cut off the time. * Before ``` Warming up -------------------------------------- query 886.000 i/100ms each 67.339k i/100ms fields 195.612k i/100ms Calculating ------------------------------------- query 9.385k (± 4.5%) i/s - 46.958k in 5.016539s each 901.633k (± 0.2%) i/s - 4.512M in 5.003951s fields 3.779M (± 0.2%) i/s - 18.974M in 5.021533s ``` * After ``` Warming up -------------------------------------- query 845.000 i/100ms each 86.916k i/100ms fields 231.527k i/100ms Calculating ------------------------------------- query 9.553k (± 2.0%) i/s - 48.320k in 5.059947s each 1.133M (± 0.3%) i/s - 5.736M in 5.062606s fields 6.319M (± 0.1%) i/s - 31.719M in 5.019960s ``` --- ext/mysql2/client.c | 17 ++++++++++------- ext/mysql2/result.c | 10 ++++++---- ext/mysql2/statement.c | 6 ++++-- 3 files changed, 20 insertions(+), 13 deletions(-) diff --git a/ext/mysql2/client.c b/ext/mysql2/client.c index c4b6c44cf..292657199 100644 --- a/ext/mysql2/client.c +++ b/ext/mysql2/client.c @@ -18,7 +18,8 @@ VALUE cMysql2Client; extern VALUE mMysql2, cMysql2Error, cMysql2TimeoutError; static VALUE sym_id, sym_version, sym_header_version, sym_async, sym_symbolize_keys, sym_as, sym_array, sym_stream; static VALUE sym_no_good_index_used, sym_no_index_used, sym_query_was_slow; -static ID intern_brackets, intern_merge, intern_merge_bang, intern_new_with_args; +static ID intern_brackets, intern_merge, intern_merge_bang, intern_new_with_args, + intern_current_query_options, intern_read_timeout; #define REQUIRE_INITIALIZED(wrapper) \ if (!wrapper->initialized) { \ @@ -579,7 +580,7 @@ static VALUE rb_mysql_client_async_result(VALUE self) { rb_raise_mysql2_error(wrapper); } - is_streaming = rb_hash_aref(rb_iv_get(self, "@current_query_options"), sym_stream); + is_streaming = rb_hash_aref(rb_ivar_get(self, intern_current_query_options), sym_stream); if (is_streaming == Qtrue) { result = (MYSQL_RES *)rb_thread_call_without_gvl(nogvl_use_result, wrapper, RUBY_UBF_IO, 0); } else { @@ -596,7 +597,7 @@ static VALUE rb_mysql_client_async_result(VALUE self) { } // Duplicate the options hash and put the copy in the Result object - current = rb_hash_dup(rb_iv_get(self, "@current_query_options")); + current = rb_hash_dup(rb_ivar_get(self, intern_current_query_options)); (void)RB_GC_GUARD(current); Check_Type(current, T_HASH); resultObj = rb_mysql_result_to_obj(self, wrapper->encoding, current, result, Qnil); @@ -639,7 +640,7 @@ static VALUE do_query(void *args) { int retval; VALUE read_timeout; - read_timeout = rb_iv_get(async_args->self, "@read_timeout"); + read_timeout = rb_ivar_get(async_args->self, intern_read_timeout); tvp = NULL; if (!NIL_P(read_timeout)) { @@ -767,7 +768,7 @@ static VALUE rb_mysql_query(VALUE self, VALUE sql, VALUE current) { (void)RB_GC_GUARD(current); Check_Type(current, T_HASH); - rb_iv_set(self, "@current_query_options", current); + rb_ivar_set(self, intern_current_query_options, current); Check_Type(sql, T_STRING); /* ensure the string is in the encoding the connection is expecting */ @@ -1179,7 +1180,7 @@ static VALUE rb_mysql_client_store_result(VALUE self) } // Duplicate the options hash and put the copy in the Result object - current = rb_hash_dup(rb_iv_get(self, "@current_query_options")); + current = rb_hash_dup(rb_ivar_get(self, intern_current_query_options)); (void)RB_GC_GUARD(current); Check_Type(current, T_HASH); resultObj = rb_mysql_result_to_obj(self, wrapper->encoding, current, result, Qnil); @@ -1265,7 +1266,7 @@ static VALUE set_read_timeout(VALUE self, VALUE value) { /* Set the instance variable here even though _mysql_client_options might not succeed, because the timeout is used in other ways elsewhere */ - rb_iv_set(self, "@read_timeout", value); + rb_ivar_set(self, intern_read_timeout, value); return _mysql_client_options(self, MYSQL_OPT_READ_TIMEOUT, value); } @@ -1471,6 +1472,8 @@ void init_mysql2_client() { intern_merge = rb_intern("merge"); intern_merge_bang = rb_intern("merge!"); intern_new_with_args = rb_intern("new_with_args"); + intern_current_query_options = rb_intern("@current_query_options"); + intern_read_timeout = rb_intern("@read_timeout"); #ifdef CLIENT_LONG_PASSWORD rb_const_set(cMysql2Client, rb_intern("LONG_PASSWORD"), diff --git a/ext/mysql2/result.c b/ext/mysql2/result.c index 74851892a..32fbe0af2 100644 --- a/ext/mysql2/result.c +++ b/ext/mysql2/result.c @@ -37,7 +37,8 @@ extern VALUE mMysql2, cMysql2Client, cMysql2Error; static VALUE cMysql2Result, cDateTime, cDate; static VALUE opt_decimal_zero, opt_float_zero, opt_time_year, opt_time_month, opt_utc_offset; static ID intern_new, intern_utc, intern_local, intern_localtime, intern_local_offset, - intern_civil, intern_new_offset, intern_merge, intern_BigDecimal; + intern_civil, intern_new_offset, intern_merge, intern_BigDecimal, + intern_query_options; static VALUE sym_symbolize_keys, sym_as, sym_array, sym_database_timezone, sym_application_timezone, sym_local, sym_utc, sym_cast_booleans, sym_cache_rows, sym_cast, sym_stream, sym_name; @@ -695,7 +696,7 @@ static VALUE rb_mysql_result_fetch_fields(VALUE self) { GET_RESULT(self); - defaults = rb_iv_get(self, "@query_options"); + defaults = rb_ivar_get(self, intern_query_options); Check_Type(defaults, T_HASH); if (rb_hash_aref(defaults, sym_symbolize_keys) == Qtrue) { symbolizeKeys = 1; @@ -818,7 +819,7 @@ static VALUE rb_mysql_result_each(int argc, VALUE * argv, VALUE self) { rb_raise(cMysql2Error, "Statement handle already closed"); } - defaults = rb_iv_get(self, "@query_options"); + defaults = rb_ivar_get(self, intern_query_options); Check_Type(defaults, T_HASH); if (rb_scan_args(argc, argv, "01&", &opts, &block) == 1) { opts = rb_funcall(defaults, intern_merge, 1, opts); @@ -951,7 +952,7 @@ VALUE rb_mysql_result_to_obj(VALUE client, VALUE encoding, VALUE options, MYSQL_ } rb_obj_call_init(obj, 0, NULL); - rb_iv_set(obj, "@query_options", options); + rb_ivar_set(obj, intern_query_options, options); /* Options that cannot be changed in results.each(...) { |row| } * should be processed here. */ @@ -980,6 +981,7 @@ void init_mysql2_result() { intern_civil = rb_intern("civil"); intern_new_offset = rb_intern("new_offset"); intern_BigDecimal = rb_intern("BigDecimal"); + intern_query_options = rb_intern("@query_options"); sym_symbolize_keys = ID2SYM(rb_intern("symbolize_keys")); sym_as = ID2SYM(rb_intern("as")); diff --git a/ext/mysql2/statement.c b/ext/mysql2/statement.c index 22e22ecfd..e51fb9fd0 100644 --- a/ext/mysql2/statement.c +++ b/ext/mysql2/statement.c @@ -3,7 +3,8 @@ extern VALUE mMysql2, cMysql2Error; static VALUE cMysql2Statement, cBigDecimal, cDateTime, cDate; static VALUE sym_stream, intern_new_with_args, intern_each, intern_to_s, intern_merge_bang; -static VALUE intern_sec_fraction, intern_usec, intern_sec, intern_min, intern_hour, intern_day, intern_month, intern_year; +static VALUE intern_sec_fraction, intern_usec, intern_sec, intern_min, intern_hour, intern_day, intern_month, intern_year, + intern_query_options; #define GET_STATEMENT(self) \ mysql_stmt_wrapper *stmt_wrapper; \ @@ -404,7 +405,7 @@ static VALUE rb_mysql_stmt_execute(int argc, VALUE *argv, VALUE self) { } // Duplicate the options hash, merge! extra opts, put the copy into the Result object - current = rb_hash_dup(rb_iv_get(stmt_wrapper->client, "@query_options")); + current = rb_hash_dup(rb_ivar_get(stmt_wrapper->client, intern_query_options)); (void)RB_GC_GUARD(current); Check_Type(current, T_HASH); @@ -599,4 +600,5 @@ void init_mysql2_statement() { intern_to_s = rb_intern("to_s"); intern_merge_bang = rb_intern("merge!"); + intern_query_options = rb_intern("@query_options"); } From a7349893f0ff1ed7e8b601a0879f6f6972f36962 Mon Sep 17 00:00:00 2001 From: Watson Date: Sat, 18 May 2019 02:15:32 +0900 Subject: [PATCH 2/2] Check only whether block was given MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `rb_scan_args(argc, argv, "01&", ...)` will generate `Proc` object from block. However, the object has used to only check whether block was given. To remove redundant object generating, this patch will use `rb_block_given_p()` to check whether block was given. * Before ``` Warming up -------------------------------------- query 845.000 i/100ms each 86.916k i/100ms fields 231.527k i/100ms Calculating ------------------------------------- query 9.553k (± 2.0%) i/s - 48.320k in 5.059947s each 1.133M (± 0.3%) i/s - 5.736M in 5.062606s fields 6.319M (± 0.1%) i/s - 31.719M in 5.019960s ``` * After ``` Warming up -------------------------------------- query 864.000 i/100ms each 106.916k i/100ms fields 251.255k i/100ms Calculating ------------------------------------- query 9.457k (± 3.8%) i/s - 47.520k in 5.032949s each 1.550M (± 0.3%) i/s - 7.805M in 5.037029s fields 6.233M (± 0.1%) i/s - 31.407M in 5.039049s ``` --- ext/mysql2/result.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/ext/mysql2/result.c b/ext/mysql2/result.c index 32fbe0af2..fbff98137 100644 --- a/ext/mysql2/result.c +++ b/ext/mysql2/result.c @@ -30,7 +30,7 @@ typedef struct { int streaming; ID db_timezone; ID app_timezone; - VALUE block_given; + int block_given; /* boolean */ } result_each_args; extern VALUE mMysql2, cMysql2Client, cMysql2Error; @@ -741,7 +741,7 @@ static VALUE rb_mysql_result_each_(VALUE self, row = fetch_row_func(self, fields, args); if (row != Qnil) { wrapper->numberOfRows++; - if (args->block_given != Qnil) { + if (args->block_given) { rb_yield(row); } } @@ -791,7 +791,7 @@ static VALUE rb_mysql_result_each_(VALUE self, return Qnil; } - if (args->block_given != Qnil) { + if (args->block_given) { rb_yield(row); } } @@ -809,7 +809,7 @@ static VALUE rb_mysql_result_each_(VALUE self, static VALUE rb_mysql_result_each(int argc, VALUE * argv, VALUE self) { result_each_args args; - VALUE defaults, opts, block, (*fetch_row_func)(VALUE, MYSQL_FIELD *fields, const result_each_args *args); + VALUE defaults, opts, (*fetch_row_func)(VALUE, MYSQL_FIELD *fields, const result_each_args *args); ID db_timezone, app_timezone, dbTz, appTz; int symbolizeKeys, asArray, castBool, cacheRows, cast; @@ -821,7 +821,10 @@ static VALUE rb_mysql_result_each(int argc, VALUE * argv, VALUE self) { defaults = rb_ivar_get(self, intern_query_options); Check_Type(defaults, T_HASH); - if (rb_scan_args(argc, argv, "01&", &opts, &block) == 1) { + + // A block can be passed to this method, but since we don't call the block directly from C, + // we don't need to capture it into a variable here with the "&" scan arg. + if (rb_scan_args(argc, argv, "01", &opts) == 1) { opts = rb_funcall(defaults, intern_merge, 1, opts); } else { opts = defaults; @@ -887,7 +890,7 @@ static VALUE rb_mysql_result_each(int argc, VALUE * argv, VALUE self) { args.cast = cast; args.db_timezone = db_timezone; args.app_timezone = app_timezone; - args.block_given = block; + args.block_given = rb_block_given_p(); if (wrapper->stmt_wrapper) { fetch_row_func = rb_mysql_result_fetch_row_stmt;