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.

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

ArrayBuilder: replace shared with unique #1155

Merged
merged 20 commits into from Dec 21, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion include/awkward/builder/ArrayBuilder.h
Expand Up @@ -233,7 +233,7 @@ namespace awkward {
/// @brief Internal function to replace the root node of the ArrayBuilder's
/// Builder tree with a new root.
void
maybeupdate(const BuilderPtr& tmp);
maybeupdate(const BuilderPtr builder);

private:
/// @brief Constant equal to `nullptr`.
Expand Down
4 changes: 2 additions & 2 deletions include/awkward/builder/BoolBuilder.h
Expand Up @@ -30,7 +30,7 @@ namespace awkward {
/// these are passed to every Builder's constructor.
/// @param buffer Contains the accumulated boolean values.
BoolBuilder(const ArrayBuilderOptions& options,
const GrowableBuffer<uint8_t>& buffer);
GrowableBuffer<uint8_t> buffer);

/// @brief User-friendly name of this class: `"BoolBuilder"`.
const std::string
Expand Down Expand Up @@ -93,7 +93,7 @@ namespace awkward {
const BuilderPtr
beginrecord(const char* name, bool check) override;

const BuilderPtr
void
field(const char* key, bool check) override;

const BuilderPtr
Expand Down
2 changes: 1 addition & 1 deletion include/awkward/builder/Builder.h
Expand Up @@ -153,7 +153,7 @@ namespace awkward {
/// Record keys are checked in round-robin order. The best performance
/// will be achieved by filling them in the same order for each record.
/// Lookup time for random order scales with the number of fields.
virtual const BuilderPtr
virtual void
field(const char* key, bool check) = 0;

/// @brief Ends a record.
Expand Down
4 changes: 2 additions & 2 deletions include/awkward/builder/Complex128Builder.h
Expand Up @@ -44,7 +44,7 @@ namespace awkward {
/// these are passed to every Builder's constructor.
/// @param buffer Contains the accumulated real numbers.
Complex128Builder(const ArrayBuilderOptions& options,
const GrowableBuffer<std::complex<double>>& buffer);
GrowableBuffer<std::complex<double>> buffer);

/// @brief User-friendly name of this class: `"Complex128Builder"`.
const std::string
Expand Down Expand Up @@ -107,7 +107,7 @@ namespace awkward {
const BuilderPtr
beginrecord(const char* name, bool check) override;

const BuilderPtr
void
field(const char* key, bool check) override;

const BuilderPtr
Expand Down
4 changes: 2 additions & 2 deletions include/awkward/builder/DatetimeBuilder.h
Expand Up @@ -27,7 +27,7 @@ namespace awkward {
/// these are passed to every Builder's constructor.
/// @param buffer Contains the accumulated integers.
DatetimeBuilder(const ArrayBuilderOptions& options,
const GrowableBuffer<int64_t>& content,
GrowableBuffer<int64_t> content,
const std::string& units);

/// @brief User-friendly name of this class: `"DatetimeBuilder"`.
Expand Down Expand Up @@ -91,7 +91,7 @@ namespace awkward {
const BuilderPtr
beginrecord(const char* name, bool check) override;

const BuilderPtr
void
field(const char* key, bool check) override;

const BuilderPtr
Expand Down
6 changes: 3 additions & 3 deletions include/awkward/builder/Float64Builder.h
Expand Up @@ -37,10 +37,10 @@ namespace awkward {
/// these are passed to every Builder's constructor.
/// @param buffer Contains the accumulated real numbers.
Float64Builder(const ArrayBuilderOptions& options,
const GrowableBuffer<double>& buffer);
GrowableBuffer<double> buffer);

/// @brief Contains the accumulated real numbers (`double`).
const GrowableBuffer<double>
const GrowableBuffer<double>&
buffer() const;

/// @brief User-friendly name of this class: `"Float64Builder"`.
Expand Down Expand Up @@ -104,7 +104,7 @@ namespace awkward {
const BuilderPtr
beginrecord(const char* name, bool check) override;

const BuilderPtr
void
field(const char* key, bool check) override;

const BuilderPtr
Expand Down
35 changes: 14 additions & 21 deletions include/awkward/builder/GrowableBuffer.h
Expand Up @@ -5,6 +5,7 @@

#include "awkward/common.h"
#include "awkward/builder/ArrayBuilderOptions.h"
#include "awkward/kernel-dispatch.h"

#include <memory>

Expand All @@ -23,22 +24,13 @@ namespace awkward {
/// data grow.
///
/// When {@link ArrayBuilder#snapshot ArrayBuilder::snapshot} is called,
/// these buffers are shared with the new Content array. The GrowableBuffer
/// can still grow because the Content array only sees the part of its
/// reservation that existed at the time of the snapshot (new elements are
/// beyond its {@link Content#length Content::length}).
/// these buffers are copied to the new Content array.
///
/// If a GrowableBuffer resizes itself by allocating a new array, it
/// decreases the reference counter for the shared buffer, but the Content
/// still owns it, and thus becomes the sole owner.
///
/// The only disadvantage to this scheme is that the Content might forever
/// have a reservation that is larger than it needs and it is unable to
/// delete or take advantage of. However, many operations require buffers
/// to be rewritten; under normal circumstances, it would soon be replaced
/// by a more appropriately sized buffer.
/// copies the buffer it owns.
template <typename T>
class LIBAWKWARD_EXPORT_SYMBOL GrowableBuffer {
using UniquePtr = kernel::UniquePtr<T>;
public:
/// @brief Creates an empty GrowableBuffer.
///
Expand All @@ -53,7 +45,7 @@ namespace awkward {
/// of `minreserve` and
/// {@link ArrayBuilderOptions#initial ArrayBuilderOptions::initial}.
static GrowableBuffer<T>
empty(const ArrayBuilderOptions& options, int64_t minreserve);
empty_reserved(const ArrayBuilderOptions& options, int64_t minreserve);

/// @brief Creates a GrowableBuffer in which all elements are initialized
/// to a given value.
Expand Down Expand Up @@ -90,7 +82,7 @@ namespace awkward {
/// Although the #length increments every time #append is called,
/// it is always less than or equal to #reserved because of reallocations.
GrowableBuffer(const ArrayBuilderOptions& options,
std::shared_ptr<T> ptr,
GrowableBuffer::UniquePtr ptr,
int64_t length,
int64_t reserved);

Expand All @@ -99,10 +91,15 @@ namespace awkward {
/// {@link ArrayBuilderOptions#initial ArrayBuilderOptions::initial}.
GrowableBuffer(const ArrayBuilderOptions& options);

/// @brief Reference-counted pointer to the array buffer.
const std::shared_ptr<T>
/// @brief Reference to a unique pointer to the array buffer.
const GrowableBuffer::UniquePtr&
ptr() const;

/// @brief FIXME: needed for uproot.cpp - remove when it's gone.
/// Note, it transfers ownership of the array buffer.
GrowableBuffer::UniquePtr
get_ptr();

/// @brief Currently used number of elements.
///
/// Although the #length increments every time #append is called,
Expand Down Expand Up @@ -135,10 +132,6 @@ namespace awkward {
/// @brief Discards accumulated data, the #reserved returns to
/// {@link ArrayBuilderOptions#initial ArrayBuilderOptions::initial},
/// and a new #ptr is allocated.
///
/// The old data are only discarded in the sense of decrementing their
/// reference count. If any old snapshots are still using the data,
/// they are not invalidated.
void
clear();

Expand All @@ -158,7 +151,7 @@ namespace awkward {
private:
const ArrayBuilderOptions options_;
// @brief See #ptr.
std::shared_ptr<T> ptr_;
UniquePtr ptr_;
// @brief See #length.
int64_t length_;
// @brief See #reserved.
Expand Down
6 changes: 3 additions & 3 deletions include/awkward/builder/Int64Builder.h
Expand Up @@ -27,10 +27,10 @@ namespace awkward {
/// these are passed to every Builder's constructor.
/// @param buffer Contains the accumulated integers.
Int64Builder(const ArrayBuilderOptions& options,
const GrowableBuffer<int64_t>& buffer);
GrowableBuffer<int64_t> buffer);

/// @brief Contains the accumulated integers.
const GrowableBuffer<int64_t>
const GrowableBuffer<int64_t>&
buffer() const;

/// @brief User-friendly name of this class: `"Int64Builder"`.
Expand Down Expand Up @@ -94,7 +94,7 @@ namespace awkward {
const BuilderPtr
beginrecord(const char* name, bool check) override;

const BuilderPtr
void
field(const char* key, bool check) override;

const BuilderPtr
Expand Down
11 changes: 5 additions & 6 deletions include/awkward/builder/ListBuilder.h
Expand Up @@ -34,7 +34,7 @@ namespace awkward {
/// #beginlist and before #endlist and is #active; if `false`,
/// it is not.
ListBuilder(const ArrayBuilderOptions& options,
const GrowableBuffer<int64_t>& offsets,
GrowableBuffer<int64_t> offsets,
const BuilderPtr& content,
bool begun);

Expand Down Expand Up @@ -100,7 +100,7 @@ namespace awkward {
const BuilderPtr
beginrecord(const char* name, bool check) override;

const BuilderPtr
void
field(const char* key, bool check) override;

const BuilderPtr
Expand All @@ -115,15 +115,14 @@ namespace awkward {

bool begun() { return begun_; }

void
maybeupdate(const BuilderPtr builder);

private:
const ArrayBuilderOptions options_;
GrowableBuffer<int64_t> offsets_;
BuilderPtr content_;
bool begun_;

public:
void
maybeupdate(const BuilderPtr& tmp);
};
}

Expand Down
15 changes: 7 additions & 8 deletions include/awkward/builder/OptionBuilder.h
Expand Up @@ -44,8 +44,8 @@ namespace awkward {
/// {@link IndexedArrayOf#index IndexedOptionArray::index}).
/// @param content Builder for the non-missing data.
OptionBuilder(const ArrayBuilderOptions& options,
const GrowableBuffer<int64_t>& index,
const BuilderPtr& content);
GrowableBuffer<int64_t> index,
const BuilderPtr content);

/// @brief User-friendly name of this class: `"OptionBuilder"`.
const std::string
Expand Down Expand Up @@ -108,26 +108,25 @@ namespace awkward {
const BuilderPtr
beginrecord(const char* name, bool check) override;

const BuilderPtr
void
field(const char* key, bool check) override;

const BuilderPtr
endrecord() override;

const GrowableBuffer<int64_t>& buffer() const { return index_; }

GrowableBuffer<int64_t>& index() { return index_; }
const GrowableBuffer<int64_t>& index() { return index_; }

const BuilderPtr builder() const { return content_; }

void
maybeupdate(const BuilderPtr builder);

private:
const ArrayBuilderOptions options_;
GrowableBuffer<int64_t> index_;
BuilderPtr content_;

public:
void
maybeupdate(const BuilderPtr& tmp);
};

}
Expand Down
13 changes: 6 additions & 7 deletions include/awkward/builder/RecordBuilder.h
Expand Up @@ -118,7 +118,7 @@ namespace awkward {
const BuilderPtr
beginrecord(const char* name, bool check) override;

const BuilderPtr
void
field(const char* key, bool check) override;

const BuilderPtr
Expand All @@ -135,11 +135,14 @@ namespace awkward {

int64_t nextindex() { return nextindex_; }

void
maybeupdate(int64_t i, const BuilderPtr builder);

private:
const BuilderPtr
void
field_fast(const char* key);

const BuilderPtr
void
field_check(const char* key);

const ArrayBuilderOptions options_;
Expand All @@ -153,10 +156,6 @@ namespace awkward {
int64_t nextindex_;
int64_t nexttotry_;
int64_t keys_size_;

public:
void
maybeupdate(int64_t i, const BuilderPtr& tmp);
};
}

Expand Down
6 changes: 3 additions & 3 deletions include/awkward/builder/StringBuilder.h
Expand Up @@ -36,8 +36,8 @@ namespace awkward {
/// if `"utf-8"`, it is encoded with variable-width UTF-8.
/// Currently, no other encodings have been defined.
StringBuilder(const ArrayBuilderOptions& options,
const GrowableBuffer<int64_t>& offsets,
const GrowableBuffer<uint8_t>& content,
GrowableBuffer<int64_t> offsets,
GrowableBuffer<uint8_t> content,
const char* encoding);

/// @brief If `nullptr`, the string is an unencoded bytestring;
Expand Down Expand Up @@ -107,7 +107,7 @@ namespace awkward {
const BuilderPtr
beginrecord(const char* name, bool check) override;

const BuilderPtr
void
field(const char* key, bool check) override;

const BuilderPtr
Expand Down
9 changes: 4 additions & 5 deletions include/awkward/builder/TupleBuilder.h
Expand Up @@ -104,7 +104,7 @@ namespace awkward {
const BuilderPtr
beginrecord(const char* name, bool check) override;

const BuilderPtr
void
field(const char* key, bool check) override;

const BuilderPtr
Expand All @@ -119,16 +119,15 @@ namespace awkward {

int64_t nextindex() { return nextindex_; }

void
maybeupdate(int64_t i, const BuilderPtr builder);

private:
const ArrayBuilderOptions options_;
std::vector<BuilderPtr> contents_;
int64_t length_;
bool begun_;
int64_t nextindex_;

public:
void
maybeupdate(int64_t i, const BuilderPtr& tmp);
};
}

Expand Down
6 changes: 3 additions & 3 deletions include/awkward/builder/UnionBuilder.h
Expand Up @@ -33,8 +33,8 @@ namespace awkward {
/// {@link UnionArrayOf#index UnionArray::index}).
/// @param contents A Builder for each of the union's possibilities.
UnionBuilder(const ArrayBuilderOptions& options,
const GrowableBuffer<int8_t>& tags,
const GrowableBuffer<int64_t>& index,
GrowableBuffer<int8_t> tags,
GrowableBuffer<int64_t> index,
std::vector<BuilderPtr>& contents);

/// @brief User-friendly name of this class: `"UnionBuilder"`.
Expand Down Expand Up @@ -99,7 +99,7 @@ namespace awkward {
const BuilderPtr
beginrecord(const char* name, bool check) override;

const BuilderPtr
void
field(const char* key, bool check) override;

const BuilderPtr
Expand Down