Bug 13388 - [js-collector-tweaks] Shrink FunctionImp / DeclaredFunctionImp by 4 bytes
Summary: [js-collector-tweaks] Shrink FunctionImp / DeclaredFunctionImp by 4 bytes
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 523.x (Safari 3)
Hardware: Mac OS X 10.4
: P2 Normal
Assignee: Maciej Stachowiak
URL:
Keywords:
Depends on:
Blocks: 13389
  Show dependency treegraph
 
Reported: 2007-04-18 00:58 PDT by Maciej Stachowiak
Modified: 2007-04-23 03:47 PDT (History)
0 users

See Also:


Attachments
08-js-gc-function-shrink.patch.txt (13.19 KB, patch)
2007-04-18 00:59 PDT, Maciej Stachowiak
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Maciej Stachowiak 2007-04-18 00:58:41 PDT
Shrink FunctionImp / DeclaredFunctionImp by 4 bytes, by moving parameter list to function body. This enables further optimizations.
Comment 1 Maciej Stachowiak 2007-04-18 00:59:25 PDT
Created attachment 14069 [details]
08-js-gc-function-shrink.patch.txt
Comment 2 Darin Adler 2007-04-18 11:27:14 PDT
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
Comment 3 Maciej Stachowiak 2007-04-18 15:04:50 PDT
(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.