Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(397)

Issue 1009543003: Slight reduction in string copying for WebRTC stats. (Closed)

Created:
5 years, 9 months ago by tommi (sloooow) - chröme
Modified:
5 years, 9 months ago
Reviewers:
perkj_chrome
CC:
chromium-reviews, darin-cc_chromium.org, feature-media-reviews_chromium.org, jam, mcasas+watch_chromium.org, mkwst+moarreviews-renderer_chromium.org, mlamouri+watch-content_chromium.org, posciak+watch_chromium.org, wjia+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Slight reduction in string copying for WebRTC stats. * Only call ToString() for types that require conversion to string. If the value is already of type string, we can just use that value. * For chrome://webrtc-internals, we can now pass the correct type in most cases (not in64), in the dictionary passed to the web page. This also lowers the overhead of the PeerConnectionTracker when no page requires this data. (I think we can still do better there though). BUG=webrtc:2822 Committed: https://crrev.com/a5c0d763fd248d5ec8a8e5b837282bccfadd303a Cr-Commit-Position: refs/heads/master@{#320772}

Patch Set 1 #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+42 lines, -10 lines) Patch
M content/browser/resources/media/stats_table.js View 1 chunk +1 line, -1 line 0 comments Download
M content/renderer/media/peer_connection_tracker.cc View 5 chunks +29 lines, -6 lines 2 comments Download
M content/renderer/media/rtc_peer_connection_handler.cc View 1 chunk +12 lines, -3 lines 0 comments Download

Messages

Total messages: 8 (2 generated)
tommi (sloooow) - chröme
5 years, 9 months ago (2015-03-13 19:12:53 UTC) #2
perkj_chrome
lgtm https://codereview.chromium.org/1009543003/diff/1/content/renderer/media/peer_connection_tracker.cc File content/renderer/media/peer_connection_tracker.cc (right): https://codereview.chromium.org/1009543003/diff/1/content/renderer/media/peer_connection_tracker.cc#newcode238 content/renderer/media/peer_connection_tracker.cc:238: case StatsReport::Value::kInt64: // int64 isn't supported, so use ...
5 years, 9 months ago (2015-03-16 10:36:39 UTC) #3
tommi (sloooow) - chröme
https://codereview.chromium.org/1009543003/diff/1/content/renderer/media/peer_connection_tracker.cc File content/renderer/media/peer_connection_tracker.cc (right): https://codereview.chromium.org/1009543003/diff/1/content/renderer/media/peer_connection_tracker.cc#newcode238 content/renderer/media/peer_connection_tracker.cc:238: case StatsReport::Value::kInt64: // int64 isn't supported, so use string. ...
5 years, 9 months ago (2015-03-16 18:07:41 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1009543003/1
5 years, 9 months ago (2015-03-16 18:08:16 UTC) #6
commit-bot: I haz the power
Committed patchset #1 (id:1)
5 years, 9 months ago (2015-03-16 19:21:21 UTC) #7
commit-bot: I haz the power
5 years, 9 months ago (2015-03-16 19:21:57 UTC) #8
Message was sent while issue was closed.
Patchset 1 (id:??) landed as
https://crrev.com/a5c0d763fd248d5ec8a8e5b837282bccfadd303a
Cr-Commit-Position: refs/heads/master@{#320772}

Powered by Google App Engine
This is Rietveld 408576698