[webkit-reviews] review granted: [Bug 67494] Use bump allocator for initial property storage : [Attachment 106754] Patch

[bugzilla-daemon at webkit.org]

Gavin Barraclough <barraclough at apple.com> has granted Oliver Hunt
<oliver at apple.com>'s request for review:
Bug 67494: Use bump allocator for initial property storage
https://bugs.webkit.org/show_bug.cgi?id=67494

Attachment 106754: Patch
https://bugs.webkit.org/attachment.cgi?id=106754&action=review

------- Additional Comments from Gavin Barraclough <barraclough at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=106754&action=review


> Source/JavaScriptCore/runtime/JSObject.cpp:-604
> -

This function can be in a number of states based on the values of usingNursery,
newPropertyStorage, and newAllocationInNursery.
I think it would be clear if you deleted the two booleans and added a state
enum with named states that better explain what's going on.

> Source/JavaScriptCore/runtime/JSObject.cpp:608
> +	   // Allocation property storage might trigger a GC that will move

(typo in comment, nursary).

> Source/JavaScriptCore/runtime/JSObject.cpp:614
> +    }

I think, at this point, either:
(1) usingNursery && newPropertyStorage && newAllocationInNursery
or:
(2) !usingNursery && !newPropertyStorage && newAllocationInNursery

> Source/JavaScriptCore/runtime/JSObject.cpp:618
> +    }

I think, at this point, either:
(1) usingNursery && newPropertyStorage && newAllocationInNursery
or:
(2) !usingNursery && newPropertyStorage && !newAllocationInNursery

> Source/JavaScriptCore/runtime/JSObject.cpp:629
> +

Based on the observations above, this check is a contradiction?  Do we ever
need to do this?