[webkit-reviews] review granted: [Bug 79608] Getting the instruction stream for a code block should not require two loads : [Attachment 128930] the patch

[bugzilla-daemon at webkit.org]

Sam Weinig <sam at webkit.org> has granted Filip Pizlo <fpizlo at apple.com>'s
request for review:
Bug 79608: Getting the instruction stream for a code block should not require
two loads
https://bugs.webkit.org/show_bug.cgi?id=79608

Attachment 128930: the patch
https://bugs.webkit.org/attachment.cgi?id=128930&action=review

------- Additional Comments from Sam Weinig <sam at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=128930&action=review


In addition to the rename and comment fix.  Please consider adding basic unit
tests to TestWebKitAPI.

> Source/JavaScriptCore/ChangeLog:16
> +	   This patch also gets rid of the bytecode discarding logic, since we
don't
> +	   use it anymore and it's unlikely to ever work right with DFG or
LLInt. And
> +	   I didn't feel like porting dead code to use CompactVector.

Sadness.

> Source/JavaScriptCore/wtf/CompactVector.h:33
> +// This implements a resizeable array for POD** values, which is optimized
for:

This isn't actually resizable.

> Source/JavaScriptCore/wtf/CompactVector.h:47
> +template<typename T>
> +class CompactVector {

I think this name is misleading.  Really, this is just an array with an
efficiently RefCounted data part.  RefCountedArray?