Skip to content

Commit 7f7eba5

Browse files
committed
refactor(container): make padded_string use pointer-width sizes
I want to be more consistent with what size type our containers use. Switch padded_string to use ptrdiff_t like bump_vector and span use. On x86_64, this improves performance slightly: Benchmark Time CPU Time Old Time New CPU Old CPU New -------------------------------------------------------------------------------------------------------------------------- benchmark_parse_file -0.0098 -0.0098 2077507 2057161 2077506 2057126 benchmark_parse_file -0.0110 -0.0110 2081085 2058265 2081055 2058225 benchmark_parse_file -0.0097 -0.0097 2081786 2061641 2081723 2061626 benchmark_parse_file -0.0110 -0.0110 2083902 2060935 2083877 2060918 benchmark_parse_file -0.0100 -0.0100 2084612 2063836 2084597 2063792 benchmark_parse_file -0.0099 -0.0099 2085052 2064490 2084987 2064432 benchmark_parse_file -0.0118 -0.0118 2085263 2060737 2085242 2060705 benchmark_parse_file -0.0080 -0.0080 2084838 2068112 2084802 2068085 benchmark_parse_file -0.0120 -0.0120 2083507 2058601 2083488 2058517 benchmark_parse_file -0.0107 -0.0107 2083857 2061491 2083813 2061443 benchmark_parse_file_pvalue 0.0002 0.0002 U Test, Repetitions: 10 vs 10 benchmark_parse_file_mean -0.0104 -0.0104 2083141 2061527 2083109 2061487 benchmark_parse_file_median -0.0109 -0.0109 2083880 2061213 2083845 2061181 benchmark_parse_file_stddev +0.3608 +0.3665 2409 3278 2402 3282 benchmark_parse_file_cv +0.3750 +0.3808 0 0 0 0 OVERALL_GEOMEAN -0.0104 -0.0104 0 0 0 0
1 parent b23f1c6 commit 7f7eba5

File tree

6 files changed

+32
-16
lines changed

6 files changed

+32
-16
lines changed

benchmark/benchmark-utf-8.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,8 @@ string8 repeat(string8_view s, int count) {
1919
void benchmark_advance_lsp_characters_in_utf_8(::benchmark::State& state,
2020
string8_view input) {
2121
padded_string padded_input(input);
22-
int total_character_count = narrow_cast<int>(
23-
count_lsp_characters_in_utf_8(&padded_input, padded_input.size()));
22+
int total_character_count = narrow_cast<int>(count_lsp_characters_in_utf_8(
23+
&padded_input, narrow_cast<int>(padded_input.size())));
2424
// Avoid count==size optimizations:
2525
int characters_to_count = total_character_count - 1;
2626

src/quick-lint-js/container/padded-string.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,14 @@
1111
#include <string>
1212

1313
namespace quick_lint_js {
14+
using padded_string_size = std::ptrdiff_t;
15+
1416
// Like std::string, but guaranteed to have several null bytes at the end.
1517
//
1618
// padded_string enables using SIMD instructions without extra bounds checking.
1719
class padded_string {
1820
public:
19-
using size_type = int;
21+
using size_type = padded_string_size;
2022

2123
static constexpr size_type padding_size = 64;
2224

src/quick-lint-js/io/file.cpp

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -65,30 +65,34 @@ platform_file_io_error file_too_large_error() {
6565
}
6666

6767
result<void, platform_file_io_error> read_file_buffered(
68-
platform_file_ref file, int buffer_size, padded_string *out_content) {
68+
platform_file_ref file, padded_string_size buffer_size,
69+
padded_string *out_content) {
6970
// TODO(strager): Use byte_buffer to avoid copying the file content every
7071
// iteration.
7172
for (;;) {
72-
int size_before = out_content->size();
73+
padded_string_size size_before = out_content->size();
7374
{
74-
std::optional<int> new_size = checked_add(size_before, buffer_size);
75+
std::optional<padded_string_size> new_size =
76+
checked_add(size_before, buffer_size);
7577
if (!new_size.has_value()) {
7678
// TODO(strager): Should we try a small buffer size?
7779
return failed_result(file_too_large_error());
7880
}
7981
out_content->resize_grow_uninitialized(size_before + buffer_size);
8082
}
8183

82-
file_read_result read_result =
83-
file.read(&out_content->data()[size_before], buffer_size);
84+
// TODO(strager): Get rid of this narrow_cast.
85+
file_read_result read_result = file.read(&out_content->data()[size_before],
86+
narrow_cast<int>(buffer_size));
8487
if (!read_result.ok()) return read_result.propagate();
8588
if (read_result.at_end_of_file()) {
8689
// We read the entire file.
8790
out_content->resize(size_before);
8891
return {};
8992
}
90-
std::optional<int> new_size =
91-
checked_add(size_before, read_result.bytes_read());
93+
// TODO(strager): Get rid of this narrow_cast.
94+
std::optional<padded_string_size> new_size = checked_add(
95+
size_before, narrow_cast<padded_string_size>(read_result.bytes_read()));
9296
QLJS_ASSERT(new_size.has_value());
9397
out_content->resize(*new_size);
9498
}

src/quick-lint-js/lsp/lsp-location.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,9 @@ const char8 *lsp_locator::from_position(lsp_position position) const noexcept {
7171
bool line_is_ascii = this->line_is_ascii_[narrow_cast<std::size_t>(line)];
7272
bool is_last_line = line == number_of_lines - 1;
7373
if (is_last_line) {
74-
offset_type line_length = this->input_.size() - line_begin_offset;
74+
// TODO(strager): Get rid of this narrow_cast.
75+
offset_type line_length =
76+
narrow_cast<offset_type>(this->input_.size() - line_begin_offset);
7577
if (line_is_ascii) {
7678
if (character > line_length) {
7779
return &this->input_[this->input_.size()];

test/test-padded-string.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -134,8 +134,8 @@ TEST(test_padded_string, move_assigning_empty_string_copies_pointers) {
134134
namespace {
135135
void expect_null_terminated(const padded_string &s) {
136136
const char8 *data = s.c_str();
137-
for (int i = 0; i < s.padding_size; ++i) {
138-
int index = s.size() + i;
137+
for (padded_string_size i = 0; i < s.padding_size; ++i) {
138+
padded_string_size index = s.size() + i;
139139
EXPECT_EQ(data[index], u8'\0') << "index=" << index;
140140
}
141141
}

test/test-utf-8.cpp

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -504,7 +504,9 @@ TEST(test_advance_lsp_characters_in_utf_8,
504504
namespace {
505505
std::ptrdiff_t count_lsp_characters_in_utf_8(
506506
padded_string_view utf_8) noexcept {
507-
return quick_lint_js::count_lsp_characters_in_utf_8(utf_8, utf_8.size());
507+
// TODO(strager): Get rid of this narrow_cast.
508+
return quick_lint_js::count_lsp_characters_in_utf_8(
509+
utf_8, narrow_cast<int>(utf_8.size()));
508510
}
509511

510512
std::ptrdiff_t count_lsp_characters_in_utf_8(
@@ -564,7 +566,10 @@ TEST(test_count_lsp_characters_in_utf_8,
564566
"\xed\xbf\xbf"_padded, // U+DFFF
565567
}) {
566568
SCOPED_TRACE(input);
567-
EXPECT_EQ(count_lsp_characters_in_utf_8(input, input.size()), input.size());
569+
// TODO(strager): Get rid of this narrow_cast.
570+
EXPECT_EQ(
571+
count_lsp_characters_in_utf_8(input, narrow_cast<int>(input.size())),
572+
input.size());
568573
}
569574
}
570575

@@ -590,7 +595,10 @@ TEST(test_count_lsp_characters_in_utf_8,
590595
"\xfc\x83\xbf\xbf\xbf\xbf"_padded, // U+03FFFFFF
591596
}) {
592597
SCOPED_TRACE(input);
593-
EXPECT_EQ(count_lsp_characters_in_utf_8(input, input.size()), input.size());
598+
// TODO(strager): Get rid of this narrow_cast.
599+
EXPECT_EQ(
600+
count_lsp_characters_in_utf_8(input, narrow_cast<int>(input.size())),
601+
input.size());
594602
}
595603
}
596604

0 commit comments

Comments
 (0)