From 884d22f28884e1decced1dee9c69ddcb25002ed1 Mon Sep 17 00:00:00 2001 From: Eugene Kosov Date: Wed, 18 Mar 2020 14:57:01 +0300 Subject: [PATCH] remove fishy reinterpret_cast from buf_page_is_zeroes() In my micro-benchmarks memcmp(4196) 3 times faster than old implementation. Also, it's generally better to use as less reinterpret_casts<> as possible. buf_is_zeroes(): renamed from buf_page_is_zeroes() and argument changed to span<> for convenience. st_::span::const_iterator: fixed page_zip-verify_checksum(): make argument byte* instead of void* --- storage/innobase/buf/buf0buf.cc | 28 ++++++++++++++-------------- storage/innobase/buf/buf0dblwr.cc | 9 ++++++--- storage/innobase/ibuf/ibuf0ibuf.cc | 11 +++++++---- storage/innobase/include/buf0buf.h | 12 ++++++------ storage/innobase/include/page0zip.h | 2 +- storage/innobase/include/span.h | 4 ++-- storage/innobase/page/page0zip.cc | 16 +++++++--------- 7 files changed, 43 insertions(+), 39 deletions(-) diff --git a/storage/innobase/buf/buf0buf.cc b/storage/innobase/buf/buf0buf.cc index 57dc54f38b4..6b8784adb89 100644 --- a/storage/innobase/buf/buf0buf.cc +++ b/storage/innobase/buf/buf0buf.cc @@ -83,6 +83,8 @@ Created 11/5/1995 Heikki Tuuri #include "lzo/lzo1x.h" #endif +using st_::span; + #ifdef HAVE_LIBNUMA #include #include @@ -461,7 +463,7 @@ buf_pool_register_chunk( @return true if temporary tablespace decrypted, false if not */ static bool buf_tmp_page_decrypt(byte* tmp_frame, byte* src_frame) { - if (buf_page_is_zeroes(src_frame, srv_page_size)) { + if (buf_is_zeroes(span(src_frame, srv_page_size))) { return true; } @@ -950,20 +952,18 @@ static uint32_t buf_page_check_crc32(const byte* page, uint32_t checksum) # define buf_page_check_crc32(page, checksum) buf_calc_page_crc32(page) #endif /* INNODB_BUG_ENDIAN_CRC32 */ -/** Check if a page is all zeroes. -@param[in] read_buf database page -@param[in] page_size page frame size -@return whether the page is all zeroes */ -bool buf_page_is_zeroes(const void* read_buf, size_t page_size) + +/** Check if a buffer is all zeroes. +@param[in] buf data to check +@return whether the buffer is all zeroes */ +bool buf_is_zeroes(span buf) { - const ulint* b = reinterpret_cast(read_buf); - const ulint* const e = b + page_size / sizeof *b; - do { - if (*b++) { - return false; - } - } while (b != e); - return true; + static const byte zeroes[4 * 1024] = {0}; + for (size_t i = 0; i < buf.size(); i += sizeof(zeroes)) { + if (memcmp(zeroes, buf.data() + i, sizeof(zeroes)) != 0) + return false; + } + return true; } /** Check if a page is corrupt. diff --git a/storage/innobase/buf/buf0dblwr.cc b/storage/innobase/buf/buf0dblwr.cc index d44bfbf2b9e..7fb4cf9a9d3 100644 --- a/storage/innobase/buf/buf0dblwr.cc +++ b/storage/innobase/buf/buf0dblwr.cc @@ -34,6 +34,8 @@ Created 2011/12/19 #include "fil0crypt.h" #include "fil0pagecompress.h" +using st_::span; + /** The doublewrite buffer */ buf_dblwr_t* buf_dblwr = NULL; @@ -581,7 +583,8 @@ buf_dblwr_process() } const page_size_t page_size(space->flags); - ut_ad(!buf_page_is_zeroes(page, page_size.physical())); + ut_ad(!buf_is_zeroes(span(page, + page_size.physical()))); /* We want to ensure that for partial reads the unread portion of the page is NUL. */ @@ -604,8 +607,8 @@ buf_dblwr_process() << "error: " << ut_strerr(err); } - const bool is_all_zero = buf_page_is_zeroes( - read_buf, page_size.physical()); + const bool is_all_zero = buf_is_zeroes( + span(read_buf, page_size.physical())); const bool expect_encrypted = space->crypt_data && space->crypt_data->type != CRYPT_SCHEME_UNENCRYPTED; diff --git a/storage/innobase/ibuf/ibuf0ibuf.cc b/storage/innobase/ibuf/ibuf0ibuf.cc index d71a37cdd0b..32f9b20a1d0 100644 --- a/storage/innobase/ibuf/ibuf0ibuf.cc +++ b/storage/innobase/ibuf/ibuf0ibuf.cc @@ -1,7 +1,7 @@ /***************************************************************************** Copyright (c) 1997, 2016, Oracle and/or its affiliates. All Rights Reserved. -Copyright (c) 2016, 2019, MariaDB Corporation. +Copyright (c) 2016, 2020 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 @@ -28,6 +28,8 @@ Created 7/19/1997 Heikki Tuuri #include "sync0sync.h" #include "btr0sea.h" +using st_::span; + #if defined UNIV_DEBUG || defined UNIV_IBUF_DEBUG my_bool srv_ibuf_disable_background_merge; #endif /* UNIV_DEBUG || UNIV_IBUF_DEBUG */ @@ -4970,7 +4972,8 @@ ibuf_check_bitmap_on_import( bitmap_page = ibuf_bitmap_get_map_page( page_id_t(space_id, page_no), page_size, &mtr); - if (buf_page_is_zeroes(bitmap_page, page_size.physical())) { + if (buf_is_zeroes(span(bitmap_page, + page_size.physical()))) { /* This means we got all-zero page instead of ibuf bitmap page. The subsequent page should be all-zero pages. */ @@ -4983,8 +4986,8 @@ ibuf_check_bitmap_on_import( page_size, RW_S_LATCH, &mtr); page_t* page = buf_block_get_frame(block); - ut_ad(buf_page_is_zeroes( - page, page_size.physical())); + ut_ad(buf_is_zeroes(span( + page, page_size.physical()))); } #endif /* UNIV_DEBUG */ ibuf_exit(&mtr); diff --git a/storage/innobase/include/buf0buf.h b/storage/innobase/include/buf0buf.h index a061d8e18f8..a04936a19cf 100644 --- a/storage/innobase/include/buf0buf.h +++ b/storage/innobase/include/buf0buf.h @@ -1,7 +1,7 @@ /***************************************************************************** Copyright (c) 1995, 2016, Oracle and/or its affiliates. All Rights Reserved. -Copyright (c) 2013, 2019, MariaDB Corporation. +Copyright (c) 2013, 2020 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 @@ -33,6 +33,7 @@ Created 11/5/1995 Heikki Tuuri #include "fil0fil.h" #include "mtr0types.h" #include "buf0types.h" +#include "span.h" #ifndef UNIV_INNOCHECKSUM #include "hash0hash.h" #include "ut0byte.h" @@ -646,11 +647,10 @@ buf_block_unfix(buf_block_t* block); # endif /* UNIV_DEBUG */ #endif /* !UNIV_INNOCHECKSUM */ -/** Check if a page is all zeroes. -@param[in] read_buf database page -@param[in] page_size page frame size -@return whether the page is all zeroes */ -bool buf_page_is_zeroes(const void* read_buf, size_t page_size); +/** Check if a buffer is all zeroes. +@param[in] buf data to check +@return whether the buffer is all zeroes */ +bool buf_is_zeroes(st_::span buf); /** Checks if the page is in crc32 checksum format. @param[in] read_buf database page diff --git a/storage/innobase/include/page0zip.h b/storage/innobase/include/page0zip.h index 63674748463..01b51ea2a0b 100644 --- a/storage/innobase/include/page0zip.h +++ b/storage/innobase/include/page0zip.h @@ -507,7 +507,7 @@ page_zip_calc_checksum( @param data ROW_FORMAT=COMPRESSED page @param size size of the page, in bytes @return whether the stored checksum matches innodb_checksum_algorithm */ -bool page_zip_verify_checksum(const void *data, size_t size); +bool page_zip_verify_checksum(const byte *data, size_t size); #ifndef UNIV_INNOCHECKSUM /**********************************************************************//** diff --git a/storage/innobase/include/span.h b/storage/innobase/include/span.h index faeb41029b8..62bcd80acd5 100644 --- a/storage/innobase/include/span.h +++ b/storage/innobase/include/span.h @@ -1,6 +1,6 @@ /***************************************************************************** -Copyright (c) 2019, MariaDB Corporation. +Copyright (c) 2019, 2020 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 @@ -34,7 +34,7 @@ public: typedef element_type& reference; typedef const element_type& const_reference; typedef pointer iterator; - typedef const pointer const_iterator; + typedef const_pointer const_iterator; typedef std::reverse_iterator reverse_iterator; typedef std::reverse_iterator const_reverse_iterator; diff --git a/storage/innobase/page/page0zip.cc b/storage/innobase/page/page0zip.cc index 6a86ad6be23..a64be931584 100644 --- a/storage/innobase/page/page0zip.cc +++ b/storage/innobase/page/page0zip.cc @@ -27,6 +27,9 @@ Created June 2005 by Marko Makela #include "page0size.h" #include "page0zip.h" +#include "span.h" + +using st_::span; /** A BLOB field reference full of zero, for use in assertions and tests. Initially, BLOB field references are set to zero, in @@ -4990,7 +4993,7 @@ page_zip_calc_checksum( @param data ROW_FORMAT=COMPRESSED page @param size size of the page, in bytes @return whether the stored checksum matches innodb_checksum_algorithm */ -bool page_zip_verify_checksum(const void *data, size_t size) +bool page_zip_verify_checksum(const byte *data, size_t size) { const srv_checksum_algorithm_t curr_algo = static_cast(srv_checksum_algorithm); @@ -4999,17 +5002,12 @@ bool page_zip_verify_checksum(const void *data, size_t size) return true; } - for (size_t i = 0; i < size; i++) { - if (static_cast(data)[i] != 0) { - goto not_all_zeroes; - } + if (buf_is_zeroes(span(data, size))) { + return true; } - return true; - -not_all_zeroes: const uint32_t stored = mach_read_from_4( - static_cast(data) + FIL_PAGE_SPACE_OR_CHKSUM); + data + FIL_PAGE_SPACE_OR_CHKSUM); uint32_t calc = page_zip_calc_checksum(data, size, curr_algo);