Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1653943002:
unroll srcover_1 for blending a single color (Closed)
Descriptionunroll srcover_1 for blending a single color
Before:
curr/maxrss loops min median mean max stddev samples config bench
8/8 MB 1 1.59ms 1.82ms 1.89ms 2.59ms 14% ▁█▃▃▃▃▃▃▃▃ nonrendering xfer4f_srcover_1_alpha_linear
8/8 MB 1 3.25ms 4.25ms 4.16ms 5.87ms 21% ▁▅▂▁▁▄█▄▅▂ nonrendering xfer4f_srcover_1_alpha_srgb
After:
curr/maxrss loops min median mean max stddev samples config bench
8/8 MB 1 915µs 915µs 946µs 1.02ms 4% █▄▇▁▁▁▆▁▁▁ nonrendering xfer4f_srcover_1_alpha_linear
8/8 MB 1 2.69ms 3.08ms 3.03ms 3.63ms 10% ▁▃▂▁▁█▄▄▄▆ nonrendering xfer4f_srcover_1_alpha_srgb
BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1653943002
Committed: https://skia.googlesource.com/skia/+/bf907e628fe245305d0f987aed8b8ecff8356374
Patch Set 1 #Patch Set 2 : #
Total comments: 9
Patch Set 3 : remove unneeded templateness, speed up linear a bit more (avoid scaling to unit on dst) #
Total comments: 2
Patch Set 4 : separate src_1 functions #Messages
Total messages: 38 (17 generated)
Description was changed from ========== unroll xfer1 procs BUG=skia: ========== to ========== unroll xfer1 procs BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ==========
Description was changed from ========== unroll xfer1 procs BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ========== to ========== unroll srcover_1 for blending a single color BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ==========
The CQ bit was checked by reed@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1653943002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1653943002/20001
reed@google.com changed reviewers: + herb@google.com, mtklein@google.com
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
good speedup? https://codereview.chromium.org/1653943002/diff/20001/src/core/SkXfermode4f.cpp File src/core/SkXfermode4f.cpp (right): https://codereview.chromium.org/1653943002/diff/20001/src/core/SkXfermode4f.c... src/core/SkXfermode4f.cpp:38: template <DstType D> Sk4f linear_to_dst_255(const Sk4f& l4) { Seems odd to template this on DstType if we're only calling it for D == kSRGB_Dst. https://codereview.chromium.org/1653943002/diff/20001/src/core/SkXfermode4f.c... src/core/SkXfermode4f.cpp:290: s4 = s4 * Sk4f(255) + Sk4f(0.5f); I find it really confusing to mutate values conditionally, especially when you need to remember that these changes happened in two places below, but not in two other places. It'd be easier to follow this if we give the new values new names, then use them only in the kLinear case: Sk4f src4_255 = s4 * Sk4f(255) + Sk4f(0.5f); Sk4f scale_255 = scale * Sk4f(255); ... if (D == kLinear_Dst) { Sk4f_ToBytes(..., src4_255 + d0 * scale_255, ... ); } else { Sk4f_ToBytes(..., linear_to_sRGB_4f(s4 + d0 * scale), ...); } The compiler will eliminate src4_255 and scale_255 completely in the sRGB case, so there's no need to be conditional around their computation unless it's so smart it starts whining that they're unused. https://codereview.chromium.org/1653943002/diff/20001/src/core/SkXfermode4f.c... src/core/SkXfermode4f.cpp:300: s4 + d0 * scale, s4 + d1 * scale, s4 + d2 * scale, s4 + d3 * scale); Might be nice to line-break after each comma here too so it's also 5 lines like the one below.
https://codereview.chromium.org/1653943002/diff/20001/src/core/SkXfermode4f.cpp File src/core/SkXfermode4f.cpp (right): https://codereview.chromium.org/1653943002/diff/20001/src/core/SkXfermode4f.c... src/core/SkXfermode4f.cpp:292: } I'm also curious to see if this hoisting can be done by the compiler if we just give it some parenthesis hints: Sk4f_ToBytes(..., (s4 * Sk4f(255) + Sk4f(0.5f)) + d0 * (scale * Sk4f(255)), (s4 * Sk4f(255) + Sk4f(0.5f)) + d1 * (scale * Sk4f(255)), (s4 * Sk4f(255) + Sk4f(0.5f)) + d2 * (scale * Sk4f(255)), (s4 * Sk4f(255) + Sk4f(0.5f)) + d3 * (scale * Sk4f(255)));
As I've tweaked the bench to make it more stable, I may land that separately so we can see the diff of this change. https://codereview.chromium.org/1653943002/diff/20001/src/core/SkXfermode4f.cpp File src/core/SkXfermode4f.cpp (right): https://codereview.chromium.org/1653943002/diff/20001/src/core/SkXfermode4f.c... src/core/SkXfermode4f.cpp:38: template <DstType D> Sk4f linear_to_dst_255(const Sk4f& l4) { On 2016/02/01 21:55:32, mtklein wrote: > Seems odd to template this on DstType if we're only calling it for D == > kSRGB_Dst. I had a bool, but it made reading profiles harder. This way the profile reads (e.g.) srcover<kSRGB> instead of srcover<true> https://codereview.chromium.org/1653943002/diff/20001/src/core/SkXfermode4f.c... src/core/SkXfermode4f.cpp:290: s4 = s4 * Sk4f(255) + Sk4f(0.5f); On 2016/02/01 21:55:32, mtklein wrote: > I find it really confusing to mutate values conditionally, especially when you > need to remember that these changes happened in two places below, but not in two > other places. > > It'd be easier to follow this if we give the new values new names, then use them > only in the kLinear case: > > Sk4f src4_255 = s4 * Sk4f(255) + Sk4f(0.5f); > Sk4f scale_255 = scale * Sk4f(255); > > ... > if (D == kLinear_Dst) { > Sk4f_ToBytes(..., src4_255 + d0 * scale_255, ... ); > } else { > Sk4f_ToBytes(..., linear_to_sRGB_4f(s4 + d0 * scale), ...); > } > > The compiler will eliminate src4_255 and scale_255 completely in the sRGB case, > so there's no need to be conditional around their computation unless it's so > smart it starts whining that they're unused. Agreed. I will give them meaningful names https://codereview.chromium.org/1653943002/diff/20001/src/core/SkXfermode4f.c... src/core/SkXfermode4f.cpp:300: s4 + d0 * scale, s4 + d1 * scale, s4 + d2 * scale, s4 + d3 * scale); On 2016/02/01 21:55:32, mtklein wrote: > Might be nice to line-break after each comma here too so it's also 5 lines like > the one below. Agreed.
https://codereview.chromium.org/1653943002/diff/20001/src/core/SkXfermode4f.cpp File src/core/SkXfermode4f.cpp (right): https://codereview.chromium.org/1653943002/diff/20001/src/core/SkXfermode4f.c... src/core/SkXfermode4f.cpp:38: template <DstType D> Sk4f linear_to_dst_255(const Sk4f& l4) { On 2016/02/02 14:04:39, reed1 wrote: > On 2016/02/01 21:55:32, mtklein wrote: > > Seems odd to template this on DstType if we're only calling it for D == > > kSRGB_Dst. > > I had a bool, but it made reading profiles harder. This way the profile reads > (e.g.) srcover<kSRGB> instead of srcover<true> No no, I mean, it's weird to template at all. Only the D == kSRGB_Dst path is being called.
https://codereview.chromium.org/1653943002/diff/20001/src/core/SkXfermode4f.cpp File src/core/SkXfermode4f.cpp (right): https://codereview.chromium.org/1653943002/diff/20001/src/core/SkXfermode4f.c... src/core/SkXfermode4f.cpp:38: template <DstType D> Sk4f linear_to_dst_255(const Sk4f& l4) { On 2016/02/02 14:26:06, mtklein wrote: > On 2016/02/02 14:04:39, reed1 wrote: > > On 2016/02/01 21:55:32, mtklein wrote: > > > Seems odd to template this on DstType if we're only calling it for D == > > > kSRGB_Dst. > > > > I had a bool, but it made reading profiles harder. This way the profile reads > > (e.g.) srcover<kSRGB> instead of srcover<true> > > No no, I mean, it's weird to template at all. Only the D == kSRGB_Dst path is > being called. Ah. It *was* being used more generically, until I did more unrolling and specializing. I think I kept it as a preemptive attempt to (eventually) create a std vocabulary of transformers for this space: - loading from either linear or srgb bytes into linear floats, w/ or w/o normalizing to 0...1 - storing linear floats to either linear or srgb bytes, w/ or w/o scaling+rounding If it is hard to read, I can remove and re-add later if it comes up again.
Description was changed from ========== unroll srcover_1 for blending a single color BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ========== to ========== unroll srcover_1 for blending a single color Before: curr/maxrss loops min median mean max stddev samples config bench 8/8 MB 1 833µs 836µs 837µs 843µs 1% ▁▇▇▁▁█▃▁▁█ nonrendering xfer4f_srcover_N_opaque_linear 8/8 MB 1 2.15ms 2.15ms 2.34ms 3.34ms 16% ▂▁▁▁▁█▄▁▁▁ nonrendering xfer4f_srcover_N_opaque_srgb 8/8 MB 1 1.93ms 2.22ms 2.3ms 2.91ms 11% ▁█▄▃▃▃▃▃▅▄ nonrendering xfer4f_srcover_N_alpha_linear 8/8 MB 1 3.55ms 4.07ms 4.12ms 5.44ms 15% ▃▃█▄▁▁▅▃▁▁ nonrendering xfer4f_srcover_N_alpha_srgb 8/8 MB 3 82.7µs 101µs 99.9µs 140µs 18% ▂▂▁█▃▁▁▄▄▄ nonrendering xfer4f_srcover_1_opaque_linear 8/8 MB 3 86.1µs 115µs 115µs 146µs 16% ▆▄▃▂▅▇█▄▄▁ nonrendering xfer4f_srcover_1_opaque_srgb 8/8 MB 1 1.59ms 1.82ms 1.89ms 2.59ms 14% ▁█▃▃▃▃▃▃▃▃ nonrendering xfer4f_srcover_1_alpha_linear 8/8 MB 1 3.25ms 4.25ms 4.16ms 5.87ms 21% ▁▅▂▁▁▄█▄▅▂ nonrendering xfer4f_srcover_1_alpha_srgb BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ==========
Description was changed from ========== unroll srcover_1 for blending a single color Before: curr/maxrss loops min median mean max stddev samples config bench 8/8 MB 1 833µs 836µs 837µs 843µs 1% ▁▇▇▁▁█▃▁▁█ nonrendering xfer4f_srcover_N_opaque_linear 8/8 MB 1 2.15ms 2.15ms 2.34ms 3.34ms 16% ▂▁▁▁▁█▄▁▁▁ nonrendering xfer4f_srcover_N_opaque_srgb 8/8 MB 1 1.93ms 2.22ms 2.3ms 2.91ms 11% ▁█▄▃▃▃▃▃▅▄ nonrendering xfer4f_srcover_N_alpha_linear 8/8 MB 1 3.55ms 4.07ms 4.12ms 5.44ms 15% ▃▃█▄▁▁▅▃▁▁ nonrendering xfer4f_srcover_N_alpha_srgb 8/8 MB 3 82.7µs 101µs 99.9µs 140µs 18% ▂▂▁█▃▁▁▄▄▄ nonrendering xfer4f_srcover_1_opaque_linear 8/8 MB 3 86.1µs 115µs 115µs 146µs 16% ▆▄▃▂▅▇█▄▄▁ nonrendering xfer4f_srcover_1_opaque_srgb 8/8 MB 1 1.59ms 1.82ms 1.89ms 2.59ms 14% ▁█▃▃▃▃▃▃▃▃ nonrendering xfer4f_srcover_1_alpha_linear 8/8 MB 1 3.25ms 4.25ms 4.16ms 5.87ms 21% ▁▅▂▁▁▄█▄▅▂ nonrendering xfer4f_srcover_1_alpha_srgb BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ========== to ========== unroll srcover_1 for blending a single color Before: curr/maxrss loops min median mean max stddev samples config bench 8/8 MB 1 1.59ms 1.82ms 1.89ms 2.59ms 14% ▁█▃▃▃▃▃▃▃▃ nonrendering xfer4f_srcover_1_alpha_linear 8/8 MB 1 3.25ms 4.25ms 4.16ms 5.87ms 21% ▁▅▂▁▁▄█▄▅▂ nonrendering xfer4f_srcover_1_alpha_srgb After: curr/maxrss loops min median mean max stddev samples config bench 8/8 MB 1 1.09ms 1.09ms 1.11ms 1.23ms 4% ▂▁▄▁▁▁▁▁▁█ nonrendering xfer4f_srcover_1_alpha_linear 8/8 MB 1 2.7ms 3.08ms 3ms 3.37ms 8% ▆▁▃▁█▆▅▅▆▁ nonrendering xfer4f_srcover_1_alpha_srgb BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ==========
Description was changed from ========== unroll srcover_1 for blending a single color Before: curr/maxrss loops min median mean max stddev samples config bench 8/8 MB 1 1.59ms 1.82ms 1.89ms 2.59ms 14% ▁█▃▃▃▃▃▃▃▃ nonrendering xfer4f_srcover_1_alpha_linear 8/8 MB 1 3.25ms 4.25ms 4.16ms 5.87ms 21% ▁▅▂▁▁▄█▄▅▂ nonrendering xfer4f_srcover_1_alpha_srgb After: curr/maxrss loops min median mean max stddev samples config bench 8/8 MB 1 1.09ms 1.09ms 1.11ms 1.23ms 4% ▂▁▄▁▁▁▁▁▁█ nonrendering xfer4f_srcover_1_alpha_linear 8/8 MB 1 2.7ms 3.08ms 3ms 3.37ms 8% ▆▁▃▁█▆▅▅▆▁ nonrendering xfer4f_srcover_1_alpha_srgb BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ========== to ========== unroll srcover_1 for blending a single color Before: curr/maxrss loops min median mean max stddev samples config bench 8/8 MB 1 1.59ms 1.82ms 1.89ms 2.59ms 14% ▁█▃▃▃▃▃▃▃▃ nonrendering xfer4f_srcover_1_alpha_linear 8/8 MB 1 3.25ms 4.25ms 4.16ms 5.87ms 21% ▁▅▂▁▁▄█▄▅▂ nonrendering xfer4f_srcover_1_alpha_srgb After: curr/maxrss loops min median mean max stddev samples config bench 8/8 MB 1 915µs 1.06ms 1.18ms 1.73ms 23% ▂▂▂▆▂▁▁█▂▅ nonrendering xfer4f_srcover_1_alpha_linear 8/8 MB 1 2.7ms 3.08ms 3ms 3.37ms 8% ▆▁▃▁█▆▅▅▆▁ nonrendering xfer4f_srcover_1_alpha_srgb BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ==========
Description was changed from ========== unroll srcover_1 for blending a single color Before: curr/maxrss loops min median mean max stddev samples config bench 8/8 MB 1 1.59ms 1.82ms 1.89ms 2.59ms 14% ▁█▃▃▃▃▃▃▃▃ nonrendering xfer4f_srcover_1_alpha_linear 8/8 MB 1 3.25ms 4.25ms 4.16ms 5.87ms 21% ▁▅▂▁▁▄█▄▅▂ nonrendering xfer4f_srcover_1_alpha_srgb After: curr/maxrss loops min median mean max stddev samples config bench 8/8 MB 1 915µs 1.06ms 1.18ms 1.73ms 23% ▂▂▂▆▂▁▁█▂▅ nonrendering xfer4f_srcover_1_alpha_linear 8/8 MB 1 2.7ms 3.08ms 3ms 3.37ms 8% ▆▁▃▁█▆▅▅▆▁ nonrendering xfer4f_srcover_1_alpha_srgb BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ========== to ========== unroll srcover_1 for blending a single color Before: curr/maxrss loops min median mean max stddev samples config bench 8/8 MB 1 1.59ms 1.82ms 1.89ms 2.59ms 14% ▁█▃▃▃▃▃▃▃▃ nonrendering xfer4f_srcover_1_alpha_linear 8/8 MB 1 3.25ms 4.25ms 4.16ms 5.87ms 21% ▁▅▂▁▁▄█▄▅▂ nonrendering xfer4f_srcover_1_alpha_srgb After: curr/maxrss loops min median mean max stddev samples config bench 8/8 MB 1 915µs 1.03ms 1.06ms 1.69ms 23% █▃▂▂▁▁▁▁▂▁ nonrendering xfer4f_srcover_1_alpha_linear 8/8 MB 1 2.73ms 3.08ms 3.13ms 3.76ms 11% ▁▇▂▁█▄▃▃▃▄ nonrendering xfer4f_srcover_1_alpha_srgb BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ==========
The CQ bit was checked by reed@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1653943002/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1653943002/40001
refactor linear -vs- srgb code more, removing (as noted by reviewer) one-side template helper, and speeding up linear a bit more. ptal
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
https://codereview.chromium.org/1653943002/diff/40001/src/core/SkXfermode4f.cpp File src/core/SkXfermode4f.cpp (left): https://codereview.chromium.org/1653943002/diff/40001/src/core/SkXfermode4f.c... src/core/SkXfermode4f.cpp:257: template <DstType D> void srcover_1(const SkXfermode::PM4fState& state, uint32_t dst[], At this point I think it'd be clearer to split srcover_1 apart into two explicit specializations: template <DstType D> void srcover_1(const SkXfermode::PM4fState& state, uint32_t dst[], const SkPM4f& src, int count, const SkAlpha aa[]); template <> void srcover_1<kLinear_Dst>(const SkXfermode::PM4fState& state, uint32_t dst[], const SkPM4f& src, int count, const SkAlpha aa[]) { ... } template <> void srcover_1<kSRGB_Dst>(const SkXfermode::PM4fState& state, uint32_t dst[], const SkPM4f& src, int count, const SkAlpha aa[]) { ... }
https://codereview.chromium.org/1653943002/diff/40001/src/core/SkXfermode4f.cpp File src/core/SkXfermode4f.cpp (left): https://codereview.chromium.org/1653943002/diff/40001/src/core/SkXfermode4f.c... src/core/SkXfermode4f.cpp:257: template <DstType D> void srcover_1(const SkXfermode::PM4fState& state, uint32_t dst[], On 2016/02/02 18:10:08, mtklein wrote: > At this point I think it'd be clearer to split srcover_1 apart into two explicit > specializations: > > template <DstType D> > void srcover_1(const SkXfermode::PM4fState& state, uint32_t dst[], const SkPM4f& > src, int count, const SkAlpha aa[]); > > template <> > void srcover_1<kLinear_Dst>(const SkXfermode::PM4fState& state, uint32_t dst[], > const SkPM4f& src, int count, const SkAlpha aa[]) { > ... > } > > template <> > void srcover_1<kSRGB_Dst>(const SkXfermode::PM4fState& state, uint32_t dst[], > const SkPM4f& src, int count, const SkAlpha aa[]) { > ... > } (otherwise lgtm)
I agree the function is 1/2 split already, but the AA section is still shared, and that isn't selectable on anything a compile-time (since it's a runtime null-check. I'll try typing it and see if we will end up with having to copy/paste that AA section or not.
Description was changed from ========== unroll srcover_1 for blending a single color Before: curr/maxrss loops min median mean max stddev samples config bench 8/8 MB 1 1.59ms 1.82ms 1.89ms 2.59ms 14% ▁█▃▃▃▃▃▃▃▃ nonrendering xfer4f_srcover_1_alpha_linear 8/8 MB 1 3.25ms 4.25ms 4.16ms 5.87ms 21% ▁▅▂▁▁▄█▄▅▂ nonrendering xfer4f_srcover_1_alpha_srgb After: curr/maxrss loops min median mean max stddev samples config bench 8/8 MB 1 915µs 1.03ms 1.06ms 1.69ms 23% █▃▂▂▁▁▁▁▂▁ nonrendering xfer4f_srcover_1_alpha_linear 8/8 MB 1 2.73ms 3.08ms 3.13ms 3.76ms 11% ▁▇▂▁█▄▃▃▃▄ nonrendering xfer4f_srcover_1_alpha_srgb BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ========== to ========== unroll srcover_1 for blending a single color Before: curr/maxrss loops min median mean max stddev samples config bench 8/8 MB 1 1.59ms 1.82ms 1.89ms 2.59ms 14% ▁█▃▃▃▃▃▃▃▃ nonrendering xfer4f_srcover_1_alpha_linear 8/8 MB 1 3.25ms 4.25ms 4.16ms 5.87ms 21% ▁▅▂▁▁▄█▄▅▂ nonrendering xfer4f_srcover_1_alpha_srgb After: curr/maxrss loops min median mean max stddev samples config bench 8/8 MB 1 915µs 915µs 946µs 1.02ms 4% █▄▇▁▁▁▆▁▁▁ nonrendering xfer4f_srcover_1_alpha_linear 8/8 MB 1 2.69ms 3.08ms 3.03ms 3.63ms 10% ▁▃▂▁▁█▄▄▄▆ nonrendering xfer4f_srcover_1_alpha_srgb BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ==========
The CQ bit was checked by reed@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1653943002/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1653943002/60001
is this better?
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
will land w/ previous review. separately writing tests for aa case, and I think there are already bugs, so I'll defer decisions about optimizing/code-share for that until I first make it correct :)
The CQ bit was checked by reed@google.com
The patchset sent to the CQ was uploaded after l-g-t-m from mtklein@google.com Link to the patchset: https://codereview.chromium.org/1653943002/#ps60001 (title: "separate src_1 functions")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1653943002/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1653943002/60001
Message was sent while issue was closed.
Description was changed from ========== unroll srcover_1 for blending a single color Before: curr/maxrss loops min median mean max stddev samples config bench 8/8 MB 1 1.59ms 1.82ms 1.89ms 2.59ms 14% ▁█▃▃▃▃▃▃▃▃ nonrendering xfer4f_srcover_1_alpha_linear 8/8 MB 1 3.25ms 4.25ms 4.16ms 5.87ms 21% ▁▅▂▁▁▄█▄▅▂ nonrendering xfer4f_srcover_1_alpha_srgb After: curr/maxrss loops min median mean max stddev samples config bench 8/8 MB 1 915µs 915µs 946µs 1.02ms 4% █▄▇▁▁▁▆▁▁▁ nonrendering xfer4f_srcover_1_alpha_linear 8/8 MB 1 2.69ms 3.08ms 3.03ms 3.63ms 10% ▁▃▂▁▁█▄▄▄▆ nonrendering xfer4f_srcover_1_alpha_srgb BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ========== to ========== unroll srcover_1 for blending a single color Before: curr/maxrss loops min median mean max stddev samples config bench 8/8 MB 1 1.59ms 1.82ms 1.89ms 2.59ms 14% ▁█▃▃▃▃▃▃▃▃ nonrendering xfer4f_srcover_1_alpha_linear 8/8 MB 1 3.25ms 4.25ms 4.16ms 5.87ms 21% ▁▅▂▁▁▄█▄▅▂ nonrendering xfer4f_srcover_1_alpha_srgb After: curr/maxrss loops min median mean max stddev samples config bench 8/8 MB 1 915µs 915µs 946µs 1.02ms 4% █▄▇▁▁▁▆▁▁▁ nonrendering xfer4f_srcover_1_alpha_linear 8/8 MB 1 2.69ms 3.08ms 3.03ms 3.63ms 10% ▁▃▂▁▁█▄▄▄▆ nonrendering xfer4f_srcover_1_alpha_srgb BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... Committed: https://skia.googlesource.com/skia/+/bf907e628fe245305d0f987aed8b8ecff8356374 ==========
Message was sent while issue was closed.
Committed patchset #4 (id:60001) as https://skia.googlesource.com/skia/+/bf907e628fe245305d0f987aed8b8ecff8356374
Message was sent while issue was closed.
sounds good |
