[Webkit-unassigned] [Bug 142032] New: Cleanup RenderSVGResourceClipper class.

[bugzilla-daemon at webkit.org]

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

            Bug ID: 142032
           Summary: Cleanup RenderSVGResourceClipper class.
    Classification: Unclassified
           Product: WebKit
           Version: 528+ (Nightly build)
          Hardware: Unspecified
                OS: Unspecified
            Status: NEW
          Severity: Normal
          Priority: P2
         Component: SVG
          Assignee: webkit-unassigned at lists.webkit.org
          Reporter: sabouhallawa at apple.com
                CC: zimmermann at kde.org

This bug to address Darin's comment in https://bugs.webkit.org/show_bug.cgi?id=141776#c5.

Comment on attachment 246862 [details]
Patch

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

> Source/WebCore/rendering/svg/RenderSVGResourceClipper.cpp:260
> +ClipperData* RenderSVGResourceClipper::addRendererToClipper(const RenderObject& object)

The pointer this returns is always non-null. This function should return a reference instead of a pointer.

> Source/WebCore/rendering/svg/RenderSVGResourceClipper.cpp:264
> +    if (!m_clipper.contains(&object))
> +        m_clipper.set(&object, std::make_unique<ClipperData>());
> +    return m_clipper.get(&object);

This code does double hash table lookups. You just moved it, but the performance problem can be fixed, like this:

    auto& slot = m_clipper.add(&object, nullptr).iterator->value;
    if (!slot)
        slot = std::make_unique<ClipperData>();
    return slot.get();

That will only do a single hash table lookup. In the future we might invent even cleaner ways to write this.

We could also consider making this a HashMap<const RenderObject*, ClipperData> instead. If we did that, then the code would be even simpler:

    return m_clipper.add(&object, ClipperData()).iterator->value;

The map would be larger, because each slot would have an entire ClipperData object. And rehashing would be slower, but we would avoid the overhead of an additional memory block for every slot that is in use.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20150226/1fb09c3f/attachment-0002.html>