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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 134603005: Clean-up: Remove the very last few uses of base::Time from src/media/ (Closed)

Created:
6 years, 11 months ago by miu
Modified:
6 years, 11 months ago
Reviewers:
DaleCurtis
CC:
chromium-reviews, feature-media-reviews_chromium.org, miu+watch_chromium.org, zhaoqin1
Visibility:
Public.

Description

Clean-up: Remove the very last few uses of base::Time from src/media/ This (finally) completes the effort to remove all uses of base::Time as a clock for tracking time passage in the media component. For more info, please see src/media/PRESUBMIT.py. Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=244419

Patch Set 1 : #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+6 lines, -11 lines) Patch
M media/audio/audio_output_controller.cc View 1 chunk +0 lines, -1 line 0 comments Download
M media/base/media_file_checker.cc View 2 chunks +2 lines, -2 lines 0 comments Download
M media/base/seekable_buffer_unittest.cc View 2 chunks +4 lines, -8 lines 1 comment Download

Messages

Total messages: 9 (0 generated)
miu
Dale: PTAL. Feel free to check commit if ell-gee-tee-em. https://codereview.chromium.org/134603005/diff/30001/media/base/seekable_buffer_unittest.cc File media/base/seekable_buffer_unittest.cc (right): https://codereview.chromium.org/134603005/diff/30001/media/base/seekable_buffer_unittest.cc#newcode32 media/base/seekable_buffer_unittest.cc:32: ...
6 years, 11 months ago (2014-01-10 22:53:53 UTC) #1
DaleCurtis
lgtm
6 years, 11 months ago (2014-01-10 23:05:50 UTC) #2
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/miu@chromium.org/134603005/30001
6 years, 11 months ago (2014-01-11 00:23:03 UTC) #3
commit-bot: I haz the power
Retried try job too often on win_rel for step(s) browser_tests http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&number=245212
6 years, 11 months ago (2014-01-11 05:04:53 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/miu@chromium.org/134603005/30001
6 years, 11 months ago (2014-01-11 05:24:39 UTC) #5
commit-bot: I haz the power
Change committed as 244419
6 years, 11 months ago (2014-01-12 02:43:27 UTC) #6
Derek Bruening
On 2014/01/12 02:43:27, I haz the power (commit-bot) wrote: > Change committed as 244419 This ...
6 years, 11 months ago (2014-01-17 18:08:49 UTC) #7
miu
My guess is that it's because I switched to using base/rand_until.h for the RNG in ...
6 years, 11 months ago (2014-01-17 19:34:22 UTC) #8
miu
6 years, 11 months ago (2014-01-17 22:37:17 UTC) #9 
Yep, it was the RNG.  Code review submitted:
https://codereview.chromium.org/139303022/


On Fri, Jan 17, 2014 at 11:34 AM, Yuri Wiitala <miu@chromium.org> wrote:

> My guess is that it's because I switched to using base/rand_until.h for
> the RNG in the test code.  I'll get a fix for this done today.
> On Jan 17, 2014 10:08 AM, <bruening@chromium.org> wrote:
>
>> On 2014/01/12 02:43:27, I haz the power (commit-bot) wrote:
>>
>>> Change committed as 244419
>>>
>>
>> This change slows down all of the media_unittests SeekableBufferTest.*
>> tests on
>> Windows by nearly 100x -- was that intentional?
>>
>> On win7:
>>
>> Here's r24415
>> http://build.chromium.org/p/chromium.win/builders/Win7%
>> 20Tests%20%281%29/builds/27088/steps/media_unittests/logs/stdio
>> [393/1422] SeekableBufferTest.RandomReadWrite (14 ms)
>> [394/1422] SeekableBufferTest.ReadWriteSeek (15 ms)
>> [395/1422] SeekableBufferTest.BufferFull (15 ms)
>> [396/1422] SeekableBufferTest.SeekBackward (14 ms)
>> [397/1422] SeekableBufferTest.GetCurrentChunk (15 ms)
>> [398/1422] SeekableBufferTest.SeekForward (15 ms)
>> [399/1422] SeekableBufferTest.AllMethods (16 ms)
>> [400/1422] SeekableBufferTest.GetTime (18 ms)
>>
>> and r244425
>> http://build.chromium.org/p/chromium.win/builders/Win7%
>> 20Tests%20%281%29/builds/27089/steps/media_unittests/logs/stdio
>> [1385/1422] SeekableBufferTest.RandomReadWrite (1525 ms)
>> [1386/1422] SeekableBufferTest.ReadWriteSeek (1310 ms)
>> [1387/1422] SeekableBufferTest.BufferFull (2274 ms)
>> [1388/1422] SeekableBufferTest.SeekBackward (1149 ms)
>> [1389/1422] SeekableBufferTest.GetCurrentChunk (1040 ms)
>> [1390/1422] SeekableBufferTest.SeekForward (1333 ms)
>> [1391/1422] SeekableBufferTest.AllMethods (1026 ms)
>> [1392/1422] SeekableBufferTest.GetTime (1034 ms)
>>
>> This orders-of-magnitude slowdown really hits Dr. Memory, adding 15
>> minutes (!)
>> to our runtime, taking it from ~5 minutes to ~20 minutes for the entire
>> media_unittests:
>>
>> r244403:
>> http://build.chromium.org/p/chromium.fyi/builders/Windows%
>> 20Unit%20%28DrMemory%20full%29%20%281%29/builds/4115
>> [----------] 8 tests from SeekableBufferTest
>> [ RUN      ] SeekableBufferTest.RandomReadWrite
>> [       OK ] SeekableBufferTest.RandomReadWrite (1264 ms)
>> [ RUN      ] SeekableBufferTest.ReadWriteSeek
>> [       OK ] SeekableBufferTest.ReadWriteSeek (1217 ms)
>> [ RUN      ] SeekableBufferTest.BufferFull
>> [       OK ] SeekableBufferTest.BufferFull (1170 ms)
>> [ RUN      ] SeekableBufferTest.SeekBackward
>> [       OK ] SeekableBufferTest.SeekBackward (1185 ms)
>> [ RUN      ] SeekableBufferTest.GetCurrentChunk
>> [       OK ] SeekableBufferTest.GetCurrentChunk (1170 ms)
>> [ RUN      ] SeekableBufferTest.SeekForward
>> [       OK ] SeekableBufferTest.SeekForward (1202 ms)
>> [ RUN      ] SeekableBufferTest.AllMethods
>> [       OK ] SeekableBufferTest.AllMethods (1170 ms)
>> [ RUN      ] SeekableBufferTest.GetTime
>> [       OK ] SeekableBufferTest.GetTime (1201 ms)
>> [----------] 8 tests from SeekableBufferTest (9579 ms total)
>>
>> r244425:
>> http://build.chromium.org/p/chromium.fyi/builders/Windows%
>> 20Unit%20%28DrMemory%20full%29%20%281%29/builds/4116
>> [----------] 8 tests from SeekableBufferTest
>> [ RUN      ] SeekableBufferTest.RandomReadWrite
>> [       OK ] SeekableBufferTest.RandomReadWrite (94210 ms)
>> [ RUN      ] SeekableBufferTest.ReadWriteSeek
>> [       OK ] SeekableBufferTest.ReadWriteSeek (94772 ms)
>> [ RUN      ] SeekableBufferTest.BufferFull
>> [       OK ] SeekableBufferTest.BufferFull (92057 ms)
>> [ RUN      ] SeekableBufferTest.SeekBackward
>> [       OK ] SeekableBufferTest.SeekBackward (93493 ms)
>> [ RUN      ] SeekableBufferTest.GetCurrentChunk
>> [       OK ] SeekableBufferTest.GetCurrentChunk (93555 ms)
>> [ RUN      ] SeekableBufferTest.SeekForward
>> [       OK ] SeekableBufferTest.SeekForward (94740 ms)
>> [ RUN      ] SeekableBufferTest.AllMethods
>> [       OK ] SeekableBufferTest.AllMethods (94148 ms)
>> [ RUN      ] SeekableBufferTest.GetTime
>> [       OK ] SeekableBufferTest.GetTime (94272 ms)
>> [----------] 8 tests from SeekableBufferTest (751263 ms total)
>>
>> Our tool's relative slowdown vs native is high for these tests, and we'll
>> look
>> at that, but we're wondering if there's some mistake in this change?  Do
>> you
>> know why this test (natively) is now 100x slower than it was?
>>
>> https://codereview.chromium.org/134603005/
>>
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

Powered by Google App Engine
This is Rietveld 408576698