Skip to content

Commit a380e5c

Browse files
authored
Fix bug for same option provided multiple times in perf test (microsoft#25716)
### Description If an option appears multiple times: Unlike `getopt` just returns it again in the parsing loop, `Abseil` processes them in order, and the last one wins (overwrites earlier values). This PR fixes the bug for `-f` free dimension override by name and `-F `free dimension override by denotation. see microsoft#25714
1 parent 1ad9f12 commit a380e5c

File tree

4 files changed

+72
-34
lines changed

4 files changed

+72
-34
lines changed

onnxruntime/test/perftest/command_args_parser.cc

Lines changed: 18 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,14 @@ static const onnxruntime::perftest::PerformanceTestConfig& DefaultPerformanceTes
3030
return default_config;
3131
}
3232

33-
ABSL_FLAG(std::string, f, "", "Specifies a free dimension by name to override to a specific value for performance optimization.");
34-
ABSL_FLAG(std::string, F, "", "Specifies a free dimension by denotation to override to a specific value for performance optimization.");
33+
ABSL_FLAG(std::string, f, "",
34+
"Specifies a free dimension by name to override to a specific value for performance optimization.\n"
35+
"[Usage]: -f \"dimension_name1:override_value1\" -f \"dimension_name2:override_value2\" ... or"
36+
" -f \"dimension_name1:override_value1 dimension_name2:override_value2 ... \". Override value must > 0.");
37+
ABSL_FLAG(std::string, F, "",
38+
"Specifies a free dimension by denotation to override to a specific value for performance optimization.\n"
39+
"[Usage]: -f \"dimension_denotation1:override_value1\" -f \"dimension_denotation2:override_value2\" ... or"
40+
" -f \"dimension_denotation1:override_value1 dimension_denotation2 : override_value2... \". Override value must > 0.");
3541
ABSL_FLAG(std::string, m, "duration", "Specifies the test mode. Value could be 'duration' or 'times'.");
3642
ABSL_FLAG(std::string, e, "cpu", "Specifies the provider 'cpu','cuda','dnnl','tensorrt', 'nvtensorrtrtx', 'openvino', 'dml', 'acl', 'nnapi', 'coreml', 'qnn', 'snpe', 'rocm', 'migraphx', 'xnnpack', 'vitisai' or 'webgpu'.");
3743
ABSL_FLAG(size_t, r, DefaultPerformanceTestConfig().run_config.repeated_times, "Specifies the repeated times if running in 'times' test mode.");
@@ -168,26 +174,6 @@ ABSL_FLAG(bool, h, false, "Print program usage.");
168174
namespace onnxruntime {
169175
namespace perftest {
170176

171-
static bool ParseDimensionOverride(std::string& dim_identifier, int64_t& override_val, const char* option) {
172-
std::basic_string<char> free_dim_str(option);
173-
size_t delimiter_location = free_dim_str.find(":");
174-
if (delimiter_location >= free_dim_str.size() - 1) {
175-
return false;
176-
}
177-
dim_identifier = free_dim_str.substr(0, delimiter_location);
178-
std::string override_val_str = free_dim_str.substr(delimiter_location + 1, std::string::npos);
179-
ORT_TRY {
180-
override_val = std::stoll(override_val_str.c_str());
181-
if (override_val <= 0) {
182-
return false;
183-
}
184-
}
185-
ORT_CATCH(...) {
186-
return false;
187-
}
188-
return true;
189-
}
190-
191177
std::string CustomUsageMessage() {
192178
std::ostringstream oss;
193179
oss << "onnxruntime_perf_test [options...] model_path [result_file]\n\n";
@@ -212,33 +198,33 @@ bool CommandLineParser::ParseArguments(PerformanceTestConfig& test_config, int a
212198
absl::SetFlagsUsageConfig(config);
213199
absl::SetProgramUsageMessage(CustomUsageMessage());
214200

215-
auto utf8_strings = utils::ConvertArgvToUtf8Strings(argc, argv);
216-
auto utf8_argv = utils::CStringsFromStrings(utf8_strings);
201+
auto utf8_argv_strings = utils::ConvertArgvToUtf8Strings(argc, argv);
202+
auto utf8_argv = utils::CStringsFromStrings(utf8_argv_strings);
217203
auto positional = absl::ParseCommandLine(static_cast<int>(utf8_argv.size()), utf8_argv.data());
218204

219205
// -f
220206
{
221207
const auto& dim_override_str = absl::GetFlag(FLAGS_f);
222208
if (!dim_override_str.empty()) {
223-
std::string dim_name;
224-
int64_t override_val;
225-
if (!ParseDimensionOverride(dim_name, override_val, dim_override_str.c_str())) {
209+
// Abseil doesn't support the same option being provided multiple times - only the last occurrence is applied.
210+
// To preserve the previous usage of '-f', where users may specify it multiple times to override different dimension names,
211+
// we need to manually parse argv.
212+
std::string option = "f";
213+
if (!ParseDimensionOverrideFromArgv(argc, utf8_argv_strings, option, test_config.run_config.free_dim_name_overrides)) {
226214
return false;
227215
}
228-
test_config.run_config.free_dim_name_overrides[dim_name] = override_val;
229216
}
230217
}
231218

232219
// -F
233220
{
234221
const auto& dim_override_str = absl::GetFlag(FLAGS_F);
235222
if (!dim_override_str.empty()) {
236-
std::string dim_denotation;
237-
int64_t override_val;
238-
if (!ParseDimensionOverride(dim_denotation, override_val, dim_override_str.c_str())) {
223+
// Same reason as '-f' above to manully parse argv.
224+
std::string option = "F";
225+
if (!ParseDimensionOverrideFromArgv(argc, utf8_argv_strings, option, test_config.run_config.free_dim_denotation_overrides)) {
239226
return false;
240227
}
241-
test_config.run_config.free_dim_denotation_overrides[dim_denotation] = override_val;
242228
}
243229
}
244230

onnxruntime/test/perftest/main.cc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ int real_main(int argc, char* argv[]) {
3535
}
3636
ORT_CATCH(const Ort::Exception& e) {
3737
ORT_HANDLE_EXCEPTION([&]() {
38-
fprintf(stderr, "Error creating environment: %s \n", e.what());
38+
std::cerr << "Error creating environment: " << e.what() << std::endl;
3939
failed = true;
4040
});
4141
}
@@ -98,7 +98,7 @@ int main(int argc, char* argv[]) {
9898
}
9999
ORT_CATCH(const std::exception& ex) {
100100
ORT_HANDLE_EXCEPTION([&]() {
101-
fprintf(stderr, "%s\n", ex.what());
101+
std::cerr << ex.what() << std::endl;
102102
retval = -1;
103103
});
104104
}

onnxruntime/test/perftest/strings_helper.cc

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,53 @@ void ParseSessionConfigs(const std::string& configs_string,
5656
}
5757
}
5858

59+
bool ParseDimensionOverride(const std::string& input, std::map<std::string, int64_t>& free_dim_override_map) {
60+
std::stringstream ss(input);
61+
std::string free_dim_str;
62+
63+
while (std::getline(ss, free_dim_str, ' ')) {
64+
if (!free_dim_str.empty()) {
65+
size_t delimiter_location = free_dim_str.find(":");
66+
if (delimiter_location >= free_dim_str.size() - 1) {
67+
return false;
68+
}
69+
std::string dim_identifier = free_dim_str.substr(0, delimiter_location);
70+
std::string override_val_str = free_dim_str.substr(delimiter_location + 1, std::string::npos);
71+
ORT_TRY {
72+
int64_t override_val = std::stoll(override_val_str.c_str());
73+
if (override_val <= 0) {
74+
return false;
75+
}
76+
free_dim_override_map[dim_identifier] = override_val;
77+
}
78+
ORT_CATCH(const std::exception& ex) {
79+
ORT_HANDLE_EXCEPTION([&]() {
80+
std::cerr << "Error parsing free dimension override value: " << override_val_str.c_str() << ", " << ex.what() << std::endl;
81+
});
82+
return false;
83+
}
84+
}
85+
}
86+
87+
return true;
88+
}
89+
90+
bool ParseDimensionOverrideFromArgv(int argc, std::vector<std::string>& argv, std::string& option, std::map<std::string, int64_t>& free_dim_override_map) {
91+
for (int i = 1; i < argc; ++i) {
92+
auto utf8_arg = argv[i];
93+
if (utf8_arg == ("-" + option) || utf8_arg == ("--" + option)) {
94+
auto value_idx = i + 1;
95+
if (value_idx >= argc || argv[value_idx][0] == '-') {
96+
std::cerr << utf8_arg << " should be followed by a key-value pair." << std::endl;
97+
return false;
98+
}
99+
100+
if (!ParseDimensionOverride(argv[value_idx], free_dim_override_map)) return false;
101+
}
102+
}
103+
return true;
104+
}
105+
59106
void ParseEpOptions(const std::string& input, std::vector<std::unordered_map<std::string, std::string>>& result) {
60107
auto tokens = utils::SplitString(input, ";", true);
61108

onnxruntime/test/perftest/strings_helper.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
// SPDX-FileCopyrightText: Copyright 2024 Arm Limited and/or its affiliates <open-source-office@arm.com>
44
// Licensed under the MIT License.
55
#include <string_view>
6+
#include <map>
67
#include <unordered_map>
78
#include <unordered_set>
89
#include <vector>
@@ -14,6 +15,10 @@ void ParseSessionConfigs(const std::string& configs_string,
1415
std::unordered_map<std::string, std::string>& session_configs,
1516
const std::unordered_set<std::string>& available_keys = {});
1617

18+
bool ParseDimensionOverride(const std::string& input, std::map<std::string, int64_t>& free_dim_override_map);
19+
20+
bool ParseDimensionOverrideFromArgv(int argc, std::vector<std::string>& argv, std::string& option, std::map<std::string, int64_t>& free_dim_override_map);
21+
1722
void ParseEpList(const std::string& input, std::vector<std::string>& result);
1823

1924
void ParseEpOptions(const std::string& input, std::vector<std::unordered_map<std::string, std::string>>& result);

0 commit comments

Comments
 (0)