Skip to content

Commit b612eb6

Browse files
stragerstrager
committed
refactor(vector): use signed size types
I want to consistently use signed sizes everywhere instead of mixing signed and signed sizes. Change vector's size to be signed. This patch seems to regress performance slightly on x86_64: Benchmark Time CPU Time Old Time New CPU Old CPU New -------------------------------------------------------------------------------------------------------------------------- benchmark_parse_file +0.0159 +0.0159 2039196 2071701 2039176 2071678 benchmark_parse_file +0.0116 +0.0116 2042598 2066377 2042551 2066320 benchmark_parse_file +0.0079 +0.0080 2045114 2061367 2045035 2061327 benchmark_parse_file +0.0081 +0.0081 2050800 2067455 2050750 2067425 benchmark_parse_file +0.0032 +0.0032 2057631 2064131 2057565 2064091 benchmark_parse_file +0.0051 +0.0051 2056919 2067384 2056875 2067325 benchmark_parse_file +0.0079 +0.0079 2054431 2070720 2054389 2070686 benchmark_parse_file +0.0057 +0.0057 2055725 2067360 2055684 2067321 benchmark_parse_file +0.0041 +0.0041 2054791 2063119 2054722 2063055 benchmark_parse_file +0.0037 +0.0037 2059054 2066720 2059031 2066667 benchmark_parse_file_pvalue 0.0002 0.0002 U Test, Repetitions: 10 vs 10 benchmark_parse_file_mean +0.0073 +0.0073 2051626 2066633 2051578 2066590 benchmark_parse_file_median +0.0060 +0.0061 2054611 2067040 2054555 2066994 benchmark_parse_file_stddev -0.5427 -0.5416 6937 3172 6935 3179 benchmark_parse_file_cv -0.5460 -0.5450 0 0 0 0 OVERALL_GEOMEAN +0.0073 +0.0073 0 0 0 0 I did not test performance on AArch64.
1 parent 0e43190 commit b612eb6

File tree

6 files changed

+40
-32
lines changed

6 files changed

+40
-32
lines changed

‎src/quick-lint-js/container/vector-profiler.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -277,9 +277,9 @@ class instrumented_vector {
277277
this->add_instrumentation_entry(vector_instrumentation::event::clear);
278278
}
279279

280-
void reserve(std::size_t new_capacity) { this->data_.reserve(new_capacity); }
280+
void reserve(size_type new_capacity) { this->data_.reserve(new_capacity); }
281281

282-
void resize(std::size_t new_size) {
282+
void resize(size_type new_size) {
283283
this->data_.resize(new_size);
284284
this->add_instrumentation_entry(vector_instrumentation::event::resize);
285285
}
@@ -326,8 +326,8 @@ class instrumented_vector {
326326
/*owner=*/this->debug_owner_,
327327
/*event=*/event,
328328
/*data_pointer=*/reinterpret_cast<std::uintptr_t>(this->data()),
329-
/*size=*/this->size(),
330-
/*capacity=*/this->capacity());
329+
/*size=*/narrow_cast<std::size_t>(this->size()),
330+
/*capacity=*/narrow_cast<std::size_t>(this->capacity()));
331331
}
332332

333333
Vector data_;

‎src/quick-lint-js/container/vector.h

Lines changed: 26 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -84,13 +84,15 @@ class uninstrumented_vector : private Vector {
8484
using Vector::release;
8585
};
8686

87+
using bump_vector_size = std::ptrdiff_t;
88+
8789
template <class T, class BumpAllocator>
8890
class raw_bump_vector {
8991
public:
9092
using value_type = T;
9193
using allocator_type = BumpAllocator *;
92-
using size_type = std::size_t;
93-
using difference_type = std::ptrdiff_t;
94+
using size_type = bump_vector_size;
95+
using difference_type = bump_vector_size;
9496
using reference = T &;
9597
using const_reference = const T &;
9698
using pointer = T *;
@@ -121,11 +123,11 @@ class raw_bump_vector {
121123
BumpAllocator *get_allocator() const noexcept { return this->allocator_; }
122124

123125
bool empty() const noexcept { return this->data_ == this->data_end_; }
124-
std::size_t size() const noexcept {
125-
return narrow_cast<std::size_t>(this->data_end_ - this->data_);
126+
size_type size() const noexcept {
127+
return narrow_cast<size_type>(this->data_end_ - this->data_);
126128
}
127-
std::size_t capacity() const noexcept {
128-
return narrow_cast<std::size_t>(this->capacity_end_ - this->data_);
129+
size_type capacity() const noexcept {
130+
return narrow_cast<size_type>(this->capacity_end_ - this->data_);
129131
}
130132

131133
QLJS_FORCE_INLINE T *data() noexcept { return this->data_; }
@@ -157,25 +159,26 @@ class raw_bump_vector {
157159
return this->data_[index];
158160
}
159161

160-
void reserve(std::size_t new_capacity) {
162+
void reserve(size_type new_capacity) {
163+
QLJS_ASSERT(new_capacity > 0);
161164
if (this->capacity() < new_capacity) {
162165
this->reserve_grow(new_capacity);
163166
}
164167
}
165168

166-
void reserve_grow(std::size_t new_capacity) {
169+
void reserve_grow(size_type new_capacity) {
167170
QLJS_ASSERT(new_capacity > this->capacity());
168171
if (this->data_) {
169172
bool grew = this->allocator_->try_grow_array_in_place(
170173
this->data_,
171-
/*old_size=*/this->capacity(),
172-
/*new_size=*/new_capacity);
174+
/*old_size=*/narrow_cast<std::size_t>(this->capacity()),
175+
/*new_size=*/narrow_cast<std::size_t>(new_capacity));
173176
if (grew) {
174177
this->capacity_end_ = this->data_ + new_capacity;
175178
} else {
176179
T *new_data =
177180
this->allocator_->template allocate_uninitialized_array<T>(
178-
new_capacity);
181+
narrow_cast<std::size_t>(new_capacity));
179182
T *new_data_end =
180183
std::uninitialized_move(this->data_, this->data_end_, new_data);
181184
this->clear();
@@ -185,7 +188,7 @@ class raw_bump_vector {
185188
}
186189
} else {
187190
this->data_ = this->allocator_->template allocate_uninitialized_array<T>(
188-
new_capacity);
191+
narrow_cast<std::size_t>(new_capacity));
189192
this->data_end_ = this->data_;
190193
this->capacity_end_ = this->data_ + new_capacity;
191194
}
@@ -248,8 +251,11 @@ class raw_bump_vector {
248251
void clear() {
249252
if (this->data_) {
250253
std::destroy(this->data_, this->data_end_);
251-
this->allocator_->deallocate(this->data_, this->size() * sizeof(T),
252-
alignof(T));
254+
this->allocator_->deallocate(
255+
this->data_,
256+
narrow_cast<std::size_t>(this->size() *
257+
static_cast<bump_vector_size>(sizeof(T))),
258+
alignof(T));
253259
this->release();
254260
}
255261
}
@@ -276,14 +282,15 @@ class raw_bump_vector {
276282
}
277283

278284
explicit operator std::basic_string_view<value_type>() const noexcept {
279-
return std::basic_string_view<value_type>(this->data_, this->size());
285+
return std::basic_string_view<value_type>(
286+
this->data_, narrow_cast<std::size_t>(this->size()));
280287
}
281288

282289
private:
283-
void reserve_grow_by_at_least(std::size_t minimum_new_entries) {
284-
std::size_t old_capacity = this->capacity();
285-
constexpr std::size_t minimum_capacity = 4;
286-
std::size_t new_size = (std::max)(
290+
void reserve_grow_by_at_least(size_type minimum_new_entries) {
291+
size_type old_capacity = this->capacity();
292+
constexpr size_type minimum_capacity = 4;
293+
size_type new_size = (std::max)(
287294
(std::max)(minimum_capacity, old_capacity + minimum_new_entries),
288295
old_capacity * 2);
289296
this->reserve_grow(new_size);

‎src/quick-lint-js/fe/lex.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1745,7 +1745,8 @@ lexer::parsed_identifier lexer::parse_identifier_slow(
17451745
normalized.append(4, u8'\0');
17461746
const char8* end =
17471747
encode_utf_8(code_point, &normalized.data()[normalized.size() - 4]);
1748-
normalized.resize(narrow_cast<std::size_t>(end - normalized.data()));
1748+
normalized.resize(
1749+
narrow_cast<bump_vector_size>(end - normalized.data()));
17491750
escape_sequences->emplace_back(escape_begin, escape.end);
17501751
}
17511752
} else {

‎src/quick-lint-js/fe/parse-expression.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -856,7 +856,7 @@ expression* parser::parse_async_expression_only(
856856
this->expressions_.allocator());
857857
call_children.emplace_back(this->make_expression<expression::variable>(
858858
async_or_await_token.identifier_name(), async_or_await_token.type));
859-
for (std::size_t i = 0; i < parameters.size(); ++i) {
859+
for (bump_vector_size i = 0; i < parameters.size(); ++i) {
860860
if (parameters.data()[i]->kind() != expression_kind::_invalid) {
861861
call_children.emplace_back(parameters.data()[i]);
862862
}

‎src/quick-lint-js/fe/parse-statement.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1183,7 +1183,7 @@ void parser::parse_and_visit_typescript_generic_parameters(
11831183
diag_typescript_generic_parameter_list_is_empty{
11841184
.expected_parameter = source_code_span::unit(less_end),
11851185
});
1186-
for (std::size_t i = 1; i < leading_commas.size(); ++i) {
1186+
for (bump_vector_size i = 1; i < leading_commas.size(); ++i) {
11871187
this->diag_reporter_->report(
11881188
diag_multiple_commas_in_generic_parameter_list{
11891189
.unexpected_comma = leading_commas[i],
@@ -1425,7 +1425,7 @@ void parser::parse_and_visit_function_declaration(
14251425
//
14261426
// We already declared the first overload (#0 above).
14271427
// The last overload (#3 above) is the real function.
1428-
for (std::size_t i = 0; i < overload_names.size() - 1; ++i) {
1428+
for (bump_vector_size i = 0; i < overload_names.size() - 1; ++i) {
14291429
identifier &overload_name = overload_names[i];
14301430
if (overload_name.normalized_name() !=
14311431
real_function_name.normalized_name()) {

‎test/test-vector.cpp

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ TEST(test_bump_vector, resize_allows_same_size) {
106106
v.emplace_back(200);
107107
std::uintptr_t old_v_data_pointer =
108108
reinterpret_cast<std::uintptr_t>(v.data());
109-
std::size_t old_capacity = v.capacity();
109+
bump_vector_size old_capacity = v.capacity();
110110

111111
v.resize(2);
112112

@@ -126,7 +126,7 @@ TEST(test_bump_vector, resize_allows_shrinking) {
126126
v.emplace_back(300);
127127
std::uintptr_t old_v_data_pointer =
128128
reinterpret_cast<std::uintptr_t>(v.data());
129-
std::size_t old_capacity = v.capacity();
129+
bump_vector_size old_capacity = v.capacity();
130130

131131
v.resize(2);
132132

@@ -146,7 +146,7 @@ TEST(test_bump_vector, resize_allows_growing_within_capacity) {
146146
v.emplace_back(200);
147147
std::uintptr_t old_v_data_pointer =
148148
reinterpret_cast<std::uintptr_t>(v.data());
149-
std::size_t old_capacity = v.capacity();
149+
bump_vector_size old_capacity = v.capacity();
150150

151151
ASSERT_GE(old_capacity, 3);
152152
v.resize(3);
@@ -223,8 +223,8 @@ TEST(test_bump_vector, move_constructor_preserves_pointers) {
223223

224224
std::uintptr_t old_v_data_pointer =
225225
reinterpret_cast<std::uintptr_t>(v.data());
226-
std::size_t old_v_capacity = v.capacity();
227-
std::size_t old_v_size = v.size();
226+
bump_vector_size old_v_capacity = v.capacity();
227+
bump_vector_size old_v_size = v.size();
228228

229229
bump_vector<int, decltype(alloc)> v2(std::move(v));
230230

0 commit comments

Comments
 (0)