Bug 99121 - RenderBR should share its constant newline string between instances.
Summary: RenderBR should share its constant newline string between instances.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Andreas Kling
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-10-11 17:34 PDT by Andreas Kling
Modified: 2012-10-12 09:58 PDT (History)
2 users (show)

See Also:

Attachments
poch (1.18 KB, patch)
2012-10-11 17:35 PDT, Andreas Kling 		no flags Details | Formatted Diff | Diff
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 Andreas Kling 2012-10-11 17:34:32 PDT 
This one is a game changer.
Comment 1 Andreas Kling 2012-10-11 17:35:12 PDT 
Created attachment 168324 [details]
poch
Comment 2 Anders Carlsson 2012-10-11 17:37:40 PDT 
Comment on attachment 168324 [details]
poch

r=gooby.\n\n\n\n
Comment 3 Eric Seidel (no email) 2012-10-11 18:47:11 PDT 
Comment on attachment 168324 [details]
poch

Really?  Wow.  Amazing.  Why not put this in the AtomicString cache?  Do we think it won't appear elsewhere?
Comment 4 Eric Seidel (no email) 2012-10-11 18:48:06 PDT 
I suspect we have a lot of Text nodes which are just whitespace.  Possibly many who are just a '\n'.  I guess Text nodes don't use AtomicStrings though, so making this atomic wouldn't help them...
Comment 5 Andreas Kling 2012-10-12 09:52:56 PDT 
(In reply to comment #3)
> (From update of attachment 168324 [details])
> Really?  Wow.  Amazing.  Why not put this in the AtomicString cache?  Do we think it won't appear elsewhere?

This way we're paying 1 global String to avoid a hash lookup on every RenderBR creation.

I don't have any data suggesting this is a bottleneck, but it seemed like the right tradeoff to go for.
Comment 6 WebKit Review Bot 2012-10-12 09:58:51 PDT 
Comment on attachment 168324 [details]
poch

Clearing flags on attachment: 168324

Committed r131195: <http://trac.webkit.org/changeset/131195>
Comment 7 WebKit Review Bot 2012-10-12 09:58:54 PDT 
All reviewed patches have been landed.  Closing bug.