Skip to content

Commit 884d22f

Browse files
committed
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<T>::const_iterator: fixed page_zip-verify_checksum(): make argument byte* instead of void*
1 parent 2bde065 commit 884d22f

File tree

7 files changed

+43
-39
lines changed

7 files changed

+43
-39
lines changed

storage/innobase/buf/buf0buf.cc

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,8 @@ Created 11/5/1995 Heikki Tuuri
8383
#include "lzo/lzo1x.h"
8484
#endif
8585

86+
using st_::span;
87+
8688
#ifdef HAVE_LIBNUMA
8789
#include <numa.h>
8890
#include <numaif.h>
@@ -461,7 +463,7 @@ buf_pool_register_chunk(
461463
@return true if temporary tablespace decrypted, false if not */
462464
static bool buf_tmp_page_decrypt(byte* tmp_frame, byte* src_frame)
463465
{
464-
if (buf_page_is_zeroes(src_frame, srv_page_size)) {
466+
if (buf_is_zeroes(span<const byte>(src_frame, srv_page_size))) {
465467
return true;
466468
}
467469

@@ -950,20 +952,18 @@ static uint32_t buf_page_check_crc32(const byte* page, uint32_t checksum)
950952
# define buf_page_check_crc32(page, checksum) buf_calc_page_crc32(page)
951953
#endif /* INNODB_BUG_ENDIAN_CRC32 */
952954

953-
/** Check if a page is all zeroes.
954-
@param[in] read_buf database page
955-
@param[in] page_size page frame size
956-
@return whether the page is all zeroes */
957-
bool buf_page_is_zeroes(const void* read_buf, size_t page_size)
955+
956+
/** Check if a buffer is all zeroes.
957+
@param[in] buf data to check
958+
@return whether the buffer is all zeroes */
959+
bool buf_is_zeroes(span<const byte> buf)
958960
{
959-
const ulint* b = reinterpret_cast<const ulint*>(read_buf);
960-
const ulint* const e = b + page_size / sizeof *b;
961-
do {
962-
if (*b++) {
963-
return false;
964-
}
965-
} while (b != e);
966-
return true;
961+
static const byte zeroes[4 * 1024] = {0};
962+
for (size_t i = 0; i < buf.size(); i += sizeof(zeroes)) {
963+
if (memcmp(zeroes, buf.data() + i, sizeof(zeroes)) != 0)
964+
return false;
965+
}
966+
return true;
967967
}
968968

969969
/** Check if a page is corrupt.

storage/innobase/buf/buf0dblwr.cc

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,8 @@ Created 2011/12/19
3434
#include "fil0crypt.h"
3535
#include "fil0pagecompress.h"
3636

37+
using st_::span;
38+
3739
/** The doublewrite buffer */
3840
buf_dblwr_t* buf_dblwr = NULL;
3941

@@ -581,7 +583,8 @@ buf_dblwr_process()
581583
}
582584

583585
const page_size_t page_size(space->flags);
584-
ut_ad(!buf_page_is_zeroes(page, page_size.physical()));
586+
ut_ad(!buf_is_zeroes(span<const byte>(page,
587+
page_size.physical())));
585588

586589
/* We want to ensure that for partial reads the
587590
unread portion of the page is NUL. */
@@ -604,8 +607,8 @@ buf_dblwr_process()
604607
<< "error: " << ut_strerr(err);
605608
}
606609

607-
const bool is_all_zero = buf_page_is_zeroes(
608-
read_buf, page_size.physical());
610+
const bool is_all_zero = buf_is_zeroes(
611+
span<const byte>(read_buf, page_size.physical()));
609612
const bool expect_encrypted = space->crypt_data
610613
&& space->crypt_data->type != CRYPT_SCHEME_UNENCRYPTED;
611614

storage/innobase/ibuf/ibuf0ibuf.cc

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
/*****************************************************************************
22
33
Copyright (c) 1997, 2016, Oracle and/or its affiliates. All Rights Reserved.
4-
Copyright (c) 2016, 2019, MariaDB Corporation.
4+
Copyright (c) 2016, 2020 MariaDB Corporation.
55
66
This program is free software; you can redistribute it and/or modify it under
77
the terms of the GNU General Public License as published by the Free Software
@@ -28,6 +28,8 @@ Created 7/19/1997 Heikki Tuuri
2828
#include "sync0sync.h"
2929
#include "btr0sea.h"
3030

31+
using st_::span;
32+
3133
#if defined UNIV_DEBUG || defined UNIV_IBUF_DEBUG
3234
my_bool srv_ibuf_disable_background_merge;
3335
#endif /* UNIV_DEBUG || UNIV_IBUF_DEBUG */
@@ -4970,7 +4972,8 @@ ibuf_check_bitmap_on_import(
49704972
bitmap_page = ibuf_bitmap_get_map_page(
49714973
page_id_t(space_id, page_no), page_size, &mtr);
49724974

4973-
if (buf_page_is_zeroes(bitmap_page, page_size.physical())) {
4975+
if (buf_is_zeroes(span<const byte>(bitmap_page,
4976+
page_size.physical()))) {
49744977
/* This means we got all-zero page instead of
49754978
ibuf bitmap page. The subsequent page should be
49764979
all-zero pages. */
@@ -4983,8 +4986,8 @@ ibuf_check_bitmap_on_import(
49834986
page_size,
49844987
RW_S_LATCH, &mtr);
49854988
page_t* page = buf_block_get_frame(block);
4986-
ut_ad(buf_page_is_zeroes(
4987-
page, page_size.physical()));
4989+
ut_ad(buf_is_zeroes(span<const byte>(
4990+
page, page_size.physical())));
49884991
}
49894992
#endif /* UNIV_DEBUG */
49904993
ibuf_exit(&mtr);

storage/innobase/include/buf0buf.h

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
/*****************************************************************************
22
33
Copyright (c) 1995, 2016, Oracle and/or its affiliates. All Rights Reserved.
4-
Copyright (c) 2013, 2019, MariaDB Corporation.
4+
Copyright (c) 2013, 2020 MariaDB Corporation.
55
66
This program is free software; you can redistribute it and/or modify it under
77
the terms of the GNU General Public License as published by the Free Software
@@ -33,6 +33,7 @@ Created 11/5/1995 Heikki Tuuri
3333
#include "fil0fil.h"
3434
#include "mtr0types.h"
3535
#include "buf0types.h"
36+
#include "span.h"
3637
#ifndef UNIV_INNOCHECKSUM
3738
#include "hash0hash.h"
3839
#include "ut0byte.h"
@@ -646,11 +647,10 @@ buf_block_unfix(buf_block_t* block);
646647
# endif /* UNIV_DEBUG */
647648
#endif /* !UNIV_INNOCHECKSUM */
648649

649-
/** Check if a page is all zeroes.
650-
@param[in] read_buf database page
651-
@param[in] page_size page frame size
652-
@return whether the page is all zeroes */
653-
bool buf_page_is_zeroes(const void* read_buf, size_t page_size);
650+
/** Check if a buffer is all zeroes.
651+
@param[in] buf data to check
652+
@return whether the buffer is all zeroes */
653+
bool buf_is_zeroes(st_::span<const byte> buf);
654654

655655
/** Checks if the page is in crc32 checksum format.
656656
@param[in] read_buf database page

storage/innobase/include/page0zip.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -507,7 +507,7 @@ page_zip_calc_checksum(
507507
@param data ROW_FORMAT=COMPRESSED page
508508
@param size size of the page, in bytes
509509
@return whether the stored checksum matches innodb_checksum_algorithm */
510-
bool page_zip_verify_checksum(const void *data, size_t size);
510+
bool page_zip_verify_checksum(const byte *data, size_t size);
511511

512512
#ifndef UNIV_INNOCHECKSUM
513513
/**********************************************************************//**

storage/innobase/include/span.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
/*****************************************************************************
22
3-
Copyright (c) 2019, MariaDB Corporation.
3+
Copyright (c) 2019, 2020 MariaDB Corporation.
44
55
This program is free software; you can redistribute it and/or modify it under
66
the terms of the GNU General Public License as published by the Free Software
@@ -34,7 +34,7 @@ template <class ElementType> class span {
3434
typedef element_type& reference;
3535
typedef const element_type& const_reference;
3636
typedef pointer iterator;
37-
typedef const pointer const_iterator;
37+
typedef const_pointer const_iterator;
3838
typedef std::reverse_iterator<iterator> reverse_iterator;
3939
typedef std::reverse_iterator<const_iterator> const_reverse_iterator;
4040

storage/innobase/page/page0zip.cc

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,9 @@ Created June 2005 by Marko Makela
2727

2828
#include "page0size.h"
2929
#include "page0zip.h"
30+
#include "span.h"
31+
32+
using st_::span;
3033

3134
/** A BLOB field reference full of zero, for use in assertions and tests.
3235
Initially, BLOB field references are set to zero, in
@@ -4990,7 +4993,7 @@ page_zip_calc_checksum(
49904993
@param data ROW_FORMAT=COMPRESSED page
49914994
@param size size of the page, in bytes
49924995
@return whether the stored checksum matches innodb_checksum_algorithm */
4993-
bool page_zip_verify_checksum(const void *data, size_t size)
4996+
bool page_zip_verify_checksum(const byte *data, size_t size)
49944997
{
49954998
const srv_checksum_algorithm_t curr_algo =
49964999
static_cast<srv_checksum_algorithm_t>(srv_checksum_algorithm);
@@ -4999,17 +5002,12 @@ bool page_zip_verify_checksum(const void *data, size_t size)
49995002
return true;
50005003
}
50015004

5002-
for (size_t i = 0; i < size; i++) {
5003-
if (static_cast<const byte*>(data)[i] != 0) {
5004-
goto not_all_zeroes;
5005-
}
5005+
if (buf_is_zeroes(span<const byte>(data, size))) {
5006+
return true;
50065007
}
50075008

5008-
return true;
5009-
5010-
not_all_zeroes:
50115009
const uint32_t stored = mach_read_from_4(
5012-
static_cast<const byte*>(data) + FIL_PAGE_SPACE_OR_CHKSUM);
5010+
data + FIL_PAGE_SPACE_OR_CHKSUM);
50135011

50145012
uint32_t calc = page_zip_calc_checksum(data, size, curr_algo);
50155013

0 commit comments

Comments
 (0)