WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
23674
Speed up some things based on profiling the page load test
https://bugs.webkit.org/show_bug.cgi?id=23674
Summary
Speed up some things based on profiling the page load test
Darin Adler
Reported
2009-02-01 09:50:27 PST
The Safari team has a page load test that we use to measure performance. It's not open source because it contains materials that are copyrighted and we don't have the right to publish. I ran it recently and profiled it and found some things I could speed up simply.
Attachments
patch
(84.38 KB, patch)
2009-02-01 09:51 PST
,
Darin Adler
no flags
Details
Formatted Diff
Diff
patch
(84.38 KB, patch)
2009-02-01 10:15 PST
,
Darin Adler
oliver
: review-
Details
Formatted Diff
Diff
patch — this time without TextResourceDecoder or reserveInitialCapacity changes as requested
(60.57 KB, patch)
2009-02-01 19:03 PST
,
Darin Adler
hyatt
: review+
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
Darin Adler
Comment 1
2009-02-01 09:51:48 PST
Created
attachment 27231
[details]
patch
Darin Adler
Comment 2
2009-02-01 10:15:06 PST
Created
attachment 27233
[details]
patch First version was missing parentheses in one place and thus didn't compile.
David Levin
Comment 3
2009-02-01 11:46:05 PST
I did a quick look out of curiosity. Two small comments: In WebCore/loader/TextResourceDecoder.cpp if (dataStart[0] == '@' && dataStart[1] == 'c' && dataStart[2] == 'h' Indent appears off for the && lines. ---- In StringImpl::createStrippingNullCharacters int foundNull = 0; for (unsigned i = 0; !foundNull && i < length; i++) { int c = characters[i]; // more efficient than using UChar here (at least on Intel Mac OS) foundNull |= !c; } It could be "foundNull = !c;" (no bitwise or) No sure if this will matter but since it mentioned as hot and things like using int instead of UChar were considered, I thought I'd point it out.
Oliver Hunt
Comment 4
2009-02-01 12:27:33 PST
Comment on
attachment 27233
[details]
patch I am concerned that this patch seems to have a large amount of seemingly unrelated and non-local changes, i'm especially concerned about the inclusion of text decoder changes.. After some consideration i'd like this patch separated out into a few parts * reserveInitialCapacity addition/use * TextDecoder changes * All the inlining changes
Darin Adler
Comment 5
2009-02-01 17:43:48 PST
(In reply to
comment #4
)
> * reserveInitialCapacity addition/use > * TextDecoder changes > * All the inlining changes
Seems fine.
Darin Adler
Comment 6
2009-02-01 19:03:31 PST
Created
attachment 27242
[details]
patch — this time without TextResourceDecoder or reserveInitialCapacity changes as requested The reserveInitialCapacity changes are now
bug 23676
. The TextResourceDecoder changes are now
bug 23677
.
Darin Adler
Comment 7
2009-02-01 19:06:30 PST
(In reply to
comment #3
)
> In StringImpl::createStrippingNullCharacters > > int foundNull = 0; > for (unsigned i = 0; !foundNull && i < length; i++) { > int c = characters[i]; // more efficient than using UChar here (at > least on Intel Mac OS) > foundNull |= !c; > } > > It could be "foundNull = !c;" (no bitwise or)
Good point. I just moved the code, didn't change it, but it seems obviously less efficient to do the |= there.
Darin Adler
Comment 8
2009-02-01 19:20:52 PST
Ollie, please let me know if you'd like this broken into even more pieces. It's quite a bit of work, but I'd be happy to do it. I have a bunch more of these ideas that I'd like to tackle once these are landed.
Dave Hyatt
Comment 9
2009-02-01 20:10:31 PST
Comment on
attachment 27242
[details]
patch — this time without TextResourceDecoder or reserveInitialCapacity changes as requested r=me, although I don't really like "firstLineStyleSlowCase" as a function name, but I can't think of anything better.
Darin Adler
Comment 10
2009-02-02 09:13:59 PST
(In reply to
comment #9
)
> I don't really like "firstLineStyleSlowCase" as a function name, > but I can't think of anything better.
In an earlier version I called it something like computeFirstLineStyle.
Darin Adler
Comment 11
2009-02-02 09:32:06 PST
trac.webkit.org/changeset/40475
Darin Adler
Comment 12
2009-02-02 16:09:00 PST
http://trac.webkit.org/changeset/40475
Regressions fixed in:
http://trac.webkit.org/changeset/40484
http://trac.webkit.org/changeset/40499
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug