Shrink RenderQuote.o by 134K

Shrink RenderQuote.o by 134K

This CL merges http://trac.webkit.org/changeset/149833, which shrinks
RenderQuote.o by 134K (from 211K to 77K) on Linux.

[Nico, 2013-05-13 20:10:05]

lgtm stamp

(is this with symbols or without?)

[abarth-chromium, 2013-05-13 20:13:26]

On 2013/05/13 20:10:05, Nico wrote:
> lgtm stamp
> 
> (is this with symbols or without?)

Whatever ninja -C ../../out/Release all_webkit builds by default on Linux.  If
there's something else I should be optimizing, please let me know.

[commit-bot: I haz the power, 2013-05-13 20:13:38]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/abarth@chromium.org/15093007/1

[Nico, 2013-05-13 20:14:27]

On Mon, May 13, 2013 at 1:13 PM, <abarth@chromium.org> wrote:

> On 2013/05/13 20:10:05, Nico wrote:
>
>> lgtm stamp
>>
>
>  (is this with symbols or without?)
>>
>
> Whatever ninja -C ../../out/Release all_webkit builds by default on Linux.
>  If
> there's something else I should be optimizing, please let me know.
>

I believe that builds without symbols. Nice :-)


>
>
https://codereview.chromium.**org/15093007/<https://codereview.chromium.org/1...
>

[eseidel, 2013-05-13 20:14:34]

Esprhen is the grand poo-ba of quotes.

[esprehn_google.com, 2013-05-13 20:16:31]

I don't like this change. It adds a lot of mess to RenderQuote that I
intentionally removed when I rewrote it. We can get the same code size
reduction without all this mess.

I also don't think this will work as is. They changed lots of stuff in
quotes recently. Did you account for all those changes?

On Monday, May 13, 2013, wrote:

> On 2013/05/13 20:10:05, Nico wrote:
>
>> lgtm stamp
>>
>
>  (is this with symbols or without?)
>>
>
> Whatever ninja -C ../../out/Release all_webkit builds by default on Linux.
>  If
> there's something else I should be optimizing, please let me know.
>
>
https://codereview.chromium.**org/15093007/<https://codereview.chromium.org/1...
>

[abarth-chromium, 2013-05-13 20:21:07]

On 2013/05/13 20:16:31, esprehn_google.com wrote:
> I don't like this change. It adds a lot of mess to RenderQuote that I
> intentionally removed when I rewrote it. We can get the same code size
> reduction without all this mess.

Would you be willing to post a CL that does that?

> I also don't think this will work as is. They changed lots of stuff in
> quotes recently. Did you account for all those changes?

I just merged the CL mechanically.  Is there are subtle issues, I'm sure I
missed them.

[esprehn_google.com, 2013-05-13 20:28:03]

Yeah I'd be happy to. We had discussed the cost of the hash table when I
rewrote it and the size was deemed fine since it added so much code
clarity. I don't think we had as many quotes in the table back then though.

On Monday, May 13, 2013, wrote:

> On 2013/05/13 20:16:31, esprehn_google.com wrote:
>
>> I don't like this change. It adds a lot of mess to RenderQuote that I
>> intentionally removed when I rewrote it. We can get the same code size
>> reduction without all this mess.
>>
>
> Would you be willing to post a CL that does that?
>
>  I also don't think this will work as is. They changed lots of stuff in
>> quotes recently. Did you account for all those changes?
>>
>
> I just merged the CL mechanically.  Is there are subtle issues, I'm sure I
> missed them.
>
>
https://codereview.chromium.**org/15093007/<https://codereview.chromium.org/1...
>

[abarth-chromium, 2013-05-13 20:35:49]

On 2013/05/13 20:28:03, esprehn_google.com wrote:
> Yeah I'd be happy to. We had discussed the cost of the hash table when I
> rewrote it and the size was deemed fine since it added so much code
> clarity. I don't think we had as many quotes in the table back then though.

Maybe we should just populate the hash table from a static data structure rather
than with code?  Binary search does seem like a better approach here, however.

[eseidel, 2013-05-14 07:34:39]

Something is wrong with HashTable::set.  Perhaps the 5 template parameters. :)

RenderQuote.o:
292k

Removing "CaseFoldingHash" from QuoteMap typedef:
288k

Making QUOTE_LANG empty:
68k

[eseidel, 2013-05-14 07:44:58]

*each* QUOTE_LANG() instance:
staticQuotesMap.set(lang, QuotesData::create(U(o1), U(c1), U(o2),
U(c2)).leakRef())
compiles to 1.52k of binary size!!?

Again, something is clearly wrong with HashTable::set.

We may need someone with some C++ template-fu to stare at our HashTable.h
implementation and help understand why it expands to infinity binary size.