[Webkit-unassigned] [Bug 64493] GC allocation fast path has too many operations

[bugzilla-daemon at webkit.org]

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





--- Comment #10 from Darin Adler <darin at apple.com>  2011-07-13 18:48:26 PST ---
(From update of attachment 100747)
View in context: https://bugs.webkit.org/attachment.cgi?id=100747&action=review

> Source/JavaScriptCore/heap/Heap.h:129
> +        void clearAllocCache();

We normally prefer not to abbreviate, so this would be named allocation cache rather than alloc cache.

> Source/JavaScriptCore/heap/Heap.h:297
> +        MarkedBlock::FreeCell* firstFree = sizeClass.firstFree;
> +        if (!firstFree)
> +            return allocateSlowCase(sizeClass);

I wonder if this branch is properly predicted. We might want to see if using UNLIKELY() speeds this up still-more.

> Source/JavaScriptCore/heap/Heap.h:300
> +        return static_cast<void*>(firstFree);

This cast should not be needed. Any pointer to a non-const non-volatile type should convert to void* without a cast.

> Source/JavaScriptCore/heap/MarkedBlock.cpp:93
> +            result = freeCell;

Is the “loop through all cells, but return the last freed one” policy here intentional? I think this is something that deserves a comment, because it’s not obvious to me at least.

> Source/JavaScriptCore/heap/MarkedBlock.cpp:107
> +        result = freeCell;

I have a similar question here.

> Source/JavaScriptCore/heap/NewSpace.h:55
> +            MarkedBlock::FreeCell* firstFree;

We normally use nouns for data members, so we would call this firstFreeCell rather than firstFree.

> Source/JavaScriptCore/heap/NewSpace.h:56
> +            MarkedBlock* curBlock;

We normally don’t abbreviate words like current to cur.

> Source/JavaScriptCore/heap/NewSpace.h:159
> +        return static_cast<void*>(firstFree);

This cast should not be needed.

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