GEODE-8756: Fix CacheableString::objectSize (#703)
- objectSize was not taking into account null-terminator character,
which as of C++11, as stated in ยง 21.4.7.1, should be included at
the end of the same string character sequence.
- Also, in those cases in which SSO applied, the returned size was
higher than the actual one.
- A parametrized UT was added to check objectSize is returning the
right size. Note that no fixed sizes are part of such test, given
that each std::string implementation could have a different SSO
size.
* Revision 2
- Added a new class called size_tracking_allocator, which is a custom
allocator used to track the size STL objects.
- Changed testing approach so it does not replicate internal logic, and
instead it instantiates a basic_string<char> with the new custom
allocator so heap size is tracked this way.diff --git a/cppcache/src/CacheableString.cpp b/cppcache/src/CacheableString.cpp
index 466566e..d51f3f7 100644
--- a/cppcache/src/CacheableString.cpp
+++ b/cppcache/src/CacheableString.cpp
@@ -119,8 +119,18 @@
}
size_t CacheableString::objectSize() const {
- auto size = sizeof(CacheableString) +
- sizeof(std::string::value_type) * m_str.capacity();
+ auto size = sizeof(CacheableString);
+
+ // This is calculated in order not to count more bytes than necessary
+ // whenever SSO applies.
+ auto delta = reinterpret_cast<const uint8_t*>(m_str.data()) -
+ reinterpret_cast<const uint8_t*>(&m_str);
+ if (delta >= static_cast<decltype(delta)>(sizeof(decltype(m_str))) ||
+ delta < 0) {
+ // Add an extra character for the null-terminator
+ size += sizeof(decltype(m_str)::value_type) * (m_str.capacity() + 1UL);
+ }
+
return size;
}
diff --git a/cppcache/src/util/size_tracking_allocator.hpp b/cppcache/src/util/size_tracking_allocator.hpp
new file mode 100644
index 0000000..e64b8e3
--- /dev/null
+++ b/cppcache/src/util/size_tracking_allocator.hpp
@@ -0,0 +1,110 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#pragma once
+
+#ifndef GEODE_UTIL_SIZE_TRACKING_ALLOCATOR_H_
+#define GEODE_UTIL_SIZE_TRACKING_ALLOCATOR_H_
+
+#include <functional>
+#include <mutex>
+
+namespace apache {
+namespace geode {
+namespace client {
+
+template <typename _Tp>
+class size_tracking_allocator {
+ public:
+ // typedefs
+ using value_type = _Tp;
+ using pointer = value_type *;
+ using size_type = std::size_t;
+ using reference = value_type &;
+ using difference_type = std::ptrdiff_t;
+ using const_pointer = const value_type *;
+ using const_reference = const value_type &;
+ using allocate_callback = std::function<void(difference_type)>;
+
+ public:
+ template <typename U>
+ class rebind {
+ public:
+ using other = size_tracking_allocator<U>;
+ };
+
+ public:
+ explicit size_tracking_allocator() = default;
+ explicit size_tracking_allocator(const allocate_callback &cb)
+ : allocate_cb_{cb} {}
+
+ size_tracking_allocator(const size_tracking_allocator &other)
+ : allocate_cb_{other.allocate_cb_} {}
+
+ template <class U>
+ explicit size_tracking_allocator(const size_tracking_allocator<U> &other)
+ : allocate_cb_{other.allocate_cb_} {}
+
+ // memory allocation
+ pointer allocate(size_type n, const void * = nullptr) {
+ auto size = n * sizeof(value_type);
+ pointer p = reinterpret_cast<pointer>(::operator new(size));
+ if (allocate_cb_) {
+ allocate_cb_(size);
+ }
+
+ return p;
+ }
+
+ void deallocate(pointer pAddress, size_type n) {
+ ::operator delete(pAddress);
+ if (allocate_cb_) {
+ allocate_cb_(-sizeof(value_type) * n);
+ }
+ }
+
+ // size
+ size_type max_size() const {
+ return std::numeric_limits<size_type>::max() / sizeof(_Tp);
+ }
+
+ // construction/destruction
+ void construct(pointer address, const value_type &object) {
+ new (address) value_type(object);
+ }
+
+ void destroy(pointer object) { object->~value_type(); }
+
+ bool operator==(size_tracking_allocator const &) { return true; }
+
+ bool operator!=(size_tracking_allocator const &other) {
+ return !operator==(other);
+ }
+
+ protected:
+ allocate_callback allocate_cb_;
+
+ private:
+ template <class U>
+ friend class size_tracking_allocator;
+};
+
+} // namespace client
+} // namespace geode
+} // namespace apache
+
+#endif // GEODE_UTIL_SIZE_TRACKING_ALLOCATOR_H_
diff --git a/cppcache/test/CacheableStringTests.cpp b/cppcache/test/CacheableStringTests.cpp
index c4a15fe..6d2979f 100644
--- a/cppcache/test/CacheableStringTests.cpp
+++ b/cppcache/test/CacheableStringTests.cpp
@@ -30,6 +30,7 @@
#include "DataOutputInternal.hpp"
#include "SerializationRegistry.hpp"
#include "gtest_extensions.h"
+#include "util/size_tracking_allocator.hpp"
namespace {
@@ -40,6 +41,8 @@
using apache::geode::client::DataOutputInternal;
using apache::geode::client::SerializationRegistry;
+using apache::geode::client::size_tracking_allocator;
+
class TestDataOutput : public DataOutputInternal {
public:
TestDataOutput()
@@ -224,4 +227,25 @@
EXPECT_EQ(utf8, str->value());
}
+class CacheableStringSizeTests : public ::testing::TestWithParam<std::size_t> {
+};
+
+INSTANTIATE_TEST_SUITE_P(CacheableStringTests, CacheableStringSizeTests,
+ ::testing::Values(0UL, 1UL, 3UL, 7UL, 15UL, 23UL, 31UL,
+ 47UL, 63UL, 1025UL, 4097UL));
+
+TEST_P(CacheableStringSizeTests, TestObjectSize) {
+ auto str_size = GetParam();
+ auto expected_size = sizeof(CacheableString);
+ size_tracking_allocator<char> allocator{
+ [&expected_size](std::ptrdiff_t size) { expected_size += size; }};
+ std::basic_string<char, std::char_traits<char>, size_tracking_allocator<char>>
+ str(str_size, '_', allocator);
+
+ auto cacheable = CacheableString::create(std::string(str_size, '_'));
+ EXPECT_TRUE(cacheable);
+
+ EXPECT_EQ(expected_size, cacheable->objectSize());
+}
+
} // namespace
