Skip to content

Commit 8e657c0

Browse files
committed
fixups
1 parent 575cec6 commit 8e657c0

File tree

2 files changed

+48
-23
lines changed

2 files changed

+48
-23
lines changed

csrc/manager.cpp

Lines changed: 40 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,10 @@
88
#include <chrono>
99
#include <cuda_runtime.h>
1010
#include <optional>
11+
#include <system_error>
12+
#include <cstdlib>
13+
#include <cerrno>
14+
#include <limits>
1115
#include <random>
1216
#include <nvtx3/nvToolsExt.h>
1317
#include <nanobind/stl/string.h>
@@ -60,32 +64,49 @@ static void trigger_gc() {
6064

6165
BenchmarkParameters read_benchmark_parameters(int input_fd) {
6266
char buf[256];
63-
FILE* sig_file = fdopen(input_fd, "r");
64-
if (!sig_file) {
65-
throw std::runtime_error("Could not open signature pipe");
67+
FILE* inp_file = fdopen(input_fd, "r");
68+
if (!inp_file) {
69+
throw std::system_error(errno, std::generic_category(), "Could not open input pipe");
6670
}
67-
if (!fgets(buf, sizeof(buf), sig_file)) {
68-
fclose(sig_file);
69-
throw std::runtime_error("Could not read signature");
70-
}
71-
std::string signature = std::string(buf);
72-
73-
if (!fgets(buf, sizeof(buf), sig_file)) {
74-
fclose(sig_file);
75-
throw std::runtime_error("Could not read seed");
76-
}
77-
std::uint64_t seed = std::strtoull(buf, nullptr, 10);
7871

72+
auto read_line = [&](const char* field_name) {
73+
if (!fgets(buf, sizeof(buf), inp_file)) {
74+
int err = errno;
75+
if (feof(inp_file)) {
76+
fclose(inp_file);
77+
throw std::runtime_error(std::string("Unexpected EOF reading ") + field_name);
78+
}
79+
fclose(inp_file);
80+
throw std::system_error(err, std::generic_category(),
81+
std::string("Could not read ") + field_name);
82+
}
83+
};
7984

80-
if (!fgets(buf, sizeof(buf), sig_file)) {
81-
fclose(sig_file);
82-
throw std::runtime_error("Could not read repeats");
85+
read_line("signature");
86+
std::string signature(buf);
87+
if (signature.empty() || signature.back() != '\n') {
88+
fclose(inp_file);
89+
throw std::invalid_argument("Malformed or empty signature");
90+
}
91+
signature.pop_back();
92+
93+
read_line("seed");
94+
char* end;
95+
std::uint64_t seed = std::strtoull(buf, &end, 10);
96+
if (end == buf || (*end != '\n' && *end != '\0')) {
97+
fclose(inp_file);
98+
throw std::invalid_argument("Invalid seed: " + std::string(buf));
8399
}
100+
101+
read_line("repeats");
84102
long repeats = std::strtol(buf, nullptr, 10);
85-
if (repeats >= std::numeric_limits<int>::max()) {
86-
throw std::runtime_error("Repeats exceeds 2^32");
103+
if (repeats >= std::numeric_limits<int>::max() || repeats < 2) {
104+
fclose(inp_file);
105+
throw std::invalid_argument(
106+
"Invalid number of repeats: " + std::to_string(repeats));
87107
}
88108

109+
fclose(inp_file);
89110
return {signature, seed, static_cast<int>(repeats)};
90111
}
91112

python/pygpubench/__init__.py

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,13 @@ class BenchmarkResult:
7272

7373
@property
7474
def success(self):
75+
"""
76+
Returns whether the benchmark was successful.
77+
A successful benchmark means that _all_ iterations completed without errors.
78+
In particular, for cases where we decide to run fewer iterations due to long running
79+
times, we want to give feedback for the running times, but we cannot consider these
80+
as valid benchmarks.
81+
"""
7582
return self.full and self.errors is None
7683

7784

@@ -167,10 +174,6 @@ def do_bench_isolated(
167174

168175
parent_tb_conn, child_tb_conn = ctx.Pipe(duplex=False)
169176

170-
# Make write_fd inheritable before creating the Process so the spawned
171-
# child receives it as a live, open fd.
172-
os.set_inheritable(write_fd, True)
173-
174177
process = ctx.Process(
175178
target=_do_bench_impl,
176179
args=(
@@ -189,6 +192,7 @@ def do_bench_isolated(
189192
process.start()
190193
child_tb_conn.close()
191194
result_child.close()
195+
sig_r.close()
192196

193197
process.join(timeout=timeout)
194198

0 commit comments

Comments
 (0)