[Webkit-unassigned] [Bug 34489] [Qt] Text codec lookup is slow

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=34489


Ariya Hidayat <ariya.hidayat at gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #47948|review?                     |
               Flag|                            |




--- Comment #2 from Ariya Hidayat <ariya.hidayat at gmail.com>  2010-02-02 11:50:12 PST ---
(From update of attachment 47948)
Hmm, seems the proper fix is to optimize QTextCodec.

Anyway, some comments:

> -    m_codec = QTextCodec::codecForName(m_encoding.name());
> +    // Very likely to reuse the same text codecs in frequent order,
> +    // QTextCodec is very slow with matching encoding name to codec, so we make a cache here
> +    // 
> +    const char* name = m_encoding.name();
> +    QString qName = QString::fromAscii(name);

This creates the unnecessary deep copy of name in the heap. However, I don't
think using QLatin1String would work either since you potentially insert the
encoding name later on to the cache.

> +    TextCodecHash::iterator it = staticTextCodecCache.find(qName);
> +    if( it != staticTextCodecCache.end() ) {
> +        m_codec = it.value();
> +        
> +    }
> +    else {
> +        m_codec = QTextCodec::codecForName(m_encoding.name());
> +        staticTextCodecCache.insert(qName, m_codec);
> +    }

A more readable version would be to use QHash::value(). Maybe something like:

m_codec = staticTextCodecCache.value(qName);
if (!m_coded) {
        m_codec = QTextCodec::codecForName(m_encoding.name());
       staticTextCodecCache.insert(qName, m_codec);
}

Also, any particular reason why the codec cache can't be a static member in
TextCodecQt class instead of a global one like that?

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.