Bug 118503 – Improve StringImpl::constructInternal() method

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Template Framework

[Mikhail Pozdnyakov]

StringImpl::constructInternal does the following  (this code had been located previously inside tryCreateUninitializedForce)

    if (sizeof(CharType) == sizeof(char))
        return adoptRef(new (NotNull, impl) StringImpl(length, Force8BitConstructor));
    return adoptRef(new (NotNull, impl) StringImpl(length));

Compiler has to compile both branches even though optimizer will than remove one of those, and as the function is inline it can affect slightly the compilation time. 
Would be nice to have compile-time polymorphism in the constructor already.

[Mikhail Pozdnyakov]

Created attachment 206316 [details]
patch

[Mikhail Pozdnyakov]

Having had a discussion with Benjamin on IRC came to conclusion that this is not worthwhile as might affect code readability.

[Mikhail Pozdnyakov]

Created attachment 206434 [details]
alternate patch

Solves the problem without affecting code readability.

[Mikhail Pozdnyakov]

EWS doe not run, might be because the bug is closed, reopening..

[Mikhail Pozdnyakov]

Created attachment 206436 [details]
same as before (try EWS)

[Benjamin Poulain]

Comment on attachment 206436 [details]
same as before (try EWS)

I am not sure why you obsess on this branch :)

But I don't see anything wrong with this. Note that the old code handle char and LChar.

[Benjamin Poulain]

Comment on attachment 206436 [details]
same as before (try EWS)

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

> Source/WTF/ChangeLog:22
> +        (WTF::StringImpl::reallocateInternal):
> +        * wtf/text/StringImpl.h:
> +        (WTF::StringImpl::StringImpl):
> +        (WTF::StringImpl::tryCreateUninitialized):
> +        (WTF::LChar):
> +        (WTF::UChar):

Oh, don't forget to fix your changelog.

[Mikhail Pozdnyakov]

Committed r152595: <http://trac.webkit.org/changeset/152595>