Bug 99121

Summary:

RenderBR should share its constant newline string between instances.

Product:

WebKit

Reporter:

Andreas Kling <kling>

Component:

Layout and Rendering

Assignee:

Andreas Kling <kling>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

eric, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
poch	none

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.