Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2751213002:
Reduce runtime of osc-negative-freq test (Closed)
|
Created:
3 years, 9 months ago by Raymond Toy Modified:
3 years, 9 months ago CC:
chromium-reviews, blink-reviews Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionReduce runtime of osc-negative-freq test
Reduce the sample rate and duration of the osc-negative-freq test to
make it run faster for MSAN tests. Tightened up the thresholds for
the new test.
On a Z620 machine running linux, the debug runtime was 1.414 sec. It
is now 0.860, roughtly 60% as long as before.
BUG=701418
TEST=osc-negative-freq.html
Review-Url: https://codereview.chromium.org/2751213002
Cr-Original-Commit-Position: refs/heads/master@{#457608}
Committed: https://chromium.googlesource.com/chromium/src/+/db917052bfb936546b2602cdb657c310778c6451
Review-Url: https://codereview.chromium.org/2751213002
Cr-Commit-Position: refs/heads/master@{#457858}
Committed: https://chromium.googlesource.com/chromium/src/+/fdaf992478720559b046d7f953c99c9728a694c4
Patch Set 1 #Patch Set 2 : Adjust threshold for mac. #
Total comments: 2
Patch Set 3 : Address review comments #Patch Set 4 : Adjust thresholds for mac10.11 retina #Messages
Total messages: 23 (11 generated)
rtoy@chromium.org changed reviewers: + hongchan@chromium.org, qyearsley@chromium.org
PTAL.
Description was changed from ========== Reduce runtime of osc-negative-freq test Reduce the sample rate and duration of the osc-negative-freq test to make it run faster. Tightened up the thresholds for the new test. On a Z620 machine running linux, the debug runtime was 1.414 sec. It is now 0.860, roughtly 60% as long as before. BUG=701418 TEST=osc-negative-freq.html ========== to ========== Reduce runtime of osc-negative-freq test Reduce the sample rate and duration of the osc-negative-freq test to make it run faster for MSAN tests. Tightened up the thresholds for the new test. On a Z620 machine running linux, the debug runtime was 1.414 sec. It is now 0.860, roughtly 60% as long as before. BUG=701418 TEST=osc-negative-freq.html ==========
On 2017/03/15 at 22:00:56, rtoy wrote: > PTAL. Not sure why the test failed on mac but not other platforms :-/ https://storage.googleapis.com/chromium-layout-test-archives/mac_chromium_rel... I don't understand what the test is doing now, so I guess I should leave review to hongchan@ :-)
On 2017/03/16 02:26:58, qyearsley wrote: > On 2017/03/15 at 22:00:56, rtoy wrote: > > PTAL. > > Not sure why the test failed on mac but not other platforms :-/ > https://storage.googleapis.com/chromium-layout-test-archives/mac_chromium_rel... > > I don't understand what the test is doing now, so I guess I should leave review > to hongchan@ :-) Thresholds are just a little too tight on Mac (because Mac uses a different FFT library) I'll update the test soon.
lgtm with nit https://codereview.chromium.org/2751213002/diff/20001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/webaudio/Oscillator/osc-negative-freq.html (right): https://codereview.chromium.org/2751213002/diff/20001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/webaudio/Oscillator/osc-negative-freq.html:17: // A failyr arbitrary duration that should have at least 1-2 sample Can we add one empty line before l.17?
https://codereview.chromium.org/2751213002/diff/20001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/webaudio/Oscillator/osc-negative-freq.html (right): https://codereview.chromium.org/2751213002/diff/20001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/webaudio/Oscillator/osc-negative-freq.html:17: // A failyr arbitrary duration that should have at least 1-2 sample On 2017/03/16 21:46:09, hongchan wrote: > Can we add one empty line before l.17? Done. And fixed the typo. :-)
The CQ bit was checked by rtoy@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from hongchan@chromium.org Link to the patchset: https://codereview.chromium.org/2751213002/#ps40001 (title: "Address review comments")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 40001, "attempt_start_ts": 1489702409869200,
"parent_rev": "6d0510b49f3fa532b29378bf9d45b08374ce1885", "commit_rev":
"db917052bfb936546b2602cdb657c310778c6451"}
Message was sent while issue was closed.
Description was changed from ========== Reduce runtime of osc-negative-freq test Reduce the sample rate and duration of the osc-negative-freq test to make it run faster for MSAN tests. Tightened up the thresholds for the new test. On a Z620 machine running linux, the debug runtime was 1.414 sec. It is now 0.860, roughtly 60% as long as before. BUG=701418 TEST=osc-negative-freq.html ========== to ========== Reduce runtime of osc-negative-freq test Reduce the sample rate and duration of the osc-negative-freq test to make it run faster for MSAN tests. Tightened up the thresholds for the new test. On a Z620 machine running linux, the debug runtime was 1.414 sec. It is now 0.860, roughtly 60% as long as before. BUG=701418 TEST=osc-negative-freq.html Review-Url: https://codereview.chromium.org/2751213002 Cr-Commit-Position: refs/heads/master@{#457608} Committed: https://chromium.googlesource.com/chromium/src/+/db917052bfb936546b2602cdb657... ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as https://chromium.googlesource.com/chromium/src/+/db917052bfb936546b2602cdb657...
Message was sent while issue was closed.
A revert of this CL (patchset #3 id:40001) has been created in https://codereview.chromium.org/2756113003/ by nhiroki@chromium.org. The reason for reverting is: After this patch, osc-negative-freq.html is consistently failing on WebKit Mac10.11 (retina): https://uberchromegw.corp.google.com/i/chromium.webkit/builders/WebKit%20Mac1....
Message was sent while issue was closed.
On 2017/03/17 at 07:19:37, nhiroki wrote: > A revert of this CL (patchset #3 id:40001) has been created in https://codereview.chromium.org/2756113003/ by nhiroki@chromium.org. > > The reason for reverting is: After this patch, osc-negative-freq.html is consistently failing on WebKit Mac10.11 (retina): > https://uberchromegw.corp.google.com/i/chromium.webkit/builders/WebKit%20Mac1.... Aw, darn! Well this is turning out to be a bit troublesome - rtoy@, if you make another patch, before submitting we can also try other Blink try bots (https://build.chromium.org/p/tryserver.blink/builders) including mac10.11_retina_blink_rel.
Message was sent while issue was closed.
On 2017/03/17 15:31:41, qyearsley wrote: > On 2017/03/17 at 07:19:37, nhiroki wrote: > > A revert of this CL (patchset #3 id:40001) has been created in > https://codereview.chromium.org/2756113003/ by mailto:nhiroki@chromium.org. > > > > The reason for reverting is: After this patch, osc-negative-freq.html is > consistently failing on WebKit Mac10.11 (retina): > > > https://uberchromegw.corp.google.com/i/chromium.webkit/builders/WebKit%20Mac1.... > > Aw, darn! > > Well this is turning out to be a bit troublesome - rtoy@, if you make another > patch, before submitting we can also try other > Blink try bots (https://build.chromium.org/p/tryserver.blink/builders) including > mac10.11_retina_blink_rel. Yep. It's just yet another case of thresholds being too tight.
Message was sent while issue was closed.
Description was changed from ========== Reduce runtime of osc-negative-freq test Reduce the sample rate and duration of the osc-negative-freq test to make it run faster for MSAN tests. Tightened up the thresholds for the new test. On a Z620 machine running linux, the debug runtime was 1.414 sec. It is now 0.860, roughtly 60% as long as before. BUG=701418 TEST=osc-negative-freq.html Review-Url: https://codereview.chromium.org/2751213002 Cr-Commit-Position: refs/heads/master@{#457608} Committed: https://chromium.googlesource.com/chromium/src/+/db917052bfb936546b2602cdb657... ========== to ========== Reduce runtime of osc-negative-freq test Reduce the sample rate and duration of the osc-negative-freq test to make it run faster for MSAN tests. Tightened up the thresholds for the new test. On a Z620 machine running linux, the debug runtime was 1.414 sec. It is now 0.860, roughtly 60% as long as before. BUG=701418 TEST=osc-negative-freq.html Review-Url: https://codereview.chromium.org/2751213002 Cr-Commit-Position: refs/heads/master@{#457608} Committed: https://chromium.googlesource.com/chromium/src/+/db917052bfb936546b2602cdb657... ==========
The CQ bit was checked by rtoy@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from hongchan@chromium.org Link to the patchset: https://codereview.chromium.org/2751213002/#ps60001 (title: "Adjust thresholds for mac10.11 retina")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 60001, "attempt_start_ts": 1489775357796500,
"parent_rev": "dd42664ce773ac1869030184955a2ef094b41f31", "commit_rev":
"fdaf992478720559b046d7f953c99c9728a694c4"}
Message was sent while issue was closed.
Description was changed from ========== Reduce runtime of osc-negative-freq test Reduce the sample rate and duration of the osc-negative-freq test to make it run faster for MSAN tests. Tightened up the thresholds for the new test. On a Z620 machine running linux, the debug runtime was 1.414 sec. It is now 0.860, roughtly 60% as long as before. BUG=701418 TEST=osc-negative-freq.html Review-Url: https://codereview.chromium.org/2751213002 Cr-Commit-Position: refs/heads/master@{#457608} Committed: https://chromium.googlesource.com/chromium/src/+/db917052bfb936546b2602cdb657... ========== to ========== Reduce runtime of osc-negative-freq test Reduce the sample rate and duration of the osc-negative-freq test to make it run faster for MSAN tests. Tightened up the thresholds for the new test. On a Z620 machine running linux, the debug runtime was 1.414 sec. It is now 0.860, roughtly 60% as long as before. BUG=701418 TEST=osc-negative-freq.html Review-Url: https://codereview.chromium.org/2751213002 Cr-Original-Commit-Position: refs/heads/master@{#457608} Committed: https://chromium.googlesource.com/chromium/src/+/db917052bfb936546b2602cdb657... Review-Url: https://codereview.chromium.org/2751213002 Cr-Commit-Position: refs/heads/master@{#457858} Committed: https://chromium.googlesource.com/chromium/src/+/fdaf992478720559b046d7f953c9... ==========
Message was sent while issue was closed.
Committed patchset #4 (id:60001) as https://chromium.googlesource.com/chromium/src/+/fdaf992478720559b046d7f953c9... |
