Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1696723002:
Documantation: nanobench (Closed)
|
Created:
4 years, 10 months ago by hal.canary Modified:
4 years, 10 months ago CC:
reviews_skia.org Base URL:
https://skia.googlesource.com/skia.git@master Target Ref:
refs/heads/master Project:
skia Visibility:
Public. |
DescriptionDocumantation: nanobench
DOCS_PREVIEW= https://skia.org/dev/testing/tests?cl=1696723002
https://skia.org/dev/testing/tests?cl=1696723002#bench
GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1696723002
NOTRY=true
Committed: https://skia.googlesource.com/skia/+/6f96cf4bde38337724b492a00dd8e0857d84d5d6
Patch Set 1 #Patch Set 2 : 2016-02-12 (Friday) 09:38:04 EST #
Total comments: 2
Patch Set 3 : random #Patch Set 4 : while #
Total comments: 2
Patch Set 5 : 2016-02-12 (Friday) 11:04:06 EST #Messages
Total messages: 18 (9 generated)
Description was changed from ========== Documantation: nanobench ========== to ========== Documantation: nanobench GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... NOTRY=true DOCS_PREVIEW= https://skia.org/?cl=1696723002 ==========
Description was changed from ========== Documantation: nanobench GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... NOTRY=true DOCS_PREVIEW= https://skia.org/?cl=1696723002 ========== to ========== Documantation: nanobench DOCS_PREVIEW= https://skia.org/dev/testing/tests?cl=1696723002 ==========
Description was changed from ========== Documantation: nanobench DOCS_PREVIEW= https://skia.org/dev/testing/tests?cl=1696723002 ========== to ========== Documantation: nanobench DOCS_PREVIEW= https://skia.org/dev/testing/tests?cl=1696723002 ==========
Description was changed from ========== Documantation: nanobench DOCS_PREVIEW= https://skia.org/dev/testing/tests?cl=1696723002 ========== to ========== Documantation: nanobench DOCS_PREVIEW= https://skia.org/dev/testing/tests?cl=1696723002 GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... NOTRY=true ==========
Description was changed from ========== Documantation: nanobench DOCS_PREVIEW= https://skia.org/dev/testing/tests?cl=1696723002 GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... NOTRY=true ========== to ========== Documantation: nanobench DOCS_PREVIEW= https://skia.org/dev/testing/tests?cl=1696723002 https://skia.org/dev/testing/tests?cl=1696723002#bench ==========
halcanary@google.com changed reviewers: + hcm@google.com, mtklein@google.com
PTAL
https://codereview.chromium.org/1696723002/diff/20001/site/dev/testing/tests.md File site/dev/testing/tests.md (right): https://codereview.chromium.org/1696723002/diff/20001/site/dev/testing/tests.... site/dev/testing/tests.md:104: canvas->drawLine(10.0f + i % 17, 10.0f + i % 13, This is probably not setting a good example. It's usually better if you don't have your benchmark code depend on which particular loop it is running. Better to use, e.g., while (loops --> 0) and SkRandom.
Description was changed from ========== Documantation: nanobench DOCS_PREVIEW= https://skia.org/dev/testing/tests?cl=1696723002 https://skia.org/dev/testing/tests?cl=1696723002#bench ========== to ========== Documantation: nanobench DOCS_PREVIEW= https://skia.org/dev/testing/tests?cl=1696723002 https://skia.org/dev/testing/tests?cl=1696723002#bench GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... NOTRY=true ==========
https://codereview.chromium.org/1696723002/diff/20001/site/dev/testing/tests.md File site/dev/testing/tests.md (right): https://codereview.chromium.org/1696723002/diff/20001/site/dev/testing/tests.... site/dev/testing/tests.md:104: canvas->drawLine(10.0f + i % 17, 10.0f + i % 13, On 2016/02/12 at 15:22:34, mtklein wrote: > This is probably not setting a good example. It's usually better if you don't have your benchmark code depend on which particular loop it is running. Better to use, e.g., while (loops --> 0) and SkRandom. done and done.
https://codereview.chromium.org/1696723002/diff/60001/site/dev/testing/tests.md File site/dev/testing/tests.md (right): https://codereview.chromium.org/1696723002/diff/60001/site/dev/testing/tests.... site/dev/testing/tests.md:106: canvas->drawLine(r.nextRangeF(0.0f, 100.0f), Oh boy, this is also a bad idea. Those calls to nextRangeF() are not ordered.
https://codereview.chromium.org/1696723002/diff/60001/site/dev/testing/tests.md File site/dev/testing/tests.md (right): https://codereview.chromium.org/1696723002/diff/60001/site/dev/testing/tests.... site/dev/testing/tests.md:106: canvas->drawLine(r.nextRangeF(0.0f, 100.0f), On 2016/02/12 at 15:59:32, mtklein wrote: > Oh boy, this is also a bad idea. Those calls to nextRangeF() are not ordered. how's this?
On 2016/02/12 15:59:32, mtklein wrote: > https://codereview.chromium.org/1696723002/diff/60001/site/dev/testing/tests.md > File site/dev/testing/tests.md (right): > > https://codereview.chromium.org/1696723002/diff/60001/site/dev/testing/tests.... > site/dev/testing/tests.md:106: canvas->drawLine(r.nextRangeF(0.0f, 100.0f), > Oh boy, this is also a bad idea. Those calls to nextRangeF() are not ordered. If you want to see what I mean, try running this program, first compiled with clang, then compiled with GCC: #include <stdio.h> #include <stdlib.h> int main(void) { printf("%d %d\n", rand(), rand()); return 0; } $ clang -Weverything test.c $ ./a.out 16807 282475249 $ gcc-6 -Wall -Wextra test.c $ ./a.out 282475249 16807
lgtm
The CQ bit was checked by halcanary@google.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1696723002/80001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1696723002/80001
Message was sent while issue was closed.
Description was changed from ========== Documantation: nanobench DOCS_PREVIEW= https://skia.org/dev/testing/tests?cl=1696723002 https://skia.org/dev/testing/tests?cl=1696723002#bench GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... NOTRY=true ========== to ========== Documantation: nanobench DOCS_PREVIEW= https://skia.org/dev/testing/tests?cl=1696723002 https://skia.org/dev/testing/tests?cl=1696723002#bench GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... NOTRY=true Committed: https://skia.googlesource.com/skia/+/6f96cf4bde38337724b492a00dd8e0857d84d5d6 ==========
Message was sent while issue was closed.
Committed patchset #5 (id:80001) as https://skia.googlesource.com/skia/+/6f96cf4bde38337724b492a00dd8e0857d84d5d6 |
