Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
3010653002:
Keep track of the capacity delay error in the FakeNetworkPipe. (Closed)
DescriptionKeep track of the capacity delay error in the FakeNetworkPipe.
The FakeNetworkPipe use millisecond precision to calculate the delay induced by
the size of the packet being sent. The problem is that it rounds the delay down
to the closest millisecond which can cause a significant error in the actual
throughput. We keep track of that error to compensate the delay cause by
subsequent packets.
BUG=webrtc:8287
Review-Url: https://codereview.webrtc.org/3010653002
Cr-Commit-Position: refs/heads/master@{#19732}
Committed: https://chromium.googlesource.com/external/webrtc/+/19f51434e84589a8065fb9b20f059ff6b52dd853
Patch Set 1 #
Total comments: 2
Patch Set 2 : Feedback #Patch Set 3 : Comment fix #
Total comments: 1
Messages
Total messages: 27 (16 generated)
philipel@webrtc.org changed reviewers: + terelius@webrtc.org
The CQ bit was checked by philipel@webrtc.org to run a CQ dry run
Dry run: 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 commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_arm64_dbg on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/linux_arm64_dbg/builds/...)
https://codereview.webrtc.org/3010653002/diff/1/webrtc/test/fake_network_pipe.cc File webrtc/test/fake_network_pipe.cc (right): https://codereview.webrtc.org/3010653002/diff/1/webrtc/test/fake_network_pipe... webrtc/test/fake_network_pipe.cc:138: const int offset_bytes = config_.link_capacity_kbps / 8 / 2; foo = config_.link_capacity_kbps / 8; capacity_delay_ms = (data_length + capacity_delay_error_bytes_ + foo/2) / foo; instead?
The CQ bit was checked by philipel@webrtc.org to run a CQ dry run
https://codereview.webrtc.org/3010653002/diff/1/webrtc/test/fake_network_pipe.cc File webrtc/test/fake_network_pipe.cc (right): https://codereview.webrtc.org/3010653002/diff/1/webrtc/test/fake_network_pipe... webrtc/test/fake_network_pipe.cc:136: // To round to closest millisecond we add half a milliseconds worth of bytes To round to the
Dry run: 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 commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by philipel@webrtc.org
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 commit-bot@chromium.org
No L-G-T-M from a valid reviewer yet. CQ run can only be started once the patch has received an L-G-T-M from a full committer. Even if an L-G-T-M may have been provided, it was from a non-committer,_not_ a full super star committer. Committers are members of the group "project-webrtc-committers". Note that this has nothing to do with OWNERS files.
philipel@webrtc.org changed reviewers: + stefan@webrtc.org
PTAL
lgtm https://codereview.webrtc.org/3010653002/diff/40001/webrtc/test/fake_network_... File webrtc/test/fake_network_pipe.cc (right): https://codereview.webrtc.org/3010653002/diff/40001/webrtc/test/fake_network_... webrtc/test/fake_network_pipe.cc:136: const int bytes_per_millisecond = config_.link_capacity_kbps / 8; nit: Would bytes_per_ms make the capacity_delay calculation fit on two lines?
ping
lgtm
The CQ bit was checked by philipel@webrtc.org
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...
CQ is committing da patch.
Bot data: {"patchset_id": 40001, "attempt_start_ts": 1504798531154540,
"parent_rev": "4389b4d96bb98cf210b761b4f152434fceb767a6", "commit_rev":
"19f51434e84589a8065fb9b20f059ff6b52dd853"}
Message was sent while issue was closed.
Description was changed from ========== Keep track of the capacity delay error in the FakeNetworkPipe. The FakeNetworkPipe use millisecond precision to calculate the delay induced by the size of the packet being sent. The problem is that it rounds the delay down to the closest millisecond which can cause a significant error in the actual throughput. We keep track of that error to compensate the delay cause by subsequent packets. BUG=None ========== to ========== Keep track of the capacity delay error in the FakeNetworkPipe. The FakeNetworkPipe use millisecond precision to calculate the delay induced by the size of the packet being sent. The problem is that it rounds the delay down to the closest millisecond which can cause a significant error in the actual throughput. We keep track of that error to compensate the delay cause by subsequent packets. BUG=None Review-Url: https://codereview.webrtc.org/3010653002 Cr-Commit-Position: refs/heads/master@{#19732} Committed: https://chromium.googlesource.com/external/webrtc/+/19f51434e84589a8065fb9b20... ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as https://chromium.googlesource.com/external/webrtc/+/19f51434e84589a8065fb9b20...
Message was sent while issue was closed.
Description was changed from ========== Keep track of the capacity delay error in the FakeNetworkPipe. The FakeNetworkPipe use millisecond precision to calculate the delay induced by the size of the packet being sent. The problem is that it rounds the delay down to the closest millisecond which can cause a significant error in the actual throughput. We keep track of that error to compensate the delay cause by subsequent packets. BUG=None Review-Url: https://codereview.webrtc.org/3010653002 Cr-Commit-Position: refs/heads/master@{#19732} Committed: https://chromium.googlesource.com/external/webrtc/+/19f51434e84589a8065fb9b20... ========== to ========== Keep track of the capacity delay error in the FakeNetworkPipe. The FakeNetworkPipe use millisecond precision to calculate the delay induced by the size of the packet being sent. The problem is that it rounds the delay down to the closest millisecond which can cause a significant error in the actual throughput. We keep track of that error to compensate the delay cause by subsequent packets. BUG=webrtc:8287 Review-Url: https://codereview.webrtc.org/3010653002 Cr-Commit-Position: refs/heads/master@{#19732} Committed: https://chromium.googlesource.com/external/webrtc/+/19f51434e84589a8065fb9b20... ========== |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
