Bug 105216 - Optimize LayoutUnit::boundedMultiply
Summary: Optimize LayoutUnit::boundedMultiply
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Emil A Eklund
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-12-17 14:16 PST by Emil A Eklund
Modified: 2012-12-18 12:13 PST (History)
4 users (show)

See Also:

Attachments
Patch (2.42 KB, patch)
2012-12-17 14:18 PST, Emil A Eklund 		no flags Details | Formatted Diff | Diff
Patch (2.62 KB, patch)
2012-12-17 14:32 PST, Emil A Eklund 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Emil A Eklund 2012-12-17 14:16:23 PST 
LayoutUnit::boundedMultiply is used for multiplication that's prone to overflow and for all LayoutUnit multiplication if SATURATED_LAYOUT_ARITHMETIC is enabled. The current approach is quite inefficient.

Change it to use a more efficient saturated multiplication implementation.
Comment 1 Emil A Eklund 2012-12-17 14:18:45 PST 
Created attachment 179801 [details]
Patch
Comment 2 Levi Weintraub 2012-12-17 14:23:09 PST 
Comment on attachment 179801 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=179801&action=review

> Source/WebCore/platform/LayoutUnit.h:490
> +    if (high != low >> 31)

It wouldn't hurt to have a comment about the significance of this compare.

> Source/WebCore/platform/LayoutUnit.h:497
> +    // FIXME: Should be bounded even in the non-subpixel case.

Thank you :)
Comment 3 Emil A Eklund 2012-12-17 14:32:37 PST 
Created attachment 179803 [details]
Patch
Comment 4 Levi Weintraub 2012-12-17 14:36:41 PST 
Comment on attachment 179803 [details]
Patch

LGTM.
Comment 5 Emil A Eklund 2012-12-18 10:47:13 PST 
Committed r138041: <http://trac.webkit.org/changeset/138041>
Comment 6 Tim Horton 2012-12-18 11:44:01 PST 
Hopefully-ok follow up build fix: http://trac.webkit.org/changeset/138046.
Comment 7 Levi Weintraub 2012-12-18 12:13:33 PST 
(In reply to comment #6)
> Hopefully-ok follow up build fix: http://trac.webkit.org/changeset/138046.

Thanks Tim!