Re: [Maria-developers] [Commits] ab7640f: my_b_fill, inline my_b_* functions instead of hairy macros
Hi Sergei, ok to push. Should we always prefer "static inline" over macroses? Regards, Sergey On Tue, May 26, 2015 at 03:38:02PM +0200, serg@mariadb.org wrote:
revision-id: ab7640f4d30f5a160d3a1c77efe9d2c459a3bda6 parent(s): d916a1dc7705ae29a916e7ec91f0c76bceab708c committer: Sergei Golubchik branch nick: maria timestamp: 2015-05-26 15:28:23 +0200 message:
my_b_fill, inline my_b_* functions instead of hairy macros
--- include/my_sys.h | 141 ++++++++++++++++++++++++++++++++++++---------------- include/thr_lock.h | 1 - mysys/mf_iocache2.c | 55 -------------------- sql/log.cc | 1 - sql/wsrep_binlog.cc | 4 -- 5 files changed, 97 insertions(+), 105 deletions(-)
diff --git a/include/my_sys.h b/include/my_sys.h index 3d930d3..8e0e34f 100644 --- a/include/my_sys.h +++ b/include/my_sys.h @@ -19,6 +19,8 @@
#include "my_global.h" /* C_MODE_START, C_MODE_END */
+#include <m_string.h> + C_MODE_START
#ifdef HAVE_AIOWAIT @@ -507,52 +509,108 @@ typedef void (*my_error_reporter)(enum loglevel level, const char *format, ...)
extern my_error_reporter my_charset_error_reporter;
- /* defines for mf_iocache */ +/* inline functions for mf_iocache */ + +extern int my_b_flush_io_cache(IO_CACHE *info, int need_append_buffer_lock); +extern int _my_b_get(IO_CACHE *info); +extern int _my_b_read(IO_CACHE *info,uchar *Buffer,size_t Count); +extern int _my_b_write(IO_CACHE *info,const uchar *Buffer,size_t Count);
- /* Test if buffer is inited */ -#define my_b_clear(info) (info)->buffer=0 -#define my_b_inited(info) (info)->buffer +/* Test if buffer is inited */ +static inline void my_b_clear(IO_CACHE *info) { info->buffer= 0; } +static inline int my_b_inited(IO_CACHE *info) { return MY_TEST(info->buffer); } #define my_b_EOF INT_MIN
-#define my_b_read(info,Buffer,Count) \ - ((info)->read_pos + (Count) <= (info)->read_end ?\ - (memcpy(Buffer,(info)->read_pos,(size_t) (Count)), \ - ((info)->read_pos+=(Count)), 0) : _my_b_read((info), (Buffer), (Count))) - -#define my_b_write(info,Buffer,Count) \ - ((info)->write_pos + (Count) <=(info)->write_end ?\ - (memcpy((info)->write_pos, (Buffer), (size_t)(Count)),\ - ((info)->write_pos+=(Count)), 0) : _my_b_write((info), (Buffer), (Count))) - -#define my_b_get(info) \ - ((info)->read_pos != (info)->read_end ?\ - ((info)->read_pos++, (int) (uchar) (info)->read_pos[-1]) :\ - _my_b_get(info)) - - /* my_b_write_byte dosn't have any err-check */ -#define my_b_write_byte(info,chr) \ - (((info)->write_pos < (info)->write_end) ?\ - ((*(info)->write_pos++)=(chr)) :\ - ((my_b_flush_io_cache(info, 1)), ((*(info)->write_pos++)=(chr)))) - -#define my_b_tell(info) ((info)->pos_in_file + \ - (size_t) (*(info)->current_pos - (info)->request_pos)) -#define my_b_write_tell(info) ((info)->pos_in_file + \ - ((info)->write_pos - (info)->write_buffer)) - -#define my_b_get_buffer_start(info) (info)->request_pos -#define my_b_get_bytes_in_buffer(info) (char*) (info)->read_end - \ - (char*) my_b_get_buffer_start(info) -#define my_b_get_pos_in_file(info) (info)->pos_in_file - -/* tell write offset in the SEQ_APPEND cache */ +static inline int my_b_read(IO_CACHE *info, uchar *Buffer, size_t Count) +{ + if (info->read_pos + Count <= info->read_end) + { + memcpy(Buffer, info->read_pos, Count); + info->read_pos+= Count; + return 0; + } + return _my_b_read(info, Buffer, Count); +} + +static inline int my_b_write(IO_CACHE *info, const uchar *Buffer, size_t Count) +{ + if (info->write_pos + Count <= info->write_end) + { + memcpy(info->write_pos, Buffer, Count); + info->write_pos+= Count; + return 0; + } + return _my_b_write(info, Buffer, Count); +} + +static inline int my_b_get(IO_CACHE *info) +{ + if (info->read_pos != info->read_end) + { + info->read_pos++; + return info->read_pos[-1]; + } + return _my_b_get(info); +} + +/* my_b_write_byte dosn't have any err-check */ +static inline void my_b_write_byte(IO_CACHE *info, uchar chr) +{ + if (info->write_pos >= info->write_end) + my_b_flush_io_cache(info, 1); + *info->write_pos++= chr; +} + +/** + Fill buffer of the cache. + + @note It assumes that you have already used all characters in the CACHE, + independent of the read_pos value! + + @returns + 0 On error or EOF (info->error = -1 on error) + # Number of characters +*/ +static inline size_t my_b_fill(IO_CACHE *info) +{ + info->read_pos= info->read_end; + return _my_b_read(info,0,0) ? 0 : info->read_end - info->read_pos; +} + +static inline my_off_t my_b_tell(const IO_CACHE *info) +{ + return info->pos_in_file + (*info->current_pos - info->request_pos); +} + +static inline my_off_t my_b_write_tell(const IO_CACHE *info) +{ + return info->pos_in_file + (info->write_pos - info->write_buffer); +} + +static inline uchar* my_b_get_buffer_start(const IO_CACHE *info) +{ + return info->request_pos; +} + +static inline size_t my_b_get_bytes_in_buffer(const IO_CACHE *info) +{ + return info->read_end - info->request_pos; +} + +static inline my_off_t my_b_get_pos_in_file(const IO_CACHE *info) +{ + return info->pos_in_file; +} + +static inline size_t my_b_bytes_in_cache(const IO_CACHE *info) +{ + return *info->current_end - *info->current_pos; +} + int my_b_copy_to_file(IO_CACHE *cache, FILE *file); my_off_t my_b_append_tell(IO_CACHE* info); my_off_t my_b_safe_tell(IO_CACHE* info); /* picks the correct tell() */
-#define my_b_bytes_in_cache(info) (size_t) (*(info)->current_end - \ - *(info)->current_pos) - typedef uint32 ha_checksum; extern ulong my_crc_dbug_check;
@@ -741,24 +799,19 @@ extern my_bool reinit_io_cache(IO_CACHE *info,enum cache_type type, my_off_t seek_offset, my_bool use_async_io, my_bool clear_cache); extern void setup_io_cache(IO_CACHE* info); -extern int _my_b_read(IO_CACHE *info,uchar *Buffer,size_t Count); extern void init_io_cache_share(IO_CACHE *read_cache, IO_CACHE_SHARE *cshare, IO_CACHE *write_cache, uint num_threads); extern void remove_io_thread(IO_CACHE *info); -extern int _my_b_get(IO_CACHE *info); extern int _my_b_async_read(IO_CACHE *info,uchar *Buffer,size_t Count); -extern int _my_b_write(IO_CACHE *info,const uchar *Buffer,size_t Count); extern int my_b_append(IO_CACHE *info,const uchar *Buffer,size_t Count); extern int my_b_safe_write(IO_CACHE *info,const uchar *Buffer,size_t Count);
extern int my_block_write(IO_CACHE *info, const uchar *Buffer, size_t Count, my_off_t pos); -extern int my_b_flush_io_cache(IO_CACHE *info, int need_append_buffer_lock);
#define flush_io_cache(info) my_b_flush_io_cache((info),1)
extern int end_io_cache(IO_CACHE *info); -extern size_t my_b_fill(IO_CACHE *info); extern void my_b_seek(IO_CACHE *info,my_off_t pos); extern size_t my_b_gets(IO_CACHE *info, char *to, size_t max_length); extern my_off_t my_b_filelength(IO_CACHE *info); diff --git a/include/thr_lock.h b/include/thr_lock.h index 2561709..bc916b8 100644 --- a/include/thr_lock.h +++ b/include/thr_lock.h @@ -166,7 +166,6 @@ void thr_set_lock_wait_callback(void (*before_wait)(void), void (*after_wait)(void));
#ifdef WITH_WSREP -#include <my_sys.h> typedef my_bool (* wsrep_thd_is_brute_force_fun)(void *, my_bool); typedef int (* wsrep_abort_thd_fun)(void *, void *, my_bool); typedef int (* wsrep_on_fun)(void *); diff --git a/mysys/mf_iocache2.c b/mysys/mf_iocache2.c index 06dfc9f..a642e11 100644 --- a/mysys/mf_iocache2.c +++ b/mysys/mf_iocache2.c @@ -61,7 +61,6 @@ my_b_copy_to_file(IO_CACHE *cache, FILE *file) if (my_fwrite(file, cache->read_pos, bytes_in_cache, MYF(MY_WME | MY_NABP)) == (size_t) -1) DBUG_RETURN(1); - cache->read_pos= cache->read_end; } while ((bytes_in_cache= my_b_fill(cache))); if(cache->error == -1) DBUG_RETURN(1); @@ -183,60 +182,6 @@ void my_b_seek(IO_CACHE *info,my_off_t pos)
/* - Fill buffer of the cache. - - NOTES - This assumes that you have already used all characters in the CACHE, - independent of the read_pos value! - - RETURN - 0 On error or EOF (info->error = -1 on error) - # Number of characters -*/ - - -size_t my_b_fill(IO_CACHE *info) -{ - my_off_t pos_in_file=(info->pos_in_file+ - (size_t) (info->read_end - info->buffer)); - size_t diff_length, length, max_length; - - if (info->seek_not_done) - { /* File touched, do seek */ - if (mysql_file_seek(info->file, pos_in_file, MY_SEEK_SET, MYF(0)) == - MY_FILEPOS_ERROR) - { - info->error= 0; - return 0; - } - info->seek_not_done=0; - } - diff_length=(size_t) (pos_in_file & (IO_SIZE-1)); - max_length=(info->read_length-diff_length); - if (max_length >= (info->end_of_file - pos_in_file)) - max_length= (size_t) (info->end_of_file - pos_in_file); - - if (!max_length) - { - info->error= 0; - return 0; /* EOF */ - } - DBUG_EXECUTE_IF ("simulate_my_b_fill_error", - {DBUG_SET("+d,simulate_file_read_error");}); - if ((length= mysql_file_read(info->file, info->buffer, max_length, - info->myflags)) == (size_t) -1) - { - info->error= -1; - return 0; - } - info->read_pos=info->buffer; - info->read_end=info->buffer+length; - info->pos_in_file=pos_in_file; - return length; -} - - -/* Read a string ended by '\n' into a buffer of 'max_length' size. Returns number of characters read, 0 on error. last byte is set to '\0' diff --git a/sql/log.cc b/sql/log.cc index 826a4d1..5d71732 100644 --- a/sql/log.cc +++ b/sql/log.cc @@ -6694,7 +6694,6 @@ int MYSQL_BIN_LOG::write_cache(THD *thd, IO_CACHE *cache) return ER_ERROR_ON_WRITE; status_var_add(thd->status_var.binlog_bytes_written, length);
- cache->read_pos=cache->read_end; // Mark buffer used up } while ((length= my_b_fill(cache)));
DBUG_ASSERT(carry == 0); diff --git a/sql/wsrep_binlog.cc b/sql/wsrep_binlog.cc index b71da70..3788a6c 100644 --- a/sql/wsrep_binlog.cc +++ b/sql/wsrep_binlog.cc @@ -71,7 +71,6 @@ int wsrep_write_cache_buf(IO_CACHE *cache, uchar **buf, size_t *buf_len)
memcpy(*buf + *buf_len, cache->read_pos, length); *buf_len = total_length; - cache->read_pos = cache->read_end; } while ((cache->file >= 0) && (length = my_b_fill(cache)));
if (reinit_io_cache(cache, WRITE_CACHE, saved_pos, 0, 0)) @@ -200,7 +199,6 @@ static int wsrep_write_cache_once(wsrep_t* const wsrep,
memcpy(buf + used, cache->read_pos, length); used = total_length; - cache->read_pos = cache->read_end; } while ((cache->file >= 0) && (length = my_b_fill(cache)));
if (used > 0) @@ -270,7 +268,6 @@ static int wsrep_write_cache_inc(wsrep_t* const wsrep, cache->read_pos, length))) goto cleanup;
- cache->read_pos = cache->read_end; } while ((cache->file >= 0) && (length = my_b_fill(cache)));
if (WSREP_OK == err) *len = total_length; @@ -397,7 +394,6 @@ void wsrep_dump_rbr_direct(THD* thd, IO_CACHE* cache) WSREP_ERROR("Failed to write file '%s'", filename); goto cleanup; } - cache->read_pos= cache->read_end; } while ((cache->file >= 0) && (bytes_in_cache= my_b_fill(cache))); if(cache->error == -1) { _______________________________________________ commits mailing list commits@mariadb.org https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits
Hi, Sergey! On Jun 02, Sergey Vojtovich wrote:
Hi Sergei,
ok to push. Should we always prefer "static inline" over macroses?
I generally prefer static inline functions to macros and only use macros when a function cannot do the job (which is still pretty often :). Back in the last millenium C compilers didn't always support inline keyword, so macros were the only portable way of doing these my_b_* things, but now static inline pretty much works everywhere. Anyway, the main goal of this patch was to remove my_b_fill function (that used mysql_file_seek and mysql_file_read, accessing IO_CACHE::file directly) an replace it with a small two-statement inline function. Regards, Sergei
participants (2)
-
Sergei Golubchik
-
Sergey Vojtovich