Bug 206400 – [WTF] AtomStringTable should be small

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Yusuke Suzuki]

[WTF] AtomStringTable should be small

[Yusuke Suzuki]

Created attachment 388014 [details]
Patch

WIP

[Yusuke Suzuki]

Putting EWS now, and running A/B tests.

[Yusuke Suzuki]

Created attachment 388138 [details]
Patch

[Yusuke Suzuki]

Created attachment 388139 [details]
Patch

[Sam Weinig]

Comment on attachment 388139 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=388139&action=review

> Source/WTF/ChangeLog:17
> +        [1]: https://bugs.webkit.org/show_bug.cgi?id=206400

This links to the same bugzilla bug as the one this patch is attached to. I think you meant it to point somewhere else?

[Sam Weinig]

How much of a win is this? 

Out of curiosity, do you know how many of the AtomStrings in the table are initialized via ProcessWarming::initializeNames()? It might also be interesting to know how many are lazily added static strings vs. atoms constructed from actual content.  

I ask mostly because if we wanted to reduce the AtomStringTable even more, there were a few ideas I had a while back about how we could consider shrinking it further based on the knowledge that many of the AtomStrings are known at compile time. For instance, one idea was to consider splitting the table in two (likely slowing down the slow case of lookup a bit), and having all the compile time known strings in a const/readonly compacted minimal perfect hash (gperf or the like).

[Yusuke Suzuki]

Comment on attachment 388139 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=388139&action=review

>> Source/WTF/ChangeLog:17
>> +        [1]: https://bugs.webkit.org/show_bug.cgi?id=206400
> 
> This links to the same bugzilla bug as the one this patch is attached to. I think you meant it to point somewhere else?

Oops, fixed https://bugs.webkit.org/show_bug.cgi?id=206469.

[Yusuke Suzuki]

(In reply to Sam Weinig from comment #6)
> How much of a win is this? 
> 
> Out of curiosity, do you know how many of the AtomStrings in the table are
> initialized via ProcessWarming::initializeNames()? It might also be
> interesting to know how many are lazily added static strings vs. atoms
> constructed from actual content.  

Not sure, but IIRC, for JavaScriptCore.framework it took 64KB (but my memory is sketchy...).
I have no data about WebCore case. It is possible WebCore allocates even more.

> 
> I ask mostly because if we wanted to reduce the AtomStringTable even more,
> there were a few ideas I had a while back about how we could consider
> shrinking it further based on the knowledge that many of the AtomStrings are
> known at compile time. For instance, one idea was to consider splitting the
> table in two (likely slowing down the slow case of lookup a bit), and having
> all the compile time known strings in a const/readonly compacted minimal
> perfect hash (gperf or the like).

I think it is possible that we can get some memory improvement. And we have a lot of compile-time-known HashMaps allocated at runtime.
I think having compile-time HashMap feature could improve things.

[Yusuke Suzuki]

Committed r254881: <https://trac.webkit.org/changeset/254881>

[Radar WebKit Bug Importer]

<rdar://problem/58772826>