Created attachment 140605 [details] Patch

Comment on attachment 140605 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=140605&action=review OK. So does this get us back the whole speed loss? > Source/WebCore/platform/FractionalLayoutUnit.h:324 > +#if ENABLE(SUBPIXEL_LAYOUT) Presumably there is still a regression for ports with this enabled?

Comment on attachment 140605 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=140605&action=review >> Source/WebCore/platform/FractionalLayoutUnit.h:324 >> +#if ENABLE(SUBPIXEL_LAYOUT) > > Presumably there is still a regression for ports with this enabled? Also, this multiply is not very bounded anymore... :)

Comment on attachment 140605 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=140605&action=review It should get us back most of the lost performance. I'll post numbers before committing. >>> Source/WebCore/platform/FractionalLayoutUnit.h:324 >>> +#if ENABLE(SUBPIXEL_LAYOUT) >> >> Presumably there is still a regression for ports with this enabled? > > Also, this multiply is not very bounded anymore... :) Yes, we plan to address that separately before turning on the flag. We have a couple of ideas but need some more time validating them.

Comment on attachment 140605 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=140605&action=review > Source/WebCore/ChangeLog:19 > + Disable the use of 64bit (long long) math ni the case where the fraction Nit: ni.

For floats_50_100_nested and floats_20_100_nested this patch seems to get back the entire speed loss. For floats_2_100_nested on the other hand we get back about 60%.

Unless anyone objects I plan to commit this and see what our perf bots thinks. If it fails to adequately address the regression I'll roll out the LayoutUnit change.

Created attachment 140613 [details] Patch for landing

Comment on attachment 140613 [details] Patch for landing Clearing flags on attachment: 140613 Committed r116392: <http://trac.webkit.org/changeset/116392>

All reviewed patches have been landed. Closing bug.

Unfortunately, this doesn't seem to have worked. :(

Comment on attachment 140613 [details] Patch for landing View in context: https://bugs.webkit.org/attachment.cgi?id=140613&action=review > Source/WebCore/ChangeLog:11 > + Fix performance regression caused by r116009 by disabling the use of > + 64bit math in FractionalLayoutUnit, simplifying the pixelSnappedMaxX/Y > + math, inlining a couple of methods and replacing the literal 0 (zero) > + with ZERO_LAYOUT_UNIT. Did you verify that the 0 -> ZERO_LAYOUT_UNIT change actually affected performance? I find that highly dubious, so I want to be sure it actually happened!

Created attachment 140820 [details] Patch Another try at addressing the performance regressions after 116009.

Attachment 140820 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1 Source/WebCore/platform/graphics/FractionalLayoutRect.h:187: Place brace on its own line for function definitions. [whitespace/braces] [4] Total errors found: 1 in 4 files If any of these errors are false positives, please file a bug against check-webkit-style.

(In reply to comment #14) > Attachment 140820 [details] did not pass style-queue: > > Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1 > Source/WebCore/platform/graphics/FractionalLayoutRect.h:187: Place brace on its own line for function definitions. [whitespace/braces] [4] > Total errors found: 1 in 4 files > > > If any of these errors are false positives, please file a bug against check-webkit-style. Thanks for the review, Ojan. I'll fix the style error and am waiting for the EWS bots to validate since I mucked with build files.

Comment on attachment 140820 [details] Patch Attachment 140820 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/12652527

<rdar://problem/11411562>

Comment on attachment 140820 [details] Patch Attachment 140820 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/12651479

Comment on attachment 140820 [details] Patch Attachment 140820 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/12647495

Comment on attachment 140820 [details] Patch Attachment 140820 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/12656470

Created attachment 140972 [details] Patch for landing

Attachment 140972 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/CMakeLists.txt', u'Source/W..." exit_code: 1 Source/WebCore/ChangeLog:18: Need whitespace between colon and description [changelog/filechangedescriptionwhitespace] [5] Total errors found: 1 in 4 files If any of these errors are false positives, please file a bug against check-webkit-style.

Created attachment 140985 [details] Patch for landing

Comment on attachment 140985 [details] Patch for landing Clearing flags on attachment: 140985 Committed r116549: <http://trac.webkit.org/changeset/116549>

All reviewed patches have been landed. Closing bug.

(In reply to comment #25) > All reviewed patches have been landed. Closing bug. I'll be monitoring the perf bots and if we don't see improvement, I'll roll back 116009 pending further investigation.