Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1936153002:
Modify LineBench for drawing straight line (Closed)
DescriptionModify LineBench for drawing
Currently we only have benchmark for lines that contains randomly generated
points. This CL modifies the benchmark which adds cases of drawing straight
lines. Also, this CL changes the call from drawPoints() to drawLines()
which measures the performance of line drawing.
BUG=skia:5243
GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1936153002
Committed: https://skia.googlesource.com/skia/+/6b27a5e7292d9a18e376f0c229ec62f7b801305a
Patch Set 1 #
Total comments: 2
Patch Set 2 : change to straightline, and fix compilation error #Patch Set 3 : add change to lineBench #Messages
Total messages: 19 (6 generated)
Description was changed from ========== Add a benchmark for SkCanvas::drawLine() for straight line Currently we only have benchmark for lines that contains randomly generated points. This CL adds a benchmark for drawing straight lines. BUG=skia:5243 ========== to ========== Add a benchmark for SkCanvas::drawLine() for straight line Currently we only have benchmark for lines that contains randomly generated points. This CL adds a benchmark for drawing straight lines. BUG=skia:5243 GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ==========
xidachen@chromium.org changed reviewers: + bsalomon@chromium.org, reed@google.com, robertphillips@chromium.org
PTAL
robertphillips@google.com changed reviewers: + robertphillips@google.com
https://codereview.chromium.org/1936153002/diff/1/bench/StraightLineBench.cpp File bench/StraightLineBench.cpp (right): https://codereview.chromium.org/1936153002/diff/1/bench/StraightLineBench.cpp... bench/StraightLineBench.cpp:32: fDoAA = doAA; I think you want this to be: straightline_%g_%s
https://codereview.chromium.org/1936153002/diff/1/bench/StraightLineBench.cpp File bench/StraightLineBench.cpp (right): https://codereview.chromium.org/1936153002/diff/1/bench/StraightLineBench.cpp... bench/StraightLineBench.cpp:32: fDoAA = doAA; On 2016/05/02 15:33:40, robertphillips wrote: > I think you want this to be: straightline_%g_%s Thank you, changed to straightline, and compilation error is also fixed.
Is there a way to share code with LineBench.cpp, maybe just pass a bool/enum to the constructor to indicated H or V (or Diagonal) lines?
Description was changed from ========== Add a benchmark for SkCanvas::drawLine() for straight line Currently we only have benchmark for lines that contains randomly generated points. This CL adds a benchmark for drawing straight lines. BUG=skia:5243 GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ========== to ========== Modify LineBench for drawing Currently we only have benchmark for lines that contains randomly generated points. This CL modifies the benchmark which adds cases of drawing straight lines. Also, this CL changes the call from drawPoints() to drawLines() which measures the performance of line drawing. BUG=skia:5243 GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ==========
I have made the change so that this CL now change the LineBench.cpp only. I run the nanobench after this change, and I realized that the case of drawing a straight line has already been optimized. For example, drawing a line of width=10, it takes 22.7us for a random line, while only 6.49us for a vertically straight line, and 3.23us for a horizontally straight line.
What was the speed diff you saw between a horizontal line and a rect?
On 2016/05/02 17:31:16, reed1 wrote: > What was the speed diff you saw between a horizontal line and a rect? What I did is like this: 1. call drawLine() to draw a vertically straight line with line_width=10, AA, config=8888, average mean is 3.59us 2. Change the bench to call drawRect() to draw a rect that is equivalent to the above line, AA config=8888, average mean is 2.19us The diff seems to be quite significant.
> 1. call drawLine() to draw a vertically straight line with line_width=10, AA, > config=8888, average mean is 3.59us Sorry, here I meant horizontally straight line.
Gentle ping. Could we land this patch? I have some code that specifically optimize the drawing of straight lines, and locally the result look like this: Without optimization: 8.14µs 8.22µs 8.36µs 9.89µs 6% █▂▁▁▁▁▁▁▁▁ 565 lines_10_AA_SV 11.5µs 11.6µs 11.7µs 12.3µs 2% █▁▁▂▁▇▁▁▁▁ 8888 lines_10_AA_SV 4.43µs 4.49µs 7.84µs 36.2µs 127% █▁▁▁▁▁▁▁▁▁ gpu lines_10_AA_SV 3.49µs 3.52µs 3.57µs 3.95µs 4% █▂▂▂▁▁▁▁▁▁ 565 lines_10_AA_SH 4.02µs 4.05µs 4.06µs 4.17µs 1% █▇▃▃▄▂▁▂▂▁ 8888 lines_10_AA_SH 3.83µs 3.87µs 3.88µs 3.98µs 1% ▇█▂▁▂▁▃▅▆▂ gpu lines_10_AA_SH With optimization: 2.59µs 2.61µs 2.8µs 4.3µs 19% ▂▁█▁▁▁▁▁▁▁ 565 lines_10_AA_SV 5.68µs 5.7µs 5.7µs 5.74µs 0% █▄▂▂▂▁▄▁▄▄ 8888 lines_10_AA_SV 2.54µs 2.74µs 4.75µs 12.2µs 75% ▁▁▁█▄▁▁▆▁▁ gpu lines_10_AA_SV 1.23µs 1.24µs 1.26µs 1.37µs 4% █▄▂▁▁▁▁▁▁▁ 565 lines_10_AA_SH 1.77µs 1.83µs 1.82µs 1.85µs 2% █▆▇▆▆▆▅▁▁▂ 8888 lines_10_AA_SH 1.13µs 1.13µs 1.13µs 1.13µs 0% ▁▆█▂▆▇▃▂█▄ gpu lines_10_AA_SH
lgtm
The CQ bit was checked by xidachen@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1936153002/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1936153002/40001
Message was sent while issue was closed.
Description was changed from ========== Modify LineBench for drawing Currently we only have benchmark for lines that contains randomly generated points. This CL modifies the benchmark which adds cases of drawing straight lines. Also, this CL changes the call from drawPoints() to drawLines() which measures the performance of line drawing. BUG=skia:5243 GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ========== to ========== Modify LineBench for drawing Currently we only have benchmark for lines that contains randomly generated points. This CL modifies the benchmark which adds cases of drawing straight lines. Also, this CL changes the call from drawPoints() to drawLines() which measures the performance of line drawing. BUG=skia:5243 GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... Committed: https://skia.googlesource.com/skia/+/6b27a5e7292d9a18e376f0c229ec62f7b801305a ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as https://skia.googlesource.com/skia/+/6b27a5e7292d9a18e376f0c229ec62f7b801305a
Message was sent while issue was closed.
A revert of this CL (patchset #3 id:40001) has been created in https://codereview.chromium.org/1952063005/ by reed@google.com. The reason for reverting is: for (int i = 0; i < loops; i++) { 54 canvas->drawPoints(SkCanvas::kLines_PointMode, PTS, fPts, paint); 68 canvas->drawLine(fStartPts[i].x(), fStartPts[i].y(), fEndPts[i].x(), fEndPts[i].y(), pai nt); This change means we index arbitrarily far into fStartPts (if loops gets big enough).. |
