Skip to content

Commit

Permalink
Bug #26927386: REDUCE COMPILATION TIME [noclose]
Browse files Browse the repository at this point in the history
Clean up some obsolete code from m_string.h, allowing us to remove
some unneeded #includes.

Reduces the build time by a second or two, and increases incrementality.

Change-Id: I23ce15c7417f55e5fb9193a769550f26e01e0e96
  • Loading branch information
Steinar H. Gunderson committed Oct 20, 2017
1 parent 928ab88 commit 1e60543
Show file tree
Hide file tree
Showing 8 changed files with 64 additions and 150 deletions.
1 change: 1 addition & 0 deletions components/logging/log_sink_syseventlog.cc
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA */
#include "log_service_imp.h"
#include "m_string.h" // native_strncasecmp()/native_strcasecmp()
#include "my_compiler.h"
#include "my_io.h"
#include "my_sys.h"
#include "mysqld_error.h" // so we can throw ER_LOG_SYSLOG_*
#ifndef _WIN32
Expand Down
2 changes: 2 additions & 0 deletions extra/regex/utils.h
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
#include <limits.h>

/* utility definitions */
#ifdef _POSIX2_RE_DUP_MAX
#define DUPMAX _POSIX2_RE_DUP_MAX /* xxx is this right? */
Expand Down
89 changes: 8 additions & 81 deletions include/m_string.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,23 +22,15 @@
*/

#include <float.h>
#include <mysql/mysql_lex_string.h>
#include <stdlib.h>
#include <string.h>
#include <stdbool.h> // IWYU pragma: keep
#include <stdint.h>
#include <stdlib.h>

#include "lex_string.h"
#include "my_byteorder.h" /* uint8korr */
#include "my_config.h"
#include "my_dbug.h"
#include "my_inttypes.h"
#include "my_macros.h"
#include "mysql_com.h"

#define bfill please_use_memset_rather_than_bfill
#define bzero please_use_memset_rather_than_bzero
#define bmove please_use_memmove_rather_than_bmove
#define strmov please_use_my_stpcpy_or_my_stpmov_rather_than_strmov
#define strnmov please_use_my_stpncpy_or_my_stpnmov_rather_than_strnmov

/**
Definition of the null string (a null pointer of type char *),
Expand Down Expand Up @@ -342,90 +334,25 @@ static inline char *ullstr(longlong value, char *buff)
#define C_STRING_WITH_LEN(X) ((char *) (X)), ((sizeof(X) - 1))

/**
Skip trailing space.
On most systems reading memory in larger chunks (ideally equal to the size of
the chinks that the machine physically reads from memory) causes fewer memory
access loops and hence increased performance.
This is why the 'int' type is used : it's closest to that (according to how
it's defined in C).
So when we determine the amount of whitespace at the end of a string we do
the following :
1. We divide the string into 3 zones :
a) from the start of the string (__start) to the first multiple
of sizeof(int) (__start_words)
b) from the end of the string (__end) to the last multiple of sizeof(int)
(__end_words)
c) a zone that is aligned to sizeof(int) and can be safely accessed
through an int *
2. We start comparing backwards from (c) char-by-char. If all we find is
space then we continue
3. If there are elements in zone (b) we compare them as unsigned ints to a
int mask (SPACE_INT) consisting of all spaces
4. Finally we compare the remaining part (a) of the string char by char.
This covers for the last non-space unsigned int from 3. (if any)
This algorithm works well for relatively larger strings, but it will slow
the things down for smaller strings (because of the additional calculations
and checks compared to the naive method). Thus the barrier of length 20
is added.
@param ptr pointer to the input string
@param len the length of the string
@return the last non-space character
*/
#if defined(__sparc) || defined(__sparcv9)
static inline const uchar *skip_trailing_space(const uchar *ptr,size_t len)
{
/* SPACE_INT is a word that contains only spaces */
#if SIZEOF_INT == 4
const unsigned SPACE_INT= 0x20202020U;
#elif SIZEOF_INT == 8
const unsigned SPACE_INT= 0x2020202020202020ULL;
#else
#error define the appropriate constant for a word full of spaces
#endif

const uchar *end= ptr + len;
Skip trailing space (ASCII spaces only).
if (len > 20)
{
const uchar *end_words= (const uchar *)(intptr)
(((ulonglong)(intptr)end) / SIZEOF_INT * SIZEOF_INT);
const uchar *start_words= (const uchar *)(intptr)
((((ulonglong)(intptr)ptr) + SIZEOF_INT - 1) / SIZEOF_INT * SIZEOF_INT);

DBUG_ASSERT(end_words > ptr);
while (end > end_words && end[-1] == 0x20)
end--;
if (end[-1] == 0x20 && start_words < end_words)
while (end > start_words && ((unsigned *)end)[-1] == SPACE_INT)
end -= SIZEOF_INT;
}
while (end > ptr && end[-1] == 0x20)
end--;
return (end);
}
#else
/*
Reads 8 bytes at a time, ignoring alignment.
We use uint8korr, which is fast (it simply reads a *ulonglong)
on all platforms, except sparc.
@return New end of the string.
*/
static inline const uchar *skip_trailing_space(const uchar *ptr, size_t len)
{
const uchar *end= ptr + len;
while (end - ptr >= 8)
{
if (uint8korr(end-8) != 0x2020202020202020ULL)
uint64_t chunk;
memcpy(&chunk, end - 8, sizeof(chunk));
if (chunk != 0x2020202020202020ULL)
break;
end-= 8;
}
while (end > ptr && end[-1] == 0x20)
end--;
return (end);
}
#endif

static inline void lex_string_set(LEX_STRING *lex_str, const char *c_str)
{
Expand Down
2 changes: 2 additions & 0 deletions rapid/unittest/gunit/xplugin/xpl/stubs/misc.cc
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@
#include "my_inttypes.h"
#include "plugin/x/src/xpl_performance_schema.h"
#include "sql/replication.h"
#include "violite.h"

#ifdef HAVE_ARPA_INET_H
#include <arpa/inet.h>
#endif
Expand Down
2 changes: 2 additions & 0 deletions storage/innobase/include/mem0mem.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ Created 6/9/1994 Heikki Tuuri
#include "ut0rnd.h"
#include "mach0data.h"

#include <limits.h>

#include <memory>

/* -------------------- MEMORY HEAPS ----------------------------- */
Expand Down
2 changes: 2 additions & 0 deletions storage/innobase/include/os0file.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ Created 10/21/1995 Heikki Tuuri
#ifndef os0file_h
#define os0file_h

#include "my_dbug.h"
#include "my_io.h"
#include "univ.i"
#include "os/file.h"

Expand Down
2 changes: 2 additions & 0 deletions strings/uca9-dump.cc
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@
use iconv to convert.
3. ucadump ja < /path/to/ja_han.txt > /path/to/yourfile
*/

#include <assert.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
Expand Down
114 changes: 45 additions & 69 deletions unittest/gunit/strings_skip_trailing-t.cc
Original file line number Diff line number Diff line change
Expand Up @@ -18,88 +18,64 @@
Bug#16395778 SUBOPTIMAL CODE IN SKIP_TRAILING_SPACE()
Below we test some alternative implementations for skip_trailing_space.
In order to do benchmarking, configure in optimized mode, and
generate a separate executable for this file:
cmake -DMERGE_UNITTESTS=0
You may want to tweak some constants below:
- experiment with num_iterations
- experiment with inserting something in front of the whitespace
- experiment with different test_values
run 'strings-t --disable-tap-output' to see timing reports for your platform.
*/

// First include (the generated) my_config.h, to get correct platform defines.
#include "my_config.h"

#include <gtest/gtest.h>
#include <string>

#include "m_string.h"
#include "unittest/gunit/benchmark.h"
#include "unittest/gunit/skip_trailing.h"

namespace skip_trailing_space_unittest {

#if defined(GTEST_HAS_PARAM_TEST)

#if !defined(DBUG_OFF)
// There is no point in benchmarking anything in debug mode.
const size_t num_iterations= 1ULL;
#else
// Set this so that each test case takes a few seconds.
// And set it back to a small value before pushing!!
// const size_t num_iterations= 200000000ULL;
const size_t num_iterations= 2ULL;
#endif

class SkipTrailingSpaceTest : public ::testing::TestWithParam<int>
static inline void benchmark_func(size_t iters,
const uchar *func(const uchar *, size_t),
size_t length)
{
protected:
virtual void SetUp()
StopBenchmarkTiming();
// Insert something else (or nothing) here,
// to see effects of alignment of data:
std::string str= "1";
str.append(length, ' ');
StartBenchmarkTiming();

for (size_t i = 0; i < iters; ++i)
{
int num_spaces= GetParam();
// Insert something else (or nothing) here,
// to see effects of alignment of data:
m_string.append("1");
for (int ix= 0 ; ix < num_spaces; ++ix)
m_string.append(" ");
m_length= m_string.length();
func(pointer_cast<const uchar *>(str.data()), length);
}
size_t m_length;
std::string m_string;
};

int test_values[]= {0, 24, 100, 150};

INSTANTIATE_TEST_CASE_P(Skip, SkipTrailingSpaceTest,
::testing::ValuesIn(test_values));

TEST_P(SkipTrailingSpaceTest, Unaligned)
{
for (size_t ix= 0; ix < num_iterations; ++ix)
skip_trailing_unalgn(reinterpret_cast<const uchar*>(m_string.c_str()),
m_length);
}

TEST_P(SkipTrailingSpaceTest, Original)
{
for (size_t ix= 0; ix < num_iterations; ++ix)
skip_trailing_orig(reinterpret_cast<const uchar*>(m_string.c_str()),
m_length);
}

TEST_P(SkipTrailingSpaceTest, FourByte)
{
for (size_t ix= 0; ix < num_iterations; ++ix)
skip_trailing_4byte(reinterpret_cast<const uchar*>(m_string.c_str()),
m_length);
}

TEST_P(SkipTrailingSpaceTest, EightByte)
{
for (size_t ix= 0; ix < num_iterations; ++ix)
skip_trailing_8byte(reinterpret_cast<const uchar*>(m_string.c_str()),
m_length);
}

#endif // GTEST_HAS_PARAM_TEST
#define INSTANTIATE_TEST(name, func, length) \
static void name(size_t iters) \
{ \
benchmark_func(iters, func, length); \
} \
BENCHMARK(name);

INSTANTIATE_TEST(BM_SkipTrailingSpaceTest_Unaligned_0, skip_trailing_unalgn, 0);
INSTANTIATE_TEST(BM_SkipTrailingSpaceTest_Unaligned_24, skip_trailing_unalgn, 24);
INSTANTIATE_TEST(BM_SkipTrailingSpaceTest_Unaligned_100, skip_trailing_unalgn, 100);
INSTANTIATE_TEST(BM_SkipTrailingSpaceTest_Unaligned_150, skip_trailing_unalgn, 150);

INSTANTIATE_TEST(BM_SkipTrailingSpaceTest_Original_0, skip_trailing_orig, 0);
INSTANTIATE_TEST(BM_SkipTrailingSpaceTest_Original_24, skip_trailing_orig, 24);
INSTANTIATE_TEST(BM_SkipTrailingSpaceTest_Original_100, skip_trailing_orig, 100);
INSTANTIATE_TEST(BM_SkipTrailingSpaceTest_Original_150, skip_trailing_orig, 150);

INSTANTIATE_TEST(BM_SkipTrailingSpaceTest_FourByte_0, skip_trailing_4byte, 0);
INSTANTIATE_TEST(BM_SkipTrailingSpaceTest_FourByte_24, skip_trailing_4byte, 24);
INSTANTIATE_TEST(BM_SkipTrailingSpaceTest_FourByte_100, skip_trailing_4byte, 100);
INSTANTIATE_TEST(BM_SkipTrailingSpaceTest_FourByte_150, skip_trailing_4byte, 150);

INSTANTIATE_TEST(BM_SkipTrailingSpaceTest_EightByte_0, skip_trailing_8byte, 0);
INSTANTIATE_TEST(BM_SkipTrailingSpaceTest_EightByte_24, skip_trailing_8byte, 24);
INSTANTIATE_TEST(BM_SkipTrailingSpaceTest_EightByte_100, skip_trailing_8byte, 100);
INSTANTIATE_TEST(BM_SkipTrailingSpaceTest_EightByte_150, skip_trailing_8byte, 150);

INSTANTIATE_TEST(BM_SkipTrailingSpaceTest_Current_0, skip_trailing_space, 0);
INSTANTIATE_TEST(BM_SkipTrailingSpaceTest_Current_24, skip_trailing_space, 24);
INSTANTIATE_TEST(BM_SkipTrailingSpaceTest_Current_100, skip_trailing_space, 100);
INSTANTIATE_TEST(BM_SkipTrailingSpaceTest_Current_150, skip_trailing_space, 150);

}

0 comments on commit 1e60543

Please sign in to comment.