Skip to content

Commit

Permalink
Address feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
@nv-hwoo
nv-hwoo committed Aug 10, 2023
1 parent a85fff0 commit abe684f
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 38 deletions.
55 changes: 30 additions & 25 deletions src/c++/perf_analyzer/command_line_parser.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -812,7 +812,7 @@ CLParser::ParseCommandLine(int argc, char** argv)
case 1: {
std::string max_threads{optarg};
if (std::stoi(max_threads) > 0) {
params_->max_threads = std::stoi(max_threads);
params_->max_threads = std::stoull(max_threads);
params_->max_threads_specified = true;
} else {
Usage("Failed to parse --max-threads. The value must be > 0.");
Expand All @@ -822,7 +822,7 @@ CLParser::ParseCommandLine(int argc, char** argv)
case 2: {
std::string sequence_length{optarg};
if (std::stoi(sequence_length) > 0) {
params_->sequence_length = std::stoi(sequence_length);
params_->sequence_length = std::stoull(sequence_length);
} else {
std::cerr << "WARNING: The sequence length must be > 0. Perf "
"Analyzer will use default value if it is measuring "
Expand Down Expand Up @@ -875,7 +875,7 @@ CLParser::ParseCommandLine(int argc, char** argv)
case 'p': {
std::string measurement_window_ms{optarg};
if (std::stoi(measurement_window_ms) > 0) {
params_->measurement_window_ms = std::stoi(measurement_window_ms);
params_->measurement_window_ms = std::stoull(measurement_window_ms);
} else {
Usage(
"Failed to parse --measurement-interval (-p). The value must "
Expand All @@ -897,10 +897,10 @@ CLParser::ParseCommandLine(int argc, char** argv)
}
int64_t val;
if (colon_pos == std::string::npos) {
val = std::stoll(arg.substr(pos, colon_pos));
val = std::stoull(arg.substr(pos, colon_pos));
pos = colon_pos;
} else {
val = std::stoll(arg.substr(pos, colon_pos - pos));
val = std::stoull(arg.substr(pos, colon_pos - pos));
pos = colon_pos + 1;
}
switch (index) {
Expand All @@ -924,9 +924,8 @@ CLParser::ParseCommandLine(int argc, char** argv)
std::string latency_threshold_ms{optarg};
if (std::stoi(latency_threshold_ms) == 0) {
params_->latency_threshold_ms = NO_LIMIT;
}
if (std::stoi(latency_threshold_ms) > 0) {
params_->latency_threshold_ms = std::stoi(latency_threshold_ms);
} else if (std::stoi(latency_threshold_ms) > 0) {
params_->latency_threshold_ms = std::stoull(latency_threshold_ms);
} else {
Usage(
"Failed to parse --latency-threshold (-l). The value must be "
Expand All @@ -949,10 +948,10 @@ CLParser::ParseCommandLine(int argc, char** argv)
case 10:
case 'r': {
std::string max_trials{optarg};
if (std::stoi(max_trials) >= 0) {
params_->max_trials = std::stoi(max_trials);
if (std::stoi(max_trials) > 0) {
params_->max_trials = std::stoull(max_trials);
} else {
Usage("Failed to parse --max-trials (-r). The value must be >= 0.");
Usage("Failed to parse --max-trials (-r). The value must be > 0.");
}
break;
}
Expand All @@ -977,7 +976,7 @@ CLParser::ParseCommandLine(int argc, char** argv)
case 12: {
std::string string_length{optarg};
if (std::stoi(string_length) > 0) {
params_->string_length = std::stoi(string_length);
params_->string_length = std::stoull(string_length);
} else {
Usage("Failed to parse --string-length. The value must be > 0");
}
Expand Down Expand Up @@ -1025,7 +1024,7 @@ CLParser::ParseCommandLine(int argc, char** argv)
case 17: {
std::string num_of_sequences{optarg};
if (std::stoi(num_of_sequences) > 0) {
params_->num_of_sequences = std::stoi(num_of_sequences);
params_->num_of_sequences = std::stoul(num_of_sequences);
} else {
Usage("Failed to parse --num-of-sequences. The value must be > 0.");
}
Expand Down Expand Up @@ -1088,7 +1087,7 @@ CLParser::ParseCommandLine(int argc, char** argv)
case 22: {
std::string output_shm_size{optarg};
if (std::stoi(output_shm_size) >= 0) {
params_->output_shm_size = std::stoi(output_shm_size);
params_->output_shm_size = std::stoull(output_shm_size);
} else {
Usage(
"Failed to parse --output-shared-memory-size. The value must "
Expand Down Expand Up @@ -1153,7 +1152,14 @@ CLParser::ParseCommandLine(int argc, char** argv)
break;
}
case 27: {
params_->measurement_request_count = std::atoi(optarg);
std::string request_count{optarg};
if (std::stoi(request_count) > 0) {
params_->measurement_request_count = std::stoull(request_count);
} else {
Usage(
"Failed to parse --measurement-request-count. The value must "
"be > 0.");
}
break;
}
case 28: {
Expand Down Expand Up @@ -1478,10 +1484,16 @@ CLParser::ParseCommandLine(int argc, char** argv)
case 'x':
params_->model_version = optarg;
break;
case 'b':
params_->batch_size = std::atoi(optarg);
params_->using_batch_size = true;
case 'b': {
std::string batch_size{optarg};
if (std::stoi(batch_size) > 0) {
params_->batch_size = std::stoull(batch_size);
params_->using_batch_size = true;
} else {
Usage("Failed to parse -b (batch size). The value must be > 0.");
}
break;
}
case 't':
params_->using_old_options = true;
params_->concurrent_request_count = std::atoi(optarg);
Expand Down Expand Up @@ -1550,13 +1562,6 @@ CLParser::VerifyOptions()
if (params_->model_name.empty()) {
Usage("Failed to parse -m (model name). The value must be specified.");
}
if (params_->batch_size <= 0) {
Usage("Failed to parse -b (batch size). The value must be > 0.");
}
if (params_->measurement_request_count <= 0) {
Usage(
"Failed to parse --measurement-request-count. The value must be > 0.");
}
if (params_->concurrency_range.start <= 0 ||
params_->concurrent_request_count < 0) {
Usage("The start of the search range must be > 0");
Expand Down
2 changes: 1 addition & 1 deletion src/c++/perf_analyzer/command_line_parser.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ struct PerfAnalyzerParameters {
std::string url{"localhost:8000"};
std::string model_name;
std::string model_version;
int32_t batch_size = 1;
uint64_t batch_size = 1;
bool using_batch_size = false;
int32_t concurrent_request_count = 1;
clientbackend::ProtocolType protocol = clientbackend::ProtocolType::HTTP;
Expand Down
34 changes: 22 additions & 12 deletions src/c++/perf_analyzer/test_command_line_parser.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -194,17 +194,6 @@ CHECK_PARAMS(PAParamsPtr act, PAParamsPtr exp)
CAPTURE(exp_val); \
} \
\
SUBCASE("set to 0") \
{ \
int argc = 5; \
char* argv[argc] = {app_name, "-m", model_name, option_name, "0"}; \
\
REQUIRE_NOTHROW(act = parser.Parse(argc, argv)); \
CHECK(!parser.UsageCalled()); \
\
exp_val = 0; \
} \
\
SUBCASE("negative value") \
{ \
int argc = 5; \
Expand Down Expand Up @@ -1175,6 +1164,16 @@ TEST_CASE("Testing Command Line Parser")
"--latency-threshold (-l)", "The value must be >= 0 msecs.");
CHECK_INT_OPTION(
"--latency-threshold", exp->latency_threshold_ms, expected_msg);

SUBCASE("set to 0")
{
int argc = 5;
char* argv[argc] = {
app_name, "-m", model_name, "--latency-threshold", "0"};

REQUIRE_NOTHROW(act = parser.Parse(argc, argv));
CHECK(!parser.UsageCalled());
}
}

SUBCASE("Option : --stability-percentage")
Expand Down Expand Up @@ -1242,8 +1241,19 @@ TEST_CASE("Testing Command Line Parser")
SUBCASE("Option : --max-trials")
{
expected_msg =
CreateUsageMessage("--max-trials (-r)", "The value must be >= 0.");
CreateUsageMessage("--max-trials (-r)", "The value must be > 0.");
CHECK_INT_OPTION("--max-trials", exp->max_trials, expected_msg);

SUBCASE("set to 0")
{
int argc = 5;
char* argv[argc] = {app_name, "-m", model_name, "--max-trials", "0"};

REQUIRE_NOTHROW(act = parser.Parse(argc, argv));
CHECK(parser.UsageCalled());

CHECK_STRING("Usage Message", parser.GetUsageMessage(), expected_msg);
}
}

SUBCASE("Option : --collect-metrics")
Expand Down

0 comments on commit abe684f

Please sign in to comment.