| Summary: | localeToScriptCodeForFontSelection should use hash tables with larger default capacity | ||||||
|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Pratik Solanki <psolanki> | ||||
| Component: | WebCore Misc. | Assignee: | Pratik Solanki <psolanki> | ||||
| Status: | RESOLVED FIXED | ||||||
| Severity: | Normal | CC: | andersca, ap, benjamin, darin, psolanki | ||||
| Priority: | P2 | ||||||
| Version: | 528+ (Nightly build) | ||||||
| Hardware: | Unspecified | ||||||
| OS: | Unspecified | ||||||
| Attachments: |
|
||||||
|
Description
Pratik Solanki
2013-08-15 17:45:03 PDT
Created attachment 208873 [details]
Patch
Comment on attachment 208873 [details]
Patch
Tweaking the size of these hash tables is OK. But a way better thing to do would be to not use a hash table for this. There is suitable code in the quotesForLanguage function in RenderQuote.cpp that we could generalize, put in WTF, and use here.
That sounds interesting. I can look into refactoring this. Should this patch be abandoned or should we check this in for now while we investigate a better approach? I don’t feel strongly either way; fine to land this. But way better to not dynamically created hash tables for this long term. Generally speaking we want to build tables at compile time. I must admit I’m not sure why we chose to use gperf for some things like this, but not for others. Maybe we should be using gperf more consistently? Comment on attachment 208873 [details]
Patch
OK to land for now, but we should do better, as I mentioned.
Thanks for the review. I'll look into factoring out the code from RenderQuote. Committed r154347: <http://trac.webkit.org/changeset/154347> |