Bug 13388 – [js-collector-tweaks] Shrink FunctionImp / DeclaredFunctionImp by 4 bytes

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Maciej Stachowiak]

Shrink FunctionImp / DeclaredFunctionImp by 4 bytes, by moving parameter list to function body. This enables further optimizations.

[Maciej Stachowiak]

Created attachment 14069 [details]
08-js-gc-function-shrink.patch.txt

[Darin Adler]

Comment on attachment 14069 [details]
08-js-gc-function-shrink.patch.txt

+UString FunctionBodyNode::paramString() const
+{
+  UString s;

I think you want to start with an empty string here, rather than a null string.

+  for(ParameterNode *p = param.get(); p != 0L; p = p->nextParam())

Missing a space after for (I know this is copy and paste), and 0L is no better than 0.

+  for(ParameterNode *p = param.get(); p != 0L; p = p->nextParam())

Again.

+    Parameter() {};

No semicolon here. Do we really need a Parameter type? How about just using Vector<Identifier>?

+    Identifier paramName(int pos) const { return m_parameters[pos].name; }

Should be size_t rather than int.

r=me

[Maciej Stachowiak]

(In reply to comment #2)
>
> Do we really need a Parameter type? How about just using Vector<Identifier>?
> 

I considered this, but Maks Orlovich's symbol table changes in the KDE version of KJS add more fields to Parameter.

Agreed on your other comments.