Bug 250676 – Use memcpy in SVGPathByteStreamSource::readType

• Component: SVG
• Status: RESOLVED FIXED

[Ahmad Saleem]

Hi Team,

While going through Blink's commit, I came across following refactoring / optimization:

Blink Commit - https://chromium.googlesource.com/chromium/blink/+/f9411437595a5782eb1adba1f719d4448530bd93

WebKit Source - https://searchfox.org/wubkat/source/Source/WebCore/svg/SVGPathByteStreamSource.h#57

'''

This avoids having the compiler beating around the bush trying to
recognize the memcpy, and results in smaller code (roughly 50% smaller
measured for GCC 4.8). All at the affordable price of upsetting a few
language purists.

'''

Appreciate if someone can share input.

Thanks!

[Radar WebKit Bug Importer]

<rdar://problem/104552389>

[Said Abou-Hallawa]

I think there is no much saving in the Blink's commit. This function just reads bool, short or float. So the for-loop in SVGPathByteStreamSource::readType() runs 1, 2 or 4 times. In my opinion using memcpy is just a cleaner solution and nice refactoring but it is not a perf or code size optimization.


So if we want to clean this area, I would suggest something like that instead:


    // SVGPathByteStreamSource.h

    template<typename DataType>
    DataType readType()
    {
        DataType data;
        size_t dataSize = sizeof(DataType);

        ASSERT(m_streamCurrent + dataSize <= m_streamEnd);
        memcpy(&data, m_streamCurrent, dataSize);
        m_streamCurrent += dataSize;
        return data;
    }


    // SVGPathByteStreamBuilder.h

    template<typename DataType>
    void writeType(DataType data)
    {
        union {
            DataType value;
            unsigned char bytes[sizeof(DataType)];
        } ByteType;

        ByteType type = { data };
        size_t typeSize = sizeof(ByteType);

        for (size_t i = 0; i < typeSize; ++i)
            m_byteStream.append(type.bytes[i]);
    }


And we can get rid of BoolByte, FloatByte and UnsignedShortByte.

[Chris Dumez]

Pull request: https://github.com/WebKit/WebKit/pull/17264

[EWS]

Committed 267523@main (e2aa0c20fc47): <https://commits.webkit.org/267523@main>

Reviewed commits have been landed. Closing PR #17264 and removing active labels.