Skip to content

Commit fc1286d

Browse files
committed
Benchmark configuration_loader
I noticed some performance problems in configuration_loader, particularly if there is no config file present. Add a benchmark to make it easier to track down the bug.
1 parent 6697f39 commit fc1286d

File tree

7 files changed

+94
-20
lines changed

7 files changed

+94
-20
lines changed

benchmark/CMakeLists.txt

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,10 @@ quick_lint_js_add_benchmark_executable(
2121
quick-lint-js-benchmark-cli-location
2222
benchmark-cli-location.cpp
2323
)
24+
quick_lint_js_add_benchmark_executable(
25+
quick-lint-js-benchmark-configuration-loader
26+
benchmark-configuration-loader.cpp
27+
)
2428
quick_lint_js_add_benchmark_executable(
2529
quick-lint-js-benchmark-document
2630
benchmark-document.cpp
Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
// Copyright (C) 2020 Matthew Glazar
2+
// See end of file for extended copyright information.
3+
4+
#include <benchmark/benchmark.h>
5+
#include <quick-lint-js/configuration-loader.h>
6+
#include <quick-lint-js/file.h>
7+
#include <quick-lint-js/narrow-cast.h>
8+
#include <quick-lint-js/options.h>
9+
#include <quick-lint-js/temporary-directory.h>
10+
#include <string>
11+
12+
namespace quick_lint_js {
13+
namespace {
14+
void benchmark_no_config_file(::benchmark::State& state) {
15+
int extra_depth = narrow_cast<int>(state.range(0));
16+
std::string temp_dir = make_temporary_directory();
17+
18+
std::string path = temp_dir;
19+
for (int i = 0; i < extra_depth; ++i) {
20+
path += "/subdir" + std::to_string(i);
21+
create_directory(path);
22+
}
23+
path += "/hello.js";
24+
write_file(path, u8"");
25+
26+
for (auto _ : state) {
27+
configuration_loader loader;
28+
configuration* config = loader.load_for_file(file_to_lint{
29+
.path = path.c_str(),
30+
.config_file = nullptr,
31+
.is_stdin = false,
32+
.vim_bufnr = std::nullopt,
33+
});
34+
::benchmark::DoNotOptimize(config);
35+
}
36+
}
37+
BENCHMARK(benchmark_no_config_file)
38+
->Arg(0)
39+
->Arg(8)
40+
->Arg(16)
41+
->Arg(24)
42+
->Arg(32)
43+
->Arg(48)
44+
->Arg(64);
45+
} // namespace
46+
} // namespace quick_lint_js
47+
48+
// quick-lint-js finds bugs in JavaScript programs.
49+
// Copyright (C) 2020 Matthew Glazar
50+
//
51+
// This file is part of quick-lint-js.
52+
//
53+
// quick-lint-js is free software: you can redistribute it and/or modify
54+
// it under the terms of the GNU General Public License as published by
55+
// the Free Software Foundation, either version 3 of the License, or
56+
// (at your option) any later version.
57+
//
58+
// quick-lint-js is distributed in the hope that it will be useful,
59+
// but WITHOUT ANY WARRANTY; without even the implied warranty of
60+
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
61+
// GNU General Public License for more details.
62+
//
63+
// You should have received a copy of the GNU General Public License
64+
// along with quick-lint-js. If not, see <https://www.gnu.org/licenses/>.

src/CMakeLists.txt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,7 @@ quick_lint_js_add_library(
112112
quick-lint-js/parse.h
113113
quick-lint-js/pipe-reader.h
114114
quick-lint-js/simd.h
115+
quick-lint-js/temporary-directory.h
115116
quick-lint-js/text-error-reporter.h
116117
quick-lint-js/translation-data.h
117118
quick-lint-js/translation.h
@@ -124,6 +125,7 @@ quick_lint_js_add_library(
124125
quick-lint-js/vim-qflist-json-error-reporter.h
125126
quick-lint-js/warning.h
126127
quick-lint-js/web-demo-location.h
128+
temporary-directory.cpp
127129
text-error-reporter.cpp
128130
translation-data.cpp
129131
translation.cpp

test/quick-lint-js/temporary-directory.h renamed to src/quick-lint-js/temporary-directory.h

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,14 @@
77
#include <string>
88

99
namespace quick_lint_js {
10+
// Crashes on failure.
1011
std::string make_temporary_directory();
11-
void delete_directory_recursive(const std::string &path);
12+
13+
// Crashes on failure.
14+
void create_directory(const std::string& path);
15+
16+
// Crashes on failure.
17+
void delete_directory_recursive(const std::string& path);
1218
}
1319

1420
#endif

test/temporary-directory.cpp renamed to src/temporary-directory.cpp

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,10 @@
2121
#include <filesystem>
2222
#endif
2323

24+
#if QLJS_HAVE_MKDTEMP
25+
#include <sys/stat.h>
26+
#endif
27+
2428
namespace quick_lint_js {
2529
#if QLJS_HAVE_MKDTEMP
2630
std::string make_temporary_directory() {
@@ -63,6 +67,18 @@ std::string make_temporary_directory() {
6367
#error "Unsupported platform"
6468
#endif
6569

70+
void create_directory(const std::string &path) {
71+
#if QLJS_HAVE_STD_FILESYSTEM
72+
std::filesystem::create_directory(path);
73+
#else
74+
if (::mkdir(path.c_str(), 0755) != 0) {
75+
std::fprintf(stderr, "error: failed to create directory %s: %s\n",
76+
path.c_str(), std::strerror(errno));
77+
std::terminate();
78+
}
79+
#endif
80+
}
81+
6682
#if QLJS_HAVE_FTS_H
6783
void delete_directory_recursive(const std::string &path) {
6884
char *paths[] = {const_cast<char *>(path.c_str()), nullptr};
@@ -117,7 +133,7 @@ void delete_directory_recursive(const std::string &path) {
117133
::fts_close(fts);
118134
}
119135
#elif QLJS_HAVE_STD_FILESYSTEM
120-
void delete_directory_recursive(const std::string &path) {
136+
void delete_directory_recursive(const std::string& path) {
121137
std::filesystem::remove_all(std::filesystem::path(path));
122138
}
123139
#endif

test/CMakeLists.txt

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,7 @@ quick_lint_js_add_executable(
2121
quick-lint-js/spy-lsp-endpoint-remote.h
2222
quick-lint-js/spy-lsp-message-parser.h
2323
quick-lint-js/spy-visitor.h
24-
quick-lint-js/temporary-directory.h
2524
spy-visitor.cpp
26-
temporary-directory.cpp
2725
test-array.cpp
2826
test-assert.cpp
2927
test-bit.cpp

test/test-configuration-loader.cpp

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,6 @@
2121
#include <filesystem>
2222
#endif
2323

24-
#if QLJS_HAVE_MKDTEMP
25-
#include <sys/stat.h>
26-
#endif
27-
2824
QLJS_WARNING_IGNORE_GCC("-Wmissing-field-initializers")
2925

3026
#define EXPECT_DEFAULT_CONFIG(config) \
@@ -50,18 +46,6 @@ using namespace std::literals::string_view_literals;
5046

5147
namespace quick_lint_js {
5248
namespace {
53-
void create_directory(const std::string& path) {
54-
#if QLJS_HAVE_STD_FILESYSTEM
55-
std::filesystem::create_directory(path);
56-
#else
57-
if (::mkdir(path.c_str(), 0755) != 0) {
58-
std::fprintf(stderr, "error: failed to create directory %s: %s\n",
59-
path.c_str(), std::strerror(errno));
60-
std::terminate();
61-
}
62-
#endif
63-
}
64-
6549
std::string get_current_working_directory() {
6650
#if QLJS_HAVE_STD_FILESYSTEM
6751
return std::filesystem::current_path().string();

0 commit comments

Comments
 (0)