Add support for unsigned parameters in FieldTrialParser

Bug: webrtc:10932
Change-Id: I3f56244a6be532065e4096cf1a289e27a032bc44
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/150886
Commit-Queue: Björn Terelius <terelius@webrtc.org>
Reviewed-by: Sebastian Jansson <srte@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#29018}
This commit is contained in:
Bjorn Terelius 2019-08-30 09:39:31 +02:00 committed by Commit Bot
parent f3a197e553
commit 9f00f0e533
7 changed files with 81 additions and 12 deletions

View file

@ -56,6 +56,7 @@ rtc_static_library("field_trial_parser") {
"../../api/units:time_delta",
"../../rtc_base:checks",
"../../rtc_base:logging",
"../../rtc_base:safe_conversions",
"../../rtc_base:stringutils",
"//third_party/abseil-cpp/absl/memory:memory",
"//third_party/abseil-cpp/absl/strings:strings",

View file

@ -9,6 +9,8 @@
*/
#include "rtc_base/experiments/field_trial_parser.h"
#include <inttypes.h>
#include <algorithm>
#include <map>
#include <type_traits>
@ -16,6 +18,7 @@
#include "rtc_base/checks.h"
#include "rtc_base/logging.h"
#include "rtc_base/numerics/safe_conversions.h"
namespace webrtc {
namespace {
@ -116,12 +119,24 @@ absl::optional<double> ParseTypedParameter<double>(std::string str) {
template <>
absl::optional<int> ParseTypedParameter<int>(std::string str) {
int value;
if (sscanf(str.c_str(), "%i", &value) == 1) {
return value;
} else {
return absl::nullopt;
int64_t value;
if (sscanf(str.c_str(), "%" SCNd64, &value) == 1) {
if (rtc::IsValueInRangeForNumericType<int, int64_t>(value)) {
return static_cast<int>(value);
}
}
return absl::nullopt;
}
template <>
absl::optional<unsigned> ParseTypedParameter<unsigned>(std::string str) {
int64_t value;
if (sscanf(str.c_str(), "%" SCNd64, &value) == 1) {
if (rtc::IsValueInRangeForNumericType<unsigned, int64_t>(value)) {
return static_cast<unsigned>(value);
}
}
return absl::nullopt;
}
template <>
@ -140,6 +155,11 @@ absl::optional<absl::optional<int>> ParseTypedParameter<absl::optional<int>>(
return ParseOptionalParameter<int>(str);
}
template <>
absl::optional<absl::optional<unsigned>>
ParseTypedParameter<absl::optional<unsigned>>(std::string str) {
return ParseOptionalParameter<unsigned>(str);
}
template <>
absl::optional<absl::optional<double>>
ParseTypedParameter<absl::optional<double>>(std::string str) {
return ParseOptionalParameter<double>(str);
@ -205,13 +225,16 @@ bool AbstractFieldTrialEnum::Parse(absl::optional<std::string> str_value) {
template class FieldTrialParameter<bool>;
template class FieldTrialParameter<double>;
template class FieldTrialParameter<int>;
template class FieldTrialParameter<unsigned>;
template class FieldTrialParameter<std::string>;
template class FieldTrialConstrained<double>;
template class FieldTrialConstrained<int>;
template class FieldTrialConstrained<unsigned>;
template class FieldTrialOptional<double>;
template class FieldTrialOptional<int>;
template class FieldTrialOptional<unsigned>;
template class FieldTrialOptional<bool>;
template class FieldTrialOptional<std::string>;

View file

@ -245,6 +245,8 @@ absl::optional<double> ParseTypedParameter<double>(std::string str);
template <>
absl::optional<int> ParseTypedParameter<int>(std::string str);
template <>
absl::optional<unsigned> ParseTypedParameter<unsigned>(std::string str);
template <>
absl::optional<std::string> ParseTypedParameter<std::string>(std::string str);
template <>
@ -254,6 +256,9 @@ template <>
absl::optional<absl::optional<int>> ParseTypedParameter<absl::optional<int>>(
std::string str);
template <>
absl::optional<absl::optional<unsigned>>
ParseTypedParameter<absl::optional<unsigned>>(std::string str);
template <>
absl::optional<absl::optional<double>>
ParseTypedParameter<absl::optional<double>>(std::string str);
@ -263,14 +268,18 @@ extern template class FieldTrialParameter<bool>;
extern template class FieldTrialParameter<double>;
// Interpreted using sscanf %i.
extern template class FieldTrialParameter<int>;
// Interpreted using sscanf %u.
extern template class FieldTrialParameter<unsigned>;
// Using the given value as is.
extern template class FieldTrialParameter<std::string>;
extern template class FieldTrialConstrained<double>;
extern template class FieldTrialConstrained<int>;
extern template class FieldTrialConstrained<unsigned>;
extern template class FieldTrialOptional<double>;
extern template class FieldTrialOptional<int>;
extern template class FieldTrialOptional<unsigned>;
extern template class FieldTrialOptional<bool>;
extern template class FieldTrialOptional<std::string>;

View file

@ -23,16 +23,19 @@ struct DummyExperiment {
FieldTrialFlag enabled = FieldTrialFlag("Enabled");
FieldTrialParameter<double> factor = FieldTrialParameter<double>("f", 0.5);
FieldTrialParameter<int> retries = FieldTrialParameter<int>("r", 5);
FieldTrialParameter<unsigned> size = FieldTrialParameter<unsigned>("s", 3);
FieldTrialParameter<bool> ping = FieldTrialParameter<bool>("p", 0);
FieldTrialParameter<std::string> hash =
FieldTrialParameter<std::string>("h", "a80");
explicit DummyExperiment(std::string field_trial) {
ParseFieldTrial({&enabled, &factor, &retries, &ping, &hash}, field_trial);
ParseFieldTrial({&enabled, &factor, &retries, &size, &ping, &hash},
field_trial);
}
DummyExperiment() {
std::string trial_string = field_trial::FindFullName(kDummyExperiment);
ParseFieldTrial({&enabled, &factor, &retries, &ping, &hash}, trial_string);
ParseFieldTrial({&enabled, &factor, &retries, &size, &ping, &hash},
trial_string);
}
};
@ -45,22 +48,24 @@ enum class CustomEnum {
} // namespace
TEST(FieldTrialParserTest, ParsesValidParameters) {
DummyExperiment exp("Enabled,f:-1.7,r:2,p:1,h:x7c");
DummyExperiment exp("Enabled,f:-1.7,r:2,s:10,p:1,h:x7c");
EXPECT_TRUE(exp.enabled.Get());
EXPECT_EQ(exp.factor.Get(), -1.7);
EXPECT_EQ(exp.retries.Get(), 2);
EXPECT_EQ(exp.size.Get(), 10u);
EXPECT_EQ(exp.ping.Get(), true);
EXPECT_EQ(exp.hash.Get(), "x7c");
}
TEST(FieldTrialParserTest, InitializesFromFieldTrial) {
test::ScopedFieldTrials field_trials(
"WebRTC-OtherExperiment/Disabled/"
"WebRTC-DummyExperiment/Enabled,f:-1.7,r:2,p:1,h:x7c/"
"WebRTC-DummyExperiment/Enabled,f:-1.7,r:2,s:10,p:1,h:x7c/"
"WebRTC-AnotherExperiment/Enabled,f:-3.1,otherstuff:beef/");
DummyExperiment exp;
EXPECT_TRUE(exp.enabled.Get());
EXPECT_EQ(exp.factor.Get(), -1.7);
EXPECT_EQ(exp.retries.Get(), 2);
EXPECT_EQ(exp.size.Get(), 10u);
EXPECT_EQ(exp.ping.Get(), true);
EXPECT_EQ(exp.hash.Get(), "x7c");
}
@ -69,6 +74,7 @@ TEST(FieldTrialParserTest, UsesDefaults) {
EXPECT_FALSE(exp.enabled.Get());
EXPECT_EQ(exp.factor.Get(), 0.5);
EXPECT_EQ(exp.retries.Get(), 5);
EXPECT_EQ(exp.size.Get(), 3u);
EXPECT_EQ(exp.ping.Get(), false);
EXPECT_EQ(exp.hash.Get(), "a80");
}
@ -77,6 +83,7 @@ TEST(FieldTrialParserTest, CanHandleMixedInput) {
EXPECT_TRUE(exp.enabled.Get());
EXPECT_EQ(exp.factor.Get(), 0.5);
EXPECT_EQ(exp.retries.Get(), 5);
EXPECT_EQ(exp.size.Get(), 3u);
EXPECT_EQ(exp.ping.Get(), true);
EXPECT_EQ(exp.hash.Get(), "");
}
@ -96,10 +103,11 @@ TEST(FieldTrialParserTest, IgnoresNewKey) {
EXPECT_EQ(exp.retries.Get(), -11);
}
TEST(FieldTrialParserTest, IgnoresInvalid) {
DummyExperiment exp("Enabled,f,p:,r:%,,:foo,h");
DummyExperiment exp("Enabled,f,p:,r:%,,s:-1,:foo,h");
EXPECT_TRUE(exp.enabled.Get());
EXPECT_EQ(exp.factor.Get(), 0.5);
EXPECT_EQ(exp.retries.Get(), 5);
EXPECT_EQ(exp.size.Get(), 3u);
EXPECT_EQ(exp.ping.Get(), false);
EXPECT_EQ(exp.hash.Get(), "a80");
}
@ -115,6 +123,10 @@ TEST(FieldTrialParserTest, IgnoresOutOfRange) {
ParseFieldTrial({&low, &high}, "low:20,high:20");
EXPECT_EQ(low.Get(), 20);
EXPECT_EQ(high.Get(), 20);
FieldTrialConstrained<unsigned> size("size", 5, 1, 10);
ParseFieldTrial({&size}, "size:0");
EXPECT_EQ(size.Get(), 5u);
}
TEST(FieldTrialParserTest, ReadsValuesFromFieldWithoutKey) {
FieldTrialFlag enabled("Enabled");
@ -136,6 +148,17 @@ TEST(FieldTrialParserTest, ParsesOptionalParameters) {
EXPECT_EQ(max_count.GetOptional().value(), 20);
ParseFieldTrial({&max_count}, "c:");
EXPECT_EQ(max_count.GetOptional().value(), 20);
FieldTrialOptional<unsigned> max_size("c", absl::nullopt);
ParseFieldTrial({&max_size}, "");
EXPECT_FALSE(max_size.GetOptional().has_value());
ParseFieldTrial({&max_size}, "c:10");
EXPECT_EQ(max_size.GetOptional().value(), 10u);
ParseFieldTrial({&max_size}, "c");
EXPECT_FALSE(max_size.GetOptional().has_value());
ParseFieldTrial({&max_size}, "c:20");
EXPECT_EQ(max_size.GetOptional().value(), 20u);
FieldTrialOptional<std::string> optional_string("s", std::string("ab"));
ParseFieldTrial({&optional_string}, "s:");
EXPECT_EQ(optional_string.GetOptional().value(), "");

View file

@ -9,6 +9,8 @@
*/
#include "rtc_base/experiments/struct_parameters_parser.h"
#include <algorithm>
#include "rtc_base/logging.h"
namespace webrtc {
@ -31,6 +33,9 @@ inline void StringEncode(std::string* target, double val) {
inline void StringEncode(std::string* target, int val) {
*target += rtc::ToString(val);
}
inline void StringEncode(std::string* target, unsigned val) {
*target += rtc::ToString(val);
}
inline void StringEncode(std::string* target, DataRate val) {
*target += webrtc::ToString(val);
}
@ -62,8 +67,10 @@ void TypedParser<T>::Encode(const void* src, std::string* target) {
template class TypedParser<bool>;
template class TypedParser<double>;
template class TypedParser<int>;
template class TypedParser<unsigned>;
template class TypedParser<absl::optional<double>>;
template class TypedParser<absl::optional<int>>;
template class TypedParser<absl::optional<unsigned>>;
template class TypedParser<DataRate>;
template class TypedParser<DataSize>;

View file

@ -53,8 +53,10 @@ class TypedParser {
extern template class TypedParser<bool>;
extern template class TypedParser<double>;
extern template class TypedParser<int>;
extern template class TypedParser<unsigned>;
extern template class TypedParser<absl::optional<double>>;
extern template class TypedParser<absl::optional<int>>;
extern template class TypedParser<absl::optional<unsigned>>;
extern template class TypedParser<DataRate>;
extern template class TypedParser<DataSize>;

View file

@ -16,6 +16,7 @@ struct DummyConfig {
bool enabled = false;
double factor = 0.5;
int retries = 5;
unsigned size = 3;
bool ping = 0;
absl::optional<TimeDelta> duration;
absl::optional<TimeDelta> latency = TimeDelta::ms(100);
@ -27,6 +28,7 @@ std::unique_ptr<StructParametersParser> DummyConfig::Parser() {
return StructParametersParser::Create("e", &enabled, //
"f", &factor, //
"r", &retries, //
"s", &size, //
"p", &ping, //
"d", &duration, //
"l", &latency);
@ -35,10 +37,11 @@ std::unique_ptr<StructParametersParser> DummyConfig::Parser() {
TEST(StructParametersParserTest, ParsesValidParameters) {
DummyConfig exp;
exp.Parser()->Parse("e:1,f:-1.7,r:2,p:1,d:8,l:,");
exp.Parser()->Parse("e:1,f:-1.7,r:2,s:7,p:1,d:8,l:,");
EXPECT_TRUE(exp.enabled);
EXPECT_EQ(exp.factor, -1.7);
EXPECT_EQ(exp.retries, 2);
EXPECT_EQ(exp.size, 7u);
EXPECT_EQ(exp.ping, true);
EXPECT_EQ(exp.duration.value().ms(), 8);
EXPECT_FALSE(exp.latency);
@ -50,6 +53,7 @@ TEST(StructParametersParserTest, UsesDefaults) {
EXPECT_FALSE(exp.enabled);
EXPECT_EQ(exp.factor, 0.5);
EXPECT_EQ(exp.retries, 5);
EXPECT_EQ(exp.size, 3u);
EXPECT_EQ(exp.ping, false);
}
@ -57,7 +61,7 @@ TEST(StructParametersParserTest, EncodeAll) {
DummyConfig exp;
auto encoded = exp.Parser()->Encode();
// All parameters are encoded.
EXPECT_EQ(encoded, "e:false,f:0.5,r:5,p:false,d:,l:100 ms");
EXPECT_EQ(encoded, "e:false,f:0.5,r:5,s:3,p:false,d:,l:100 ms");
}
} // namespace webrtc