|
|
DescriptionProbingEstimator: Erase history based on time threshold
Erases history based on time threshold instead of retaining really old cluster data. Also does a bunch of clean up.
BUG=
R=danilchap@webrtc.org, philipel@webrtc.org, stefan@webrtc.org
Committed: https://crrev.com/f99a9de06948494b2708a5accb6a5667a000926c
Cr-Commit-Position: refs/heads/master@{#13870}
Patch Set 1 #
Total comments: 1
Patch Set 2 : Improvements #
Total comments: 8
Patch Set 3 : Address nits #
Total comments: 9
Patch Set 4 : Reduced cluster history to 1s #
Total comments: 20
Patch Set 5 : Addressed feedback #
Total comments: 2
Patch Set 6 : Addressed comment #Patch Set 7 : rebase #
Messages
Total messages: 32 (13 generated)
Description was changed from ========== ProbingEstimator: Enhancements for startup probing We add support to specify minimum clusters and also to measure highest bandwidth received across all clusters. BUG= ========== to ========== ProbingEstimator: Enhancements for startup probing We add support to specify minimum clusters and also to measure highest bandwidth received across all clusters. BUG= ==========
isheriff@chromium.org changed reviewers: + philipel@webrtc.org, stefan@webrtc.org
isheriff@chromium.org changed reviewers: + terelius@webrtc.org
PTAL. Splitting up the changes into small chunks. This one adds some enhancements to enable start-up probing work.
isheriff@chromium.org changed reviewers: - philipel@webrtc.org, stefan@webrtc.org, terelius@webrtc.org
isheriff@chromium.org changed reviewers: + philipel@webrtc.org, stefan@webrtc.org, terelius@webrtc.org
The purpose of the ProbeBitrateEstimator class is to keep track of feedback received relevant to probing packets and to calculate/validate the result when enough information for a given cluster has been received. If we want to keep track of the history of probing results then that should be done at a higher level. I agree that the ProbingResult class is not really necessary at this point and should be removed to make the interface clearer. https://codereview.webrtc.org/2239143002/diff/1/webrtc/modules/congestion_con... File webrtc/modules/congestion_controller/probe_bitrate_estimator.cc (right): https://codereview.webrtc.org/2239143002/diff/1/webrtc/modules/congestion_con... webrtc/modules/congestion_controller/probe_bitrate_estimator.cc:41: last_valid_cluster_id_ = packet_info.probe_cluster_id; |last_valid_cluster_id_| tracks the last cluster with a valid estimate and at this point we don't know if this will result in a valid cluster. There is also a risk that we receive feedback in the wrong order which could result in a valid probing attempt to be ignored.
PTAL. I have also simplified the tests a bit.
https://codereview.chromium.org/2239143002/diff/20001/webrtc/modules/congesti... File webrtc/modules/congestion_controller/probe_bitrate_estimator.cc (right): https://codereview.chromium.org/2239143002/diff/20001/webrtc/modules/congesti... webrtc/modules/congestion_controller/probe_bitrate_estimator.cc:31: // on the side side as well as the receive side. sender side https://codereview.chromium.org/2239143002/diff/20001/webrtc/modules/congesti... webrtc/modules/congestion_controller/probe_bitrate_estimator.cc:54: cluster->num_probes++; ++cluster...; https://codereview.chromium.org/2239143002/diff/20001/webrtc/modules/congesti... webrtc/modules/congestion_controller/probe_bitrate_estimator.cc:121: valid_clusters_--; --valid_clusters_; https://codereview.chromium.org/2239143002/diff/20001/webrtc/modules/congesti... webrtc/modules/congestion_controller/probe_bitrate_estimator.cc:129: int ProbeBitrateEstimator::HighestBitrateOnClusters() { Our previous reasoning has been that if a probe cluster at, say, 1000 kbps fails and the next cluster at 2000 kbps succeeds, both are considered failures. This could perhaps be a too cautious decision. What's the rationale for accepting the higher bitrate probes as we do with this change?
https://codereview.chromium.org/2239143002/diff/20001/webrtc/modules/congesti... File webrtc/modules/congestion_controller/probe_bitrate_estimator.cc (right): https://codereview.chromium.org/2239143002/diff/20001/webrtc/modules/congesti... webrtc/modules/congestion_controller/probe_bitrate_estimator.cc:31: // on the side side as well as the receive side. On 2016/08/13 10:05:10, stefan-webrtc (holmer) wrote: > sender side Done. https://codereview.chromium.org/2239143002/diff/20001/webrtc/modules/congesti... webrtc/modules/congestion_controller/probe_bitrate_estimator.cc:54: cluster->num_probes++; On 2016/08/13 10:05:10, stefan-webrtc (holmer) wrote: > ++cluster...; Done. https://codereview.chromium.org/2239143002/diff/20001/webrtc/modules/congesti... webrtc/modules/congestion_controller/probe_bitrate_estimator.cc:121: valid_clusters_--; On 2016/08/13 10:05:11, stefan-webrtc (holmer) wrote: > --valid_clusters_; Done. https://codereview.chromium.org/2239143002/diff/20001/webrtc/modules/congesti... webrtc/modules/congestion_controller/probe_bitrate_estimator.cc:129: int ProbeBitrateEstimator::HighestBitrateOnClusters() { On 2016/08/13 10:05:11, stefan-webrtc (holmer) wrote: > Our previous reasoning has been that if a probe cluster at, say, 1000 kbps fails > and the next cluster at 2000 kbps succeeds, both are considered failures. This > could perhaps be a too cautious decision. What's the rationale for accepting the > higher bitrate probes as we do with this change? Given that probing uses 5 samples to get a quick idea of the BW, a failure (we discard measured data for several reasons) indicates that the probing was unsuccessful. A success at a higher bitrate does indicate that the bottleneck on the end-to-end path can handle that particular bitrate and the measured bitrate should be useful as a starting point. Also, the overuse/underuse signals stay active during probing to handle a real issue. Happy to chat more over VC on this.
https://codereview.webrtc.org/2239143002/diff/40001/webrtc/modules/congestion... File webrtc/modules/congestion_controller/probe_bitrate_estimator.cc (right): https://codereview.webrtc.org/2239143002/diff/40001/webrtc/modules/congestion... webrtc/modules/congestion_controller/probe_bitrate_estimator.cc:61: ++valid_clusters_; At this point we don't know if this is a valid cluster or not, move to line 89. https://codereview.webrtc.org/2239143002/diff/40001/webrtc/modules/congestion... webrtc/modules/congestion_controller/probe_bitrate_estimator.cc:129: int ProbeBitrateEstimator::HighestBitrateOnClusters() { Shouldn't this iterate over only the last |min_clusters| from HandleProbeAndEstimateBitrate? https://codereview.webrtc.org/2239143002/diff/40001/webrtc/modules/congestion... File webrtc/modules/congestion_controller/probe_bitrate_estimator.h (right): https://codereview.webrtc.org/2239143002/diff/40001/webrtc/modules/congestion... webrtc/modules/congestion_controller/probe_bitrate_estimator.h:46: int HighestBitrateOnClusters(); I'm still not sure why we need this functionality, is it some type of filtering or validation?
https://codereview.chromium.org/2239143002/diff/40001/webrtc/modules/congesti... File webrtc/modules/congestion_controller/probe_bitrate_estimator.cc (right): https://codereview.chromium.org/2239143002/diff/40001/webrtc/modules/congesti... webrtc/modules/congestion_controller/probe_bitrate_estimator.cc:28: constexpr int kMaxClusterHistoryInMs = 5000; This may be to high if we want to do probing more often in some scenarios. I am planning to reduce to 1s. lmk if you feel strongly on a value here. https://codereview.chromium.org/2239143002/diff/40001/webrtc/modules/congesti... webrtc/modules/congestion_controller/probe_bitrate_estimator.cc:61: ++valid_clusters_; On 2016/08/16 15:21:06, philipel wrote: > At this point we don't know if this is a valid cluster or not, move to line 89. valid cluster indicates that we have received enough probes (kMinNumProbesValidCluster) to evaluate it. We have this to allow waiting on probes to be received on multiple clusters (3x + 6x scenario at init) before evaluating bitrate and reporting back paced_sender. Moving this down at line 89 would mean that if the first cluster had an unsuccessful probing (due to timing issues), we would not even look at the second cluster at all at line 56. And we do want to evaluate even if one cluster has successful probing. https://codereview.chromium.org/2239143002/diff/40001/webrtc/modules/congesti... webrtc/modules/congestion_controller/probe_bitrate_estimator.cc:129: int ProbeBitrateEstimator::HighestBitrateOnClusters() { On 2016/08/16 15:21:06, philipel wrote: > Shouldn't this iterate over only the last |min_clusters| from > HandleProbeAndEstimateBitrate? The |min_clusters| indicates minimum number of clusters we want to look at. We could also turn it into an exact requirement (instead of min) and in that case look at that many clusters, but do you see an issue with this ? On a related note, am considering to reduce the max cluster history to 1 second. https://codereview.chromium.org/2239143002/diff/40001/webrtc/modules/congesti... File webrtc/modules/congestion_controller/probe_bitrate_estimator.h (right): https://codereview.chromium.org/2239143002/diff/40001/webrtc/modules/congesti... webrtc/modules/congestion_controller/probe_bitrate_estimator.h:46: int HighestBitrateOnClusters(); On 2016/08/16 15:21:06, philipel wrote: > I'm still not sure why we need this functionality, is it some type of filtering > or validation? The initial phase has probing on multiple clusters at the same time (3x and 6x times the estimate). We need to evaluate the highest across clusters at start up before we move on with one cluster evaluation during exponential probing phase. Let me know if this helps clarify.
isheriff@chromium.org changed reviewers: + danilchap@webrtc.org
Ping ?
From my point of view this lgtm. But please get an lg from philip before landing.
https://codereview.webrtc.org/2239143002/diff/60001/webrtc/modules/congestion... File webrtc/modules/congestion_controller/probe_bitrate_estimator.cc (right): https://codereview.webrtc.org/2239143002/diff/60001/webrtc/modules/congestion... webrtc/modules/congestion_controller/probe_bitrate_estimator.cc:28: constexpr int kMaxClusterHistoryInMs = 1000; maybe kMaxClusterHistoryMs (i.e. drop 'In') https://codereview.webrtc.org/2239143002/diff/60001/webrtc/modules/congestion... webrtc/modules/congestion_controller/probe_bitrate_estimator.cc:32: constexpr int kMaxProbeIntervalInMs = 1000; same here https://codereview.webrtc.org/2239143002/diff/60001/webrtc/modules/congestion... webrtc/modules/congestion_controller/probe_bitrate_estimator.cc:42: RTC_DCHECK(packet_info.probe_cluster_id != PacketInfo::kNotAProbe); RTC_DCHECK_NE(packet_info.probe_cluster_id, PacketInfo::kNotAProbe); https://codereview.webrtc.org/2239143002/diff/60001/webrtc/modules/congestion... webrtc/modules/congestion_controller/probe_bitrate_estimator.cc:66: EraseOldClusters(packet_info.arrival_time_ms - kMaxClusterHistoryInMs); calling this function might (in theory) change the highest bitrate by removing old cluster, but HandleProbeAndEstimateBitrate still can return -1 after this statement. Is it expected? https://codereview.webrtc.org/2239143002/diff/60001/webrtc/modules/congestion... webrtc/modules/congestion_controller/probe_bitrate_estimator.cc:80: if (send_interval_ms <= 0 || receive_interval_ms <= 0 || may be reorder to make it clearer bounds are checked here: if (send_interval_ms <= 0 || send_interval_ms > kMaxProbeIntervalInMs || receive_interval_ms <= 0 || receive_interval_ms > kMaxProbeIntervalInMs) { https://codereview.webrtc.org/2239143002/diff/60001/webrtc/modules/congestion... webrtc/modules/congestion_controller/probe_bitrate_estimator.cc:132: if (kv.second.bps > highest_bps) shouldn't you skip invalid clusters here? (i.e. add check it->second.num_probes >= kMinNumProbesValidCluster) https://codereview.webrtc.org/2239143002/diff/60001/webrtc/modules/congestion... File webrtc/modules/congestion_controller/probe_bitrate_estimator.h (right): https://codereview.webrtc.org/2239143002/diff/60001/webrtc/modules/congestion... webrtc/modules/congestion_controller/probe_bitrate_estimator.h:29: int HandleProbeAndEstimateBitrate(const PacketInfo& packet_info, this function returns -1 when bitrate doesn't change and the bitrate when it may be changed (when it is recalculated, probably to the same cluster). May it would be cleaner if the funciton always return highest (cached) bitrate, even if is obviously same as before. Or return bool to indicate the bitrate may be changed and have an output parameter or accessor to get highest bitrate so far. https://codereview.webrtc.org/2239143002/diff/60001/webrtc/modules/congestion... File webrtc/modules/congestion_controller/probe_bitrate_estimator_unittest.cc (right): https://codereview.webrtc.org/2239143002/diff/60001/webrtc/modules/congestion... webrtc/modules/congestion_controller/probe_bitrate_estimator_unittest.cc:18: #include "webrtc/base/logging.h" is it needed? https://codereview.webrtc.org/2239143002/diff/60001/webrtc/modules/congestion... webrtc/modules/congestion_controller/probe_bitrate_estimator_unittest.cc:34: measured_bps_ = bps > 0 ? bps : 0; is it needed to check if bps > 0? Shouldn't last AddPacketFeedback in all tests collect valid bitrate? May be cleaner to remove measured_bps_ and make AddPacketFeedback return bps instead.
https://codereview.webrtc.org/2239143002/diff/40001/webrtc/modules/congestion... File webrtc/modules/congestion_controller/probe_bitrate_estimator.cc (right): https://codereview.webrtc.org/2239143002/diff/40001/webrtc/modules/congestion... webrtc/modules/congestion_controller/probe_bitrate_estimator.cc:61: ++valid_clusters_; On 2016/08/16 17:02:10, Irfan wrote: > On 2016/08/16 15:21:06, philipel wrote: > > At this point we don't know if this is a valid cluster or not, move to line > 89. > > valid cluster indicates that we have received enough probes > (kMinNumProbesValidCluster) to evaluate it. > > We have this to allow waiting on probes to be received on multiple clusters (3x > + 6x scenario at init) before evaluating bitrate and reporting back > paced_sender. > > Moving this down at line 89 would mean that if the first cluster had an > unsuccessful probing (due to timing issues), we would not even look at the > second cluster at all at line 56. And we do want to evaluate even if one cluster > has successful probing. This can still cause us to ignore the second probe cluster if we don't receive enough feedback about the first probe cluster. I would suggest that we either keep the old logic that only report results about the current or newer probing attempt, or, we expand the ProbeResult class with more information about the probing cluster (like cluster id or maybe send/receive timestamps) so that the decision can be made somewhere else. https://codereview.webrtc.org/2239143002/diff/40001/webrtc/modules/congestion... File webrtc/modules/congestion_controller/probe_bitrate_estimator.h (right): https://codereview.webrtc.org/2239143002/diff/40001/webrtc/modules/congestion... webrtc/modules/congestion_controller/probe_bitrate_estimator.h:46: int HighestBitrateOnClusters(); On 2016/08/16 17:02:10, Irfan wrote: > On 2016/08/16 15:21:06, philipel wrote: > > I'm still not sure why we need this functionality, is it some type of > filtering > > or validation? > > The initial phase has probing on multiple clusters at the same time (3x and 6x > times the estimate). We need to evaluate the highest across clusters at start up > before we move on with one cluster evaluation during exponential probing phase. > > Let me know if this helps clarify. Why don't we just return the newest result possible? If we for example have a 800 kbps connection and probe at 900 and 1800, and the result from the first probe is 950 and the result from the second probe is 750, should we still report 950 as the result from the second probe? https://codereview.webrtc.org/2239143002/diff/60001/webrtc/modules/congestion... File webrtc/modules/congestion_controller/probe_bitrate_estimator.cc (right): https://codereview.webrtc.org/2239143002/diff/60001/webrtc/modules/congestion... webrtc/modules/congestion_controller/probe_bitrate_estimator.cc:28: constexpr int kMaxClusterHistoryInMs = 1000; I think cleaning up probes based on how old they are is a good approach, but If we want to get the result from the last X seconds then I think we should either have a max age parameter to the HandeProbeAndEstimateBitrate or implement some function where you can query for the highest bitrate over the last X seconds.
https://codereview.webrtc.org/2239143002/diff/60001/webrtc/modules/congestion... File webrtc/modules/congestion_controller/probe_bitrate_estimator.cc (right): https://codereview.webrtc.org/2239143002/diff/60001/webrtc/modules/congestion... webrtc/modules/congestion_controller/probe_bitrate_estimator.cc:28: constexpr int kMaxClusterHistoryInMs = 1000; On 2016/08/19 11:46:30, danilchap wrote: > maybe kMaxClusterHistoryMs (i.e. drop 'In') Done. https://codereview.webrtc.org/2239143002/diff/60001/webrtc/modules/congestion... webrtc/modules/congestion_controller/probe_bitrate_estimator.cc:28: constexpr int kMaxClusterHistoryInMs = 1000; On 2016/08/19 13:41:31, philipel wrote: > I think cleaning up probes based on how old they are is a good approach, but If > we want to get the result from the last X seconds then I think we should either > have a max age parameter to the HandeProbeAndEstimateBitrate or implement some > function where you can query for the highest bitrate over the last X seconds. Latest PS does clean up and leaves bitrate handling to caller. https://codereview.webrtc.org/2239143002/diff/60001/webrtc/modules/congestion... webrtc/modules/congestion_controller/probe_bitrate_estimator.cc:32: constexpr int kMaxProbeIntervalInMs = 1000; On 2016/08/19 11:46:30, danilchap wrote: > same here Done. https://codereview.webrtc.org/2239143002/diff/60001/webrtc/modules/congestion... webrtc/modules/congestion_controller/probe_bitrate_estimator.cc:42: RTC_DCHECK(packet_info.probe_cluster_id != PacketInfo::kNotAProbe); On 2016/08/19 11:46:30, danilchap wrote: > RTC_DCHECK_NE(packet_info.probe_cluster_id, PacketInfo::kNotAProbe); Done. https://codereview.webrtc.org/2239143002/diff/60001/webrtc/modules/congestion... webrtc/modules/congestion_controller/probe_bitrate_estimator.cc:66: EraseOldClusters(packet_info.arrival_time_ms - kMaxClusterHistoryInMs); On 2016/08/19 11:46:30, danilchap wrote: > calling this function might (in theory) change the highest bitrate by removing > old cluster, but HandleProbeAndEstimateBitrate still can return -1 after this > statement. Is it expected? Moved this up so it is clear it is cleaning up before handling the new probe packet received. If the probe ends up being old, it will not return any bitrate and will end up getting cleaned up on the next call. https://codereview.webrtc.org/2239143002/diff/60001/webrtc/modules/congestion... webrtc/modules/congestion_controller/probe_bitrate_estimator.cc:80: if (send_interval_ms <= 0 || receive_interval_ms <= 0 || On 2016/08/19 11:46:30, danilchap wrote: > may be reorder to make it clearer bounds are checked here: > if (send_interval_ms <= 0 || > send_interval_ms > kMaxProbeIntervalInMs || > receive_interval_ms <= 0 || > receive_interval_ms > kMaxProbeIntervalInMs) { Done. https://codereview.webrtc.org/2239143002/diff/60001/webrtc/modules/congestion... webrtc/modules/congestion_controller/probe_bitrate_estimator.cc:132: if (kv.second.bps > highest_bps) On 2016/08/19 11:46:30, danilchap wrote: > shouldn't you skip invalid clusters here? (i.e. add check it->second.num_probes > >= kMinNumProbesValidCluster) This is gone now https://codereview.webrtc.org/2239143002/diff/60001/webrtc/modules/congestion... File webrtc/modules/congestion_controller/probe_bitrate_estimator.h (right): https://codereview.webrtc.org/2239143002/diff/60001/webrtc/modules/congestion... webrtc/modules/congestion_controller/probe_bitrate_estimator.h:29: int HandleProbeAndEstimateBitrate(const PacketInfo& packet_info, On 2016/08/19 11:46:30, danilchap wrote: > this function returns -1 when bitrate doesn't change and > the bitrate when it may be changed (when it is recalculated, probably to the > same cluster). > > May it would be cleaner if the funciton always return highest (cached) bitrate, > even if is obviously same as before. > Or return bool to indicate the bitrate may be changed and have an output > parameter or accessor to get highest bitrate so far. Given that I have removed the highest across clusters functionality now, I think this is pretty clean now in that it returns bitrate for the cluster to which the probe belongs to.
Description was changed from ========== ProbingEstimator: Enhancements for startup probing We add support to specify minimum clusters and also to measure highest bandwidth received across all clusters. BUG= ========== to ========== ProbingEstimator: Erase history based on time threshold Erases history based on time threshold instead of retaining really old cluster data. Also does a bunch of clean up. BUG= ==========
Patchset #5 (id:80001) has been deleted
PTAL. PS#5 is a simplified version and this CL now is mostly a clean up and not necessary for exponential start-up probing. https://codereview.chromium.org/2239143002/diff/60001/webrtc/modules/congesti... File webrtc/modules/congestion_controller/probe_bitrate_estimator_unittest.cc (right): https://codereview.chromium.org/2239143002/diff/60001/webrtc/modules/congesti... webrtc/modules/congestion_controller/probe_bitrate_estimator_unittest.cc:18: #include "webrtc/base/logging.h" On 2016/08/19 11:46:30, danilchap wrote: > is it needed? Done. https://codereview.chromium.org/2239143002/diff/60001/webrtc/modules/congesti... webrtc/modules/congestion_controller/probe_bitrate_estimator_unittest.cc:34: measured_bps_ = bps > 0 ? bps : 0; On 2016/08/19 11:46:30, danilchap wrote: > is it needed to check if bps > 0? > Shouldn't last AddPacketFeedback in all tests collect valid bitrate? > May be cleaner to remove measured_bps_ and make AddPacketFeedback return bps > instead. Not necessarily. The HandleProbeAndEstimateBitrate() returns a valid bitrate if the given probe completes a valid cluster and -1 otherwise.
lgtm
lgtm https://codereview.webrtc.org/2239143002/diff/100001/webrtc/modules/congestio... File webrtc/modules/congestion_controller/probe_bitrate_estimator_unittest.cc (right): https://codereview.webrtc.org/2239143002/diff/100001/webrtc/modules/congestio... webrtc/modules/congestion_controller/probe_bitrate_estimator_unittest.cc:33: measured_bps_ = bps > 0 ? bps : 0; bps > 0 ? bps : 0 converts invalid value '-1' into invalid value '0'. May be tests can check directly for -1 or HandleProbeAndEstimateBitrate can use 0 for 'no bitrate' value.
https://codereview.webrtc.org/2239143002/diff/100001/webrtc/modules/congestio... File webrtc/modules/congestion_controller/probe_bitrate_estimator_unittest.cc (right): https://codereview.webrtc.org/2239143002/diff/100001/webrtc/modules/congestio... webrtc/modules/congestion_controller/probe_bitrate_estimator_unittest.cc:33: measured_bps_ = bps > 0 ? bps : 0; On 2016/08/23 13:27:34, danilchap wrote: > bps > 0 ? bps : 0 > converts invalid value '-1' into invalid value '0'. > May be tests can check directly for -1 > or HandleProbeAndEstimateBitrate can use 0 for 'no bitrate' value. Done.
The CQ bit was checked by isheriff@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from stefan@webrtc.org, philipel@webrtc.org, danilchap@webrtc.org Link to the patchset: https://codereview.webrtc.org/2239143002/#ps140001 (title: "rebase")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...
The CQ bit was unchecked by isheriff@chromium.org
Message was sent while issue was closed.
Description was changed from ========== ProbingEstimator: Erase history based on time threshold Erases history based on time threshold instead of retaining really old cluster data. Also does a bunch of clean up. BUG= ========== to ========== ProbingEstimator: Erase history based on time threshold Erases history based on time threshold instead of retaining really old cluster data. Also does a bunch of clean up. BUG= R=danilchap@webrtc.org, philipel@webrtc.org, stefan@webrtc.org Committed: https://crrev.com/f99a9de06948494b2708a5accb6a5667a000926c Cr-Commit-Position: refs/heads/master@{#13870} ==========
Message was sent while issue was closed.
Description was changed from ========== ProbingEstimator: Erase history based on time threshold Erases history based on time threshold instead of retaining really old cluster data. Also does a bunch of clean up. BUG= R=danilchap@webrtc.org, philipel@webrtc.org, stefan@webrtc.org Committed: https://crrev.com/f99a9de06948494b2708a5accb6a5667a000926c Cr-Commit-Position: refs/heads/master@{#13870} ========== to ========== ProbingEstimator: Erase history based on time threshold Erases history based on time threshold instead of retaining really old cluster data. Also does a bunch of clean up. BUG= R=danilchap@webrtc.org, philipel@webrtc.org, stefan@webrtc.org Committed: https://chromium.googlesource.com/external/webrtc/+/f99a9de06948494b2708a5acc... ==========
Message was sent while issue was closed.
Patchset 7 (id:??) landed as https://crrev.com/f99a9de06948494b2708a5accb6a5667a000926c Cr-Commit-Position: refs/heads/master@{#13870}
Message was sent while issue was closed.
Committed patchset #7 (id:140001) manually as f99a9de06948494b2708a5accb6a5667a000926c (presubmit successful). |