cleanup: replace ad-hoc [0, 1] value types with UnitFloat #14081
Conversation
Add IntervalValue and its specialization UnitFloat, and use them to guarantee that ScaledMinimum's scale_factor_ is in the range [0, 1]. Also do the same for OverloadActionState, and replace ScaledTimerManagerImpl::DurationScaleFactor with UnitFloat, which is a functional no-op. Signed-off-by: Alex Konradi <akonradi@google.com>
| namespace Envoy { | ||
|
|
||
| // Template helper type that represents a closed interval with the given minimum and maximum values. | ||
| template <typename T, T MinValue, T MaxValue> struct Interval { |
There was a problem hiding this comment.
super nit: Could call this closedInterval since it represents a closed interval in particular
There was a problem hiding this comment.
Seems like the name is still "Interval"
There was a problem hiding this comment.
I opted to make the change to the value type since it didn't seem helpful to give the interval type an opinion on whether it is open or closed - that is, the value type is templated over a thing with a min and a max, which sounds like it could apply to both an open or closed interval (though "bounds" might be better for an open interval).
Signed-off-by: Alex Konradi <akonradi@google.com>
|
/assign @antoniovicente |
| T value_; | ||
| }; | ||
|
|
||
| using UnitFloat = ClosedIntervalValue<float, Interval<int, 0, 1>>; |
There was a problem hiding this comment.
Could you avoid mixing float and int in this definition?
There was a problem hiding this comment.
No; float template parameters aren't allowed until C++20.
There was a problem hiding this comment.
Darn. Ok.
Worth a comment/todo to clean this up once C++ 20 is here?
| namespace Envoy { | ||
|
|
||
| // Template helper type that represents a closed interval with the given minimum and maximum values. | ||
| template <typename T, T MinValue, T MaxValue> struct Interval { |
There was a problem hiding this comment.
Seems like the name is still "Interval"
| T value_; | ||
| }; | ||
|
|
||
| using UnitFloat = ClosedIntervalValue<float, Interval<int, 0, 1>>; |
There was a problem hiding this comment.
Darn. Ok.
Worth a comment/todo to clean this up once C++ 20 is here?
Signed-off-by: Alex Konradi <akonradi@google.com>
Signed-off-by: Alex Konradi <akonradi@google.com>
CI complained about coverage, so try adding tests Signed-off-by: Alex Konradi <akonradi@google.com>
…t-interval-type Signed-off-by: Alex Konradi <akonradi@google.com>
|
/retest |
|
Retrying Azure Pipelines: |
|
@jmarantz can you assign a non-Google reviewer or merge if this seems trivial enough? |
|
Pretty cool stuff. I think @mattklein123 might want to take a look; maybe there. would be other applications of this new type. |
* master: (70 commits) upstream: avoid reset after end_stream in TCP HTTP upstream (envoyproxy#14106) bazelci: add fuzz coverage (envoyproxy#14179) dependencies: allowlist CVE-2020-8277 to prevent false positives. (envoyproxy#14228) cleanup: replace ad-hoc [0, 1] value types with UnitFloat (envoyproxy#14081) Update docs for skywalking tracer (envoyproxy#14210) Fix some errors in the switch statement when decode dubbo response (envoyproxy#14207) Windows: enable tests and envoy-static.exe pdb file (envoyproxy#13688) http: add Kill Request HTTP filter (envoyproxy#14170) dependencies: fix release_dates error behavior. (envoyproxy#14216) thrift filter: support skip decoding data after metadata in the thrift message (envoyproxy#13592) update cares (envoyproxy#14213) docs: clarify behavior of hedge_on_per_try_timeout (envoyproxy#12983) repokitteh: add support for randomized auto-assign. (envoyproxy#14185) [grpc] validate grpc config for illegal characters (envoyproxy#14129) server: Return nullopt when process_context is nullptr (envoyproxy#14181) [Windows] Fix thrift proxy tests (envoyproxy#13220) kafka: add missing unit tests (envoyproxy#14195) doc: mention gperftools explicitly in PPROF.md (envoyproxy#14199) Removed `--use-fake-symbol-table` option. (envoyproxy#14178) filter contract: clarification around local replies (envoyproxy#14193) ... Signed-off-by: Michael Puncel <mpuncel@squareup.com>
Commit Message: Use specialized IntervalType to guarantee bounds
Additional Description:
Add IntervalValue and its specialization UnitFloat, and use them to
guarantee that ScaledMinimum's scale_factor_ is in the range [0, 1].
Also do the same for OverloadActionState, and replace
ScaledTimerManagerImpl::DurationScaleFactor with UnitFloat, which reduces
space usage since double precision wasn't necessary.
Risk Level: low
Testing: ran tests
Docs Changes: n/a
Release Notes: n/a
Platform Specific Features: n/a
/assign @KBaichoo