[webkit-reviews] review granted: [Bug 21123] using "arguments" in a function should not force creation of an activation object : [Attachment 23983] Proposed patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Oct 1 13:09:58 PDT 2008

Darin Adler <darin at apple.com> has granted Cameron Zwarich (cpst)
<cwzwarich at uwaterloo.ca>'s request for review:
Bug 21123: using "arguments" in a function should not force creation of an
activation object

Attachment 23983: Proposed patch

------- Additional Comments from Darin Adler <darin at apple.com>
+	     m_jit.movl_mr(RegisterFile::OptionalCalleeActivation *
static_cast<int>(sizeof(Register)), X86::edi, X86::eax);
+	     m_jit.orl_mr(RegisterFile::OptionalCalleeArguments *
static_cast<int>(sizeof(Register)), X86::edi, X86::eax);

I'm disappointed that we have to make the fast case slower here by examining
both of these. I wonder if some day we can find a way to make it faster.

+    } else if (JSValue* arguments =
r[RegisterFile::OptionalCalleeArguments].getJSValue()) {
+	 if (arguments->isObject(&Arguments::info))
+	     static_cast<Arguments*>(arguments)->copyRegisters();
+    }

Why is this isObject check needed? When does OptionalCalleeArguments contains
something that's non-zero but is not an Arguments object?

+					 { $$ =
*$1, *$2, $4.m_node.head, $7, LEXER->sourceRange($6, $8)), $4.m_featureInfo |
ClosureFeature, 0); DBG($7, @6, @8); $7->setUsesArguments($7->usesArguments() |
($4.m_featureInfo & ArgumentsFeature)); if (!$$.m_node) YYABORT; }

I'd expect to have the new code be before the DBG rather than after. As these
lines of code get super-long, I'd interested in having some of this logic move
into a helper function so it's not all on one giant source line in the grammar.
Or some other way of formatting this with multiple lines so it's easier to

This is a great speedup!


More information about the webkit-reviews mailing list