You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by mp...@apache.org on 2016/09/12 05:54:59 UTC
kudu git commit: util: avoid redundant copies in various macros
Repository: kudu
Updated Branches:
refs/heads/master 30099cf9e -> 708f38b2b
util: avoid redundant copies in various macros
A lot of macros have the form:
#define FOO(arg) \
Foo arg_ = (arg); \
... do something with arg_ ...
Of course the purpose of the 'arg_' local is to avoid double-evaluation
of the argument in the case that it's something expensive or something
with a side effect. However, this pattern results in an extra copy of
'arg' in the case that it's actually a local, eg in the pattern:
Foo f = ...;
FOO(f);
This expands out to a redundant copy 'Foo arg_ = f;'.
It turns out that the solution is to make 'Foo arg_' a const reference
'const Foo& arg_'. At first, this seems wrong: references to a temporary
are often bad. However, it turns out that C++ has a rule that const
references to temporaries on the stack keep that temporary alive as long as
the reference stays in scope. So, this is safe, and avoids an extra
copy.
Change-Id: I6057751d5c6790b2d350e77636b58079e2093cb7
Reviewed-on: http://gerrit.cloudera.org:8080/4379
Tested-by: Kudu Jenkins
Reviewed-by: Mike Percy <mp...@apache.org>
Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/708f38b2
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/708f38b2
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/708f38b2
Branch: refs/heads/master
Commit: 708f38b2b932ef88dda2f990ac95a8de1cd226cb
Parents: 30099cf
Author: Todd Lipcon <to...@apache.org>
Authored: Sun Sep 11 21:34:08 2016 -0700
Committer: Mike Percy <mp...@apache.org>
Committed: Mon Sep 12 05:54:51 2016 +0000
----------------------------------------------------------------------
src/kudu/util/status.h | 14 +++++++-------
src/kudu/util/test_macros.h | 12 ++++++------
2 files changed, 13 insertions(+), 13 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/kudu/blob/708f38b2/src/kudu/util/status.h
----------------------------------------------------------------------
diff --git a/src/kudu/util/status.h b/src/kudu/util/status.h
index 21f47e7..af02c73 100644
--- a/src/kudu/util/status.h
+++ b/src/kudu/util/status.h
@@ -28,14 +28,14 @@
/// @brief Return the given status if it is not @c OK.
#define KUDU_RETURN_NOT_OK(s) do { \
- ::kudu::Status _s = (s); \
+ const ::kudu::Status& _s = (s); \
if (PREDICT_FALSE(!_s.ok())) return _s; \
} while (0);
/// @brief Return the given status if it is not OK, but first clone it and
/// prepend the given message.
#define KUDU_RETURN_NOT_OK_PREPEND(s, msg) do { \
- ::kudu::Status _s = (s); \
+ const ::kudu::Status& _s = (s); \
if (PREDICT_FALSE(!_s.ok())) return _s.CloneAndPrepend(msg); \
} while (0);
@@ -43,13 +43,13 @@
/// The substitution for 'to_return' may reference the variable
/// @c s for the bad status.
#define KUDU_RETURN_NOT_OK_RET(to_call, to_return) do { \
- ::kudu::Status s = (to_call); \
+ const ::kudu::Status& s = (to_call); \
if (PREDICT_FALSE(!s.ok())) return (to_return); \
} while (0);
/// @brief Emit a warning if @c to_call returns a bad status.
#define KUDU_WARN_NOT_OK(to_call, warning_prefix) do { \
- ::kudu::Status _s = (to_call); \
+ const ::kudu::Status& _s = (to_call); \
if (PREDICT_FALSE(!_s.ok())) { \
KUDU_LOG(WARNING) << (warning_prefix) << ": " << _s.ToString(); \
} \
@@ -57,7 +57,7 @@
/// @brief Log the given status and return immediately.
#define KUDU_LOG_AND_RETURN(level, status) do { \
- ::kudu::Status _s = (status); \
+ const ::kudu::Status& _s = (status); \
KUDU_LOG(level) << _s.ToString(); \
return _s; \
} while (0);
@@ -65,8 +65,8 @@
/// @brief If @c to_call returns a bad status, CHECK immediately with
/// a logged message of @c msg followed by the status.
#define KUDU_CHECK_OK_PREPEND(to_call, msg) do { \
- ::kudu::Status _s = (to_call); \
- KUDU_CHECK(_s.ok()) << (msg) << ": " << _s.ToString(); \
+ const ::kudu::Status& _s = (to_call); \
+ KUDU_CHECK(_s.ok()) << (msg) << ": " << _s.ToString(); \
} while (0);
/// @brief If the status is bad, CHECK immediately, appending the status to the
http://git-wip-us.apache.org/repos/asf/kudu/blob/708f38b2/src/kudu/util/test_macros.h
----------------------------------------------------------------------
diff --git a/src/kudu/util/test_macros.h b/src/kudu/util/test_macros.h
index c4fdca0..ce28aa0 100644
--- a/src/kudu/util/test_macros.h
+++ b/src/kudu/util/test_macros.h
@@ -24,7 +24,7 @@
#define NO_FATALS ASSERT_NO_FATAL_FAILURE
#define ASSERT_OK(status) do { \
- Status _s = status; \
+ const Status& _s = status; \
if (_s.ok()) { \
SUCCEED(); \
} else { \
@@ -33,7 +33,7 @@
} while (0);
#define EXPECT_OK(status) do { \
- Status _s = status; \
+ const Status& _s = status; \
if (_s.ok()) { \
SUCCEED(); \
} else { \
@@ -44,14 +44,14 @@
// Like the above, but doesn't record successful
// tests.
#define ASSERT_OK_FAST(status) do { \
- Status _s = status; \
+ const Status& _s = status; \
if (!_s.ok()) { \
FAIL() << "Bad status: " << _s.ToString(); \
} \
} while (0);
#define ASSERT_STR_CONTAINS(str, substr) do { \
- std::string _s = (str); \
+ const std::string& _s = (str); \
if (_s.find((substr)) == std::string::npos) { \
FAIL() << "Expected to find substring '" << (substr) \
<< "'. Got: '" << _s << "'"; \
@@ -98,13 +98,13 @@
} while (0)
#define ASSERT_FILE_EXISTS(env, path) do { \
- std::string _s = path; \
+ const std::string& _s = path; \
ASSERT_TRUE(env->FileExists(_s)) \
<< "Expected file to exist: " << _s; \
} while (0);
#define ASSERT_FILE_NOT_EXISTS(env, path) do { \
- std::string _s = path; \
+ const std::string& _s = path; \
ASSERT_FALSE(env->FileExists(_s)) \
<< "Expected file not to exist: " << _s; \
} while (0);