Commit cf6fa796 authored by barraclough@apple.com's avatar barraclough@apple.com

How many copies of the parameters do you need?

https://bugs.webkit.org/show_bug.cgi?id=28701

Reviewed by Darin Adler.

The function parameters in JSC get copied a lot - and unnecessarily so.

Originally this happened due to duplicating FunctionBodyNodes on recompilation,
though the problem has been exacerbated by copying the parameters from the
original function body onto the executable, then back onto the real body that
will be generated (this happens on every function).  And this is all made worse
since the data structures in question are a little ugly - C style arrays of C++
objects containing ref counts, so they need a full copy-construct (rather than
a simple memcpy).

This can all be greatly simplified by just punting the parameters off into
their own ref-counted object, and forgoing all the copying.

~no performance change, possible slight progression.

* bytecompiler/BytecodeGenerator.cpp:
(JSC::BytecodeGenerator::BytecodeGenerator):
* bytecompiler/BytecodeGenerator.h:
(JSC::BytecodeGenerator::makeFunction):
* parser/Nodes.cpp:
(JSC::FunctionParameters::FunctionParameters):
(JSC::FunctionBodyNode::FunctionBodyNode):
(JSC::FunctionBodyNode::finishParsing):
* parser/Nodes.h:
(JSC::FunctionBodyNode::parameters):
(JSC::FunctionBodyNode::parameterCount):
* runtime/Executable.cpp:
(JSC::FunctionExecutable::~FunctionExecutable):
(JSC::FunctionExecutable::compile):
(JSC::FunctionExecutable::reparseExceptionInfo):
(JSC::FunctionExecutable::fromGlobalCode):
(JSC::FunctionExecutable::paramString):
* runtime/Executable.h:
(JSC::FunctionExecutable::FunctionExecutable):
(JSC::FunctionExecutable::parameterCount):



git-svn-id: http://svn.webkit.org/repository/webkit/trunk@47775 268f45cc-cd09-0410-ab3c-d52691b4dbfc
parent f706b11d
2009-08-24 Gavin Barraclough <barraclough@apple.com>
Reviewed by Darin Adler.
How many copies of the parameters do you need?
https://bugs.webkit.org/show_bug.cgi?id=28701
The function parameters in JSC get copied a lot - and unnecessarily so.
Originally this happened due to duplicating FunctionBodyNodes on recompilation,
though the problem has been exacerbated by copying the parameters from the
original function body onto the executable, then back onto the real body that
will be generated (this happens on every function). And this is all made worse
since the data structures in question are a little ugly - C style arrays of C++
objects containing ref counts, so they need a full copy-construct (rather than
a simple memcpy).
This can all be greatly simplified by just punting the parameters off into
their own ref-counted object, and forgoing all the copying.
~no performance change, possible slight progression.
* bytecompiler/BytecodeGenerator.cpp:
(JSC::BytecodeGenerator::BytecodeGenerator):
* bytecompiler/BytecodeGenerator.h:
(JSC::BytecodeGenerator::makeFunction):
* parser/Nodes.cpp:
(JSC::FunctionParameters::FunctionParameters):
(JSC::FunctionBodyNode::FunctionBodyNode):
(JSC::FunctionBodyNode::finishParsing):
* parser/Nodes.h:
(JSC::FunctionBodyNode::parameters):
(JSC::FunctionBodyNode::parameterCount):
* runtime/Executable.cpp:
(JSC::FunctionExecutable::~FunctionExecutable):
(JSC::FunctionExecutable::compile):
(JSC::FunctionExecutable::reparseExceptionInfo):
(JSC::FunctionExecutable::fromGlobalCode):
(JSC::FunctionExecutable::paramString):
* runtime/Executable.h:
(JSC::FunctionExecutable::FunctionExecutable):
(JSC::FunctionExecutable::parameterCount):
2009-08-25 Brent Fulgham <bfulgham@webkit.org>
Reviewed by NOBODY (Buildfix).
......
......@@ -349,8 +349,8 @@ BytecodeGenerator::BytecodeGenerator(FunctionBodyNode* functionBody, const Debug
for (size_t i = 0; i < varStack.size(); ++i)
addVar(*varStack[i].first, varStack[i].second & DeclarationStacks::IsConstant);
const Identifier* parameters = functionBody->parameters();
size_t parameterCount = functionBody->parameterCount();
FunctionParameters& parameters = *functionBody->parameters();
size_t parameterCount = parameters.size();
m_nextParameterIndex = -RegisterFile::CallFrameHeaderSize - parameterCount - 1;
m_parameters.grow(1 + parameterCount); // reserve space for "this"
......
......@@ -419,7 +419,7 @@ namespace JSC {
PassRefPtr<FunctionExecutable> makeFunction(FunctionBodyNode* body)
{
return adoptRef(new FunctionExecutable(body->ident(), body->source(), body->usesArguments(), body->copyParameters(), body->parameterCount(), body->lineNo(), body->lastLine()));
return FunctionExecutable::create(body->ident(), body->source(), body->usesArguments(), body->parameters(), body->lineNo(), body->lastLine());
}
Vector<Instruction>& instructions() { return m_codeBlock->instructions(); }
......
......@@ -1980,43 +1980,32 @@ RegisterID* EvalNode::emitBytecode(BytecodeGenerator& generator, RegisterID*)
// ------------------------------ FunctionBodyNode -----------------------------
FunctionParameters::FunctionParameters(ParameterNode* firstParameter)
{
for (ParameterNode* parameter = firstParameter; parameter; parameter = parameter->nextParam())
append(parameter->ident());
}
inline FunctionBodyNode::FunctionBodyNode(JSGlobalData* globalData)
: ScopeNode(globalData)
, m_parameters(0)
, m_parameterCount(0)
{
}
inline FunctionBodyNode::FunctionBodyNode(JSGlobalData* globalData, SourceElements* children, VarStack* varStack, FunctionStack* funcStack, const SourceCode& sourceCode, CodeFeatures features, int numConstants)
: ScopeNode(globalData, sourceCode, children, varStack, funcStack, features, numConstants)
, m_parameters(0)
, m_parameterCount(0)
{
}
FunctionBodyNode::~FunctionBodyNode()
{
for (size_t i = 0; i < m_parameterCount; ++i)
m_parameters[i].~Identifier();
fastFree(m_parameters);
}
void FunctionBodyNode::finishParsing(const SourceCode& source, ParameterNode* firstParameter, const Identifier& ident)
{
Vector<Identifier> parameters;
for (ParameterNode* parameter = firstParameter; parameter; parameter = parameter->nextParam())
parameters.append(parameter->ident());
size_t count = parameters.size();
setSource(source);
finishParsing(parameters.releaseBuffer(), count, ident);
finishParsing(FunctionParameters::create(firstParameter), ident);
}
void FunctionBodyNode::finishParsing(Identifier* parameters, size_t parameterCount, const Identifier& ident)
void FunctionBodyNode::finishParsing(PassRefPtr<FunctionParameters> parameters, const Identifier& ident)
{
ASSERT(!source().isNull());
m_parameters = parameters;
m_parameterCount = parameterCount;
m_ident = ident;
}
......@@ -2053,13 +2042,6 @@ RegisterID* FunctionBodyNode::emitBytecode(BytecodeGenerator& generator, Registe
return 0;
}
Identifier* FunctionBodyNode::copyParameters()
{
Identifier* parameters = static_cast<Identifier*>(fastMalloc(m_parameterCount * sizeof(Identifier)));
VectorCopier<false, Identifier>::uninitializedCopy(m_parameters, m_parameters + m_parameterCount, parameters);
return parameters;
}
// ------------------------------ FuncDeclNode ---------------------------------
RegisterID* FuncDeclNode::emitBytecode(BytecodeGenerator& generator, RegisterID* dst)
......
......@@ -1445,21 +1445,26 @@ namespace JSC {
virtual RegisterID* emitBytecode(BytecodeGenerator&, RegisterID* = 0);
};
class FunctionParameters : public Vector<Identifier>, public RefCounted<FunctionParameters> {
public:
static PassRefPtr<FunctionParameters> create(ParameterNode* firstParameter) { return adoptRef(new FunctionParameters(firstParameter)); }
private:
FunctionParameters(ParameterNode*);
};
class FunctionBodyNode : public ScopeNode {
friend class JIT;
public:
static FunctionBodyNode* create(JSGlobalData*);
static PassRefPtr<FunctionBodyNode> create(JSGlobalData*, SourceElements*, VarStack*, FunctionStack*, const SourceCode&, CodeFeatures, int numConstants);
virtual ~FunctionBodyNode();
const Identifier* parameters() const { return m_parameters; }
size_t parameterCount() const { return m_parameterCount; }
Identifier* copyParameters();
FunctionParameters* parameters() const { return m_parameters.get(); }
size_t parameterCount() const { return m_parameters->size(); }
virtual RegisterID* emitBytecode(BytecodeGenerator&, RegisterID* = 0);
void finishParsing(const SourceCode&, ParameterNode*, const Identifier& ident);
void finishParsing(Identifier* parameters, size_t parameterCount, const Identifier& ident);
void finishParsing(const SourceCode&, ParameterNode*, const Identifier&);
void finishParsing(PassRefPtr<FunctionParameters>, const Identifier&);
const Identifier& ident() { return m_ident; }
......@@ -1470,8 +1475,7 @@ namespace JSC {
FunctionBodyNode(JSGlobalData*, SourceElements*, VarStack*, FunctionStack*, const SourceCode&, CodeFeatures, int numConstants);
Identifier m_ident;
Identifier* m_parameters;
size_t m_parameterCount;
RefPtr<FunctionParameters> m_parameters;
};
class FuncExprNode : public ExpressionNode {
......
......@@ -56,9 +56,6 @@ ProgramExecutable::~ProgramExecutable()
FunctionExecutable::~FunctionExecutable()
{
for (int i = 0; i < m_parameterCount; ++i)
m_parameters[i].~Identifier();
fastFree(m_parameters);
delete m_codeBlock;
}
......@@ -120,7 +117,7 @@ void FunctionExecutable::compile(ExecState*, ScopeChainNode* scopeChainNode)
RefPtr<FunctionBodyNode> body = globalData->parser->parse<FunctionBodyNode>(globalData, 0, 0, m_source);
if (m_forceUsesArguments)
body->setUsesArguments();
body->finishParsing(copyParameters(), m_parameterCount, m_name);
body->finishParsing(m_parameters, m_name);
recordParse(body->features(), body->lineNo(), body->lastLine());
ScopeChain scopeChain(scopeChainNode);
......@@ -185,7 +182,7 @@ ExceptionInfo* FunctionExecutable::reparseExceptionInfo(JSGlobalData* globalData
RefPtr<FunctionBodyNode> newFunctionBody = globalData->parser->parse<FunctionBodyNode>(globalData, 0, 0, m_source);
if (m_forceUsesArguments)
newFunctionBody->setUsesArguments();
newFunctionBody->finishParsing(copyParameters(), m_parameterCount, m_name);
newFunctionBody->finishParsing(m_parameters, m_name);
ScopeChain scopeChain(scopeChainNode);
JSGlobalObject* globalObject = scopeChain.globalObject();
......@@ -262,25 +259,17 @@ PassRefPtr<FunctionExecutable> FunctionExecutable::fromGlobalCode(const Identifi
FunctionBodyNode* body = static_cast<FuncExprNode*>(funcExpr)->body();
ASSERT(body);
return adoptRef(new FunctionExecutable(functionName, body->source(), body->usesArguments(), body->copyParameters(), body->parameterCount(), body->lineNo(), body->lastLine()));
}
Identifier* FunctionExecutable::copyParameters()
{
// This code uses the internal vector copier to make copy-constructed copies of the data in the array
// (the array contains Identfiers which reference count Ustring::Reps, which must be ref'ed correctly).
Identifier* parameters = static_cast<Identifier*>(fastMalloc(m_parameterCount * sizeof(Identifier)));
WTF::VectorCopier<false, Identifier>::uninitializedCopy(m_parameters, m_parameters + m_parameterCount, parameters);
return parameters;
return FunctionExecutable::create(functionName, body->source(), body->usesArguments(), body->parameters(), body->lineNo(), body->lastLine());
}
UString FunctionExecutable::paramString() const
{
FunctionParameters& parameters = *m_parameters;
UString s("");
for (int pos = 0; pos < m_parameterCount; ++pos) {
for (size_t pos = 0; pos < parameters.size(); ++pos) {
if (!s.isEmpty())
s += ", ";
s += m_parameters[pos].ustring();
s += parameters[pos].ustring();
}
return s;
......
......@@ -221,17 +221,9 @@ namespace JSC {
class FunctionExecutable : public ScriptExecutable {
friend class JIT;
public:
FunctionExecutable(const Identifier& name, const SourceCode& source, bool forceUsesArguments, Identifier* parameters, int parameterCount, int firstLine, int lastLine)
: ScriptExecutable(source)
, m_forceUsesArguments(forceUsesArguments)
, m_parameters(parameters)
, m_parameterCount(parameterCount)
, m_codeBlock(0)
, m_name(name)
, m_numVariables(0)
static PassRefPtr<FunctionExecutable> create(const Identifier& name, const SourceCode& source, bool forceUsesArguments, FunctionParameters* parameters, int firstLine, int lastLine)
{
m_firstLine = firstLine;
m_lastLine = lastLine;
return adoptRef(new FunctionExecutable(name, source, forceUsesArguments, parameters, firstLine, lastLine));
}
~FunctionExecutable();
......@@ -261,7 +253,7 @@ namespace JSC {
}
const Identifier& name() { return m_name; }
size_t parameterCount() const { return m_parameterCount; }
size_t parameterCount() const { return m_parameters->size(); }
size_t variableCount() const { return m_numVariables; }
UString paramString() const;
......@@ -271,12 +263,22 @@ namespace JSC {
static PassRefPtr<FunctionExecutable> fromGlobalCode(const Identifier&, ExecState*, Debugger*, const SourceCode&, int* errLine = 0, UString* errMsg = 0);
private:
FunctionExecutable(const Identifier& name, const SourceCode& source, bool forceUsesArguments, FunctionParameters* parameters, int firstLine, int lastLine)
: ScriptExecutable(source)
, m_forceUsesArguments(forceUsesArguments)
, m_parameters(parameters)
, m_codeBlock(0)
, m_name(name)
, m_numVariables(0)
{
m_firstLine = firstLine;
m_lastLine = lastLine;
}
void compile(ExecState*, ScopeChainNode*);
Identifier* copyParameters();
bool m_forceUsesArguments;
Identifier* m_parameters;
int m_parameterCount;
RefPtr<FunctionParameters> m_parameters;
CodeBlock* m_codeBlock;
Identifier m_name;
size_t m_numVariables;
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment