[webkit-reviews] review granted: [Bug 108093] Static size inference for JavaScript objects : [Attachment 185041] Patch

[bugzilla-daemon at webkit.org]

Filip Pizlo <fpizlo at apple.com> has granted Geoffrey Garen <ggaren at apple.com>'s
request for review:
Bug 108093: Static size inference for JavaScript objects
https://bugs.webkit.org/show_bug.cgi?id=108093

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

------- Additional Comments from Filip Pizlo <fpizlo at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=185041&action=review


> Source/JavaScriptCore/ChangeLog:128
> +	   (JSC::DFG::AbstractState::execute): Upated for rename.

Typo "Upated"

> Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:757
> +UnlinkedValueProfile BytecodeGenerator::emitProfiledOpcode(int dst, OpcodeID
opcodeID)
>  {
> +    m_staticPropertyAnalyzer.kill(dst);

Seems like it would be good to change the name of emitProfiledOpcode, since it
does things to staticPropertyAnalyzer now.  Not sure what a good name would be.


> Source/JavaScriptCore/bytecompiler/StaticPropertyAnalyzer.h:38
> +// Used for flow-insensitive static analysis of the number of properties
assigned to an object.
> +class StaticPropertyAnalyzer {
> +public:
> +    StaticPropertyAnalyzer(Vector<UnlinkedInstruction>*);
> +

Is it the case that this analysis can be wrong, and even if it is, the program
will execute "correctly" albeit possibly less efficiently?

If so, then you should add a comment about this.  It's useful to be able to
remember which analyses have to be sound at any cost (DFG::AbstractState) and
which can be wrong (this one).