Reduce locking when collecting receive statistics.

Reduce locking when collecting receive statistic

BUG=None

Review-Url: https://codereview.webrtc.org/2997803002
Cr-Commit-Position: refs/heads/master@{#19336}
Committed: https://chromium.googlesource.com/external/webrtc/+/ec86be0962506903ee2d642fb2f8bc6e4f44c555

[danilchap, 2017-08-11 17:42:24]

The CQ bit was checked by danilchap@webrtc.org to run a CQ dry run

[commit-bot: I haz the power, 2017-08-11 17:42:32]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[danilchap, 2017-08-11 18:13:54]

Description was changed from

==========
Reduce locking when collecting and retrieving receive statistic

BUG=None
==========

to

==========
Reduce locking when collecting receive statistic

BUG=None
==========

[danilchap, 2017-08-11 18:19:59]

danilchap@webrtc.org changed reviewers:
+ sprang@webrtc.org

[danilchap, 2017-08-11 18:20:01]

(some of it was added in https://webrtc-codereview.appspot.com/7649004)

[sprang_webrtc, 2017-08-14 09:43:06]

lgtm with nit

https://codereview.webrtc.org/2997803002/diff/60001/webrtc/modules/rtp_rtcp/s...
File webrtc/modules/rtp_rtcp/source/receive_statistics_impl.h (right):

https://codereview.webrtc.org/2997803002/diff/60001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/receive_statistics_impl.h:61: void
NotifyRtcpCallback(const RtcpStatistics&) LOCKS_EXCLUDED(stream_lock_);
Nit: These don't seem to be very useful anymore, can we just add a guarded_by to
the callbacks pointers and call them directly?

[danilchap, 2017-08-14 10:44:28]

Patchset #5 (id:80001) has been deleted

[danilchap, 2017-08-14 10:45:32]

https://codereview.webrtc.org/2997803002/diff/60001/webrtc/modules/rtp_rtcp/s...
File webrtc/modules/rtp_rtcp/source/receive_statistics_impl.h (right):

https://codereview.webrtc.org/2997803002/diff/60001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/receive_statistics_impl.h:61: void
NotifyRtcpCallback(const RtcpStatistics&) LOCKS_EXCLUDED(stream_lock_);
On 2017/08/14 09:43:06, sprang_webrtc wrote:
> Nit: These don't seem to be very useful anymore, can we just add a guarded_by
to
> the callbacks pointers and call them directly?

Checked documentation for LOCKS_EXCLUDED - it doesn't apply here.
so removing it together with the functions.

unfortunately I'm not aware of GUARDED_BY-like macro to annotate lock should NOT
be held when accessing a variable.

[danilchap, 2017-08-14 10:47:31]

The CQ bit was checked by danilchap@webrtc.org to run a CQ dry run

[commit-bot: I haz the power, 2017-08-14 10:47:36]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2017-08-14 10:52:06]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-08-14 10:52:06]

Dry run: Try jobs failed on following builders:
  presubmit on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/presubmit/builds/20101)

[sprang_webrtc, 2017-08-14 11:25:39]

lgtm

https://codereview.webrtc.org/2997803002/diff/60001/webrtc/modules/rtp_rtcp/s...
File webrtc/modules/rtp_rtcp/source/receive_statistics_impl.h (right):

https://codereview.webrtc.org/2997803002/diff/60001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/receive_statistics_impl.h:61: void
NotifyRtcpCallback(const RtcpStatistics&) LOCKS_EXCLUDED(stream_lock_);
On 2017/08/14 10:45:32, danilchap wrote:
> On 2017/08/14 09:43:06, sprang_webrtc wrote:
> > Nit: These don't seem to be very useful anymore, can we just add a
guarded_by
> to
> > the callbacks pointers and call them directly?
> 
> Checked documentation for LOCKS_EXCLUDED - it doesn't apply here.
> so removing it together with the functions.
> 
> unfortunately I'm not aware of GUARDED_BY-like macro to annotate lock should
NOT
> be held when accessing a variable.

Ah, shoot. Just assumed there was.

[danilchap, 2017-08-14 11:57:01]

The CQ bit was checked by danilchap@webrtc.org

[commit-bot: I haz the power, 2017-08-14 11:57:07]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2017-08-14 12:51:03]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1502711821787540,
"parent_rev": "0ba43b5a2045b0b6de4c8e374895462961330b85", "commit_rev":
"ec86be0962506903ee2d642fb2f8bc6e4f44c555"}

[commit-bot: I haz the power, 2017-08-14 12:51:08]

Description was changed from

==========
Reduce locking when collecting receive statistic

BUG=None
==========

to

==========
Reduce locking when collecting receive statistic

BUG=None

Review-Url: https://codereview.webrtc.org/2997803002
Cr-Commit-Position: refs/heads/master@{#19336}
Committed:
https://chromium.googlesource.com/external/webrtc/+/ec86be0962506903ee2d642fb...
==========

[commit-bot: I haz the power, 2017-08-14 12:51:10]

Committed patchset #5 (id:100001) as
https://chromium.googlesource.com/external/webrtc/+/ec86be0962506903ee2d642fb...