From 55251c3d49752383097fcfefd1e25fe4b098f8b4 Mon Sep 17 00:00:00 2001 From: Sebastian Jansson Date: Thu, 8 Aug 2019 11:14:51 +0200 Subject: [PATCH] Adds struct parameters parser/encoder. This is similar to the field trial parser but it uses a normal struct with normal fields as underlying storage. This makes it easier to understand and use as only the encoding and parsing uses non- standard constructs. Additionally, it makes it easier to use the struct as a regular config struct when the values are not set using field trials. Bug: webrtc:9883 Change-Id: I5b16c2a71875b6f478383decff18fbaa62bc404a Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/145203 Reviewed-by: Jonas Olsson Commit-Queue: Sebastian Jansson Cr-Commit-Position: refs/heads/master@{#28810} --- rtc_base/experiments/BUILD.gn | 5 + rtc_base/experiments/field_trial_parser.cc | 16 ++ rtc_base/experiments/field_trial_parser.h | 27 +++ rtc_base/experiments/field_trial_units.cc | 16 ++ rtc_base/experiments/field_trial_units.h | 8 + .../experiments/struct_parameters_parser.cc | 67 ++++++ .../experiments/struct_parameters_parser.h | 214 ++++++++++++++++++ .../struct_parameters_parser_unittest.cc | 84 +++++++ 8 files changed, 437 insertions(+) create mode 100644 rtc_base/experiments/struct_parameters_parser.cc create mode 100644 rtc_base/experiments/struct_parameters_parser.h create mode 100644 rtc_base/experiments/struct_parameters_parser_unittest.cc diff --git a/rtc_base/experiments/BUILD.gn b/rtc_base/experiments/BUILD.gn index daffb22a2f..927e8c7d16 100644 --- a/rtc_base/experiments/BUILD.gn +++ b/rtc_base/experiments/BUILD.gn @@ -47,6 +47,8 @@ rtc_static_library("field_trial_parser") { "field_trial_parser.h", "field_trial_units.cc", "field_trial_units.h", + "struct_parameters_parser.cc", + "struct_parameters_parser.h", ] deps = [ "../../api/units:data_rate", @@ -55,6 +57,8 @@ rtc_static_library("field_trial_parser") { "../../rtc_base:checks", "../../rtc_base:logging", "../../rtc_base:stringutils", + "//third_party/abseil-cpp/absl/memory:memory", + "//third_party/abseil-cpp/absl/strings:strings", "//third_party/abseil-cpp/absl/types:optional", ] } @@ -208,6 +212,7 @@ if (rtc_include_tests) { "quality_scaling_experiment_unittest.cc", "rate_control_settings_unittest.cc", "rtt_mult_experiment_unittest.cc", + "struct_parameters_parser_unittest.cc", ] deps = [ ":balanced_degradation_settings", diff --git a/rtc_base/experiments/field_trial_parser.cc b/rtc_base/experiments/field_trial_parser.cc index b2c48c94ac..4f68e5c592 100644 --- a/rtc_base/experiments/field_trial_parser.cc +++ b/rtc_base/experiments/field_trial_parser.cc @@ -129,6 +129,22 @@ absl::optional ParseTypedParameter(std::string str) { return std::move(str); } +template <> +absl::optional> ParseTypedParameter>( + std::string str) { + return ParseOptionalParameter(str); +} +template <> +absl::optional> ParseTypedParameter>( + std::string str) { + return ParseOptionalParameter(str); +} +template <> +absl::optional> +ParseTypedParameter>(std::string str) { + return ParseOptionalParameter(str); +} + FieldTrialFlag::FieldTrialFlag(std::string key) : FieldTrialFlag(key, false) {} FieldTrialFlag::FieldTrialFlag(std::string key, bool default_value) diff --git a/rtc_base/experiments/field_trial_parser.h b/rtc_base/experiments/field_trial_parser.h index 73b6ca837a..3c3731c6bc 100644 --- a/rtc_base/experiments/field_trial_parser.h +++ b/rtc_base/experiments/field_trial_parser.h @@ -228,6 +228,33 @@ class FieldTrialFlag : public FieldTrialParameterInterface { bool value_; }; +template +absl::optional> ParseOptionalParameter(std::string str) { + if (str.empty()) + return absl::optional(); + auto parsed = ParseTypedParameter(str); + if (parsed.has_value()) + return parsed; + return absl::nullopt; +} + +template <> +absl::optional ParseTypedParameter(std::string str); +template <> +absl::optional ParseTypedParameter(std::string str); +template <> +absl::optional ParseTypedParameter(std::string str); + +template <> +absl::optional> ParseTypedParameter>( + std::string str); +template <> +absl::optional> ParseTypedParameter>( + std::string str); +template <> +absl::optional> +ParseTypedParameter>(std::string str); + // Accepts true, false, else parsed with sscanf %i, true if != 0. extern template class FieldTrialParameter; // Interpreted using sscanf %lf. diff --git a/rtc_base/experiments/field_trial_units.cc b/rtc_base/experiments/field_trial_units.cc index a7cd13f544..9c9cf434d9 100644 --- a/rtc_base/experiments/field_trial_units.cc +++ b/rtc_base/experiments/field_trial_units.cc @@ -84,6 +84,22 @@ absl::optional ParseTypedParameter(std::string str) { return absl::nullopt; } +template <> +absl::optional> +ParseTypedParameter>(std::string str) { + return ParseOptionalParameter(str); +} +template <> +absl::optional> +ParseTypedParameter>(std::string str) { + return ParseOptionalParameter(str); +} +template <> +absl::optional> +ParseTypedParameter>(std::string str) { + return ParseOptionalParameter(str); +} + template class FieldTrialParameter; template class FieldTrialParameter; template class FieldTrialParameter; diff --git a/rtc_base/experiments/field_trial_units.h b/rtc_base/experiments/field_trial_units.h index 353c87bbf8..d85b2f04ba 100644 --- a/rtc_base/experiments/field_trial_units.h +++ b/rtc_base/experiments/field_trial_units.h @@ -16,6 +16,14 @@ #include "rtc_base/experiments/field_trial_parser.h" namespace webrtc { + +template <> +absl::optional ParseTypedParameter(std::string str); +template <> +absl::optional ParseTypedParameter(std::string str); +template <> +absl::optional ParseTypedParameter(std::string str); + extern template class FieldTrialParameter; extern template class FieldTrialParameter; extern template class FieldTrialParameter; diff --git a/rtc_base/experiments/struct_parameters_parser.cc b/rtc_base/experiments/struct_parameters_parser.cc new file mode 100644 index 0000000000..cef9386544 --- /dev/null +++ b/rtc_base/experiments/struct_parameters_parser.cc @@ -0,0 +1,67 @@ +/* + * Copyright (c) 2019 The WebRTC project authors. All Rights Reserved. + * + * Use of this source code is governed by a BSD-style license + * that can be found in the LICENSE file in the root of the source + * tree. An additional intellectual property rights grant can be found + * in the file PATENTS. All contributing project authors may + * be found in the AUTHORS file in the root of the source tree. + */ +#include "rtc_base/experiments/struct_parameters_parser.h" + +#include + +#include "rtc_base/logging.h" +#include "rtc_base/strings/string_builder.h" + +namespace webrtc { +namespace struct_parser_impl { +namespace { +size_t FindOrEnd(absl::string_view str, size_t start, char delimiter) { + size_t pos = str.find(delimiter, start); + pos = (pos == std::string::npos) ? str.length() : pos; + return pos; +} +} // namespace + +void ParseConfigParams( + absl::string_view config_str, + std::map> field_map) { + size_t i = 0; + while (i < config_str.length()) { + size_t val_end = FindOrEnd(config_str, i, ','); + size_t colon_pos = FindOrEnd(config_str, i, ':'); + size_t key_end = std::min(val_end, colon_pos); + size_t val_begin = key_end + 1u; + std::string key(config_str.substr(i, key_end - i)); + absl::string_view opt_value; + if (val_end >= val_begin) + opt_value = config_str.substr(val_begin, val_end - val_begin); + i = val_end + 1u; + auto field = field_map.find(key); + if (field != field_map.end()) { + if (!field->second(opt_value)) { + RTC_LOG(LS_WARNING) << "Failed to read field with key: '" << key + << "' in trial: \"" << config_str << "\""; + } + } else { + RTC_LOG(LS_INFO) << "No field with key: '" << key + << "' (found in trial: \"" << config_str << "\")"; + } + } +} + +std::string EncodeStringStringMap(std::map mapping) { + rtc::StringBuilder sb; + bool first = true; + for (const auto& kv : mapping) { + if (!first) + sb << ","; + sb << kv.first << ":" << kv.second; + first = false; + } + return sb.Release(); +} +} // namespace struct_parser_impl + +} // namespace webrtc diff --git a/rtc_base/experiments/struct_parameters_parser.h b/rtc_base/experiments/struct_parameters_parser.h new file mode 100644 index 0000000000..f6728f6ea5 --- /dev/null +++ b/rtc_base/experiments/struct_parameters_parser.h @@ -0,0 +1,214 @@ +/* + * Copyright (c) 2019 The WebRTC project authors. All Rights Reserved. + * + * Use of this source code is governed by a BSD-style license + * that can be found in the LICENSE file in the root of the source + * tree. An additional intellectual property rights grant can be found + * in the file PATENTS. All contributing project authors may + * be found in the AUTHORS file in the root of the source tree. + */ +#ifndef RTC_BASE_EXPERIMENTS_STRUCT_PARAMETERS_PARSER_H_ +#define RTC_BASE_EXPERIMENTS_STRUCT_PARAMETERS_PARSER_H_ + +#include +#include +#include +#include +#include +#include + +#include "absl/memory/memory.h" +#include "absl/strings/string_view.h" +#include "absl/types/optional.h" +#include "rtc_base/experiments/field_trial_parser.h" +#include "rtc_base/experiments/field_trial_units.h" +#include "rtc_base/string_encode.h" + +namespace webrtc { +namespace struct_parser_impl { +inline std::string StringEncode(bool val) { + return rtc::ToString(val); +} +inline std::string StringEncode(double val) { + return rtc::ToString(val); +} +inline std::string StringEncode(int val) { + return rtc::ToString(val); +} +inline std::string StringEncode(std::string val) { + return val; +} +inline std::string StringEncode(DataRate val) { + return ToString(val); +} +inline std::string StringEncode(DataSize val) { + return ToString(val); +} +inline std::string StringEncode(TimeDelta val) { + return ToString(val); +} + +template +inline std::string StringEncode(absl::optional val) { + if (val) + return StringEncode(*val); + return ""; +} + +template +struct LambdaTraits : public LambdaTraits {}; + +template +struct LambdaTraits { + using ret = RetType; + using src = SourceType; +}; + +void ParseConfigParams( + absl::string_view config_str, + std::map> field_map); + +std::string EncodeStringStringMap(std::map mapping); + +template +class StructParameterParser { + public: + virtual bool Parse(absl::string_view src, StructType* target) const = 0; + virtual bool Changed(const StructType& src, const StructType& base) const = 0; + virtual std::string Encode(const StructType& src) const = 0; + virtual ~StructParameterParser() = default; +}; + +template +class StructParameterImpl : public StructParameterParser { + public: + explicit StructParameterImpl(std::function field_getter) + : field_getter_(std::move(field_getter)) {} + bool Parse(absl::string_view src, StructType* target) const override { + auto parsed = ParseTypedParameter(std::string(src)); + if (parsed.has_value()) + *field_getter_(target) = *parsed; + return parsed.has_value(); + } + bool Changed(const StructType& src, const StructType& base) const override { + T base_value = *field_getter_(const_cast(&base)); + T value = *field_getter_(const_cast(&src)); + return value != base_value; + } + std::string Encode(const StructType& src) const override { + T value = *field_getter_(const_cast(&src)); + return struct_parser_impl::StringEncode(value); + } + + private: + const std::function field_getter_; +}; + +template +struct StructParameter { + std::string key; + StructParameterParser* parser; +}; + +template ::ret> +void AddParameters(std::vector>* out, + std::string key, + Closure getter) { + auto* parser = new StructParameterImpl(getter); + out->push_back(StructParameter{std::move(key), parser}); +} + +template ::ret, + typename... Args> +void AddParameters(std::vector>* out, + std::string key, + Closure getter, + Args... args) { + AddParameters(out, key, getter); + AddParameters(out, args...); +} + +} // namespace struct_parser_impl + +template +class StructParametersParser { + public: + ~StructParametersParser() { + for (auto& param : parameters_) { + delete param.parser; + } + } + + void Parse(StructType* target, absl::string_view src) { + std::map> field_parsers; + for (const auto& param : parameters_) { + field_parsers.emplace(param.key, [target, param](absl::string_view src) { + return param.parser->Parse(src, target); + }); + } + struct_parser_impl::ParseConfigParams(src, std::move(field_parsers)); + } + + StructType Parse(absl::string_view src) { + StructType res; + Parse(&res, src); + return res; + } + + std::string EncodeChanged(const StructType& src) { + static StructType base; + std::map pairs; + for (const auto& param : parameters_) { + if (param.parser->Changed(src, base)) + pairs[param.key] = param.parser->Encode(src); + } + return struct_parser_impl::EncodeStringStringMap(pairs); + } + + std::string EncodeAll(const StructType& src) { + std::map pairs; + for (const auto& param : parameters_) { + pairs[param.key] = param.parser->Encode(src); + } + return struct_parser_impl::EncodeStringStringMap(pairs); + } + + private: + template + friend std::unique_ptr> + CreateStructParametersParser(std::string, C, Args...); + + explicit StructParametersParser( + std::vector> parameters) + : parameters_(parameters) {} + + std::vector> parameters_; +}; + +// Creates a struct parameters parser based on interleaved key and field +// accessor arguments, where the field accessor converts a struct pointer to a +// member pointer: FieldType*(StructType*). See the unit tests for example +// usage. Note that the struct type is inferred from the field getters. Beware +// of providing incorrect arguments to this, such as mixing the struct type or +// incorrect return values, as this will cause very confusing compile errors. +template ::src, + typename... Args> +std::unique_ptr> CreateStructParametersParser( + std::string first_key, + Closure first_getter, + Args... args) { + std::vector> parameters; + struct_parser_impl::AddParameters(¶meters, std::move(first_key), + first_getter, args...); + // absl::make_unique can't be used since the StructParametersParser + // constructor is only visible to this create function. + return absl::WrapUnique(new StructParametersParser(std::move(parameters))); +} +} // namespace webrtc + +#endif // RTC_BASE_EXPERIMENTS_STRUCT_PARAMETERS_PARSER_H_ diff --git a/rtc_base/experiments/struct_parameters_parser_unittest.cc b/rtc_base/experiments/struct_parameters_parser_unittest.cc new file mode 100644 index 0000000000..1e88f8cfc1 --- /dev/null +++ b/rtc_base/experiments/struct_parameters_parser_unittest.cc @@ -0,0 +1,84 @@ +/* + * Copyright (c) 2019 The WebRTC project authors. All Rights Reserved. + * + * Use of this source code is governed by a BSD-style license + * that can be found in the LICENSE file in the root of the source + * tree. An additional intellectual property rights grant can be found + * in the file PATENTS. All contributing project authors may + * be found in the AUTHORS file in the root of the source tree. + */ +#include "rtc_base/experiments/struct_parameters_parser.h" +#include "rtc_base/gunit.h" + +namespace webrtc { +namespace { +struct DummyConfig { + bool enabled = false; + double factor = 0.5; + int retries = 5; + bool ping = 0; + std::string hash = "a80"; + absl::optional duration; + absl::optional latency = TimeDelta::ms(100); + static StructParametersParser* Parser(); +}; + +StructParametersParser* DummyConfig::Parser() { + using C = DummyConfig; + // The empty comments ensures that each pair is on a separate line. + static auto parser = CreateStructParametersParser( + "e", [](C* c) { return &c->enabled; }, // + "f", [](C* c) { return &c->factor; }, // + "r", [](C* c) { return &c->retries; }, // + "p", [](C* c) { return &c->ping; }, // + "h", [](C* c) { return &c->hash; }, // + "d", [](C* c) { return &c->duration; }, // + "l", [](C* c) { return &c->latency; }); // + return parser.get(); +} +} // namespace + +TEST(StructParametersParserTest, ParsesValidParameters) { + DummyConfig exp = + DummyConfig::Parser()->Parse("e:1,f:-1.7,r:2,p:1,h:x7c,d:8,l:,"); + EXPECT_TRUE(exp.enabled); + EXPECT_EQ(exp.factor, -1.7); + EXPECT_EQ(exp.retries, 2); + EXPECT_EQ(exp.ping, true); + EXPECT_EQ(exp.duration.value().ms(), 8); + EXPECT_FALSE(exp.latency); +} + +TEST(StructParametersParserTest, UsesDefaults) { + DummyConfig exp = DummyConfig::Parser()->Parse(""); + EXPECT_FALSE(exp.enabled); + EXPECT_EQ(exp.factor, 0.5); + EXPECT_EQ(exp.retries, 5); + EXPECT_EQ(exp.ping, false); + EXPECT_EQ(exp.hash, "a80"); +} + +TEST(StructParametersParserTest, EmptyDefaults) { + DummyConfig exp; + auto encoded = DummyConfig::Parser()->EncodeChanged(exp); + // Unchanged parameters are not encoded. + EXPECT_EQ(encoded, ""); +} + +TEST(StructParametersParserTest, EncodeAll) { + DummyConfig exp; + auto encoded = DummyConfig::Parser()->EncodeAll(exp); + // All parameters are encoded. + EXPECT_EQ(encoded, "d:,e:false,f:0.5,h:a80,l:100 ms,p:false,r:5"); +} + +TEST(StructParametersParserTest, EncodeChanged) { + DummyConfig exp; + exp.ping = true; + exp.retries = 4; + auto encoded = DummyConfig::Parser()->EncodeChanged(exp); + // We expect the changed parameters to be encoded in alphabetical order. + EXPECT_EQ(encoded, "p:true,r:4"); +} + +} // namespace webrtc