You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by wz...@apache.org on 2021/11/11 07:43:16 UTC
[impala] branch master updated (df42225 -> cb0018e)
This is an automated email from the ASF dual-hosted git repository.
wzhou pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git.
from df42225 IMPALA-10984: Improve TimestampValue to String casting
new 4d97895 IMPALA-10943: Add test to verify support for multiple resource and executor pools
new e344457 IMPALA-10940: Pick parts of recent gutil changes from Kudu repo
new cb0018e IMPALA-11011: Impala crashes in OrcStructReader::NumElements()
The 3 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
be/src/exec/orc-column-readers.h | 4 +-
be/src/gutil/cpu.cc | 19 +-
be/src/gutil/cpu.h | 8 +
be/src/gutil/integral_types.h | 1 +
be/src/gutil/macros.h | 5 +-
be/src/gutil/map-util.h | 204 ++++++++++++++++-----
be/src/gutil/port.h | 79 ++++++--
be/src/gutil/stringprintf.cc | 14 +-
be/src/gutil/stringprintf.h | 13 +-
be/src/gutil/strings/numbers.cc | 70 +++++--
be/src/gutil/strings/numbers.h | 168 +++++++++--------
be/src/gutil/strtoint.cc | 5 +-
be/src/gutil/strtoint.h | 9 +-
.../queries/QueryTest/struct-in-select-list.test | 21 +++
tests/custom_cluster/test_executor_groups.py | 93 +++++++++-
15 files changed, 517 insertions(+), 196 deletions(-)
[impala] 02/03: IMPALA-10940: Pick parts of recent gutil changes
from Kudu repo
Posted by wz...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
wzhou pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
commit e344457f0108f2874759283bd292eac4a7970ab2
Author: wzhou-code <wz...@cloudera.com>
AuthorDate: Sun Oct 3 18:16:07 2021 -0700
IMPALA-10940: Pick parts of recent gutil changes from Kudu repo
There are various fixes and improvements in gutil that have been made
on the Kudu repo. These changes are required for rebase of be/src/kudu.
Testing:
- Passed core debug build and exhaustive release build.
- Passed core ASAN build.
Change-Id: I9230923111af3c1028f0e71cd37cb5ee6fbec654
Reviewed-on: http://gerrit.cloudera.org:8080/17897
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
be/src/gutil/cpu.cc | 19 ++--
be/src/gutil/cpu.h | 8 ++
be/src/gutil/integral_types.h | 1 +
be/src/gutil/macros.h | 5 +-
be/src/gutil/map-util.h | 204 ++++++++++++++++++++++++++++++++--------
be/src/gutil/port.h | 79 ++++++++++++----
be/src/gutil/stringprintf.cc | 14 +--
be/src/gutil/stringprintf.h | 13 ++-
be/src/gutil/strings/numbers.cc | 70 ++++++++++----
be/src/gutil/strings/numbers.h | 168 ++++++++++++++++++---------------
be/src/gutil/strtoint.cc | 5 +-
be/src/gutil/strtoint.h | 9 +-
12 files changed, 407 insertions(+), 188 deletions(-)
diff --git a/be/src/gutil/cpu.cc b/be/src/gutil/cpu.cc
index d59a19c..1df8f2f 100644
--- a/be/src/gutil/cpu.cc
+++ b/be/src/gutil/cpu.cc
@@ -4,13 +4,12 @@
#include "gutil/cpu.h"
-#include <stdlib.h>
-#include <string.h>
+#ifndef __aarch64__
+#include <cstring>
+#include <utility>
-#include <algorithm>
-
-#include "gutil/basictypes.h"
-#include "gutil/strings/stringpiece.h"
+#include "gutil/integral_types.h"
+#endif //__aarch64__
#if defined(__x86_64__)
#if defined(_MSC_VER)
@@ -33,12 +32,16 @@ CPU::CPU()
has_sse_(false),
has_sse2_(false),
has_sse3_(false),
+ has_pclmulqdq_(false),
has_ssse3_(false),
has_sse41_(false),
has_sse42_(false),
+ has_popcnt_(false),
has_avx_(false),
has_avx2_(false),
has_aesni_(false),
+ has_bmi_(false),
+ has_bmi2_(false),
has_non_stop_time_stamp_counter_(false),
has_broken_neon_(false),
cpu_vendor_("unknown") {
@@ -222,9 +225,11 @@ void CPU::Initialize() {
has_sse_ = (cpu_info[3] & 0x02000000) != 0;
has_sse2_ = (cpu_info[3] & 0x04000000) != 0;
has_sse3_ = (cpu_info[2] & 0x00000001) != 0;
+ has_pclmulqdq_ = (cpu_info[2] & 0x00000002) != 0;
has_ssse3_ = (cpu_info[2] & 0x00000200) != 0;
has_sse41_ = (cpu_info[2] & 0x00080000) != 0;
has_sse42_ = (cpu_info[2] & 0x00100000) != 0;
+ has_popcnt_ = (cpu_info[2] & 0x00800000) != 0;
// AVX instructions will generate an illegal instruction exception unless
// a) they are supported by the CPU,
// b) XSAVE is supported by the CPU and
@@ -242,6 +247,8 @@ void CPU::Initialize() {
(_xgetbv(0) & 6) == 6 /* XSAVE enabled by kernel */;
has_aesni_ = (cpu_info[2] & 0x02000000) != 0;
has_avx2_ = has_avx_ && (cpu_info7[1] & 0x00000020) != 0;
+ has_bmi_ = cpu_info7[1] & (1 << 3);
+ has_bmi2_ = cpu_info7[1] & (1 << 8);
}
// Get the brand string of the cpu.
diff --git a/be/src/gutil/cpu.h b/be/src/gutil/cpu.h
index 6549814..6462642 100644
--- a/be/src/gutil/cpu.h
+++ b/be/src/gutil/cpu.h
@@ -41,12 +41,16 @@ class CPU {
bool has_sse() const { return has_sse_; }
bool has_sse2() const { return has_sse2_; }
bool has_sse3() const { return has_sse3_; }
+ bool has_pclmulqdq() const { return has_pclmulqdq_; }
bool has_ssse3() const { return has_ssse3_; }
bool has_sse41() const { return has_sse41_; }
bool has_sse42() const { return has_sse42_; }
+ bool has_popcnt() const { return has_popcnt_; }
bool has_avx() const { return has_avx_; }
bool has_avx2() const { return has_avx2_; }
bool has_aesni() const { return has_aesni_; }
+ bool has_bmi() const { return has_bmi_; }
+ bool has_bmi2() const { return has_bmi2_; }
bool has_non_stop_time_stamp_counter() const {
return has_non_stop_time_stamp_counter_;
}
@@ -73,12 +77,16 @@ class CPU {
bool has_sse_;
bool has_sse2_;
bool has_sse3_;
+ bool has_pclmulqdq_;
bool has_ssse3_;
bool has_sse41_;
bool has_sse42_;
+ bool has_popcnt_;
bool has_avx_;
bool has_avx2_;
bool has_aesni_;
+ bool has_bmi_;
+ bool has_bmi2_;
bool has_non_stop_time_stamp_counter_;
bool has_broken_neon_;
std::string cpu_vendor_;
diff --git a/be/src/gutil/integral_types.h b/be/src/gutil/integral_types.h
index cbcf917..e84a42e 100644
--- a/be/src/gutil/integral_types.h
+++ b/be/src/gutil/integral_types.h
@@ -29,6 +29,7 @@ typedef __int64 int64;
#else
typedef int64_t int64;
#endif /* _MSC_VER */
+typedef __int128 int128;
// NOTE: unsigned types are DANGEROUS in loops and other arithmetical
// places. Use the signed types unless your variable represents a bit
diff --git a/be/src/gutil/macros.h b/be/src/gutil/macros.h
index da7ea13..346abde 100644
--- a/be/src/gutil/macros.h
+++ b/be/src/gutil/macros.h
@@ -39,8 +39,6 @@ template <bool>
struct CompileAssert {
};
-#ifndef COMPILE_ASSERT
-
#define COMPILE_ASSERT(expr, msg) \
typedef CompileAssert<(bool(expr))> msg[bool(expr) ? 1 : -1] ATTRIBUTE_UNUSED
@@ -84,7 +82,6 @@ struct CompileAssert {
//
// This is to avoid running into a bug in MS VC 7.1, which
// causes ((0.0) ? 1 : -1) to incorrectly evaluate to 1.
-#endif // COMPILE_ASSERT
// A macro to disallow the copy constructor and operator= functions
@@ -99,7 +96,7 @@ struct CompileAssert {
#ifndef DISALLOW_COPY_AND_ASSIGN
#define DISALLOW_COPY_AND_ASSIGN(TypeName) \
TypeName(const TypeName&) = delete; \
- void operator=(const TypeName&) = delete
+ TypeName& operator=(const TypeName&) = delete
#endif
// An older, politically incorrect name for the above.
diff --git a/be/src/gutil/map-util.h b/be/src/gutil/map-util.h
index fb8d5c8..d33c9b0 100644
--- a/be/src/gutil/map-util.h
+++ b/be/src/gutil/map-util.h
@@ -62,16 +62,15 @@
#ifndef UTIL_GTL_MAP_UTIL_H_
#define UTIL_GTL_MAP_UTIL_H_
-#include <stddef.h>
-#include <string>
-using std::string;
+#include <cstddef>
+#include <tuple>
#include <utility>
using std::make_pair;
using std::pair;
#include <vector>
using std::vector;
-#include <common/logging.h>
+#include <glog/logging.h>
#include "gutil/logging-inl.h"
@@ -295,8 +294,7 @@ bool FindCopy(const Collection& collection,
// Returns true iff the given collection contains the given key.
template <class Collection, class Key>
bool ContainsKey(const Collection& collection, const Key& key) {
- auto it = collection.find(key);
- return it != collection.end();
+ return collection.find(key) != collection.end();
}
// Returns true iff the given collection contains the given key-value pair.
@@ -305,7 +303,7 @@ bool ContainsKeyValuePair(const Collection& collection,
const Key& key,
const Value& value) {
typedef typename Collection::const_iterator const_iterator;
- pair<const_iterator, const_iterator> range = collection.equal_range(key);
+ std::pair<const_iterator, const_iterator> range = collection.equal_range(key);
for (const_iterator it = range.first; it != range.second; ++it) {
if (it->second == value) {
return true;
@@ -324,7 +322,7 @@ bool ContainsKeyValuePair(const Collection& collection,
template <class Collection>
bool InsertOrUpdate(Collection* const collection,
const typename Collection::value_type& vt) {
- pair<typename Collection::iterator, bool> ret = collection->insert(vt);
+ std::pair<typename Collection::iterator, bool> ret = collection->insert(vt);
if (!ret.second) {
// update
ret.first->second = vt.second;
@@ -361,7 +359,7 @@ bool InsertAndDeleteExisting(
Collection* const collection,
const typename Collection::key_type& key,
const typename Collection::mapped_type& value) {
- pair<typename Collection::iterator, bool> ret =
+ std::pair<typename Collection::iterator, bool> ret =
collection->insert(typename Collection::value_type(key, value));
if (!ret.second) {
delete ret.first->second;
@@ -435,13 +433,78 @@ typename Collection::mapped_type& InsertKeyOrDie(
Collection* const collection,
const typename Collection::key_type& key) {
typedef typename Collection::value_type value_type;
- pair<typename Collection::iterator, bool> res =
+ std::pair<typename Collection::iterator, bool> res =
collection->insert(value_type(key, typename Collection::mapped_type()));
CHECK(res.second) << "duplicate key: " << key;
return res.first->second;
}
//
+// Emplace*()
+//
+
+// Dancing with std::enable_if() is necessary to make these two functions
+// below work for both dictionary-like and set-like containers as well.
+// The idea is that for dictionary-like containers Collection::value_type
+// is always pair<const key_type, mapped_type>, so it cannot be the same
+// as key type.
+template <class Collection, class... Args>
+typename std::enable_if<
+ std::is_same<typename Collection::key_type,
+ typename Collection::value_type>::value,
+ bool>::type
+EmplaceIfNotPresent(Collection* const collection, Args&&... args) {
+ return collection->emplace(std::forward<Args>(args)...).second;
+}
+
+template <class Collection, class... Args>
+typename std::enable_if<
+ !std::is_same<typename Collection::key_type,
+ typename Collection::value_type>::value,
+ bool>::type
+EmplaceIfNotPresent(Collection* const collection, Args&&... args) {
+ return collection->try_emplace(std::forward<Args>(args)...).second;
+}
+
+// Emplaces the given key-value pair into the collection. Returns true if the
+// given key didn't previously exist. If the given key already existed in the
+// map, its value is changed to the given "value" and false is returned.
+template <class Collection>
+bool EmplaceOrUpdate(Collection* const collection,
+ const typename Collection::key_type& key,
+ typename Collection::mapped_type&& value) {
+ return collection->insert_or_assign(
+ key, std::forward<typename Collection::mapped_type>(value)).second;
+}
+
+// Given the key and parameters to construct the mapped object in-place,
+// EmplaceOrDie() returns reference to the mapped object for dictionary-like
+// containers or constant reference to the element itself for set-like ones.
+// See the comment for EmplaceIfNotPresent() for details behind the template
+// meta-programming details.
+template <class Collection, class... Args>
+typename std::enable_if<
+ std::is_same<typename Collection::key_type,
+ typename Collection::value_type>::value,
+ const typename Collection::value_type&>::type
+EmplaceOrDie(Collection* const collection, Args&&... args) {
+ auto res = collection->emplace(std::forward<Args>(args)...);
+ CHECK(res.second) << "duplicate value";
+ return *res.first;
+}
+
+template <class Collection, class... Args>
+typename std::enable_if<
+ !std::is_same<typename Collection::key_type,
+ typename Collection::value_type>::value,
+ typename Collection::mapped_type&>::type
+EmplaceOrDie(Collection* const collection, Args&&... args) {
+ auto res = collection->emplace(std::forward<Args>(args)...);
+ CHECK(res.second) << "duplicate value";
+ return res.first->second;
+}
+
+//
// Lookup*()
//
@@ -465,6 +528,21 @@ LookupOrInsert(Collection* const collection,
collection, typename Collection::value_type(key, value));
}
+// It's similar to LookupOrInsert() but uses the emplace and r-value mechanics
+// to achieve the desired results, constructing the element in-place in the
+// container. The constructor of the new element is called with exactly the same
+// arguments as supplied to emplace, forwarded via std::forward<Args>(args).
+// The element is constructed only if there was no element with the specified
+// key in the container.
+// For details, see
+// https://en.cppreference.com/w/cpp/container/map/try_emplace
+// https://en.cppreference.com/w/cpp/container/unordered_map/try_emplace
+template <class Collection, class... Args>
+typename Collection::mapped_type&
+LookupOrEmplace(Collection* const collection, Args&&... args) {
+ return collection->try_emplace(std::forward<Args>(args)...).first->second;
+}
+
// Counts the number of equivalent elements in the given "sequence", and stores
// the results in "count_map" with element as the key and count as the value.
//
@@ -511,7 +589,7 @@ template <class Collection>
typename Collection::mapped_type&
LookupOrInsertNew(Collection* const collection,
const typename Collection::key_type& key) {
- pair<typename Collection::iterator, bool> ret =
+ std::pair<typename Collection::iterator, bool> ret =
collection->insert(
typename Collection::value_type(key,
static_cast<typename Collection::mapped_type>(NULL)));
@@ -530,7 +608,7 @@ typename Collection::mapped_type&
LookupOrInsertNew(Collection* const collection,
const typename Collection::key_type& key,
const Arg& arg) {
- pair<typename Collection::iterator, bool> ret =
+ std::pair<typename Collection::iterator, bool> ret =
collection->insert(
typename Collection::value_type(
key,
@@ -563,7 +641,7 @@ LookupOrInsertNewSharedPtr(
const typename Collection::key_type& key) {
typedef typename Collection::mapped_type SharedPtr;
typedef typename Collection::mapped_type::element_type Element;
- pair<typename Collection::iterator, bool> ret =
+ std::pair<typename Collection::iterator, bool> ret =
collection->insert(typename Collection::value_type(key, SharedPtr()));
if (ret.second) {
ret.first->second.reset(new Element());
@@ -572,22 +650,22 @@ LookupOrInsertNewSharedPtr(
}
// A variant of LookupOrInsertNewSharedPtr where the value is constructed using
-// a single-parameter constructor. Note: the constructor argument is computed
-// even if it will not be used, so only values cheap to compute should be passed
-// here. On the other hand it does not matter how expensive the construction of
-// the actual stored value is, as that only occurs if necessary.
-template <class Collection, class Arg>
+// constructor arguments. Note: the constructor arguments are computed even if
+// they will not be used, so only values cheap to compute should be passed
+// here. On the other hand it does not matter how expensive the construction
+// of the actual stored value is, as that only occurs if necessary.
+template <class Collection, class... Args>
typename Collection::mapped_type&
LookupOrInsertNewSharedPtr(
Collection* const collection,
const typename Collection::key_type& key,
- const Arg& arg) {
+ const Args&... args) {
typedef typename Collection::mapped_type SharedPtr;
typedef typename Collection::mapped_type::element_type Element;
- pair<typename Collection::iterator, bool> ret =
+ std::pair<typename Collection::iterator, bool> ret =
collection->insert(typename Collection::value_type(key, SharedPtr()));
if (ret.second) {
- ret.first->second.reset(new Element(arg));
+ ret.first->second.reset(new Element(args...));
}
return ret.first->second;
}
@@ -608,7 +686,7 @@ bool UpdateReturnCopy(Collection* const collection,
const typename Collection::key_type& key,
const typename Collection::mapped_type& value,
typename Collection::mapped_type* previous) {
- pair<typename Collection::iterator, bool> ret =
+ std::pair<typename Collection::iterator, bool> ret =
collection->insert(typename Collection::value_type(key, value));
if (!ret.second) {
// update
@@ -626,7 +704,7 @@ template <class Collection>
bool UpdateReturnCopy(Collection* const collection,
const typename Collection::value_type& vt,
typename Collection::mapped_type* previous) {
- pair<typename Collection::iterator, bool> ret =
+ std::pair<typename Collection::iterator, bool> ret =
collection->insert(vt);
if (!ret.second) {
// update
@@ -650,7 +728,7 @@ template <class Collection>
typename Collection::mapped_type* const
InsertOrReturnExisting(Collection* const collection,
const typename Collection::value_type& vt) {
- pair<typename Collection::iterator, bool> ret = collection->insert(vt);
+ std::pair<typename Collection::iterator, bool> ret = collection->insert(vt);
if (ret.second) {
return NULL; // Inserted, no existing previous value.
} else {
@@ -694,7 +772,7 @@ void ReverseMap(const Collection& collection,
// delete EraseKeyReturnValuePtr(&my_map, "abc");
//
// Use returned value:
-// gscoped_ptr<MyType> value_ptr(EraseKeyReturnValuePtr(&my_map, "abc"));
+// unique_ptr<MyType> value_ptr(EraseKeyReturnValuePtr(&my_map, "abc"));
// if (value_ptr.get())
// value_ptr->DoSomething();
//
@@ -750,7 +828,7 @@ void AppendKeysFromMap(const MapContainer& map_container,
// without the complexity of a SFINAE-based solution.)
template <class MapContainer, class KeyType>
void AppendKeysFromMap(const MapContainer& map_container,
- vector<KeyType>* key_container) {
+ std::vector<KeyType>* key_container) {
CHECK(key_container != NULL);
// We now have the opportunity to call reserve(). Calling reserve() every
// time is a bad idea for some use cases: libstdc++'s implementation of
@@ -785,6 +863,19 @@ void AppendValuesFromMap(const MapContainer& map_container,
}
}
+template <class MapContainer, class ValueContainer>
+void EmplaceValuesFromMap(MapContainer&& map_container,
+ ValueContainer* value_container) {
+ CHECK(value_container != nullptr);
+ // See AppendKeysFromMap for why this is done.
+ if (value_container->empty()) {
+ value_container->reserve(map_container.size());
+ }
+ for (auto&& entry : map_container) {
+ value_container->emplace_back(std::move(entry.second));
+ }
+}
+
// A more specialized overload of AppendValuesFromMap to optimize reallocations
// for the common case in which we're appending values to a vector and hence
// can (and sometimes should) call reserve() first.
@@ -794,15 +885,8 @@ void AppendValuesFromMap(const MapContainer& map_container,
// without the complexity of a SFINAE-based solution.)
template <class MapContainer, class ValueType>
void AppendValuesFromMap(const MapContainer& map_container,
- vector<ValueType>* value_container) {
- CHECK(value_container != NULL);
- // See AppendKeysFromMap for why this is done.
- if (value_container->empty()) {
- value_container->reserve(map_container.size());
- }
- for (const auto& entry : map_container) {
- value_container->push_back(entry.second);
- }
+ std::vector<ValueType>* value_container) {
+ EmplaceValuesFromMap(map_container, value_container);
}
// Compute and insert new value if it's absent from the map. Return a pair with a reference to the
@@ -825,19 +909,47 @@ void AppendValuesFromMap(const MapContainer& map_container,
// MyValue* const value = result.first;
// if (result.second) ....
//
+// The ComputePair* variants expect a lambda that creates a pair<k, v>. This
+// can be useful if the key is a StringPiece pointing to external state to
+// avoid excess memory for the keys, while being safer in multi-threaded
+// contexts, e.g. in case the key goes out of scope before the container does.
+//
+// Example usage:
+//
+// map<StringPiece, int, GoodFastHash<StringPiece>> string_to_idx;
+// vector<unique_ptr<StringPB>> pbs;
+// auto result = ComputePairIfAbsentReturnAbsense(&string_to_idx, my_key,
+// [&]() {
+// unique_ptr<StringPB> s = new StringPB();
+// s->set_string(my_key);
+// int idx = pbs.size();
+// pbs.emplace_back(s.release());
+// return make_pair(StringPiece(pbs.back()->string()), idx);
+// });
template <class MapContainer, typename Function>
-pair<typename MapContainer::mapped_type* const, bool>
-ComputeIfAbsentReturnAbsense(MapContainer* container,
- const typename MapContainer::key_type& key,
- Function compute_func) {
+std::pair<typename MapContainer::mapped_type* const, bool>
+ComputePairIfAbsentReturnAbsense(MapContainer* container,
+ const typename MapContainer::key_type& key,
+ Function compute_pair_func) {
typename MapContainer::iterator iter = container->find(key);
bool new_value = iter == container->end();
if (new_value) {
- pair<typename MapContainer::iterator, bool> result = container->emplace(key, compute_func());
+ auto p = compute_pair_func();
+ std::pair<typename MapContainer::iterator, bool> result =
+ container->emplace(std::move(p.first), std::move(p.second));
DCHECK(result.second) << "duplicate key: " << key;
iter = result.first;
}
- return make_pair(&iter->second, new_value);
+ return std::make_pair(&iter->second, new_value);
+}
+template <class MapContainer, typename Function>
+std::pair<typename MapContainer::mapped_type* const, bool>
+ComputeIfAbsentReturnAbsense(MapContainer* container,
+ const typename MapContainer::key_type& key,
+ Function compute_func) {
+ return ComputePairIfAbsentReturnAbsense(container, key, [&key, &compute_func] {
+ return std::make_pair(key, compute_func());
+ });
};
// Like the above but doesn't return a pair, just returns a pointer to the value.
@@ -855,4 +967,14 @@ ComputeIfAbsent(MapContainer* container,
return ComputeIfAbsentReturnAbsense(container, key, compute_func).first;
};
+template <class MapContainer, typename Function>
+typename MapContainer::mapped_type* const
+ComputePairIfAbsent(MapContainer* container,
+ const typename MapContainer::key_type& key,
+ Function compute_pair_func) {
+ return ComputePairIfAbsentReturnAbsense<MapContainer, Function>(
+ container, key, compute_pair_func)
+ .first;
+};
+
#endif // UTIL_GTL_MAP_UTIL_H_
diff --git a/be/src/gutil/port.h b/be/src/gutil/port.h
index 964e009..1ef153f 100644
--- a/be/src/gutil/port.h
+++ b/be/src/gutil/port.h
@@ -9,8 +9,8 @@
#define BASE_PORT_H_
#include <limits.h> // So we can set the bounds of our types
-#include <string.h> // for memcpy()
#include <stdlib.h> // for free()
+#include <string.h> // for memcpy()
#if defined(__APPLE__)
#include <unistd.h> // for getpagesize() on mac
@@ -18,6 +18,8 @@
#include <malloc.h> // for memalign()
#endif
+#include <type_traits>
+
#include "gutil/integral_types.h"
// Must happens before inttypes.h inclusion */
@@ -231,9 +233,6 @@ inline size_t strnlen(const char *s, size_t maxlen) {
return maxlen;
}
-namespace std {} // Avoid error if we didn't see std.
-using namespace std; // Just like VC++, we need a using here.
-
// Doesn't exist on OSX; used in google.cc for send() to mean "no flags".
#define MSG_NOSIGNAL 0
@@ -325,8 +324,8 @@ inline void* memrchr(const void* bytes, int find_char, size_t len) {
// TODO(user) This is the L1 D-cache line size of our Power7 machines.
// Need to check if this is appropriate for other PowerPC64 systems.
#define CACHELINE_SIZE 128
-#elif defined(__aarch64__) && defined(__linux__)
-#define CACHELINE_SIZE CACHELINESIZE_AARCH64
+#elif defined(__aarch64__)
+#define CACHELINE_SIZE 64
#elif defined(__arm__)
// Cache line sizes for ARM: These values are not strictly correct since
// cache line sizes depend on implementations, not architectures. There
@@ -964,7 +963,7 @@ inline int isinf(double x) {
return 0;
}
-// #include "conflict-signal.h"
+// #include "kudu/conflict-signal.h"
typedef void (*sig_t)(int);
// These actually belong in errno.h but there's a name confilict in errno
@@ -1008,7 +1007,7 @@ typedef short int16_t;
#endif // _MSC_VER
#ifdef STL_MSVC // not always the same as _MSC_VER
-#include "base/port_hash.h"
+#include "kudu/base/port_hash.h"
#else
struct PortableHashBase { };
#endif
@@ -1165,23 +1164,63 @@ inline void UNALIGNED_STORE64(void *p, uint64 v) {
#if defined(__cplusplus)
-inline void UnalignedCopy16(const void *src, void *dst) {
- UNALIGNED_STORE16(dst, UNALIGNED_LOAD16(src));
+namespace port_internal {
+
+template<class T>
+constexpr bool LoadByReinterpretCast() {
+#ifndef NEED_ALIGNED_LOADS
+ // Per above, it's safe to use reinterpret_cast on x86 for types int64 and smaller.
+ return sizeof(T) <= 8;
+#else
+ return false;
+#endif
}
-inline void UnalignedCopy32(const void *src, void *dst) {
- UNALIGNED_STORE32(dst, UNALIGNED_LOAD32(src));
+// Enable UnalignedLoad and UnalignedStore for numeric types (floats and ints) including
+// int128. We don't allow these functions for other types, even if they are POD and <= 16
+// bits.
+template<class T>
+using enable_if_numeric = std::enable_if<
+ std::is_arithmetic<T>::value || std::is_same<T, __int128>::value, T>;
+
+} // namespace port_internal
+
+
+// Load an integer from pointer 'src'.
+//
+// This is a safer equivalent of *reinterpret_cast<const T*>(src) that properly handles
+// the case of larger types such as int128 which require alignment.
+//
+// Usage:
+// int32_t x = UnalignedLoad<int32_t>(void_ptr);
+//
+template<typename T,
+ typename port_internal::enable_if_numeric<T>::type* = nullptr>
+inline T UnalignedLoad(const void* src) {
+ if (port_internal::LoadByReinterpretCast<T>()) {
+ return *reinterpret_cast<const T*>(src);
+ }
+ T ret;
+ memcpy(&ret, src, sizeof(T));
+ return ret;
}
-inline void UnalignedCopy64(const void *src, void *dst) {
- if (sizeof(void *) == 8) {
- UNALIGNED_STORE64(dst, UNALIGNED_LOAD64(src));
- } else {
- const char *src_char = reinterpret_cast<const char *>(src);
- char *dst_char = reinterpret_cast<char *>(dst);
- UNALIGNED_STORE32(dst_char, UNALIGNED_LOAD32(src_char));
- UNALIGNED_STORE32(dst_char + 4, UNALIGNED_LOAD32(src_char + 4));
+// Store the integer 'src' in the pointer 'dst'.
+//
+// Usage:
+// int32_t foo = 123;
+// UnalignedStore(my_void_ptr, foo);
+//
+// NOTE: this reverses the usual style-guide-suggested order of arguments
+// to match the more natural "*p = v;" ordering of a normal store.
+template<typename T,
+ typename port_internal::enable_if_numeric<T>::type* = nullptr>
+inline void UnalignedStore(void* dst, const T& src) {
+ if (port_internal::LoadByReinterpretCast<T>()) {
+ *reinterpret_cast<T*>(dst) = src;
+ } else {
+ memcpy(dst, &src, sizeof(T));
}
}
diff --git a/be/src/gutil/stringprintf.cc b/be/src/gutil/stringprintf.cc
index 13939fe..ec3b937 100644
--- a/be/src/gutil/stringprintf.cc
+++ b/be/src/gutil/stringprintf.cc
@@ -2,15 +2,17 @@
#include "gutil/stringprintf.h"
-#include <errno.h>
-#include <stdarg.h> // For va_list and related operations
-#include <stdio.h> // MSVC requires this for _vsnprintf
+#include <cstdio> // MSVC requires this for _vsnprintf
+#include <ostream>
#include <vector>
-using std::vector;
-#include <common/logging.h>
-#include "gutil/logging-inl.h"
+
+#include <glog/logging.h>
+
#include "gutil/macros.h"
+using std::string;
+using std::vector;
+
#ifdef _MSC_VER
enum { IS__MSC_VER = 1 };
#else
diff --git a/be/src/gutil/stringprintf.h b/be/src/gutil/stringprintf.h
index e486e72..359c5b9 100644
--- a/be/src/gutil/stringprintf.h
+++ b/be/src/gutil/stringprintf.h
@@ -12,30 +12,28 @@
#include <stdarg.h>
#include <string>
-using std::string;
#include <vector>
-using std::vector;
#include "gutil/port.h"
// Return a C++ string
-extern string StringPrintf(const char* format, ...)
+extern std::string StringPrintf(const char* format, ...)
// Tell the compiler to do printf format string checking.
PRINTF_ATTRIBUTE(1,2);
// Store result into a supplied string and return it
-extern const string& SStringPrintf(string* dst, const char* format, ...)
+extern const std::string& SStringPrintf(std::string* dst, const char* format, ...)
// Tell the compiler to do printf format string checking.
PRINTF_ATTRIBUTE(2,3);
// Append result to a supplied string
-extern void StringAppendF(string* dst, const char* format, ...)
+extern void StringAppendF(std::string* dst, const char* format, ...)
// Tell the compiler to do printf format string checking.
PRINTF_ATTRIBUTE(2,3);
// Lower-level routine that takes a va_list and appends to a specified
// string. All other routines are just convenience wrappers around it.
-extern void StringAppendV(string* dst, const char* format, va_list ap);
+extern void StringAppendV(std::string* dst, const char* format, va_list ap);
// The max arguments supported by StringPrintfVector
extern const int kStringPrintfVectorMaxArgs;
@@ -43,6 +41,7 @@ extern const int kStringPrintfVectorMaxArgs;
// You can use this version when all your arguments are strings, but
// you don't know how many arguments you'll have at compile time.
// StringPrintfVector will LOG(FATAL) if v.size() > kStringPrintfVectorMaxArgs
-extern string StringPrintfVector(const char* format, const vector<string>& v);
+extern std::string StringPrintfVector(
+ const char* format, const std::vector<std::string>& v);
#endif /* _BASE_STRINGPRINTF_H */
diff --git a/be/src/gutil/strings/numbers.cc b/be/src/gutil/strings/numbers.cc
index c7f7d19..801af88 100644
--- a/be/src/gutil/strings/numbers.cc
+++ b/be/src/gutil/strings/numbers.cc
@@ -6,28 +6,33 @@
#include "gutil/strings/numbers.h"
-#include <assert.h>
-#include <ctype.h>
-#include <errno.h>
-#include <float.h> // for DBL_DIG and FLT_DIG
-#include <math.h> // for HUGE_VAL
-#include <stdio.h>
-#include <stdlib.h>
-#include <string.h>
+#include <sys/types.h>
+
+#include <cassert>
+#include <cctype>
+#include <cerrno>
+#include <cfloat> // for DBL_DIG and FLT_DIG
+#include <cmath> // for HUGE_VAL
+#include <cstdio>
+#include <cstdlib>
+#include <cstring>
#include <limits>
-using std::numeric_limits;
+#include <memory>
+#include <ostream>
#include <string>
-using std::string;
+
+#include <glog/logging.h>
#include "gutil/int128.h"
#include "gutil/integral_types.h"
-#include <common/logging.h>
-#include "gutil/logging-inl.h"
-#include "gutil/gscoped_ptr.h"
#include "gutil/stringprintf.h"
#include "gutil/strtoint.h"
#include "gutil/strings/ascii_ctype.h"
+using std::numeric_limits;
+using std::string;
+using std::unique_ptr;
+
// Reads a <double> in *text, which may not be whitespace-initiated.
// *len is the length, or -1 if text is '\0'-terminated, which is more
// efficient. Sets *text to the end of the double, and val to the
@@ -78,7 +83,7 @@ static inline bool EatADouble(const char** text, int* len, bool allow_question,
retval = strtod(pos, &end_nonconst);
} else {
// not '\0'-terminated & no obvious terminator found. must copy.
- gscoped_array<char> buf(new char[rem + 1]);
+ unique_ptr<char[]> buf(new char[rem + 1]);
memcpy(buf.get(), pos, rem);
buf[rem] = '\0';
retval = strtod(buf.get(), &end_nonconst);
@@ -1010,10 +1015,7 @@ char* FastInt32ToBufferLeft(int32 i, char* buffer) {
uint32 u = i;
if (i < 0) {
*buffer++ = '-';
- // We need to do the negation in modular (i.e., "unsigned")
- // arithmetic; MSVC++ apprently warns for plain "-u", so
- // we write the equivalent expression "0 - u" instead.
- u = 0 - u;
+ u = ~u + 1;
}
return FastUInt32ToBufferLeft(u, buffer);
}
@@ -1064,11 +1066,41 @@ char* FastInt64ToBufferLeft(int64 i, char* buffer) {
uint64 u = i;
if (i < 0) {
*buffer++ = '-';
- u = 0 - u;
+ u = ~u + 1;
}
return FastUInt64ToBufferLeft(u, buffer);
}
+char* FastUInt128ToBufferLeft(unsigned __int128 i, char* buffer) {
+ static const unsigned __int128 TWENTY_DIGITS =
+ static_cast<unsigned __int128>(10000000000)
+ * static_cast<unsigned __int128>(10000000000);
+
+ uint64 u = static_cast<uint64>(i);
+ if (u == i) return FastUInt64ToBufferLeft(u, buffer);
+
+ unsigned __int128 top_19_digits = i / TWENTY_DIGITS;
+ buffer = FastUInt64ToBufferLeft(top_19_digits, buffer);
+ unsigned __int128 rem128 = i - (top_19_digits * TWENTY_DIGITS);
+
+ unsigned __int128 middle_19_digits = rem128 / 10;
+ buffer = FastUInt64ToBufferLeft(middle_19_digits, buffer);
+ u = rem128 - (middle_19_digits * 10);
+
+ buffer = FastUInt32ToBufferLeft(u, buffer);
+
+ return buffer;
+}
+
+char* FastInt128ToBufferLeft(__int128 i, char* buffer) {
+ unsigned __int128 u = i;
+ if (i < 0) {
+ *buffer++ = '-';
+ u = ~u + 1;
+ }
+ return FastUInt128ToBufferLeft(u, buffer);
+}
+
int HexDigitsPrefix(const char* buf, int num_digits) {
for (int i = 0; i < num_digits; i++)
if (!ascii_isxdigit(buf[i]))
diff --git a/be/src/gutil/strings/numbers.h b/be/src/gutil/strings/numbers.h
index f035f23..e1a7645 100644
--- a/be/src/gutil/strings/numbers.h
+++ b/be/src/gutil/strings/numbers.h
@@ -6,10 +6,9 @@
#ifndef STRINGS_NUMBERS_H_
#define STRINGS_NUMBERS_H_
-#include <stddef.h>
-#include <stdlib.h>
-#include <string.h>
-#include <time.h>
+#include <cstddef>
+#include <cinttypes>
+#include <ctime>
#include <functional>
using std::binary_function;
using std::less;
@@ -26,16 +25,15 @@ using std::vector;
#include "gutil/port.h"
#include "gutil/stringprintf.h"
-
// START DOXYGEN NumbersFunctions grouping
/* @defgroup NumbersFunctions
* @{ */
// Convert a fingerprint to 16 hex digits.
-string FpToString(Fprint fp);
+std::string FpToString(Fprint fp);
// Formats a uint128 as a 32-digit hex string.
-string Uint128ToHexString(uint128 ui128);
+std::string Uint128ToHexString(uint128 ui128);
// Convert strings to numeric values, with strict error checking.
// Leading and trailing spaces are allowed.
@@ -52,12 +50,12 @@ bool safe_strtou64(const char* str, uint64* value);
bool safe_strtof(const char* str, float* value);
bool safe_strtod(const char* str, double* value);
-bool safe_strto32(const string& str, int32* value);
-bool safe_strto64(const string& str, int64* value);
-bool safe_strtou32(const string& str, uint32* value);
-bool safe_strtou64(const string& str, uint64* value);
-bool safe_strtof(const string& str, float* value);
-bool safe_strtod(const string& str, double* value);
+bool safe_strto32(const std::string& str, int32* value);
+bool safe_strto64(const std::string& str, int64* value);
+bool safe_strtou32(const std::string& str, uint32* value);
+bool safe_strtou64(const std::string& str, uint64* value);
+bool safe_strtof(const std::string& str, float* value);
+bool safe_strtod(const std::string& str, double* value);
// Parses buffer_size many characters from startptr into value.
bool safe_strto32(const char* startptr, int buffer_size, int32* value);
@@ -71,10 +69,10 @@ bool safe_strto64_base(const char* str, int64* value, int base);
bool safe_strtou32_base(const char* str, uint32* value, int base);
bool safe_strtou64_base(const char* str, uint64* value, int base);
-bool safe_strto32_base(const string& str, int32* value, int base);
-bool safe_strto64_base(const string& str, int64* value, int base);
-bool safe_strtou32_base(const string& str, uint32* value, int base);
-bool safe_strtou64_base(const string& str, uint64* value, int base);
+bool safe_strto32_base(const std::string& str, int32* value, int base);
+bool safe_strto64_base(const std::string& str, int64* value, int base);
+bool safe_strtou32_base(const std::string& str, uint32* value, int base);
+bool safe_strtou64_base(const std::string& str, uint64* value, int base);
bool safe_strto32_base(const char* startptr, int buffer_size,
int32* value, int base);
@@ -92,7 +90,7 @@ size_t u64tostr_base36(uint64 number, size_t buf_size, char* buffer);
// Similar to atoi(s), except s could be like "16k", "32M", "2G", "4t".
uint64 atoi_kmgt(const char* s);
-inline uint64 atoi_kmgt(const string& s) { return atoi_kmgt(s.c_str()); }
+inline uint64 atoi_kmgt(const std::string& s) { return atoi_kmgt(s.c_str()); }
// ----------------------------------------------------------------------
// FastIntToBuffer()
@@ -157,6 +155,8 @@ inline char* FastUIntToBuffer(unsigned int i, char* buffer) {
// FastUInt32ToBufferLeft()
// FastInt64ToBufferLeft()
// FastUInt64ToBufferLeft()
+// FastInt128ToBufferLeft()
+// FastUInt128ToBufferLeft()
//
// Like the Fast*ToBuffer() functions above, these are intended for speed.
// Unlike the Fast*ToBuffer() functions, however, these functions write
@@ -172,6 +172,8 @@ char* FastInt32ToBufferLeft(int32 i, char* buffer); // at least 12 bytes
char* FastUInt32ToBufferLeft(uint32 i, char* buffer); // at least 12 bytes
char* FastInt64ToBufferLeft(int64 i, char* buffer); // at least 22 bytes
char* FastUInt64ToBufferLeft(uint64 i, char* buffer); // at least 22 bytes
+char* FastInt128ToBufferLeft(__int128 i, char* buffer);
+char* FastUInt128ToBufferLeft(unsigned __int128 i, char* buffer);
// Just define these in terms of the above.
inline char* FastUInt32ToBuffer(uint32 i, char* buffer) {
@@ -195,7 +197,7 @@ int HexDigitsPrefix(const char* buf, int num_digits);
// ConsumeStrayLeadingZeroes
// Eliminates all leading zeroes (unless the string itself is composed
// of nothing but zeroes, in which case one is kept: 0...0 becomes 0).
-void ConsumeStrayLeadingZeroes(string* str);
+void ConsumeStrayLeadingZeroes(std::string* str);
// ----------------------------------------------------------------------
// ParseLeadingInt32Value
@@ -206,7 +208,7 @@ void ConsumeStrayLeadingZeroes(string* str);
// treated as octal. If you know it's decimal, use ParseLeadingDec32Value.
// --------------------------------------------------------------------
int32 ParseLeadingInt32Value(const char* str, int32 deflt);
-inline int32 ParseLeadingInt32Value(const string& str, int32 deflt) {
+inline int32 ParseLeadingInt32Value(const std::string& str, int32 deflt) {
return ParseLeadingInt32Value(str.c_str(), deflt);
}
@@ -218,7 +220,7 @@ inline int32 ParseLeadingInt32Value(const string& str, int32 deflt) {
// treated as octal. If you know it's decimal, use ParseLeadingUDec32Value.
// --------------------------------------------------------------------
uint32 ParseLeadingUInt32Value(const char* str, uint32 deflt);
-inline uint32 ParseLeadingUInt32Value(const string& str, uint32 deflt) {
+inline uint32 ParseLeadingUInt32Value(const std::string& str, uint32 deflt) {
return ParseLeadingUInt32Value(str.c_str(), deflt);
}
@@ -232,7 +234,7 @@ inline uint32 ParseLeadingUInt32Value(const string& str, uint32 deflt) {
// See also: ParseLeadingDec64Value
// --------------------------------------------------------------------
int32 ParseLeadingDec32Value(const char* str, int32 deflt);
-inline int32 ParseLeadingDec32Value(const string& str, int32 deflt) {
+inline int32 ParseLeadingDec32Value(const std::string& str, int32 deflt) {
return ParseLeadingDec32Value(str.c_str(), deflt);
}
@@ -245,7 +247,7 @@ inline int32 ParseLeadingDec32Value(const string& str, int32 deflt) {
// See also: ParseLeadingUDec64Value
// --------------------------------------------------------------------
uint32 ParseLeadingUDec32Value(const char* str, uint32 deflt);
-inline uint32 ParseLeadingUDec32Value(const string& str, uint32 deflt) {
+inline uint32 ParseLeadingUDec32Value(const std::string& str, uint32 deflt) {
return ParseLeadingUDec32Value(str.c_str(), deflt);
}
@@ -260,23 +262,23 @@ inline uint32 ParseLeadingUDec32Value(const string& str, uint32 deflt) {
// valid integer is found; else returns deflt
// --------------------------------------------------------------------
uint64 ParseLeadingUInt64Value(const char* str, uint64 deflt);
-inline uint64 ParseLeadingUInt64Value(const string& str, uint64 deflt) {
+inline uint64 ParseLeadingUInt64Value(const std::string& str, uint64 deflt) {
return ParseLeadingUInt64Value(str.c_str(), deflt);
}
int64 ParseLeadingInt64Value(const char* str, int64 deflt);
-inline int64 ParseLeadingInt64Value(const string& str, int64 deflt) {
+inline int64 ParseLeadingInt64Value(const std::string& str, int64 deflt) {
return ParseLeadingInt64Value(str.c_str(), deflt);
}
uint64 ParseLeadingHex64Value(const char* str, uint64 deflt);
-inline uint64 ParseLeadingHex64Value(const string& str, uint64 deflt) {
+inline uint64 ParseLeadingHex64Value(const std::string& str, uint64 deflt) {
return ParseLeadingHex64Value(str.c_str(), deflt);
}
int64 ParseLeadingDec64Value(const char* str, int64 deflt);
-inline int64 ParseLeadingDec64Value(const string& str, int64 deflt) {
+inline int64 ParseLeadingDec64Value(const std::string& str, int64 deflt) {
return ParseLeadingDec64Value(str.c_str(), deflt);
}
uint64 ParseLeadingUDec64Value(const char* str, uint64 deflt);
-inline uint64 ParseLeadingUDec64Value(const string& str, uint64 deflt) {
+inline uint64 ParseLeadingUDec64Value(const std::string& str, uint64 deflt) {
return ParseLeadingUDec64Value(str.c_str(), deflt);
}
@@ -287,7 +289,7 @@ inline uint64 ParseLeadingUDec64Value(const string& str, uint64 deflt) {
// check if str is entirely consumed.
// --------------------------------------------------------------------
double ParseLeadingDoubleValue(const char* str, double deflt);
-inline double ParseLeadingDoubleValue(const string& str, double deflt) {
+inline double ParseLeadingDoubleValue(const std::string& str, double deflt) {
return ParseLeadingDoubleValue(str.c_str(), deflt);
}
@@ -299,7 +301,7 @@ inline double ParseLeadingDoubleValue(const string& str, double deflt) {
// 0/1, false/true, no/yes, n/y
// --------------------------------------------------------------------
bool ParseLeadingBoolValue(const char* str, bool deflt);
-inline bool ParseLeadingBoolValue(const string& str, bool deflt) {
+inline bool ParseLeadingBoolValue(const std::string& str, bool deflt) {
return ParseLeadingBoolValue(str.c_str(), deflt);
}
@@ -337,29 +339,29 @@ bool StrictAutoDigitLessThan(const char* a, int alen,
const char* b, int blen);
struct autodigit_less
- : public binary_function<const string&, const string&, bool> {
- bool operator()(const string& a, const string& b) const {
+ : public std::binary_function<const std::string&, const std::string&, bool> {
+ bool operator()(const std::string& a, const std::string& b) const {
return AutoDigitLessThan(a.data(), a.size(), b.data(), b.size());
}
};
struct autodigit_greater
- : public binary_function<const string&, const string&, bool> {
- bool operator()(const string& a, const string& b) const {
+ : public std::binary_function<const std::string&, const std::string&, bool> {
+ bool operator()(const std::string& a, const std::string& b) const {
return AutoDigitLessThan(b.data(), b.size(), a.data(), a.size());
}
};
struct strict_autodigit_less
- : public binary_function<const string&, const string&, bool> {
- bool operator()(const string& a, const string& b) const {
+ : public std::binary_function<const std::string&, const std::string&, bool> {
+ bool operator()(const std::string& a, const std::string& b) const {
return StrictAutoDigitLessThan(a.data(), a.size(), b.data(), b.size());
}
};
struct strict_autodigit_greater
- : public binary_function<const string&, const string&, bool> {
- bool operator()(const string& a, const string& b) const {
+ : public std::binary_function<const std::string&, const std::string&, bool> {
+ bool operator()(const std::string& a, const std::string& b) const {
return StrictAutoDigitLessThan(b.data(), b.size(), a.data(), a.size());
}
};
@@ -371,26 +373,36 @@ struct strict_autodigit_greater
//
// Return value: string
// ----------------------------------------------------------------------
-inline string SimpleItoa(int32 i) {
+inline std::string SimpleItoa(int32 i) {
char buf[16]; // Longest is -2147483648
- return string(buf, FastInt32ToBufferLeft(i, buf));
+ return std::string(buf, FastInt32ToBufferLeft(i, buf));
}
// We need this overload because otherwise SimpleItoa(5U) wouldn't compile.
-inline string SimpleItoa(uint32 i) {
+inline std::string SimpleItoa(uint32 i) {
char buf[16]; // Longest is 4294967295
- return string(buf, FastUInt32ToBufferLeft(i, buf));
+ return std::string(buf, FastUInt32ToBufferLeft(i, buf));
}
-inline string SimpleItoa(int64 i) {
+inline std::string SimpleItoa(int64 i) {
char buf[32]; // Longest is -9223372036854775808
- return string(buf, FastInt64ToBufferLeft(i, buf));
+ return std::string(buf, FastInt64ToBufferLeft(i, buf));
}
// We need this overload because otherwise SimpleItoa(5ULL) wouldn't compile.
-inline string SimpleItoa(uint64 i) {
+inline std::string SimpleItoa(uint64 i) {
char buf[32]; // Longest is 18446744073709551615
- return string(buf, FastUInt64ToBufferLeft(i, buf));
+ return std::string(buf, FastUInt64ToBufferLeft(i, buf));
+}
+
+inline std::string SimpleItoa(__int128 i) {
+ char buf[64]; // Longest is -170141183460469231731687303715884105728
+ return std::string(buf, FastInt128ToBufferLeft(i, buf));
+}
+
+inline std::string SimpleItoa(unsigned __int128 i) {
+ char buf[64]; // Longest is 340282366920938463463374607431768211455
+ return std::string(buf, FastUInt128ToBufferLeft(i, buf));
}
// SimpleAtoi converts a string to an integer.
@@ -421,7 +433,7 @@ bool MUST_USE_RESULT SimpleAtoi(const char* s, int_type* out) {
}
template <typename int_type>
-bool MUST_USE_RESULT SimpleAtoi(const string& s, int_type* out) {
+bool MUST_USE_RESULT SimpleAtoi(const std::string& s, int_type* out) {
return SimpleAtoi(s.c_str(), out);
}
@@ -444,8 +456,8 @@ bool MUST_USE_RESULT SimpleAtoi(const string& s, int_type* out) {
//
// Return value: string
// ----------------------------------------------------------------------
-string SimpleDtoa(double value);
-string SimpleFtoa(float value);
+std::string SimpleDtoa(double value);
+std::string SimpleFtoa(float value);
char* DoubleToBuffer(double i, char* buffer);
char* FloatToBuffer(float i, char* buffer);
@@ -464,10 +476,10 @@ static const int kFloatToBufferSize = 24;
//
// Return value: string
// ----------------------------------------------------------------------
-string SimpleItoaWithCommas(int32 i);
-string SimpleItoaWithCommas(uint32 i);
-string SimpleItoaWithCommas(int64 i);
-string SimpleItoaWithCommas(uint64 i);
+std::string SimpleItoaWithCommas(int32 i);
+std::string SimpleItoaWithCommas(uint32 i);
+std::string SimpleItoaWithCommas(int64 i);
+std::string SimpleItoaWithCommas(uint64 i);
// ----------------------------------------------------------------------
// ItoaKMGT()
@@ -478,7 +490,7 @@ string SimpleItoaWithCommas(uint64 i);
//
// Return value: string
// ----------------------------------------------------------------------
-string ItoaKMGT(int64 i);
+std::string ItoaKMGT(int64 i);
// ----------------------------------------------------------------------
// ParseDoubleRange()
@@ -540,36 +552,36 @@ bool ParseDoubleRange(const char* text, int len, const char** end,
// These functions are deprecated.
// Do not use in new code.
-// // DEPRECATED(wadetregaskis). Just call StringPrintf or SimpleFtoa.
-// string FloatToString(float f, const char* format);
+// DEPRECATED(wadetregaskis). Just call StringPrintf or SimpleFtoa.
+std::string FloatToString(float f, const char* format);
-// // DEPRECATED(wadetregaskis). Just call StringPrintf or SimpleItoa.
-// string IntToString(int i, const char* format);
+// DEPRECATED(wadetregaskis). Just call StringPrintf or SimpleItoa.
+std::string IntToString(int i, const char* format);
-// // DEPRECATED(wadetregaskis). Just call StringPrintf or SimpleItoa.
-// string Int64ToString(int64 i64, const char* format);
+// DEPRECATED(wadetregaskis). Just call StringPrintf or SimpleItoa.
+std::string Int64ToString(int64 i64, const char* format);
-// // DEPRECATED(wadetregaskis). Just call StringPrintf or SimpleItoa.
-// string UInt64ToString(uint64 ui64, const char* format);
+// DEPRECATED(wadetregaskis). Just call StringPrintf or SimpleItoa.
+std::string UInt64ToString(uint64 ui64, const char* format);
-// // DEPRECATED(wadetregaskis). Just call StringPrintf.
-// inline string FloatToString(float f) {
-// return StringPrintf("%7f", f);
-// }
+// DEPRECATED(wadetregaskis). Just call StringPrintf.
+inline std::string FloatToString(float f) {
+ return StringPrintf("%7f", f);
+}
-// // DEPRECATED(wadetregaskis). Just call StringPrintf.
-// inline string IntToString(int i) {
-// return StringPrintf("%7d", i);
-// }
+// DEPRECATED(wadetregaskis). Just call StringPrintf.
+inline std::string IntToString(int i) {
+ return StringPrintf("%7d", i);
+}
-// // DEPRECATED(wadetregaskis). Just call StringPrintf.
-// inline string Int64ToString(int64 i64) {
-// return StringPrintf("%7" PRId64, i64);
-// }
+// DEPRECATED(wadetregaskis). Just call StringPrintf.
+inline std::string Int64ToString(int64 i64) {
+ return StringPrintf("%7" PRId64, i64);
+}
-// // DEPRECATED(wadetregaskis). Just call StringPrintf.
-// inline string UInt64ToString(uint64 ui64) {
-// return StringPrintf("%7" PRIu64, ui64);
-// }
+// DEPRECATED(wadetregaskis). Just call StringPrintf.
+inline std::string UInt64ToString(uint64 ui64) {
+ return StringPrintf("%7" PRIu64, ui64);
+}
#endif // STRINGS_NUMBERS_H_
diff --git a/be/src/gutil/strtoint.cc b/be/src/gutil/strtoint.cc
index 9df29a3..bde71a1 100644
--- a/be/src/gutil/strtoint.cc
+++ b/be/src/gutil/strtoint.cc
@@ -4,8 +4,9 @@
// See strtoint.h for details on how to use this component.
//
-#include <errno.h>
-#include "gutil/port.h"
+#include <cerrno>
+#include <climits>
+
#include "gutil/strtoint.h"
// Replacement strto[u]l functions that have identical overflow and underflow
diff --git a/be/src/gutil/strtoint.h b/be/src/gutil/strtoint.h
index 581ebf9..a010e04 100644
--- a/be/src/gutil/strtoint.h
+++ b/be/src/gutil/strtoint.h
@@ -30,12 +30,11 @@
#ifndef BASE_STRTOINT_H_
#define BASE_STRTOINT_H_
-#include <stdlib.h> // For strtol* functions.
+#include <cstdlib> // For strtol* functions.
#include <string>
-using std::string;
+
#include "gutil/integral_types.h"
#include "gutil/macros.h"
-#include "gutil/port.h"
// Adapter functions for handling overflow and errno.
int32 strto32_adapter(const char *nptr, char **endptr, int base);
@@ -82,11 +81,11 @@ inline int64 atoi64(const char *nptr) {
}
// Convenience versions of the above that take a string argument.
-inline int32 atoi32(const string &s) {
+inline int32 atoi32(const std::string& s) {
return atoi32(s.c_str());
}
-inline int64 atoi64(const string &s) {
+inline int64 atoi64(const std::string& s) {
return atoi64(s.c_str());
}
[impala] 03/03: IMPALA-11011: Impala crashes in
OrcStructReader::NumElements()
Posted by wz...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
wzhou pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
commit cb0018e679f4b48e98d20590679bcd157ef36ee2
Author: Daniel Becker <da...@cloudera.com>
AuthorDate: Tue Nov 9 17:21:06 2021 +0100
IMPALA-11011: Impala crashes in OrcStructReader::NumElements()
Running the query
select inner_arr.ITEM
from functional_orc_def.complextypestbl_non_transactional.nested_struct.c.d.ITEM
as inner_arr;
crashes Impala because in OrcStructReader::NumElements() 'vbatch_' is
NULL and we dereference it.
This commit adds a NULL check and if 'vbatch_' is NULL, NumElements()
returns 0.
Testing:
- added a regression test in
'testdata/workloads/functional-query/queries/QueryTest/struct-in-select-list.test'
that runs the above query.
Change-Id: I19cea7afdd1b3542a20a81b9f212fa320f3c1427
Reviewed-on: http://gerrit.cloudera.org:8080/18007
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
be/src/exec/orc-column-readers.h | 4 +++-
.../queries/QueryTest/struct-in-select-list.test | 21 +++++++++++++++++++++
2 files changed, 24 insertions(+), 1 deletion(-)
diff --git a/be/src/exec/orc-column-readers.h b/be/src/exec/orc-column-readers.h
index ea87c6a..2dd5663 100644
--- a/be/src/exec/orc-column-readers.h
+++ b/be/src/exec/orc-column-readers.h
@@ -600,7 +600,9 @@ class OrcStructReader : public OrcComplexColumnReader {
int GetChildBatchOffset(int row_idx) const override { return row_idx; }
int NumElements() const final {
- if (MaterializeTuple()) return vbatch_->numElements;
+ if (MaterializeTuple()) {
+ return vbatch_ ? vbatch_->numElements : 0;
+ }
DCHECK_EQ(children().size(), 1);
OrcColumnReader* child = children()[0];
return child->NumElements();
diff --git a/testdata/workloads/functional-query/queries/QueryTest/struct-in-select-list.test b/testdata/workloads/functional-query/queries/QueryTest/struct-in-select-list.test
index 4e9327a..ec290c8 100644
--- a/testdata/workloads/functional-query/queries/QueryTest/struct-in-select-list.test
+++ b/testdata/workloads/functional-query/queries/QueryTest/struct-in-select-list.test
@@ -288,6 +288,27 @@ from functional_orc_def.complextypestbl.nested_struct.c.d.ITEM as inner_arr;
STRING,INT,STRING
====
---- QUERY
+# Similar to the above, but on a non-transactional version of the table.
+# Regression test for IMPALA-11011.
+select inner_arr.ITEM
+from functional_orc_def.complextypestbl_non_transactional.nested_struct.c.d.ITEM as inner_arr;
+---- RESULTS
+'{"e":-1,"f":"nonnullable"}'
+'{"e":10,"f":"aaa"}'
+'{"e":-10,"f":"bbb"}'
+'{"e":11,"f":"c"}'
+'{"e":null,"f":null}'
+'{"e":10,"f":"aaa"}'
+'{"e":null,"f":null}'
+'{"e":-10,"f":"bbb"}'
+'{"e":null,"f":null}'
+'{"e":11,"f":"c"}'
+'NULL'
+'NULL'
+---- TYPES
+STRING
+====
+---- QUERY
# Querying a struct that is inside a nested array. Referencing the inner array through a
# join with the base table.
select tbl.id, inner_arr.ITEM
[impala] 01/03: IMPALA-10943: Add test to verify support for
multiple resource and executor pools
Posted by wz...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
wzhou pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
commit 4d97895c6c22c4802cf97187bd4fe1e69ebc1c12
Author: Bikramjeet Vig <bi...@gmail.com>
AuthorDate: Thu Sep 30 18:04:08 2021 -0700
IMPALA-10943: Add test to verify support for multiple resource and
executor pools
This patch adds a test to verify that admission control accounting
works when using multiple coordinators and multiple executor groups
mapped to different resource pools and having different sizes.
Change-Id: If76d386d8de5730da937674ddd9a69aa1aa1355e
Reviewed-on: http://gerrit.cloudera.org:8080/17891
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
tests/custom_cluster/test_executor_groups.py | 93 +++++++++++++++++++++++++---
1 file changed, 86 insertions(+), 7 deletions(-)
diff --git a/tests/custom_cluster/test_executor_groups.py b/tests/custom_cluster/test_executor_groups.py
index 750ff0a..d89cd76 100644
--- a/tests/custom_cluster/test_executor_groups.py
+++ b/tests/custom_cluster/test_executor_groups.py
@@ -22,6 +22,7 @@ from tests.util.concurrent_workload import ConcurrentWorkload
import json
import logging
+import os
import pytest
from time import sleep
@@ -30,6 +31,8 @@ LOG = logging.getLogger("test_auto_scaling")
# Non-trivial query that gets scheduled on all executors within a group.
TEST_QUERY = "select count(*) from functional.alltypes where month + random() < 3"
+DEFAULT_RESOURCE_POOL = "default-pool"
+
class TestExecutorGroups(CustomClusterTestSuite):
"""This class contains tests that exercise the logic related to scaling clusters up and
down by adding and removing groups of executors. All tests start with a base cluster
@@ -47,13 +50,14 @@ class TestExecutorGroups(CustomClusterTestSuite):
super(TestExecutorGroups, self).setup_method(method)
self.coordinator = self.cluster.impalads[0]
- def _group_name(self, name):
+ def _group_name(self, resource_pool, name_suffix):
# By convention, group names must start with their associated resource pool name
- # followed by a "-". Tests in this class all use the default resource pool.
- return "default-pool-%s" % name
+ # followed by a "-". Tests in this class mostly use the default resource pool.
+ return "%s-%s" % (resource_pool, name_suffix)
def _add_executor_group(self, name_suffix, min_size, num_executors=0,
- admission_control_slots=0, extra_args=None):
+ admission_control_slots=0, extra_args=None,
+ resource_pool=DEFAULT_RESOURCE_POOL):
"""Adds an executor group to the cluster. 'min_size' specifies the minimum size for
the new group to be considered healthy. 'num_executors' specifies the number of
executors to start and defaults to 'min_size' but can be different from 'min_size' to
@@ -64,7 +68,7 @@ class TestExecutorGroups(CustomClusterTestSuite):
if num_executors == 0:
num_executors = min_size
self.num_impalads += num_executors
- name = self._group_name(name_suffix)
+ name = self._group_name(resource_pool, name_suffix)
LOG.info("Adding %s executors to group %s with minimum size %s" %
(num_executors, name, min_size))
cluster_args = ["--impalad_args=-admission_control_slots=%s" %
@@ -131,7 +135,7 @@ class TestExecutorGroups(CustomClusterTestSuite):
None is returned if the group has no executors or does not exist."""
METRIC_PREFIX = "admission-controller.executor-group.num-queries-executing.{0}"
return self.coordinator.service.get_metric_value(
- METRIC_PREFIX.format(self._group_name(group_name_suffix)))
+ METRIC_PREFIX.format(self._group_name(DEFAULT_RESOURCE_POOL, group_name_suffix)))
def _assert_eventually_in_profile(self, query_handle, expected_str):
"""Assert with a timeout of 60 sec and a polling interval of 1 sec that the
@@ -456,7 +460,7 @@ class TestExecutorGroups(CustomClusterTestSuite):
client.cancel(q3)
self.coordinator.service.wait_for_metric_value(
"admission-controller.executor-group.num-queries-executing.{0}".format(
- self._group_name(group_names[0])), 0, timeout=30)
+ self._group_name(DEFAULT_RESOURCE_POOL, group_names[0])), 0, timeout=30)
@pytest.mark.execute_serially
def test_join_strategy_single_executor(self):
@@ -584,3 +588,78 @@ class TestExecutorGroups(CustomClusterTestSuite):
assert "queue reason: Not enough memory available on host" in profile, profile
self.close_query(handle_for_first)
second_coord_client.close_query(handle_for_second)
+
+ @pytest.mark.execute_serially
+ def test_admission_control_with_multiple_coords_and_exec_groups(self):
+ """This test verifies that admission control accounting works when using multiple
+ coordinators and multiple executor groups mapped to different resource pools and
+ having different sizes."""
+ # A long running query that runs on every executor
+ LONG_QUERY = "select * from functional_parquet.alltypes \
+ where month < 3 and id + random() < sleep(100);"
+ # The path to resources directory which contains the admission control config files.
+ RESOURCES_DIR = os.path.join(os.environ['IMPALA_HOME'], "fe", "src", "test",
+ "resources")
+ fs_allocation_path = os.path.join(RESOURCES_DIR, "fair-scheduler-allocation.xml")
+ llama_site_path = os.path.join(RESOURCES_DIR, "llama-site-empty.xml")
+ # Start with a regular admission config with multiple pools and no resource limits.
+ self._restart_coordinators(num_coordinators=2,
+ extra_args="-vmodule admission-controller=3 "
+ "-fair_scheduler_allocation_path %s "
+ "-llama_site_path %s" % (
+ fs_allocation_path, llama_site_path))
+
+ # Create fresh clients
+ second_coord_client = self.create_client_for_nth_impalad(1)
+ self.create_impala_clients()
+ # Add an exec group with a single admission slot and 2 executors.
+ self._add_executor_group("group", 2, admission_control_slots=1,
+ resource_pool="root.queue1", extra_args="-mem_limit=2g")
+ # Add an exec group with a single admission slot and only 1 executor.
+ self._add_executor_group("group", 1, admission_control_slots=1,
+ resource_pool="root.queue2", extra_args="-mem_limit=2g")
+ assert self._get_num_executor_groups(only_healthy=True) == 2
+
+ # Execute a long running query on group 'queue1'
+ self.client.set_configuration({'request_pool': 'queue1'})
+ handle_long_running_queue1 = self.execute_query_async(LONG_QUERY)
+ self.coordinator.service.wait_for_metric_value(
+ "admission-controller.executor-group.num-queries-executing.root.queue1-group",
+ 1, timeout=30)
+ profile = self.client.get_runtime_profile(handle_long_running_queue1)
+ "Executor Group: root.queue1-group" in profile
+
+ # Try to execute another query on group 'queue1'. This one should queue.
+ handle_queued_query_queue1 = self.execute_query_async(TEST_QUERY)
+ self.coordinator.service.wait_for_metric_value(
+ "admission-controller.local-num-queued.root.queue1", 1, timeout=30)
+ profile = self.client.get_runtime_profile(handle_queued_query_queue1)
+ assert "queue reason: Not enough admission control slots available on host" in \
+ profile, profile
+
+ # Execute a query on group 'queue2'. This one will run as its running in another pool.
+ result = self.execute_query_expect_success(self.client, TEST_QUERY,
+ query_options={'request_pool': 'queue2'})
+ assert "Executor Group: root.queue2-group" in str(result.runtime_profile)
+
+ # Verify that multiple coordinators' accounting still works correctly in case of
+ # multiple executor groups.
+
+ # Run a query in group 'queue2' on the second coordinator
+ second_coord_client.set_configuration({'request_pool': 'queue2'})
+ second_coord_client.execute_async(LONG_QUERY)
+ # Verify that the first coordinator knows about the query running on the second
+ self.coordinator.service.wait_for_metric_value(
+ "admission-controller.agg-num-running.root.queue2", 1, timeout=30)
+
+ # Check that attempting to run another query in 'queue2' will queue the query.
+ self.client.set_configuration({'request_pool': 'queue2'})
+ handle_queued_query_queue2 = self.execute_query_async(TEST_QUERY)
+ self.coordinator.service.wait_for_metric_value(
+ "admission-controller.local-num-queued.root.queue2", 1, timeout=30)
+ profile = self.client.get_runtime_profile(handle_queued_query_queue2)
+ assert "queue reason: Not enough admission control slots available on host" in \
+ profile, profile
+
+ self.client.close()
+ second_coord_client.close()