Skip to content

Commit 5a6b211

Browse files
authored
[REACTOR] Cleanup and improve map array containers (#462)
This PR runs the following refactor of the map and array containers. - Remove InplaceArrayBase since it is not as useful - Extract MapObj into map_base.h so it can be used for other different subclass and variant of map if needed. - Add extra helper methods such as clean We also reorganized SmallBaseMapObj after DenseMapObj as SmallBaseMapObj is a specialization, which can help possible future refactors to make SmallBaseMapObj be aware of DenseMapObj. The split out of map_base.h is roughly manual copy and a few extra manual edits to reduce unexpected surprises.
1 parent b648c5d commit 5a6b211

5 files changed

Lines changed: 1460 additions & 1478 deletions

File tree

include/tvm/ffi/container/array.h

Lines changed: 2 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -43,18 +43,8 @@ namespace tvm {
4343
namespace ffi {
4444

4545
/*! \brief Array node content in array */
46-
class ArrayObj : public SeqBaseObj, public details::InplaceArrayBase<ArrayObj, TVMFFIAny> {
47-
// InplaceArrayBase<ArrayObj, TVMFFIAny>'s destructor is compiled out because TVMFFIAny is
48-
// trivial. SeqBaseObj::~SeqBaseObj() handles element destruction. Changing the ElemType to a
49-
// non-trivial type would cause double-destruction.
50-
static_assert(std::is_trivially_destructible_v<TVMFFIAny>,
51-
"InplaceArrayBase<ArrayObj, TVMFFIAny> must use a trivially destructible "
52-
"element type to avoid double-destruction with SeqBaseObj::~SeqBaseObj()");
53-
46+
class ArrayObj : public SeqBaseObj {
5447
public:
55-
// Bring SeqBaseObj names into ArrayObj scope to hide InplaceArrayBase's versions
56-
using SeqBaseObj::operator[];
57-
using SeqBaseObj::EmplaceInit;
5848
/*!
5949
* \brief Constructs a container and copy from another
6050
* \param cap The capacity of the container
@@ -132,7 +122,7 @@ class ArrayObj : public SeqBaseObj, public details::InplaceArrayBase<ArrayObj, T
132122
ObjectPtr<ArrayObj> p = make_inplace_array_object<ArrayObj, Any>(n);
133123
p->TVMFFISeqCell::capacity = n;
134124
p->TVMFFISeqCell::size = 0;
135-
p->data = p->AddressOf(0);
125+
p->data = reinterpret_cast<char*>(p.get()) + sizeof(ArrayObj);
136126
p->data_deleter = nullptr;
137127
return p;
138128
}
@@ -161,9 +151,6 @@ class ArrayObj : public SeqBaseObj, public details::InplaceArrayBase<ArrayObj, T
161151
/*! \brief Expansion factor of the Array */
162152
static constexpr int64_t kIncFactor = 2;
163153

164-
// CRTP parent class
165-
friend InplaceArrayBase<ArrayObj, Any>;
166-
167154
// Reference class
168155
template <typename, typename>
169156
friend class Array;

include/tvm/ffi/container/container_details.h

Lines changed: 0 additions & 128 deletions
Original file line numberDiff line numberDiff line change
@@ -36,134 +36,6 @@
3636
namespace tvm {
3737
namespace ffi {
3838
namespace details {
39-
/*!
40-
* \brief Base template for classes with array like memory layout.
41-
*
42-
* It provides general methods to access the memory. The memory
43-
* layout is ArrayType + [ElemType]. The alignment of ArrayType
44-
* and ElemType is handled by the memory allocator.
45-
*
46-
* \tparam ArrayType The array header type, contains object specific metadata.
47-
* \tparam ElemType The type of objects stored in the array right after
48-
* ArrayType.
49-
*
50-
* \code{.cpp}
51-
* // Example usage of the template to define a simple array wrapper
52-
* class ArrayObj : public tvm::ffi::details::InplaceArrayBase<ArrayObj, Elem> {
53-
* public:
54-
* // Wrap EmplaceInit to initialize the elements
55-
* template <typename Iterator>
56-
* void Init(Iterator begin, Iterator end) {
57-
* size_t num_elems = std::distance(begin, end);
58-
* auto it = begin;
59-
* this->size = 0;
60-
* for (size_t i = 0; i < num_elems; ++i) {
61-
* InplaceArrayBase::EmplaceInit(i, *it++);
62-
* this->size++;
63-
* }
64-
* }
65-
* }
66-
*
67-
* void test_function() {
68-
* vector<Elem> fields;
69-
* auto ptr = make_inplace_array_object<ArrayObj, Elem>(fields.size());
70-
* ptr->Init(fields.begin(), fields.end());
71-
*
72-
* // Access the 0th element in the array.
73-
* assert(ptr->operator[](0) == fields[0]);
74-
* }
75-
* \endcode
76-
*/
77-
template <typename ArrayType, typename ElemType>
78-
class InplaceArrayBase {
79-
public:
80-
/*!
81-
* \brief Access element at index
82-
* \param idx The index of the element.
83-
* \return Const reference to ElemType at the index.
84-
*/
85-
const ElemType& operator[](size_t idx) const {
86-
size_t size = Self()->GetSize();
87-
if (idx > size) {
88-
TVM_FFI_THROW(IndexError) << "Index " << idx << " out of bounds " << size;
89-
}
90-
return *(reinterpret_cast<ElemType*>(AddressOf(idx)));
91-
}
92-
93-
/*!
94-
* \brief Access element at index
95-
* \param idx The index of the element.
96-
* \return Reference to ElemType at the index.
97-
*/
98-
ElemType& operator[](size_t idx) {
99-
size_t size = Self()->GetSize();
100-
if (idx > size) {
101-
TVM_FFI_THROW(IndexError) << "Index " << idx << " out of bounds " << size;
102-
}
103-
return *(reinterpret_cast<ElemType*>(AddressOf(idx)));
104-
}
105-
106-
/*!
107-
* \brief Destroy the Inplace Array Base object
108-
*/
109-
~InplaceArrayBase() {
110-
if constexpr (!(std::is_standard_layout_v<ElemType> && std::is_trivial_v<ElemType>)) {
111-
size_t size = Self()->GetSize();
112-
for (size_t i = 0; i < size; ++i) {
113-
ElemType* fp = reinterpret_cast<ElemType*>(AddressOf(i));
114-
fp->ElemType::~ElemType();
115-
}
116-
}
117-
}
118-
119-
private:
120-
InplaceArrayBase() = default;
121-
friend ArrayType;
122-
123-
protected:
124-
/*!
125-
* \brief Construct a value in place with the arguments.
126-
*
127-
* \tparam Args Type parameters of the arguments.
128-
* \param idx Index of the element.
129-
* \param args Arguments to construct the new value.
130-
*
131-
* \note Please make sure ArrayType::GetSize returns 0 before first call of
132-
* EmplaceInit, and increment GetSize by 1 each time EmplaceInit succeeds.
133-
*/
134-
template <typename... Args>
135-
void EmplaceInit(size_t idx, Args&&... args) {
136-
void* field_ptr = AddressOf(idx);
137-
new (field_ptr) ElemType(std::forward<Args>(args)...);
138-
}
139-
140-
/*!
141-
* \brief Return the self object for the array.
142-
*
143-
* \return Pointer to ArrayType.
144-
*/
145-
inline ArrayType* Self() const {
146-
return static_cast<ArrayType*>(const_cast<InplaceArrayBase*>(this));
147-
}
148-
149-
/*!
150-
* \brief Return the raw pointer to the element at idx.
151-
*
152-
* \param idx The index of the element.
153-
* \return Raw pointer to the element.
154-
*/
155-
void* AddressOf(size_t idx) const {
156-
static_assert(
157-
alignof(ArrayType) % alignof(ElemType) == 0 && sizeof(ArrayType) % alignof(ElemType) == 0,
158-
"The size and alignment of ArrayType should respect "
159-
"ElemType's alignment.");
160-
161-
size_t kDataStart = sizeof(ArrayType);
162-
ArrayType* self = Self();
163-
char* data_start = reinterpret_cast<char*>(self) + kDataStart;
164-
return data_start + idx * sizeof(ElemType);
165-
}
166-
};
16739

16840
/*!
16941
* \brief iterator adapter that adapts TIter to return another type.

0 commit comments

Comments
 (0)