Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

added get_serialized_size_bytes() #428

Merged
merged 5 commits into from
Mar 23, 2024
Merged

added get_serialized_size_bytes() #428

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a667763
added get_serialized_size_bytes()
AlexanderSaydakov Mar 12, 2024
4e2b8b6
added necessary side effect
AlexanderSaydakov Mar 13, 2024
c785fc6
values only in buffer, optional serialization with buffer
AlexanderSaydakov Mar 15, 2024
82f5bd5
changed and simplified sizing considering temporary buffer for merging
AlexanderSaydakov Mar 18, 2024
a693442
removed duplicate code
AlexanderSaydakov Mar 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 10 additions & 3 deletions tdigest/include/tdigest.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,9 @@
#ifndef _TDIGEST_HPP_
#define _TDIGEST_HPP_

#include <type_traits>
#include <cstddef>
#include <limits>
#include <type_traits>

#include "common_defs.hpp"

Expand Down Expand Up @@ -165,6 +166,12 @@ class tdigest {
*/
string<Allocator> to_string(bool print_centroids = false) const;

/**
* Computes size needed to serialize the current state.
* @return size in bytes needed to serialize this tdigest
*/
size_t get_serialized_size_bytes() const;

/**
* This method serializes t-Digest into a given stream in a binary form
* @param os output stream
Expand Down Expand Up @@ -222,12 +229,12 @@ class tdigest {
enum flags { IS_EMPTY, IS_SINGLE_VALUE, REVERSE_MERGE };

bool is_single_value() const;
uint8_t get_preamble_longs() const;
void merge_buffered();

// for deserialize
tdigest(bool reverse_merge, uint16_t k, T min, T max, vector_centroid&& centroids, uint64_t total_weight_, const Allocator& allocator);

void merge_buffered();

static double weighted_average(double x1, double w1, double x2, double w2);

// for compatibility with format of the reference implementation
Expand Down
19 changes: 14 additions & 5 deletions tdigest/include/tdigest_impl.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -312,15 +312,24 @@ void tdigest<T, A>::serialize(std::ostream& os) const {
}

template<typename T, typename A>
auto tdigest<T, A>::serialize(unsigned header_size_bytes) const -> vector_bytes {
uint8_t tdigest<T, A>::get_preamble_longs() const {
return is_empty() || is_single_value() ? PREAMBLE_LONGS_EMPTY_OR_SINGLE : PREAMBLE_LONGS_MULTIPLE;
}

template<typename T, typename A>
size_t tdigest<T, A>::get_serialized_size_bytes() const {
const_cast<tdigest*>(this)->merge_buffered(); // side effect
const uint8_t preamble_longs = is_empty() || is_single_value() ? PREAMBLE_LONGS_EMPTY_OR_SINGLE : PREAMBLE_LONGS_MULTIPLE;
const size_t size_bytes = preamble_longs * sizeof(uint64_t) +
return get_preamble_longs() * sizeof(uint64_t) +
(is_empty() ? 0 : (is_single_value() ? sizeof(T) : sizeof(T) * 2 + sizeof(centroid) * centroids_.size()));
vector_bytes bytes(size_bytes, 0, allocator_);
}

template<typename T, typename A>
auto tdigest<T, A>::serialize(unsigned header_size_bytes) const -> vector_bytes {
const_cast<tdigest*>(this)->merge_buffered(); // side effect
vector_bytes bytes(get_serialized_size_bytes(), 0, allocator_);
uint8_t* ptr = bytes.data() + header_size_bytes;

*ptr++ = preamble_longs;
*ptr++ = get_preamble_longs();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One of the more aggressive warning flags complains about *ptr++ as possibly incrementing a null ptr

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see how this would happen.
I thought about encapsulating this keeping track of the offset. or, perhaps, using something like std::strstream

*ptr++ = SERIAL_VERSION;
*ptr++ = SKETCH_TYPE;
ptr += copy_to_mem(k_, ptr);
Expand Down
Loading