Commit a23898508743b8fc69d66ff214427f9a436089c6
Committed by
GitHub
Merge pull request #1277 from m-holger/objtable
Refactor ObjTable
Showing
4 changed files
with
112 additions
and
20 deletions
libqpdf/QPDFWriter.cc
| @@ -2077,8 +2077,8 @@ void | @@ -2077,8 +2077,8 @@ void | ||
| 2077 | QPDFWriter::initializeTables(size_t extra) | 2077 | QPDFWriter::initializeTables(size_t extra) |
| 2078 | { | 2078 | { |
| 2079 | auto size = QIntC::to_size(QPDF::Writer::tableSize(m->pdf) + 100) + extra; | 2079 | auto size = QIntC::to_size(QPDF::Writer::tableSize(m->pdf) + 100) + extra; |
| 2080 | - m->obj.initialize(size); | ||
| 2081 | - m->new_obj.initialize(size); | 2080 | + m->obj.resize(size); |
| 2081 | + m->new_obj.resize(size); | ||
| 2082 | } | 2082 | } |
| 2083 | 2083 | ||
| 2084 | void | 2084 | void |
libqpdf/qpdf/ObjTable.hh
| @@ -28,6 +28,7 @@ template <class T> | @@ -28,6 +28,7 @@ template <class T> | ||
| 28 | class ObjTable: public std::vector<T> | 28 | class ObjTable: public std::vector<T> |
| 29 | { | 29 | { |
| 30 | public: | 30 | public: |
| 31 | + using reference = T&; | ||
| 31 | ObjTable() = default; | 32 | ObjTable() = default; |
| 32 | ObjTable(const ObjTable&) = delete; | 33 | ObjTable(const ObjTable&) = delete; |
| 33 | ObjTable(ObjTable&&) = delete; | 34 | ObjTable(ObjTable&&) = delete; |
| @@ -99,17 +100,30 @@ class ObjTable: public std::vector<T> | @@ -99,17 +100,30 @@ class ObjTable: public std::vector<T> | ||
| 99 | return element(id); | 100 | return element(id); |
| 100 | } | 101 | } |
| 101 | 102 | ||
| 103 | + // emplace_back to the end of the vector. If there are any conflicting sparse elements, emplace | ||
| 104 | + // them to the back of the vector before adding the current element. | ||
| 105 | + template <class... Args> | ||
| 106 | + inline T& | ||
| 107 | + emplace_back(Args&&... args) | ||
| 108 | + { | ||
| 109 | + if (min_sparse == std::vector<T>::size()) { | ||
| 110 | + return emplace_back_large(std::forward<Args&&...>(args...)); | ||
| 111 | + } | ||
| 112 | + return std::vector<T>::emplace_back(std::forward<Args&&...>(args...)); | ||
| 113 | + } | ||
| 114 | + | ||
| 102 | void | 115 | void |
| 103 | - initialize(size_t idx) | ||
| 104 | - { | ||
| 105 | - if (std::vector<T>::size() > 0 || sparse_elements.size() > 0) { | ||
| 106 | - throw ::std::logic_error("ObjTable accessed before initialization"); | ||
| 107 | - } else if ( | ||
| 108 | - idx >= static_cast<size_t>(std::numeric_limits<int>::max()) || | ||
| 109 | - idx >= std::vector<T>::max_size()) { | ||
| 110 | - throw std::runtime_error("Invalid maximum object id initializing ObjTable."); | ||
| 111 | - } else { | ||
| 112 | - std::vector<T>::resize(++idx); | 116 | + resize(size_t a_size) |
| 117 | + { | ||
| 118 | + std::vector<T>::resize(a_size); | ||
| 119 | + if (a_size > min_sparse) { | ||
| 120 | + auto it = sparse_elements.begin(); | ||
| 121 | + auto end = sparse_elements.end(); | ||
| 122 | + while (it != end && it->first < a_size) { | ||
| 123 | + std::vector<T>::operator[](it->first) = std::move(it->second); | ||
| 124 | + it = sparse_elements.erase(it); | ||
| 125 | + } | ||
| 126 | + min_sparse = (it == end) ? std::numeric_limits<size_t>::max() : it->first; | ||
| 113 | } | 127 | } |
| 114 | } | 128 | } |
| 115 | 129 | ||
| @@ -127,30 +141,62 @@ class ObjTable: public std::vector<T> | @@ -127,30 +141,62 @@ class ObjTable: public std::vector<T> | ||
| 127 | 141 | ||
| 128 | private: | 142 | private: |
| 129 | std::map<size_t, T> sparse_elements; | 143 | std::map<size_t, T> sparse_elements; |
| 144 | + // Smallest id present in sparse_elements. | ||
| 145 | + size_t min_sparse{std::numeric_limits<size_t>::max()}; | ||
| 130 | 146 | ||
| 131 | inline T& | 147 | inline T& |
| 132 | element(size_t idx) | 148 | element(size_t idx) |
| 133 | { | 149 | { |
| 134 | if (idx < std::vector<T>::size()) { | 150 | if (idx < std::vector<T>::size()) { |
| 135 | return std::vector<T>::operator[](idx); | 151 | return std::vector<T>::operator[](idx); |
| 136 | - } else if (idx < static_cast<size_t>(std::numeric_limits<int>::max())) { | 152 | + } |
| 153 | + return large_element(idx); | ||
| 154 | + } | ||
| 155 | + | ||
| 156 | + // Must only be called by element. Separated out from element to keep inlined code tight. | ||
| 157 | + T& | ||
| 158 | + large_element(size_t idx) | ||
| 159 | + { | ||
| 160 | + static const size_t max_size = std::vector<T>::max_size(); | ||
| 161 | + if (idx < min_sparse) { | ||
| 162 | + min_sparse = idx; | ||
| 163 | + } | ||
| 164 | + if (idx < max_size) { | ||
| 137 | return sparse_elements[idx]; | 165 | return sparse_elements[idx]; |
| 138 | } | 166 | } |
| 139 | - throw std::runtime_error("Invalid object id accessing ObjTable."); | 167 | + throw std::runtime_error("Impossibly large object id encountered accessing ObjTable"); |
| 140 | return element(0); // doesn't return | 168 | return element(0); // doesn't return |
| 141 | } | 169 | } |
| 142 | 170 | ||
| 143 | inline T const& | 171 | inline T const& |
| 144 | element(size_t idx) const | 172 | element(size_t idx) const |
| 145 | { | 173 | { |
| 174 | + static const size_t max_size = std::vector<T>::max_size(); | ||
| 146 | if (idx < std::vector<T>::size()) { | 175 | if (idx < std::vector<T>::size()) { |
| 147 | return std::vector<T>::operator[](idx); | 176 | return std::vector<T>::operator[](idx); |
| 148 | - } else if (idx < static_cast<size_t>(std::numeric_limits<int>::max())) { | 177 | + } |
| 178 | + if (idx < max_size) { | ||
| 149 | return sparse_elements.at(idx); | 179 | return sparse_elements.at(idx); |
| 150 | } | 180 | } |
| 151 | - throw std::runtime_error("Invalid object id accessing ObjTable."); | 181 | + throw std::runtime_error("Impossibly large object id encountered accessing ObjTable"); |
| 152 | return element(0); // doesn't return | 182 | return element(0); // doesn't return |
| 153 | } | 183 | } |
| 184 | + | ||
| 185 | + // Must only be called by emplace_back. Separated out from emplace_back to keep inlined code | ||
| 186 | + // tight. | ||
| 187 | + template <class... Args> | ||
| 188 | + T& | ||
| 189 | + emplace_back_large(Args&&... args) | ||
| 190 | + { | ||
| 191 | + auto it = sparse_elements.begin(); | ||
| 192 | + auto end = sparse_elements.end(); | ||
| 193 | + while (it != end && it->first == std::vector<T>::size()) { | ||
| 194 | + std::vector<T>::emplace_back(std::move(it->second)); | ||
| 195 | + it = sparse_elements.erase(it); | ||
| 196 | + } | ||
| 197 | + min_sparse = (it == end) ? std::numeric_limits<size_t>::max() : it->first; | ||
| 198 | + return std::vector<T>::emplace_back(std::forward<Args&&...>(args...)); | ||
| 199 | + } | ||
| 154 | }; | 200 | }; |
| 155 | 201 | ||
| 156 | #endif // OBJTABLE_HH | 202 | #endif // OBJTABLE_HH |
libtests/obj_table.cc
| @@ -2,6 +2,11 @@ | @@ -2,6 +2,11 @@ | ||
| 2 | 2 | ||
| 3 | struct Test | 3 | struct Test |
| 4 | { | 4 | { |
| 5 | + Test() = default; | ||
| 6 | + Test(int value) : | ||
| 7 | + value(value) | ||
| 8 | + { | ||
| 9 | + } | ||
| 5 | int value{0}; | 10 | int value{0}; |
| 6 | }; | 11 | }; |
| 7 | 12 | ||
| @@ -10,7 +15,7 @@ class Table: public ObjTable<Test> | @@ -10,7 +15,7 @@ class Table: public ObjTable<Test> | ||
| 10 | public: | 15 | public: |
| 11 | Table() | 16 | Table() |
| 12 | { | 17 | { |
| 13 | - initialize(5); | 18 | + resize(5); |
| 14 | } | 19 | } |
| 15 | 20 | ||
| 16 | void | 21 | void |
| @@ -20,9 +25,23 @@ class Table: public ObjTable<Test> | @@ -20,9 +25,23 @@ class Table: public ObjTable<Test> | ||
| 20 | (*this)[i].value = 2 * i; | 25 | (*this)[i].value = 2 * i; |
| 21 | (*this)[1000 + i].value = 2 * (1000 + i); | 26 | (*this)[1000 + i].value = 2 * (1000 + i); |
| 22 | } | 27 | } |
| 28 | + for (int i: {50, 60, 70, 98, 99, 100, 101, 150, 198, 199, 200, 201}) { | ||
| 29 | + (*this)[i].value = 2 * i; | ||
| 30 | + } | ||
| 31 | + resize(100); | ||
| 32 | + for (int i: {1, 99, 100, 105, 110, 120, 205, 206, 207, 210}) { | ||
| 33 | + (*this)[i].value = 3 * i; | ||
| 34 | + } | ||
| 35 | + resize(200); | ||
| 36 | + | ||
| 37 | + for (int i = 1; i < 10; ++i) { | ||
| 38 | + emplace_back(i); | ||
| 39 | + } | ||
| 23 | 40 | ||
| 24 | forEach([](auto i, auto const& item) -> void { | 41 | forEach([](auto i, auto const& item) -> void { |
| 25 | - std::cout << std::to_string(i) << " : " << std::to_string(item.value) << "\n"; | 42 | + if (item.value) { |
| 43 | + std::cout << std::to_string(i) << " : " << std::to_string(item.value) << "\n"; | ||
| 44 | + } | ||
| 26 | }); | 45 | }); |
| 27 | 46 | ||
| 28 | std::cout << "2000 : " << std::to_string((*this)[2000].value) << "\n"; | 47 | std::cout << "2000 : " << std::to_string((*this)[2000].value) << "\n"; |
libtests/qtest/obj_table/obj_table.out
| 1 | -0 : 0 | ||
| 2 | -1 : 2 | 1 | +1 : 3 |
| 3 | 2 : 4 | 2 | 2 : 4 |
| 4 | 3 : 6 | 3 | 3 : 6 |
| 5 | 4 : 8 | 4 | 4 : 8 |
| @@ -8,6 +7,34 @@ | @@ -8,6 +7,34 @@ | ||
| 8 | 7 : 14 | 7 | 7 : 14 |
| 9 | 8 : 16 | 8 | 8 : 16 |
| 10 | 9 : 18 | 9 | 9 : 18 |
| 10 | +50 : 100 | ||
| 11 | +60 : 120 | ||
| 12 | +70 : 140 | ||
| 13 | +98 : 196 | ||
| 14 | +99 : 297 | ||
| 15 | +100 : 300 | ||
| 16 | +101 : 202 | ||
| 17 | +105 : 315 | ||
| 18 | +110 : 330 | ||
| 19 | +120 : 360 | ||
| 20 | +150 : 300 | ||
| 21 | +198 : 396 | ||
| 22 | +199 : 398 | ||
| 23 | +200 : 400 | ||
| 24 | +201 : 402 | ||
| 25 | +202 : 1 | ||
| 26 | +203 : 2 | ||
| 27 | +204 : 3 | ||
| 28 | +205 : 615 | ||
| 29 | +206 : 618 | ||
| 30 | +207 : 621 | ||
| 31 | +208 : 4 | ||
| 32 | +209 : 5 | ||
| 33 | +210 : 630 | ||
| 34 | +211 : 6 | ||
| 35 | +212 : 7 | ||
| 36 | +213 : 8 | ||
| 37 | +214 : 9 | ||
| 11 | 1000 : 2000 | 38 | 1000 : 2000 |
| 12 | 1001 : 2002 | 39 | 1001 : 2002 |
| 13 | 1002 : 2004 | 40 | 1002 : 2004 |