Re: [Maria-developers] [Commits] f380e7b5535: MDEV-12072 Do not unnecessarily construct rec_printer objects in debug builds

ok to push. On Wed, Feb 15, 2017 at 4:02 PM, <marko.makela@mariadb.com> wrote:

revision-id: f380e7b5535358bd3d7caf81d22579ef97f6ca24 (mariadb-10.2.3-271- gf380e7b5535) parent(s): 24911cee4e3be988848f6eab4d768849709f5256 author: Marko Mäkelä committer: Marko Mäkelä timestamp: 2017-02-15 16:02:00 +0200 message:

MDEV-12072 Do not unnecessarily construct rec_printer objects in debug builds

I introduced the rec_printer object in MySQL to pretty-print raw InnoDB records and index tuples in diagnostic messages. These objects are being constructed unconditionally, even though the DBUG_PRINT is not enabled.

The unnecessary work would be avoided by simply passing rec_printer(…).str() to the DBUG_LOG that was introduced in MDEV-11713.

--- storage/innobase/btr/btr0cur.cc | 54 ++++++++---------------- storage/innobase/row/row0log.cc | 58 ++++++++------------------ storage/innobase/row/row0merge.cc | 88 +++++++++++++++--------------- --------- 3 files changed, 68 insertions(+), 132 deletions(-)

diff --git a/storage/innobase/btr/btr0cur.cc b/storage/innobase/btr/ btr0cur.cc index 9033c399ce2..bdf3bd2eca7 100644 --- a/storage/innobase/btr/btr0cur.cc +++ b/storage/innobase/btr/btr0cur.cc @@ -3012,18 +3012,10 @@ btr_cur_optimistic_insert(

page_cursor = btr_cur_get_page_cur(cursor);

-#ifdef UNIV_DEBUG - { - rec_printer p(entry); - DBUG_PRINT("ib_cur", ("insert %s (" IB_ID_FMT ") by " IB_ID_FMT " %s", - index->name(), index->id, - thr != NULL - ? trx_get_id_for_print(thr_get_trx(thr)) - : 0, - p.str().c_str())); - } -#endif - + DBUG_LOG("ib_cur", + "insert " << index->name << " (" << index->id << ") by " + << ib::hex(thr ? trx_get_id_for_print(thr_get_trx(thr)) : 0) + << ' ' << rec_printer(entry).str()); DBUG_EXECUTE_IF("do_page_reorganize", btr_page_reorganize(page_cursor, index, mtr););

@@ -3635,14 +3627,10 @@ btr_cur_update_in_place( ut_ad(fil_page_index_page_check(btr_cur_get_page(cursor))); ut_ad(btr_page_get_index_id(btr_cur_get_page(cursor)) == index->id);

-#ifdef UNIV_DEBUG - { - rec_printer p(rec, offsets); - DBUG_PRINT("ib_cur", ("update-in-place %s (" IB_ID_FMT ") by " IB_ID_FMT ": %s", - index->name(), index->id, trx_id, - p.str().c_str())); - } -#endif + DBUG_LOG("ib_cur", + "update-in-place " << index->name << " (" << index->id + << ") by " << ib::hex(trx_id) << ": " + << rec_printer(rec, offsets).str());

block = btr_cur_get_block(cursor); page_zip = buf_block_get_page_zip(block); @@ -3842,14 +3830,10 @@ btr_cur_optimistic_update( } }

-#ifdef UNIV_DEBUG - { - rec_printer p(rec, *offsets); - DBUG_PRINT("ib_cur", ("update %s (" IB_ID_FMT ") by " IB_ID_FMT ": %s", - index->name(), index->id, trx_id, - p.str().c_str())); - } -#endif + DBUG_LOG("ib_cur", + "update " << index->name << " (" << index->id << ") by " + << ib::hex(trx_id) << ": " + << rec_printer(rec, *offsets).str());

page_cursor = btr_cur_get_page_cur(cursor);

@@ -4669,15 +4653,11 @@ btr_cur_del_mark_set_clust_rec( ut_ad(trx_state_eq(trx, TRX_STATE_ACTIVE)); ut_ad(!trx->in_rollback);

-#ifdef UNIV_DEBUG - { - rec_printer p(rec, offsets); - DBUG_PRINT("ib_cur", ("delete-mark clust %s (" IB_ID_FMT ") by " IB_ID_FMT ": %s", - index->table_name, index->id, - trx_get_id_for_print(trx), - p.str().c_str())); - } -#endif + DBUG_LOG("ib_cur", + "delete-mark clust " << index->table->name + << " (" << index->id << ") by " + << ib::hex(trx_get_id_for_print(trx)) << ": " + << rec_printer(rec, offsets).str());

if (dict_index_is_online_ddl(index)) { row_log_table_delete(rec, entry, index, offsets, NULL); diff --git a/storage/innobase/row/row0log.cc b/storage/innobase/row/ row0log.cc index d43fb333cdc..0fc1dd097cc 100644 --- a/storage/innobase/row/row0log.cc +++ b/storage/innobase/row/row0log.cc @@ -1,6 +1,7 @@ /*********************************************************** ******************

Copyright (c) 2011, 2016, Oracle and/or its affiliates. All Rights Reserved. +Copyright (c) 2017, MariaDB Corporation.

This program is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free Software @@ -1641,15 +1642,9 @@ row_log_table_apply_insert_low( ut_ad(dtuple_validate(row)); ut_ad(trx_id);

-#ifdef UNIV_DEBUG - { - rec_printer p(row); - DBUG_PRINT("ib_alter_table", - ("insert table " IB_ID_FMT " (index " IB_ID_FMT "): %s", - index->table->id, index->id, - p.str().c_str())); - } -#endif + DBUG_LOG("ib_alter_table", + "insert table " << index->table->id << " (index " + << index->id << "): " << rec_printer(row).str());

static const ulint flags = (BTR_CREATE_FLAG @@ -1777,15 +1772,10 @@ row_log_table_apply_delete_low(

ut_ad(dict_index_is_clust(index));

-#ifdef UNIV_DEBUG - { - rec_printer p(btr_pcur_get_rec(pcur), offsets); - DBUG_PRINT("ib_alter_table", - ("delete table " IB_ID_FMT " (index " IB_ID_FMT "): %s", - index->table->id, index->id, - p.str().c_str())); - } -#endif + DBUG_LOG("ib_alter_table", + "delete table " << index->table->id << " (index " + << index->id << "): " + << rec_printer(btr_pcur_get_rec(pcur), offsets).str());

if (dict_table_get_next_index(index)) { /* Build a row template for purging secondary index entries. */ @@ -2266,17 +2256,11 @@ row_log_table_apply_update( cur_offsets, NULL, NULL, NULL, &old_ext, heap); ut_ad(old_row);

-#ifdef UNIV_DEBUG - { - rec_printer old(old_row); - rec_printer new_row(row); - DBUG_PRINT("ib_alter_table", - ("update table " IB_ID_FMT " (index " IB_ID_FMT "): %s to %s", - index->table->id, index->id, - old.str().c_str(), - new_row.str().c_str())); - } -#endif + DBUG_LOG("ib_alter_table", + "update table " << index->table->id + << " (index " << index->id + << ": " << rec_printer(old_row).str() + << " to " << rec_printer(row).str()); } else { old_row = NULL; old_ext = NULL; @@ -3298,17 +3282,11 @@ row_log_apply_op_low( ut_ad(!dict_index_is_corrupted(index)); ut_ad(trx_id != 0 || op == ROW_OP_DELETE);

-#ifdef UNIV_DEBUG - { - rec_printer p(entry); - DBUG_PRINT("ib_create_index", - ("%s %s index " IB_ID_FMT "," IB_ID_FMT ": %s", - op == ROW_OP_INSERT ? "insert" : "delete", - has_index_lock ? "locked" : "unlocked", - index->id, trx_id, - p.str().c_str())); - } -#endif + DBUG_LOG("ib_create_index", + (op == ROW_OP_INSERT ? "insert " : "delete ") + << (has_index_lock ? "locked index " : "unlocked index ") + << index->id << ',' << ib::hex(trx_id) << ": " + << rec_printer(entry).str());

mtr_start(&mtr); mtr.set_named_space(index->space); diff --git a/storage/innobase/row/row0merge.cc b/storage/innobase/row/ row0merge.cc index 57dd8f75e8e..85f1e0b6ecc 100644 --- a/storage/innobase/row/row0merge.cc +++ b/storage/innobase/row/row0merge.cc @@ -1,6 +1,7 @@ /*********************************************************** ******************

Copyright (c) 2005, 2016, Oracle and/or its affiliates. All Rights Reserved. +Copyright (c) 2017, MariaDB Corporation.

This program is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free Software @@ -1100,16 +1101,11 @@ row_merge_buf_write( row_merge_buf_encode(&b, index, entry, n_fields); ut_ad(b < &block[srv_sort_buf_size]);

-#ifdef UNIV_DEBUG - { - rec_printer p(entry->fields, n_fields); - DBUG_PRINT("ib_merge_sort", - ("%p,fd=%d,%lu %lu: %s", - reinterpret_cast<const void*>(b), of->fd, - (ulint)of->offset, (ulint)i, - p.str().c_str())); - } -#endif + DBUG_LOG("ib_merge_sort", + reinterpret_cast<const void*>(b) << ',' + << of->fd << ',' << of->offset << ' ' << + i << ": " << + rec_printer(entry->fields, n_fields).str()); }

/* Write an "end-of-chunk" marker. */ @@ -1121,10 +1117,9 @@ row_merge_buf_write( to avoid bogus warnings. */ memset(b, 0xff, &block[srv_sort_buf_size] - b); #endif /* UNIV_DEBUG_VALGRIND */ - DBUG_PRINT("ib_merge_sort", - ("write %p,%d,%lu EOF", - reinterpret_cast<const void*>(b), of->fd, - (ulint)of->offset)); + DBUG_LOG("ib_merge_sort", + "write " << b << ',' << of->fd << ',' << of->offset << + " EOF"); DBUG_VOID_RETURN; }

@@ -1177,7 +1172,7 @@ row_merge_read( os_offset_t ofs = ((os_offset_t) offset) * srv_sort_buf_size;

DBUG_ENTER("row_merge_read"); - DBUG_PRINT("ib_merge_sort", ("fd=%d ofs=" UINT64PF, fd, ofs)); + DBUG_LOG("ib_merge_sort", "fd=" << fd << " ofs=" << ofs); DBUG_EXECUTE_IF("row_merge_read_failure", DBUG_RETURN(FALSE););

IORequest request; @@ -1224,7 +1219,7 @@ row_merge_write( void* out_buf = (void *)buf;

DBUG_ENTER("row_merge_write"); - DBUG_PRINT("ib_merge_sort", ("fd=%d ofs=" UINT64PF, fd, ofs)); + DBUG_LOG("ib_merge_sort", "fd=" << fd << " ofs=" << ofs); DBUG_EXECUTE_IF("row_merge_write_failure", DBUG_RETURN(FALSE););

IORequest request(IORequest::WRITE); @@ -1296,11 +1291,10 @@ row_merge_read_rec( if (UNIV_UNLIKELY(!extra_size)) { /* End of list */ *mrec = NULL; - DBUG_PRINT("ib_merge_sort", - ("read %p,%p,%d,%lu EOF\n", - reinterpret_cast<const void*>(b), - reinterpret_cast<const void*>(block), - fd, ulint(*foffs))); + DBUG_LOG("ib_merge_sort", + "read " << reinterpret_cast<const void*>(b) << ',' << + reinterpret_cast<const void*>(block) << ',' << + fd << ',' << *foffs << " EOF"); DBUG_RETURN(NULL); }

@@ -1413,18 +1407,11 @@ row_merge_read_rec( b += extra_size + data_size - avail_size;

func_exit: - -#ifdef UNIV_DEBUG - { - rec_printer p(*mrec, 0, offsets); - DBUG_PRINT("ib_merge_sort", - ("%p,%p,fd=%d,%lu: %s", - reinterpret_cast<const void*>(b), - reinterpret_cast<const void*>(block), - fd, ulint(*foffs), - p.str().c_str())); - } -#endif + DBUG_LOG("ib_merge_sort", + reinterpret_cast<const void*>(b) << ',' << + reinterpret_cast<const void*>(block) + << ",fd=" << fd << ',' << *foffs << ": " + << rec_printer(*mrec, 0, offsets).str()); DBUG_RETURN(b); }

@@ -1455,15 +1442,9 @@ row_merge_write_rec_low( #endif /* DBUG_OFF */ DBUG_ASSERT(e == rec_offs_extra_size(offsets) + 1);

-#ifdef UNIV_DEBUG - { - rec_printer p(mrec, 0, offsets); - DBUG_PRINT("ib_merge_sort", - ("%p,fd=%d,%lu: %s", - reinterpret_cast<const void*>(b), fd, ulint(foffs), - p.str().c_str())); - } -#endif + DBUG_LOG("ib_merge_sort", + reinterpret_cast<const void*>(b) << ",fd=" << fd << ',' + << foffs << ": " << rec_printer(mrec, 0, offsets).str());

if (e < 0x80) { *b++ = (byte) e; @@ -1573,11 +1554,10 @@ row_merge_write_eof( ut_ad(foffs);

DBUG_ENTER("row_merge_write_eof"); - DBUG_PRINT("ib_merge_sort", - ("%p,%p,fd=%d,%lu", - reinterpret_cast<const void*>(b), - reinterpret_cast<const void*>(block), - fd, ulint(*foffs))); + DBUG_LOG("ib_merge_sort", + reinterpret_cast<const void*>(b) << ',' << + reinterpret_cast<const void*>(block) << + ",fd=" << fd << ',' << *foffs);

if (b == &block[0]) { b+= ROW_MERGE_RESERVE_SIZE; @@ -2937,10 +2917,9 @@ row_merge_blocks( ulint* offsets1;/* offsets of mrec1 */

DBUG_ENTER("row_merge_blocks"); - DBUG_PRINT("ib_merge_sort", - ("fd=%d,%lu+%lu to fd=%d,%lu", - file->fd, ulint(*foffs0), ulint(*foffs1), - of->fd, ulint(of->offset))); + DBUG_LOG("ib_merge_sort", + "fd=" << file->fd << ',' << *foffs0 << '+' << *foffs1 + << " to fd=" << of->fd << ',' << of->offset);

heap = row_merge_heap_create(dup->index, &buf, &offsets0, &offsets1);

@@ -3050,10 +3029,9 @@ row_merge_blocks_copy( ulint* offsets1;/* dummy offsets */

DBUG_ENTER("row_merge_blocks_copy"); - DBUG_PRINT("ib_merge_sort", - ("fd=%d,%lu to fd=%d,%lu", - file->fd, ulint(foffs0), - of->fd, ulint(of->offset))); + DBUG_LOG("ib_merge_sort", + "fd=" << file->fd << ',' << foffs0 + << " to fd=" << of->fd << ',' << of->offset);

heap = row_merge_heap_create(index, &buf, &offsets0, &offsets1);

_______________________________________________ commits mailing list commits@mariadb.org https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits