Bug 250523 – Use shifts to speedup floor() and round()

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Ahmad Saleem]

Hi Team,

While going through Blink's commit, came across another potential performance optimization:

Blink Commit - https://chromium.googlesource.com/chromium/blink/+/143faa0607149758526318a98757a3055197cb0f

WebKit Commit - https://searchfox.org/wubkat/source/Source/WebCore/platform/LayoutUnit.h#61 & https://searchfox.org/wubkat/source/Source/WebCore/platform/LayoutUnit.h#161 & https://searchfox.org/wubkat/source/Source/WebCore/platform/LayoutUnit.h#170

Just wanted to raise it for input. Thanks!

[Radar WebKit Bug Importer]

<rdar://problem/104441420>

[Brent Fulgham]

Interesting. I wonder if this is a measurable performance improvement.

[Brent Fulgham]

I started an A/B test to see if it's measurable.

[Brent Fulgham]

Pull request: https://github.com/Webkit/WebKit/pull/8853

[Simon Fraser (smfr)]

Was the speedup confirmed?

[Brent Fulgham]

(In reply to Simon Fraser (smfr) from comment #5)
> Was the speedup confirmed?

A/B test is running now. I wanted to put the PR up to check correctness.

[Brent Fulgham]

I messed up the perf test, and am running them again now. :-(

[Brent Fulgham]

MotionMark has a small (0.18%) improvement, but is not statistically significant.

Waiting for PLT results.

[Brent Fulgham]

Performance A/B Results:

iOS 16 (iPad Pro)
* MotionMark: 0.18% better (insignificant)
* PLT5: 0.52% better (insignificant)

macOS 13, M1 MacBook Air
* MotionMark: 1.79% better (insignificant)
* PLT5: 0.23% better (insignificant)

macOS 13, Intel iMac
* MotionMark: 0.28% worse (insignificant)
* PLT5: 0.17% better (insignificant)

[Brent Fulgham]

There doesn't seem to be a measurable win on this change, so let's not add the complexity (and less-readable code).

[EWS]

Committed 260422@main (3fdb8cf70b8a): <https://commits.webkit.org/260422@main>

Reviewed commits have been landed. Closing PR #8853 and removing active labels.

[Alexey Proskuryakov]

Looks like we ended up landing this anyway. Should it be reverted then?

[Chris Dumez]

(In reply to Alexey Proskuryakov from comment #12)
> Looks like we ended up landing this anyway. Should it be reverted then?

Sorry, that's my bad. I hadn't seen the discussion on the bugzilla side.

I'll post micro-benchmark numbers today. The new code is simpler (less branching).
We can decide whether to revert or not then.

[zalan]

Not surprised we haven't seen any perf improvements on non-micro benchmarks as I  think these functions are not used too much anymore after going full-subpixel layout/rendering (other than some in some DOM api) and their usage should be trending down.

[Chris Dumez]

With 100000000 random values (ran 3 times each):

* floor:
Before: 112.52 ms
After: 79.46ms (29% faster)

* round:
Before: 192.19ms
After: 97.04ms (50% faster)