Bug 33876

Summary: [Qt] Shortcut for Font::floatWidthForComplexText
Product: WebKit Reporter: Holger Freyther <zecke>
Component: WebKit QtAssignee: Holger Freyther <zecke>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, eric, laszlo.gombos, skyul
Priority: P2 Keywords: Qt
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
Bug Depends on:    
Bug Blocks: 34208    
Attachments:
Description Flags
First patch ariya.hidayat: review+

Holger Freyther
Reported 2010-01-19 16:52:52 PST
For text which only consists of one charachter we can avoid using harfbuzz and try QFontMetrics::width instead. In the case of loading google.com/news (english version) 23% of all calls to the floatWidthForComplexText is from a space. It is also true for other sites with less links. I'm currently doing a reduction and looking into the possible performance gain.
Attachments
First patch (2.39 KB, patch)
2010-01-28 04:29 PST, Holger Freyther 		ariya.hidayat: review+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Holger Freyther
Comment 1 2010-01-28 04:29:01 PST
Created attachment 47605 [details] First patch Only ' ', '\t' has the benefit of being fast with QFontMetrics::width. Any other charachter might or might not have different rounding and about the same speed as harfbuzz. This is a significant improvement on the pageloading test.
Benjamin Poulain
Comment 2 2010-01-29 02:29:07 PST
Here are the results of the painting benchmark on a regular N900: Before the patch: benchmark: http://en.wikipedia.org/wiki/Main_Page mean: 30 msecs +/- 1 msecs, +/- 3.333333 % avg: 30 msecs 31, 30, 30, 30, 31, 30, 30, 32, 30, 32, benchmark: http://en.wikipedia.org/wiki/Nokia mean: 244 msecs +/- 1 msecs, +/- 0.409836 % avg: 243 msecs 244, 243, 242, 245, 244, 246, 242, 242, 244, 244, benchmark: http://buzz.blogger.com/ mean: 182 msecs +/- 1 msecs, +/- 0.549451 % avg: 181 msecs 181, 182, 181, 181, 179, 183, 181, 182, 182, 182, benchmark: http://googleblog.blogspot.com/ mean: 82 msecs +/- 0 msecs, +/- 0.000000 % avg: 81 msecs 81, 81, 81, 82, 82, 81, 82, 80, 82, 82, benchmark: http://news.163.com/ mean: 299 msecs +/- 1 msecs, +/- 0.334448 % avg: 299 msecs 299, 301, 300, 299, 299, 298, 300, 301, 299, 299, benchmark: http://www.imdb.com/title/tt0468569/ mean: 49 msecs +/- 0 msecs, +/- 0.000000 % avg: 49 msecs 49, 48, 50, 50, 48, 49, 50, 50, 49, 49, benchmark: http://www.flickr.com/photos/rocio-ponce/3209550712/ mean: 54 msecs +/- 0 msecs, +/- 0.000000 % avg: 53 msecs 54, 54, 53, 52, 54, 53, 55, 54, 54, 55, benchmark: http://www.amazon.com/Kindle-Wireless-Reading-Display-Generation/dp/B0015TG12Q/ mean: 296 msecs +/- 2 msecs, +/- 0.675676 % avg: 294 msecs 296, 294, 292, 296, 299, 292, 292, 294, 297, 296, With the page: benchmark: http://en.wikipedia.org/wiki/Main_Page mean: 30 msecs +/- 0 msecs, +/- 0.000000 % avg: 30 msecs 31, 30, 31, 30, 31, 30, 30, 30, 31, 30, benchmark: http://en.wikipedia.org/wiki/Nokia mean: 242 msecs +/- 3 msecs, +/- 1.239669 % avg: 242 msecs 241, 241, 242, 251, 241, 243, 242, 242, 242, 240, benchmark: http://buzz.blogger.com/ mean: 181 msecs +/- 0 msecs, +/- 0.000000 % avg: 180 msecs 181, 180, 181, 180, 181, 179, 181, 181, 181, 182, benchmark: http://googleblog.blogspot.com/ mean: 81 msecs +/- 0 msecs, +/- 0.000000 % avg: 81 msecs 81, 81, 81, 81, 80, 81, 83, 80, 81, 81, benchmark: http://news.163.com/ mean: 297 msecs +/- 0 msecs, +/- 0.000000 % avg: 296 msecs 297, 297, 298, 297, 297, 296, 297, 297, 296, 297, benchmark: http://www.imdb.com/title/tt0468569/ mean: 49 msecs +/- 0 msecs, +/- 0.000000 % avg: 48 msecs 49, 49, 49, 49, 49, 48, 48, 49, 49, 49, benchmark: http://www.flickr.com/photos/rocio-ponce/3209550712/ mean: 53 msecs +/- 0 msecs, +/- 0.000000 % avg: 53 msecs 53, 53, 52, 54, 53, 53, 54, 53, 52, 53, benchmark: http://www.amazon.com/Kindle-Wireless-Reading-Display-Generation/dp/B0015TG12Q/ mean: 292 msecs +/- 1 msecs, +/- 0.342466 % avg: 292 msecs 295, 291, 292, 291, 294, 292, 292, 294, 294, 290, Good job Holger.
Holger Freyther
Comment 3 2010-01-29 02:37:34 PST
Oh, even a small win on painting. You would see a bigger win on the tst_cycler benchmark as the WebCore::Font method is called more often there.
Ariya Hidayat
Comment 4 2010-01-31 15:52:31 PST
Comment on attachment 47605 [details] First patch LGTM.
Eric Seidel (no email)
Comment 5 2010-02-01 16:12:36 PST
Attachment 47605 [details] was posted by a committer and has review+, assigning to Holger Freyther for commit.
Laszlo Gombos
Comment 6 2010-02-22 21:44:24 PST
Ping; Holger can you mark this cq+, if you think it is ready ? Thanks.
Holger Freyther
Comment 7 2010-03-02 02:19:54 PST
Landed in r55406. We will have to see how it behaves on all the fonts out there..
Note You need to log in before you can comment on or make changes to this bug.