[Webkit-unassigned] [Bug 73712] PODIntervalTree takes 1.7MB memory on www.nytimes.com

[bugzilla-daemon at webkit.org]

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


Darin Adler <darin at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #117731|review?                     |review+
               Flag|                            |




--- Comment #7 from Darin Adler <darin at apple.com>  2011-12-08 17:18:16 PST ---
(From update of attachment 117731)
View in context: https://bugs.webkit.org/attachment.cgi?id=117731&action=review

> Source/WebCore/platform/PODArena.h:110
> +        ChunkVector::const_iterator it = m_chunks.begin();
> +        while (it != end) {
> +            if ((*it)->contains(ptr)) {
> +                (*it)->free(ptr);
> +                break;
> +            }
> +            ++it;
> +        }

We normally use a for loop for something like this rather than a while loop.

> Source/WebCore/platform/PODArena.h:170
> +                ChunkVector::const_iterator it = m_chunks.begin();
> +                while (it < end) {
> +                    if ((*it)->hasFreeList()) {
> +                        ptr = (*it)->allocate(roundedSize);
> +                        break;
> +                    }
> +                    ++it;
> +                }

Same comment about for loop rather than while loop. Also better to use != rather than < for the end check.

If the allocation fails, don’t we want to keep iterating to use the free space in a different chunk? I think it should be if (ptr) break; rather than just break;

> Source/WebCore/platform/PODArena.h:225
> +                void* p = m_freeList;
> +                m_freeList = m_freeList->m_next;
> +                return p;

We normally prefer to use a word rather than a letter for a local variable like “p”.

> Source/WebCore/platform/PODArena.h:244
> +            FreeCell* cell = (FreeCell*)ptr;

This should be static_cast rather than a C-style cast.

> Source/WebCore/platform/PODArena.h:251
> +            return (ptr >= m_base && ptr < m_base + m_size);

No need for the parentheses here.

> Source/WebCore/platform/PODRedBlackTree.h:171
> +    void initIfNeeded(PassRefPtr<PODArena> arena)
> +    {
> +        if (!m_arena)
> +            m_arena = arena;
> +    }

It would be slightly more efficient if this took a raw pointer instead of PassRefPtr because this does not always take ownership of the passed in arena, and when it does not, the PassRefPtr produces unnecessary reference count churn.

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