New issue
Advanced search Search tips

Issue 781425 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Nov 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

Network Quality observations synthesized from connection type should be discarded when cached observations are available

Project Member Reported by tbansal@chromium.org, Nov 3 2017

Issue description

Currently, network quality estimator takes into account (i) the cached observations; (ii) observations synthesized from the platform based on the connection type; and (iii) observations from the external estimate provider to generate a network quality estimate at startup. This ensures that a reasonable estimate is available even before any traffic is observed.

However, we know that cached observations are typically more accurate than the other two sources. In the case when cached estimate is available, Network Quality Estimator (NQE) should disregard observations from the other two sources.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Nov 6 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2577549570f819d97b590c635dd270d1eaea37d1

commit 2577549570f819d97b590c635dd270d1eaea37d1
Author: Tarun Bansal <tbansal@chromium.org>
Date: Mon Nov 06 19:47:53 2017

When cached estimate is available, do not add observations from the
platform or the external estimate provider.

In Network Quality Estimator (NQE), if a cached estimate is available
from the prefs, then it is not necessary to add other synthesized
observations such as from the platform, or the ones available
from the external estimate provider.

Cq-Include-Trybots: master.tryserver.chromium.android:android_cronet_tester;master.tryserver.chromium.mac:ios-simulator-cronet
Bug:  781425 
Change-Id: Ief8648b239d1ad190ec076025654c33cd8d8c912
Reviewed-on: https://chromium-review.googlesource.com/753641
Reviewed-by: Ryan Sturm <ryansturm@chromium.org>
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#514211}
[modify] https://crrev.com/2577549570f819d97b590c635dd270d1eaea37d1/net/nqe/network_quality_estimator.cc
[modify] https://crrev.com/2577549570f819d97b590c635dd270d1eaea37d1/net/nqe/network_quality_estimator.h
[modify] https://crrev.com/2577549570f819d97b590c635dd270d1eaea37d1/net/nqe/network_quality_estimator_test_util.h
[modify] https://crrev.com/2577549570f819d97b590c635dd270d1eaea37d1/net/nqe/network_quality_estimator_unittest.cc

Owner: tbansal@chromium.org
Status: Started (was: Untriaged)
Labels: -Pri-3 Pri-2
Project Member

Comment 4 by bugdroid1@chromium.org, Nov 7 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/395d0048c8821cb676d38b601e84ec193aa1ed24

commit 395d0048c8821cb676d38b601e84ec193aa1ed24
Author: Tarun Bansal <tbansal@chromium.org>
Date: Tue Nov 07 21:09:07 2017

Remove observations when the cached estimate is available in the Network
Quality Estimator (NQE).

Remove observations from the external estimate provider and the observations
synthesized from the platform once the cached estimate is available.

Bug:  781425 
Cq-Include-Trybots: master.tryserver.chromium.android:android_cronet_tester;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: If9096388bfaf4eb6506c7fe2a35bd1e06559fc8c
Reviewed-on: https://chromium-review.googlesource.com/648946
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Reviewed-by: Ryan Sturm <ryansturm@chromium.org>
Cr-Commit-Position: refs/heads/master@{#514592}
[modify] https://crrev.com/395d0048c8821cb676d38b601e84ec193aa1ed24/net/nqe/network_quality_estimator.cc
[modify] https://crrev.com/395d0048c8821cb676d38b601e84ec193aa1ed24/net/nqe/network_quality_estimator.h
[modify] https://crrev.com/395d0048c8821cb676d38b601e84ec193aa1ed24/net/nqe/network_quality_estimator_unittest.cc
[modify] https://crrev.com/395d0048c8821cb676d38b601e84ec193aa1ed24/net/nqe/observation_buffer.cc
[modify] https://crrev.com/395d0048c8821cb676d38b601e84ec193aa1ed24/net/nqe/observation_buffer.h
[modify] https://crrev.com/395d0048c8821cb676d38b601e84ec193aa1ed24/net/nqe/observation_buffer_unittest.cc

Status: Fixed (was: Started)
Project Member

Comment 6 by bugdroid1@chromium.org, Nov 7 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7ae8b2057eb9e99800769c03d414d3ab69d2e0af

commit 7ae8b2057eb9e99800769c03d414d3ab69d2e0af
Author: Tarun Bansal <tbansal@chromium.org>
Date: Tue Nov 07 21:25:38 2017

Revert "Remove observations when the cached estimate is available in the Network"

This reverts commit 395d0048c8821cb676d38b601e84ec193aa1ed24.

Reason for revert:  Breaks compilation.

Original change's description:
> Remove observations when the cached estimate is available in the Network
> Quality Estimator (NQE).
> 
> Remove observations from the external estimate provider and the observations
> synthesized from the platform once the cached estimate is available.
> 
> Bug:  781425 
> Cq-Include-Trybots: master.tryserver.chromium.android:android_cronet_tester;master.tryserver.chromium.mac:ios-simulator-cronet
> Change-Id: If9096388bfaf4eb6506c7fe2a35bd1e06559fc8c
> Reviewed-on: https://chromium-review.googlesource.com/648946
> Commit-Queue: Tarun Bansal <tbansal@chromium.org>
> Reviewed-by: Ryan Sturm <ryansturm@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#514592}

TBR=tbansal@chromium.org,ryansturm@chromium.org

Change-Id: Ic041009699e619880c4bfc30fe60740e8a98429a
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  781425 
Cq-Include-Trybots: master.tryserver.chromium.android:android_cronet_tester;master.tryserver.chromium.mac:ios-simulator-cronet
Reviewed-on: https://chromium-review.googlesource.com/756937
Reviewed-by: Tarun Bansal <tbansal@chromium.org>
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#514598}
[modify] https://crrev.com/7ae8b2057eb9e99800769c03d414d3ab69d2e0af/net/nqe/network_quality_estimator.cc
[modify] https://crrev.com/7ae8b2057eb9e99800769c03d414d3ab69d2e0af/net/nqe/network_quality_estimator.h
[modify] https://crrev.com/7ae8b2057eb9e99800769c03d414d3ab69d2e0af/net/nqe/network_quality_estimator_unittest.cc
[modify] https://crrev.com/7ae8b2057eb9e99800769c03d414d3ab69d2e0af/net/nqe/observation_buffer.cc
[modify] https://crrev.com/7ae8b2057eb9e99800769c03d414d3ab69d2e0af/net/nqe/observation_buffer.h
[modify] https://crrev.com/7ae8b2057eb9e99800769c03d414d3ab69d2e0af/net/nqe/observation_buffer_unittest.cc

Project Member

Comment 7 by bugdroid1@chromium.org, Nov 7 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/756049dccf48bb1caf380990d2a2cf9bdc562585

commit 756049dccf48bb1caf380990d2a2cf9bdc562585
Author: Brian White <bcwhite@chromium.org>
Date: Tue Nov 07 21:32:38 2017

Revert "Remove observations when the cached estimate is available in the Network"

This reverts commit 395d0048c8821cb676d38b601e84ec193aa1ed24.

Reason for revert: Linux Builder is failing
https://logs.chromium.org/v/?s=chromium%2Fbb%2Fchromium.linux%2FLinux_Builder%2F94045%2F%2B%2Frecipes%2Fsteps%2Fcompile%2F0%2Fstdout


Original change's description:
> Remove observations when the cached estimate is available in the Network
> Quality Estimator (NQE).
> 
> Remove observations from the external estimate provider and the observations
> synthesized from the platform once the cached estimate is available.
> 
> Bug:  781425 
> Cq-Include-Trybots: master.tryserver.chromium.android:android_cronet_tester;master.tryserver.chromium.mac:ios-simulator-cronet
> Change-Id: If9096388bfaf4eb6506c7fe2a35bd1e06559fc8c
> Reviewed-on: https://chromium-review.googlesource.com/648946
> Commit-Queue: Tarun Bansal <tbansal@chromium.org>
> Reviewed-by: Ryan Sturm <ryansturm@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#514592}

TBR=tbansal@chromium.org,ryansturm@chromium.org

Change-Id: Idcc489cfbe1ee6dfe1aa8e2dddb3e4f933424575
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  781425 
Cq-Include-Trybots: master.tryserver.chromium.android:android_cronet_tester;master.tryserver.chromium.mac:ios-simulator-cronet
Reviewed-on: https://chromium-review.googlesource.com/757580
Reviewed-by: Brian White <bcwhite@chromium.org>
Commit-Queue: Brian White <bcwhite@chromium.org>
Cr-Commit-Position: refs/heads/master@{#514599}

Project Member

Comment 8 by bugdroid1@chromium.org, Nov 8 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/31b07c588dfe61fef87b1f54fc60b92fe575ec2b

commit 31b07c588dfe61fef87b1f54fc60b92fe575ec2b
Author: Tarun Bansal <tbansal@chromium.org>
Date: Wed Nov 08 02:29:47 2017

Remove observations when the cached estimate is available in the Network
Quality Estimator (NQE).

This is a reland of the original CL from http://shortn/_tl5pp7UlOS which was reverted.

Remove observations from the external estimate provider and the observations
synthesized from the platform once the cached estimate is available.

Bug:  781425 
Change-Id: I816a1a9883739f81328f4ec792cbf2042848a258
Cq-Include-Trybots: master.tryserver.chromium.android:android_cronet_tester;master.tryserver.chromium.mac:ios-simulator-cronet
TBR: ryansturm@chromium.org
Reviewed-on: https://chromium-review.googlesource.com/756974
Reviewed-by: Tarun Bansal <tbansal@chromium.org>
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#514716}
[modify] https://crrev.com/31b07c588dfe61fef87b1f54fc60b92fe575ec2b/net/nqe/network_quality_estimator.cc
[modify] https://crrev.com/31b07c588dfe61fef87b1f54fc60b92fe575ec2b/net/nqe/network_quality_estimator.h
[modify] https://crrev.com/31b07c588dfe61fef87b1f54fc60b92fe575ec2b/net/nqe/network_quality_estimator_unittest.cc
[modify] https://crrev.com/31b07c588dfe61fef87b1f54fc60b92fe575ec2b/net/nqe/observation_buffer.cc
[modify] https://crrev.com/31b07c588dfe61fef87b1f54fc60b92fe575ec2b/net/nqe/observation_buffer.h
[modify] https://crrev.com/31b07c588dfe61fef87b1f54fc60b92fe575ec2b/net/nqe/observation_buffer_unittest.cc

Sign in to add a comment