Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2280593002:
Significantly increased max_num_buffers_ of Vp9FrameBufferPool. (Closed)
|
Created:
4 years, 3 months ago by hbos Modified:
4 years, 3 months ago Reviewers:
stefan-webrtc CC:
webrtc-reviews_webrtc.org, tterriberry_mozilla.com, zhengzhonghou_agora.io, video-team_agora.io, stefan-webrtc, mflodman Target Ref:
refs/pending/heads/master Project:
webrtc Visibility:
Public. |
DescriptionSignificantly increased max_num_buffers_ of Vp9FrameBufferPool.
Changed from 10 to 68.
This is to avoid a flake where the limit is exceeded, see
crbug.com/638554. Our performance tests should flag performance
regressions, we shouldn't rely on crashing because the number of
referenced buffers is not tiny to detect this. However, if a really big
number of buffers (>68) are referenced without being dereferenced it is
likely that we have a bug and frames are leaking in which case we can
DCHECK-crash.
BUG=chromium:638554
Committed: https://crrev.com/c928453fd03a25d9d66d1c5052fcd1e4ade3dbc7
Cr-Commit-Position: refs/heads/master@{#14084}
Patch Set 1 #Patch Set 2 : Updated comment and max num buffers #Messages
Total messages: 16 (10 generated)
hbos@webrtc.org changed reviewers: + stefan@webrtc.org
Please take a look, stefan.
Description was changed from ========== Significantly increased max_num_buffers_ of Vp9FrameBufferPool. Changed from 10 to 100. This is to avoid a flake where the limit is exceeded, see crbug.com/638554. Our performance tests should flag performance regressions, we shouldn't rely on crashing because the number of referenced buffers is not tiny to detect this. However, if a really big number of buffers (>100) are referenced without being dereferenced it is likely that we have a bug and frames are leaking in which case we can DCHECK-crash. BUG=chromium:638554 ========== to ========== Significantly increased max_num_buffers_ of Vp9FrameBufferPool. Changed from 10 to 100. This is to avoid a flake where the limit is exceeded, see crbug.com/638554. Our performance tests should flag performance regressions, we shouldn't rely on crashing because the number of referenced buffers is not tiny to detect this. However, if a really big number of buffers (>68) are referenced without being dereferenced it is likely that we have a bug and frames are leaking in which case we can DCHECK-crash. BUG=chromium:638554 ==========
The CQ bit was checked by hbos@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: android_dbg on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/android_dbg/builds/16083)
Do we have a warning logged when we reach a large number in release builds? Update the description from 100 to 68. lgtm
Description was changed from ========== Significantly increased max_num_buffers_ of Vp9FrameBufferPool. Changed from 10 to 100. This is to avoid a flake where the limit is exceeded, see crbug.com/638554. Our performance tests should flag performance regressions, we shouldn't rely on crashing because the number of referenced buffers is not tiny to detect this. However, if a really big number of buffers (>68) are referenced without being dereferenced it is likely that we have a bug and frames are leaking in which case we can DCHECK-crash. BUG=chromium:638554 ========== to ========== Significantly increased max_num_buffers_ of Vp9FrameBufferPool. Changed from 10 to 68. This is to avoid a flake where the limit is exceeded, see crbug.com/638554. Our performance tests should flag performance regressions, we shouldn't rely on crashing because the number of referenced buffers is not tiny to detect this. However, if a really big number of buffers (>68) are referenced without being dereferenced it is likely that we have a bug and frames are leaking in which case we can DCHECK-crash. BUG=chromium:638554 ==========
On 2016/09/05 19:13:58, stefan-webrtc (holmer) wrote: > Do we have a warning logged when we reach a large number in release builds? > > Update the description from 100 to 68. > > lgtm Yes, LOG(LS_WARNING)
The CQ bit was checked by hbos@webrtc.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...
Message was sent while issue was closed.
Description was changed from ========== Significantly increased max_num_buffers_ of Vp9FrameBufferPool. Changed from 10 to 68. This is to avoid a flake where the limit is exceeded, see crbug.com/638554. Our performance tests should flag performance regressions, we shouldn't rely on crashing because the number of referenced buffers is not tiny to detect this. However, if a really big number of buffers (>68) are referenced without being dereferenced it is likely that we have a bug and frames are leaking in which case we can DCHECK-crash. BUG=chromium:638554 ========== to ========== Significantly increased max_num_buffers_ of Vp9FrameBufferPool. Changed from 10 to 68. This is to avoid a flake where the limit is exceeded, see crbug.com/638554. Our performance tests should flag performance regressions, we shouldn't rely on crashing because the number of referenced buffers is not tiny to detect this. However, if a really big number of buffers (>68) are referenced without being dereferenced it is likely that we have a bug and frames are leaking in which case we can DCHECK-crash. BUG=chromium:638554 ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== Significantly increased max_num_buffers_ of Vp9FrameBufferPool. Changed from 10 to 68. This is to avoid a flake where the limit is exceeded, see crbug.com/638554. Our performance tests should flag performance regressions, we shouldn't rely on crashing because the number of referenced buffers is not tiny to detect this. However, if a really big number of buffers (>68) are referenced without being dereferenced it is likely that we have a bug and frames are leaking in which case we can DCHECK-crash. BUG=chromium:638554 ========== to ========== Significantly increased max_num_buffers_ of Vp9FrameBufferPool. Changed from 10 to 68. This is to avoid a flake where the limit is exceeded, see crbug.com/638554. Our performance tests should flag performance regressions, we shouldn't rely on crashing because the number of referenced buffers is not tiny to detect this. However, if a really big number of buffers (>68) are referenced without being dereferenced it is likely that we have a bug and frames are leaking in which case we can DCHECK-crash. BUG=chromium:638554 Committed: https://crrev.com/c928453fd03a25d9d66d1c5052fcd1e4ade3dbc7 Cr-Commit-Position: refs/heads/master@{#14084} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/c928453fd03a25d9d66d1c5052fcd1e4ade3dbc7 Cr-Commit-Position: refs/heads/master@{#14084} |
