Issue metadata
Sign in to add a comment
|
223.1% regression in content_browsertests at 499336:499354 |
||||||||||||||||||||
Issue descriptionSeems to be introduced by https://chromium.googlesource.com/chromium/src/+/d70094cd62f6ad2482f66734d61845ded77d58bb
,
Sep 5 2017
I believe this test [1] measures the average duration of executing the method VideoCaptureDeviceClient::OnIncomingCapturedData() [2] over the course of a 10s video capture example run. The reason why it takes longer after the suspected CL is probably because it now maps and unmaps the shared memory for each frame, while before the CL the shared memory was mapped only once when the buffer pool created the buffer. miu@: Do you think we should consider adding functionality to allow clients to tell the buffer pool to "pin" the memory mapping for cases where we know that we are going to request in-process access over and over again? [1] https://cs.chromium.org/chromium/src/content/browser/webrtc/webrtc_getusermedia_browsertest.cc?type=cs&l=662 [2] https://cs.chromium.org/chromium/src/media/capture/video/video_capture_device_client.cc?l=143
,
Sep 5 2017
The regression is 150 microseconds, and is very likely being caused by the extra syscalls around mapping and unmapping for each frame...But, this does not seem like much of a regression...? It would add 150 microseconds to the latency in the video capture pipeline, but this is insignificant compared to downstream parts of the pipeline like video encoding and network transmission (20,000 to 500,000 microseconds). However, there's a perf test measuring this, and so I feel like I'm missing something important. :) Since I don't have the time to investigate or monitor any other possible related fall-out from this, I'll go ahead and put together a patch to fix this regression: The "lazy map once" scheme--what the code in services/video_capture/public/cpp/receiver_media_to_mojo_adapter.cc did before--would work just fine for what I needed and it would be a fairly simple change that restores the "mmap once for the long term" behavior.
,
Sep 5 2017
Fix up for review: https://chromium-review.googlesource.com/c/chromium/src/+/651506
,
Sep 6 2017
,
Sep 6 2017
Issue 762255 has been merged into this issue.
,
Sep 6 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/121c12240e81599bcbcea88a87da740c641d69f2 commit 121c12240e81599bcbcea88a87da740c641d69f2 Author: Yuri Wiitala <miu@chromium.org> Date: Wed Sep 06 21:33:36 2017 Video Capture SharedMemoryHandleProvider: mmap() once, lazy Fixes the performance regression caused by recent re-workings of the way shared memory is managed in the Video Capture stack. Instead of only mapping shared memory while it is being mutated (i.e., mapped and unmapped for each video frame produced), this change restores the prior behavior of mapping upon first use and then only unmapping way later, when the buffer pool is destroyed. Bug: 762042 Change-Id: I18775d02696a24863dae3f21932c544cc10c2432 Reviewed-on: https://chromium-review.googlesource.com/651506 Commit-Queue: Yuri Wiitala <miu@chromium.org> Reviewed-by: Yuri Wiitala <miu@chromium.org> Reviewed-by: Christian Fremerey <chfremer@chromium.org> Cr-Commit-Position: refs/heads/master@{#500096} [modify] https://crrev.com/121c12240e81599bcbcea88a87da740c641d69f2/media/capture/video/shared_memory_handle_provider.cc [modify] https://crrev.com/121c12240e81599bcbcea88a87da740c641d69f2/media/capture/video/shared_memory_handle_provider.h
,
Sep 21 2017
|
|||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||
Comment 1 by 42576172...@developer.gserviceaccount.com
, Sep 5 2017