From f8a90353a393b3ec6e0aa143b99a28bb8519f392 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B8rgen=20Edelbo?= Date: Wed, 11 Jan 2023 13:09:43 +0100 Subject: [PATCH 1/4] Optimize size of ArrayDecimal128 --- CHANGELOG.md | 2 +- Package.swift | 1 - .../IntelRDFPMathLib20U2/CMakeLists.txt | 1 + src/realm/array_decimal128.cpp | 247 ++++++++++++++++-- src/realm/array_decimal128.hpp | 20 +- src/realm/decimal128.cpp | 90 ++++++- src/realm/decimal128.hpp | 16 +- test/test_decimal128.cpp | 63 ++++- 8 files changed, 412 insertions(+), 28 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0bf56984bf1..fd547d1085c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ ### Enhancements * (PR [#????](https://github.com/realm/realm-core/pull/????)) -* None. +* Storage of Decimal128 properties has been optimised so that the individual values will take up 0 bits (if all nulls), 32 bits, 64 bits or 128 bits depending on what is needed. (PR [#6111]https://github.com/realm/realm-core/pull/6111)) ### Fixed * ([#????](https://github.com/realm/realm-core/issues/????), since v?.?.?) diff --git a/Package.swift b/Package.swift index 1611fdaee4e..10bee1897fe 100644 --- a/Package.swift +++ b/Package.swift @@ -253,7 +253,6 @@ let bidExcludes: [String] = [ "bid32_tan.c", "bid32_tanh.c", "bid32_tgamma.c", - "bid32_to_bid128.c", "bid32_to_bid64.c", "bid32_to_int16.c", "bid32_to_int32.c", diff --git a/src/external/IntelRDFPMathLib20U2/CMakeLists.txt b/src/external/IntelRDFPMathLib20U2/CMakeLists.txt index b0536af9f9e..d8f6c508f7e 100644 --- a/src/external/IntelRDFPMathLib20U2/CMakeLists.txt +++ b/src/external/IntelRDFPMathLib20U2/CMakeLists.txt @@ -7,6 +7,7 @@ LIBRARY/src/bid128_add.c LIBRARY/src/bid128_fma.c LIBRARY/src/bid128_string.c LIBRARY/src/bid128_2_str_tables.c +LIBRARY/src/bid32_to_bid128.c LIBRARY/src/bid64_to_bid128.c LIBRARY/src/bid128_to_int64.c LIBRARY/src/bid128_quantize.c diff --git a/src/realm/array_decimal128.cpp b/src/realm/array_decimal128.cpp index abfc2c42754..58150265d17 100644 --- a/src/realm/array_decimal128.cpp +++ b/src/realm/array_decimal128.cpp @@ -21,40 +21,127 @@ namespace realm { +namespace { + +uint8_t min_width(const Decimal128& value) +{ + Decimal128::Bid128 coefficient; + int exponent; + bool sign; + + if (value.is_null()) { + return 0; + } + + value.unpack(coefficient, exponent, sign); + if (coefficient.w[1] == 0) { + if (coefficient.w[0] < (1ull << 23) && exponent > -95 && exponent < 96) { + return 4; + } + if (coefficient.w[0] < (1ull << 53) && exponent > -383 && exponent < 384) { + return 8; + } + } + return 16; +} +} // namespace + void ArrayDecimal128::set(size_t ndx, Decimal128 value) { REALM_ASSERT(ndx < m_size); copy_on_write(); - auto values = reinterpret_cast(m_data); - values[ndx] = value; + switch (upgrade_leaf(min_width(value))) { + case 0: + break; + case 4: { + auto values = reinterpret_cast(m_data); + auto val = value.to_bid32(); + REALM_ASSERT(val); + values[ndx] = *val; + break; + } + case 8: { + auto values = reinterpret_cast(m_data); + auto val = value.to_bid64(); + REALM_ASSERT(val); + values[ndx] = *val; + break; + } + case 16: { + auto values = reinterpret_cast(m_data); + values[ndx] = value; + break; + } + } } void ArrayDecimal128::insert(size_t ndx, Decimal128 value) { REALM_ASSERT(ndx <= m_size); // Allocate room for the new value - alloc(m_size + 1, sizeof(Decimal128)); // Throws + switch (upgrade_leaf(min_width(value))) { + case 0: + m_size += 1; + Array::copy_on_write(); + set_header_size(m_size); + break; + case 4: { + alloc(m_size + 1, 4); // Throws - auto src = reinterpret_cast(m_data) + ndx; - auto dst = src + 1; + auto src = reinterpret_cast(m_data) + ndx; + auto dst = src + 1; - // Make gap for new value - memmove(dst, src, sizeof(Decimal128) * (m_size - 1 - ndx)); + // Make gap for new value + memmove(dst, src, sizeof(Decimal::Bid32) * (m_size - 1 - ndx)); - // Set new value - *src = value; + // Set new value + auto val = value.to_bid32(); + REALM_ASSERT(val); + *src = *val; + break; + } + case 8: { + alloc(m_size + 1, 8); // Throws + + auto src = reinterpret_cast(m_data) + ndx; + auto dst = src + 1; + + // Make gap for new value + memmove(dst, src, sizeof(Decimal::Bid64) * (m_size - 1 - ndx)); + + // Set new value + auto val = value.to_bid64(); + REALM_ASSERT(val); + *src = *val; + break; + } + case 16: { + alloc(m_size + 1, sizeof(Decimal128)); // Throws + + auto src = reinterpret_cast(m_data) + ndx; + auto dst = src + 1; + + // Make gap for new value + memmove(dst, src, sizeof(Decimal128) * (m_size - 1 - ndx)); + + // Set new value + *src = value; + break; + } + } } void ArrayDecimal128::erase(size_t ndx) { + REALM_ASSERT(ndx < m_size); copy_on_write(); - Decimal128* dst = reinterpret_cast(m_data) + ndx; - Decimal128* src = dst + 1; - - memmove(dst, src, sizeof(Decimal128) * (m_size - ndx)); + if (m_width) { + auto dst = m_data + ndx * m_width; + memmove(dst, dst + m_width, m_width * (m_size - ndx)); + } // Update size (also in header) m_size -= 1; @@ -65,10 +152,16 @@ void ArrayDecimal128::move(ArrayDecimal128& dst_arr, size_t ndx) { size_t elements_to_move = m_size - ndx; if (elements_to_move) { + if (m_width >= dst_arr.m_width) { + dst_arr.upgrade_leaf(m_width); + } + else { + upgrade_leaf(dst_arr.m_width); + } const auto old_dst_size = dst_arr.m_size; - dst_arr.alloc(old_dst_size + elements_to_move, sizeof(Decimal128)); - Decimal128* dst = reinterpret_cast(dst_arr.m_data) + old_dst_size; - Decimal128* src = reinterpret_cast(m_data) + ndx; + dst_arr.alloc(old_dst_size + elements_to_move, m_width); + auto dst = dst_arr.m_data + old_dst_size * m_width; + auto src = m_data + ndx * m_width; memmove(dst, src, elements_to_move * sizeof(Decimal128)); } truncate(ndx); @@ -81,11 +174,45 @@ size_t ArrayDecimal128::find_first(Decimal128 value, size_t start, size_t end) c end = sz; REALM_ASSERT(start <= sz && end <= sz && start <= end); - auto values = reinterpret_cast(this->m_data); - for (size_t i = start; i < end; i++) { - if (values[i] == value) - return i; + auto width = min_width(value); + switch (m_width) { + case 0: + if (value.is_null()) { + return 0; + } + break; + case 4: + if (width <= 4) { + // Worth optimizing here + auto optval32 = value.to_bid32(); + REALM_ASSERT(optval32); + auto val32 = *optval32; + auto values = reinterpret_cast(this->m_data); + for (size_t i = start; i < end; i++) { + if (values[i] == val32) + return i; + } + } + break; + case 8: + if (width <= 8) { + auto values = reinterpret_cast(this->m_data); + for (size_t i = start; i < end; i++) { + if (Decimal128(values[i]) == value) + return i; + } + } + break; + case 16: { + auto values = reinterpret_cast(this->m_data); + for (size_t i = start; i < end; i++) { + if (values[i] == value) + return i; + } + break; + } } + return realm::npos; } @@ -95,4 +222,84 @@ Mixed ArrayDecimal128::get_any(size_t ndx) const } +size_t ArrayDecimal128::upgrade_leaf(uint8_t width) +{ + if (m_width == 16) { + return 16; + } + if (width <= m_width) { + return m_width; + } + + if (m_size == 0) { + alloc(m_size, width); + return width; + } + + if (m_width == 8) { + // Upgrade to 16 bytes + alloc(m_size, 16); + auto src = reinterpret_cast(m_data); + auto dst = reinterpret_cast(m_data); + for (size_t i = m_size; i > 0; --i) { + auto val = Decimal128(src[i - 1]); + dst[i - 1] = *val.raw(); + } + return 16; + } + + if (m_width == 4) { + alloc(m_size, width); + auto src = reinterpret_cast(m_data); + if (width == 8) { + // Upgrade to 8 bytes + auto dst = reinterpret_cast(m_data); + for (size_t i = m_size; i > 0; --i) { + auto val = Decimal128(src[i - 1]); + dst[i - 1] = *val.to_bid64(); + } + } + else if (width == 16) { + // Upgrade to 16 bytes + auto dst = reinterpret_cast(m_data); + for (size_t i = m_size; i > 0; --i) { + auto val = Decimal128(src[i - 1]); + dst[i - 1] = *val.raw(); + } + } + return width; + } + + // Width is zero + if (width == 4) { + // Upgrade to 4 bytes + alloc(m_size, 4); + auto values = reinterpret_cast(m_data); + auto zero = *Decimal128(realm::null()).to_bid32(); + for (size_t i = 0; i < m_size; i++) { + values[i] = zero; + } + return 4; + } + else if (width == 8) { + // Upgrade to 8 bytes + alloc(m_size, 8); + auto values = reinterpret_cast(m_data); + auto zero = *Decimal128(realm::null()).to_bid64(); + for (size_t i = 0; i < m_size; i++) { + values[i] = zero; + } + return 8; + } + + alloc(m_size, 16); + auto values = reinterpret_cast(m_data); + auto zero = Decimal128(realm::null()); + for (size_t i = 0; i < m_size; i++) { + values[i] = zero; + } + return 16; +} + + } // namespace realm diff --git a/src/realm/array_decimal128.hpp b/src/realm/array_decimal128.hpp index 28657314af5..a1a4d0b499d 100644 --- a/src/realm/array_decimal128.hpp +++ b/src/realm/array_decimal128.hpp @@ -67,8 +67,23 @@ class ArrayDecimal128 : public ArrayPayload, private Array { Decimal128 get(size_t ndx) const { REALM_ASSERT(ndx < m_size); - auto values = reinterpret_cast(this->m_data); - return values[ndx]; + switch (m_width) { + case 0: + return Decimal128(realm::null()); + case 4: { + auto values = reinterpret_cast(this->m_data); + return Decimal128(values[ndx]); + } + case 8: { + auto values = reinterpret_cast(this->m_data); + return Decimal128(values[ndx]); + } + case 16: { + auto values = reinterpret_cast(this->m_data); + return values[ndx]; + } + } + return {}; } Mixed get_any(size_t ndx) const override; @@ -99,6 +114,7 @@ class ArrayDecimal128 : public ArrayPayload, private Array { { return num_items * sizeof(Decimal128) + header_size; } + size_t upgrade_leaf(uint8_t width); }; } // namespace realm diff --git a/src/realm/decimal128.cpp b/src/realm/decimal128.cpp index af59828175c..bf6f630c370 100644 --- a/src/realm/decimal128.cpp +++ b/src/realm/decimal128.cpp @@ -1437,8 +1437,25 @@ Decimal128::Decimal128(uint64_t val) noexcept memcpy(this, &expanded, sizeof(*this)); } +Decimal128::Decimal128(Bid32 val) noexcept +{ + if (val.u == DECIMAL_NULL_32) { + m_value = DECIMAL_NULL_128; + return; + } + unsigned flags = 0; + BID_UINT32 x(val.u); + BID_UINT128 tmp; + bid32_to_bid128(&tmp, &x, &flags); + memcpy(this, &tmp, sizeof(*this)); +} + Decimal128::Decimal128(Bid64 val) noexcept { + if (val.w == DECIMAL_NULL_64) { + m_value = DECIMAL_NULL_128; + return; + } unsigned flags = 0; BID_UINT64 x(val.w); BID_UINT128 tmp; @@ -1730,14 +1747,31 @@ std::string Decimal128::to_string() const noexcept return ret; } -auto Decimal128::to_bid64() const -> Bid64 +auto Decimal128::to_bid32() const noexcept -> std::optional { + if (is_null()) { + return DECIMAL_NULL_32; + } + unsigned flags = 0; + BID_UINT32 buffer; + BID_UINT128 tmp = to_BID_UINT128(*this); + bid128_to_bid32(&buffer, &tmp, &flags); + if (flags & ~BID_INEXACT_EXCEPTION) + return {}; + return Bid32(buffer); +} + +auto Decimal128::to_bid64() const noexcept -> std::optional +{ + if (is_null()) { + return DECIMAL_NULL_64; + } unsigned flags = 0; BID_UINT64 buffer; BID_UINT128 tmp = to_BID_UINT128(*this); bid128_to_bid64(&buffer, &tmp, &flags); if (flags & ~BID_INEXACT_EXCEPTION) - throw std::overflow_error("Decimal128::to_bid64 failed"); + return {}; return Bid64(buffer); } @@ -1751,4 +1785,56 @@ void Decimal128::unpack(Bid128& coefficient, int& exponent, bool& sign) const no coefficient.w[1] = get_coefficient_high(); } +bool operator==(Decimal128::Bid32 lhs, Decimal128::Bid32 rhs) noexcept +{ + static constexpr int DECIMAL_COEFF_BITS_32 = 23; + static constexpr int DECIMAL_EXP_BITS_32 = 8; + static constexpr unsigned MASK_COEFF_32 = (1 << DECIMAL_COEFF_BITS_32) - 1; + static constexpr unsigned MASK_EXP_32 = ((1 << DECIMAL_EXP_BITS_32) - 1) << DECIMAL_COEFF_BITS_32; + static constexpr unsigned MASK_SIGN_32 = 1 << (DECIMAL_COEFF_BITS_32 + DECIMAL_EXP_BITS_32); + + uint32_t x = lhs.u; + uint32_t y = rhs.u; + + if (x == y) { + return true; + } + + unsigned sig_x = (x & MASK_COEFF_32); + unsigned sig_y = (y & MASK_COEFF_32); + bool x_is_zero = (sig_x == 0); + bool y_is_zero = (sig_y == 0); + + if (x_is_zero && y_is_zero) { + return true; + } + else if ((x_is_zero && !y_is_zero) || (!x_is_zero && y_is_zero)) { + return false; + } + + // Check if sign differs + if ((x ^ y) & MASK_SIGN_32) { + return false; + } + + int exp_x = (x & MASK_EXP_32) >> DECIMAL_COEFF_BITS_32; + int exp_y = (y & MASK_EXP_32) >> DECIMAL_COEFF_BITS_32; + + // Make exp_y biggest + if (exp_x > exp_y) { + std::swap(exp_x, exp_y); + std::swap(sig_x, sig_y); + } + if (exp_y - exp_x > 6) { + return false; + } + for (int lcv = 0; lcv < (exp_y - exp_x); lcv++) { + sig_y = sig_y * 10; + if (sig_y > 9999999) { + return false; + } + } + return (sig_y == sig_x); +} + } // namespace realm diff --git a/src/realm/decimal128.hpp b/src/realm/decimal128.hpp index ae7860f9920..274814882d2 100644 --- a/src/realm/decimal128.hpp +++ b/src/realm/decimal128.hpp @@ -35,6 +35,13 @@ class Decimal128 { // expected result. enum class RoundTo { Digits7 = 0, Digits15 = 1 }; + struct Bid32 { + Bid32(uint32_t x) + : u(x) + { + } + uint32_t u; + }; struct Bid64 { Bid64(uint64_t x) : w(x) @@ -55,6 +62,7 @@ class Decimal128 { { } Decimal128(Bid128 coefficient, int exponent, bool sign) noexcept; + explicit Decimal128(Bid32) noexcept; explicit Decimal128(Bid64) noexcept; explicit Decimal128(StringData) noexcept; explicit Decimal128(Bid128 val) noexcept @@ -111,7 +119,8 @@ class Decimal128 { } std::string to_string() const noexcept; - Bid64 to_bid64() const; + std::optional to_bid32() const noexcept; + std::optional to_bid64() const noexcept; const Bid128* raw() const noexcept { return &m_value; @@ -127,6 +136,9 @@ class Decimal128 { static constexpr int DECIMAL_EXPONENT_BIAS_128 = 6176; static constexpr int DECIMAL_COEFF_HIGH_BITS = 49; static constexpr int DECIMAL_EXP_BITS = 14; + static constexpr uint32_t DECIMAL_NULL_32 = 0x7c0000aa; + static constexpr uint64_t DECIMAL_NULL_64 = 0x7c000000000000aa; + static constexpr Bid128 DECIMAL_NULL_128 = {0xaa, 0x7c00000000000000}; static constexpr uint64_t MASK_COEFF = (1ull << DECIMAL_COEFF_HIGH_BITS) - 1; static constexpr uint64_t MASK_EXP = ((1ull << DECIMAL_EXP_BITS) - 1) << DECIMAL_COEFF_HIGH_BITS; static constexpr uint64_t MASK_SIGN = 1ull << (DECIMAL_COEFF_HIGH_BITS + DECIMAL_EXP_BITS); @@ -143,6 +155,8 @@ class Decimal128 { } }; +bool operator==(Decimal128::Bid32 lhs, Decimal128::Bid32 rhs) noexcept; + inline std::ostream& operator<<(std::ostream& ostr, const Decimal128& id) { ostr << id.to_string(); diff --git a/test/test_decimal128.cpp b/test/test_decimal128.cpp index 9844aa5e02e..89f6efc645f 100644 --- a/test/test_decimal128.cpp +++ b/test/test_decimal128.cpp @@ -191,12 +191,15 @@ TEST(Decimal_Array) arr.add(Decimal128(str0)); arr.add(Decimal128(str1)); arr.insert(1, Decimal128(str2)); + arr.add(Decimal128(realm::null())); Decimal128 id2(str2); CHECK_EQUAL(arr.get(0), Decimal128(str0)); CHECK_EQUAL(arr.get(1), id2); CHECK_EQUAL(arr.get(2), Decimal128(str1)); CHECK_EQUAL(arr.find_first(id2), 1); + CHECK_EQUAL(arr.find_first(Decimal128("1000")), 2); + CHECK_EQUAL(arr.find_first(Decimal128(realm::null())), 3); arr.erase(1); CHECK_EQUAL(arr.get(1), Decimal128(str1)); @@ -206,13 +209,71 @@ TEST(Decimal_Array) arr.move(arr1, 1); CHECK_EQUAL(arr.size(), 1); - CHECK_EQUAL(arr1.size(), 1); + CHECK_EQUAL(arr1.size(), 2); CHECK_EQUAL(arr1.get(0), Decimal128(str1)); + CHECK_EQUAL(arr1.get(1), Decimal128(realm::null())); + + arr.clear(); + CHECK_EQUAL(arr.size(), 0); + + arr.add(Decimal128(0)); + arr.add(Decimal128(realm::null())); + CHECK_NOT(arr.is_null(0)); + CHECK(arr.is_null(1)); arr.destroy(); arr1.destroy(); } +TEST(Decimal_ArrayUpdgrade) +{ + Decimal128 size_0{realm::null()}; + Decimal128 size_4{"100"}; + Decimal128 size_8{"123.456e100"}; + Decimal128 size_16{"3.141592653589793238462643"}; + + ArrayDecimal128 arr(Allocator::get_default()); + arr.create(); + + arr.add(size_0); + CHECK_EQUAL(arr.get(0), size_0); + arr.add(size_4); // 0 -> 4 + CHECK_EQUAL(arr.get(0), size_0); + CHECK_EQUAL(arr.get(1), size_4); + arr.add(size_8); // 4 -> 8 + CHECK_EQUAL(arr.get(0), size_0); + CHECK_EQUAL(arr.get(1), size_4); + CHECK_EQUAL(arr.get(2), size_8); + arr.add(size_16); // 8 -> 16 + CHECK_EQUAL(arr.get(0), size_0); + CHECK_EQUAL(arr.get(1), size_4); + CHECK_EQUAL(arr.get(2), size_8); + CHECK_EQUAL(arr.get(3), size_16); + + arr.clear(); + arr.add(size_4); + CHECK_EQUAL(arr.get(0), size_4); + arr.add(size_16); // 4 -> 16 + CHECK_EQUAL(arr.get(0), size_4); + CHECK_EQUAL(arr.get(1), size_16); + + arr.clear(); + arr.add(size_0); + CHECK_EQUAL(arr.get(0), size_0); + arr.add(size_8); // 0 -> 8 + CHECK_EQUAL(arr.get(0), size_0); + CHECK_EQUAL(arr.get(1), size_8); + + arr.clear(); + arr.add(size_0); + CHECK_EQUAL(arr.get(0), size_0); + arr.add(size_16); // 0 -> 16 + CHECK_EQUAL(arr.get(0), size_0); + CHECK_EQUAL(arr.get(1), size_16); + + arr.destroy(); +} + TEST(Decimal_Table) { const char str0[] = "12345.67"; From cfede136ae9f376101883cd9fe878d56e7a174fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B8rgen=20Edelbo?= Date: Fri, 13 Jan 2023 14:42:39 +0100 Subject: [PATCH 2/4] Update after review --- src/realm/array_decimal128.cpp | 18 +++++++++------- src/realm/array_decimal128.hpp | 7 ++++++- test/test_decimal128.cpp | 38 +++++++++++++++++++++++++++++++--- 3 files changed, 51 insertions(+), 12 deletions(-) diff --git a/src/realm/array_decimal128.cpp b/src/realm/array_decimal128.cpp index 58150265d17..90342ba4e9b 100644 --- a/src/realm/array_decimal128.cpp +++ b/src/realm/array_decimal128.cpp @@ -35,10 +35,10 @@ uint8_t min_width(const Decimal128& value) value.unpack(coefficient, exponent, sign); if (coefficient.w[1] == 0) { - if (coefficient.w[0] < (1ull << 23) && exponent > -95 && exponent < 96) { + if (coefficient.w[0] < (1ull << 23) && exponent > -91 && exponent < 91) { return 4; } - if (coefficient.w[0] < (1ull << 53) && exponent > -383 && exponent < 384) { + if (coefficient.w[0] < (1ull << 53) && exponent > -370 && exponent < 370) { return 8; } } @@ -154,15 +154,17 @@ void ArrayDecimal128::move(ArrayDecimal128& dst_arr, size_t ndx) if (elements_to_move) { if (m_width >= dst_arr.m_width) { dst_arr.upgrade_leaf(m_width); + const auto old_dst_size = dst_arr.m_size; + dst_arr.alloc(old_dst_size + elements_to_move, m_width); + auto dst = dst_arr.m_data + old_dst_size * m_width; + auto src = m_data + ndx * m_width; + memmove(dst, src, elements_to_move * m_width); } else { - upgrade_leaf(dst_arr.m_width); + for (size_t i = 0; i < elements_to_move; i++) { + dst_arr.add(get(ndx + i)); + } } - const auto old_dst_size = dst_arr.m_size; - dst_arr.alloc(old_dst_size + elements_to_move, m_width); - auto dst = dst_arr.m_data + old_dst_size * m_width; - auto src = m_data + ndx * m_width; - memmove(dst, src, elements_to_move * sizeof(Decimal128)); } truncate(ndx); } diff --git a/src/realm/array_decimal128.hpp b/src/realm/array_decimal128.hpp index a1a4d0b499d..95eb9152dad 100644 --- a/src/realm/array_decimal128.hpp +++ b/src/realm/array_decimal128.hpp @@ -61,7 +61,7 @@ class ArrayDecimal128 : public ArrayPayload, private Array { bool is_null(size_t ndx) const { - return this->get_width() == 0 || get(ndx).is_null(); + return m_width == 0 || get(ndx).is_null(); } Decimal128 get(size_t ndx) const @@ -109,6 +109,11 @@ class ArrayDecimal128 : public ArrayPayload, private Array { size_t find_first(Decimal128 value, size_t begin = 0, size_t end = npos) const noexcept; + uint8_t get_width() const noexcept + { + return m_width; + } + protected: size_t calc_byte_len(size_t num_items, size_t) const override { diff --git a/test/test_decimal128.cpp b/test/test_decimal128.cpp index 89f6efc645f..0aac979a0f2 100644 --- a/test/test_decimal128.cpp +++ b/test/test_decimal128.cpp @@ -184,6 +184,7 @@ TEST(Decimal_Array) const char str0[] = "12345.67"; const char str1[] = "1000.00"; const char str2[] = "-45"; + const char str3[] = "123.456e100"; ArrayDecimal128 arr(Allocator::get_default()); arr.create(); @@ -213,6 +214,17 @@ TEST(Decimal_Array) CHECK_EQUAL(arr1.get(0), Decimal128(str1)); CHECK_EQUAL(arr1.get(1), Decimal128(realm::null())); + arr.add(Decimal128(str3)); // size 8 + arr1.move(arr, 1); + + CHECK_EQUAL(arr.size(), 3); + CHECK_EQUAL(arr1.size(), 1); + CHECK_EQUAL(arr.get(0), Decimal128(str0)); + CHECK_EQUAL(arr.get(1), Decimal128(str3)); + CHECK_EQUAL(arr.get(2), Decimal128(realm::null())); + CHECK_EQUAL(arr1.get(0), Decimal128(str1)); + CHECK_EQUAL(arr.find_first(Decimal128("123.456000e100")), 1); + arr.clear(); CHECK_EQUAL(arr.size(), 0); @@ -229,7 +241,11 @@ TEST(Decimal_ArrayUpdgrade) { Decimal128 size_0{realm::null()}; Decimal128 size_4{"100"}; + Decimal128 large_size_4("8388607e90"); + Decimal128 small_size_4("8388607e-90"); Decimal128 size_8{"123.456e100"}; + Decimal128 large_size_8("9007199254740991e369"); + Decimal128 small_size_8("9007199254740991e-369"); Decimal128 size_16{"3.141592653589793238462643"}; ArrayDecimal128 arr(Allocator::get_default()); @@ -238,17 +254,33 @@ TEST(Decimal_ArrayUpdgrade) arr.add(size_0); CHECK_EQUAL(arr.get(0), size_0); arr.add(size_4); // 0 -> 4 + arr.add(large_size_4); + arr.add(small_size_4); + CHECK_EQUAL(arr.get_width(), 4); CHECK_EQUAL(arr.get(0), size_0); CHECK_EQUAL(arr.get(1), size_4); + CHECK_EQUAL(arr.get(2), large_size_4); + CHECK_EQUAL(arr.get(3), small_size_4); arr.add(size_8); // 4 -> 8 + arr.add(large_size_8); + arr.add(small_size_8); + CHECK_EQUAL(arr.get_width(), 8); CHECK_EQUAL(arr.get(0), size_0); CHECK_EQUAL(arr.get(1), size_4); - CHECK_EQUAL(arr.get(2), size_8); + CHECK_EQUAL(arr.get(2), large_size_4); + CHECK_EQUAL(arr.get(3), small_size_4); + CHECK_EQUAL(arr.get(4), size_8); + CHECK_EQUAL(arr.get(5), large_size_8); + CHECK_EQUAL(arr.get(6), small_size_8); arr.add(size_16); // 8 -> 16 CHECK_EQUAL(arr.get(0), size_0); CHECK_EQUAL(arr.get(1), size_4); - CHECK_EQUAL(arr.get(2), size_8); - CHECK_EQUAL(arr.get(3), size_16); + CHECK_EQUAL(arr.get(2), large_size_4); + CHECK_EQUAL(arr.get(3), small_size_4); + CHECK_EQUAL(arr.get(4), size_8); + CHECK_EQUAL(arr.get(5), large_size_8); + CHECK_EQUAL(arr.get(6), small_size_8); + CHECK_EQUAL(arr.get(7), size_16); arr.clear(); arr.add(size_4); From 6f518011d329d8b5cd11c298b3e7c20447aa19e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B8rgen=20Edelbo?= Date: Fri, 20 Jan 2023 11:20:38 +0100 Subject: [PATCH 3/4] Adaptive interpretation of zero width --- src/realm/array_decimal128.cpp | 45 +++++++++++++++++++++++----------- src/realm/array_decimal128.hpp | 2 +- test/test_decimal128.cpp | 17 +++++++++++++ 3 files changed, 49 insertions(+), 15 deletions(-) diff --git a/src/realm/array_decimal128.cpp b/src/realm/array_decimal128.cpp index 90342ba4e9b..39b26d66f74 100644 --- a/src/realm/array_decimal128.cpp +++ b/src/realm/array_decimal128.cpp @@ -23,18 +23,21 @@ namespace realm { namespace { -uint8_t min_width(const Decimal128& value) +uint8_t min_width(const Decimal128& value, bool zero_width_is_zero) { Decimal128::Bid128 coefficient; int exponent; bool sign; if (value.is_null()) { - return 0; + return zero_width_is_zero ? 4 : 0; } value.unpack(coefficient, exponent, sign); if (coefficient.w[1] == 0) { + if (coefficient.w[0] == 0) { + return zero_width_is_zero ? 0 : 4; + } if (coefficient.w[0] < (1ull << 23) && exponent > -91 && exponent < 91) { return 4; } @@ -50,7 +53,7 @@ void ArrayDecimal128::set(size_t ndx, Decimal128 value) { REALM_ASSERT(ndx < m_size); copy_on_write(); - switch (upgrade_leaf(min_width(value))) { + switch (upgrade_leaf(min_width(value, Array::get_context_flag()))) { case 0: break; case 4: { @@ -78,8 +81,13 @@ void ArrayDecimal128::set(size_t ndx, Decimal128 value) void ArrayDecimal128::insert(size_t ndx, Decimal128 value) { REALM_ASSERT(ndx <= m_size); + if (m_size == 0 && value == Decimal128()) { + // zero width should be interpreted as 0 + Array::copy_on_write(); + Array::set_context_flag(true); + } // Allocate room for the new value - switch (upgrade_leaf(min_width(value))) { + switch (upgrade_leaf(min_width(value, Array::get_context_flag()))) { case 0: m_size += 1; Array::copy_on_write(); @@ -176,11 +184,19 @@ size_t ArrayDecimal128::find_first(Decimal128 value, size_t start, size_t end) c end = sz; REALM_ASSERT(start <= sz && end <= sz && start <= end); - auto width = min_width(value); + bool zero_width_is_zero = Array::get_context_flag(); + auto width = min_width(value, zero_width_is_zero); switch (m_width) { case 0: - if (value.is_null()) { - return 0; + if (zero_width_is_zero) { + if (value == Decimal128()) { + return 0; + } + } + else { + if (value.is_null()) { + return 0; + } } break; case 4: @@ -272,14 +288,16 @@ size_t ArrayDecimal128::upgrade_leaf(uint8_t width) return width; } - // Width is zero + // Upgrade from zero width. Fill with either 0 or null. + Decimal128 fill_value = get_context_flag() ? Decimal128(0) : Decimal128(realm::null()); + if (width == 4) { // Upgrade to 4 bytes alloc(m_size, 4); auto values = reinterpret_cast(m_data); - auto zero = *Decimal128(realm::null()).to_bid32(); + auto fill = *fill_value.to_bid32(); for (size_t i = 0; i < m_size; i++) { - values[i] = zero; + values[i] = fill; } return 4; } @@ -287,18 +305,17 @@ size_t ArrayDecimal128::upgrade_leaf(uint8_t width) // Upgrade to 8 bytes alloc(m_size, 8); auto values = reinterpret_cast(m_data); - auto zero = *Decimal128(realm::null()).to_bid64(); + auto fill = *fill_value.to_bid64(); for (size_t i = 0; i < m_size; i++) { - values[i] = zero; + values[i] = fill; } return 8; } alloc(m_size, 16); auto values = reinterpret_cast(m_data); - auto zero = Decimal128(realm::null()); for (size_t i = 0; i < m_size; i++) { - values[i] = zero; + values[i] = fill_value; } return 16; } diff --git a/src/realm/array_decimal128.hpp b/src/realm/array_decimal128.hpp index 95eb9152dad..5b6d5573cc6 100644 --- a/src/realm/array_decimal128.hpp +++ b/src/realm/array_decimal128.hpp @@ -69,7 +69,7 @@ class ArrayDecimal128 : public ArrayPayload, private Array { REALM_ASSERT(ndx < m_size); switch (m_width) { case 0: - return Decimal128(realm::null()); + return get_context_flag() ? Decimal128() : Decimal128(realm::null()); case 4: { auto values = reinterpret_cast(this->m_data); return Decimal128(values[ndx]); diff --git a/test/test_decimal128.cpp b/test/test_decimal128.cpp index 0aac979a0f2..e6ac946167e 100644 --- a/test/test_decimal128.cpp +++ b/test/test_decimal128.cpp @@ -188,7 +188,24 @@ TEST(Decimal_Array) ArrayDecimal128 arr(Allocator::get_default()); arr.create(); + arr.add(Decimal128(realm::null())); + CHECK_EQUAL(arr.get_width(), 0); + CHECK_EQUAL(arr.get(0), Decimal128(realm::null())); + arr.add(Decimal128()); + CHECK_EQUAL(arr.get_width(), 4); + CHECK_EQUAL(arr.get(0), Decimal128(realm::null())); + CHECK_EQUAL(arr.get(1), Decimal128()); + arr.clear(); + arr.add(Decimal128()); + CHECK_EQUAL(arr.get_width(), 0); + CHECK_EQUAL(arr.get(0), Decimal128()); + arr.add(Decimal128(realm::null())); + CHECK_EQUAL(arr.get_width(), 4); + CHECK_EQUAL(arr.get(0), Decimal128()); + CHECK_EQUAL(arr.get(1), Decimal128(realm::null())); + + arr.clear(); arr.add(Decimal128(str0)); arr.add(Decimal128(str1)); arr.insert(1, Decimal128(str2)); From 48756f3dd56e830c59f7fa30a3e0bb7182f4ee75 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B8rgen=20Edelbo?= Date: Wed, 1 Feb 2023 16:14:52 +0100 Subject: [PATCH 4/4] Fix ArrayDecimal128::is_null() --- src/realm/array_decimal128.hpp | 5 ++++- test/test_decimal128.cpp | 6 ++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/src/realm/array_decimal128.hpp b/src/realm/array_decimal128.hpp index 5b6d5573cc6..07d3dc332bf 100644 --- a/src/realm/array_decimal128.hpp +++ b/src/realm/array_decimal128.hpp @@ -61,7 +61,10 @@ class ArrayDecimal128 : public ArrayPayload, private Array { bool is_null(size_t ndx) const { - return m_width == 0 || get(ndx).is_null(); + if (m_width == 0) { + return !get_context_flag(); + } + return get(ndx).is_null(); } Decimal128 get(size_t ndx) const diff --git a/test/test_decimal128.cpp b/test/test_decimal128.cpp index e6ac946167e..5792301343e 100644 --- a/test/test_decimal128.cpp +++ b/test/test_decimal128.cpp @@ -191,19 +191,25 @@ TEST(Decimal_Array) arr.add(Decimal128(realm::null())); CHECK_EQUAL(arr.get_width(), 0); CHECK_EQUAL(arr.get(0), Decimal128(realm::null())); + CHECK(arr.is_null(0)); arr.add(Decimal128()); CHECK_EQUAL(arr.get_width(), 4); CHECK_EQUAL(arr.get(0), Decimal128(realm::null())); CHECK_EQUAL(arr.get(1), Decimal128()); + CHECK(arr.is_null(0)); + CHECK_NOT(arr.is_null(1)); arr.clear(); arr.add(Decimal128()); CHECK_EQUAL(arr.get_width(), 0); CHECK_EQUAL(arr.get(0), Decimal128()); + CHECK_NOT(arr.is_null(0)); arr.add(Decimal128(realm::null())); CHECK_EQUAL(arr.get_width(), 4); CHECK_EQUAL(arr.get(0), Decimal128()); CHECK_EQUAL(arr.get(1), Decimal128(realm::null())); + CHECK_NOT(arr.is_null(0)); + CHECK(arr.is_null(1)); arr.clear(); arr.add(Decimal128(str0));