Skip to content

Commit 3a2f206

Browse files
geoffreygarengeoffreygaren
committed
Unreviewed, rolling back in r190450
https://bugs.webkit.org/show_bug.cgi?id=149727 This time for sure? The cause of the leak was an invalidated compilation. There was vestigial manual memory management code that eagerly removed a CodeBlock from the set of CodeBlocks if compilation was invalidated. That's not cool since we rely on the set of CodeBlocks when we run destructors. The fix is to remove the vestigial code. I ran the leaks, correctness, and performance tests locally and did not see any problems. Restored changesets: "CodeBlock should be a GC object" https://bugs.webkit.org/show_bug.cgi?id=149727 http://trac.webkit.org/changeset/190450 Canonical link: https://commits.webkit.org/168039@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@190694 268f45cc-cd09-0410-ab3c-d52691b4dbfc
1 parent 6cf43b9 commit 3a2f206

57 files changed

Lines changed: 886 additions & 736 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎Source/JavaScriptCore/ChangeLog‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,28 @@
1+
2015-10-07 Geoffrey Garen <ggaren@apple.com>
2+
3+
Unreviewed, rolling back in r190450
4+
https://bugs.webkit.org/show_bug.cgi?id=149727
5+
6+
This time for sure?
7+
8+
The cause of the leak was an invalidated compilation.
9+
10+
There was vestigial manual memory management code that eagerly removed
11+
a CodeBlock from the set of CodeBlocks if compilation was invalidated.
12+
That's not cool since we rely on the set of CodeBlocks when we run
13+
destructors.
14+
15+
The fix is to remove the vestigial code.
16+
17+
I ran the leaks, correctness, and performance tests locally and did not
18+
see any problems.
19+
20+
Restored changesets:
21+
22+
"CodeBlock should be a GC object"
23+
https://bugs.webkit.org/show_bug.cgi?id=149727
24+
http://trac.webkit.org/changeset/190450
25+
126
2015-10-07 Mark Lam <mark.lam@apple.com>
227

328
Disable tail calls because it is breaking some sites.

‎Source/JavaScriptCore/bytecode/CodeBlock.cpp‎

Lines changed: 277 additions & 174 deletions
Large diffs are not rendered by default.

‎Source/JavaScriptCore/bytecode/CodeBlock.h‎

Lines changed: 229 additions & 94 deletions
Large diffs are not rendered by default.

‎Source/JavaScriptCore/bytecode/CodeOrigin.cpp‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ bool CodeOrigin::isApproximatelyEqualTo(const CodeOrigin& other) const
7575
if (!a.inlineCallFrame)
7676
return true;
7777

78-
if (a.inlineCallFrame->executable.get() != b.inlineCallFrame->executable.get())
78+
if (a.inlineCallFrame->baselineCodeBlock.get() != b.inlineCallFrame->baselineCodeBlock.get())
7979
return false;
8080

8181
a = a.inlineCallFrame->directCaller;
@@ -98,7 +98,7 @@ unsigned CodeOrigin::approximateHash() const
9898
if (!codeOrigin.inlineCallFrame)
9999
return result;
100100

101-
result += WTF::PtrHash<JSCell*>::hash(codeOrigin.inlineCallFrame->executable.get());
101+
result += WTF::PtrHash<JSCell*>::hash(codeOrigin.inlineCallFrame->baselineCodeBlock.get());
102102

103103
codeOrigin = codeOrigin.inlineCallFrame->directCaller;
104104
}
@@ -115,11 +115,11 @@ Vector<CodeOrigin> CodeOrigin::inlineStack() const
115115
return result;
116116
}
117117

118-
ScriptExecutable* CodeOrigin::codeOriginOwner() const
118+
CodeBlock* CodeOrigin::codeOriginOwner() const
119119
{
120120
if (!inlineCallFrame)
121121
return 0;
122-
return inlineCallFrame->executable.get();
122+
return inlineCallFrame->baselineCodeBlock.get();
123123
}
124124

125125
int CodeOrigin::stackOffset() const
@@ -143,7 +143,7 @@ void CodeOrigin::dump(PrintStream& out) const
143143
out.print(" --> ");
144144

145145
if (InlineCallFrame* frame = stack[i].inlineCallFrame) {
146-
out.print(frame->briefFunctionInformation(), ":<", RawPointer(frame->executable.get()), "> ");
146+
out.print(frame->briefFunctionInformation(), ":<", RawPointer(frame->baselineCodeBlock.get()), "> ");
147147
if (frame->isClosureCall)
148148
out.print("(closure) ");
149149
}

‎Source/JavaScriptCore/bytecode/CodeOrigin.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ struct CodeOrigin {
8787

8888
// If the code origin corresponds to inlined code, gives you the heap object that
8989
// would have owned the code if it had not been inlined. Otherwise returns 0.
90-
ScriptExecutable* codeOriginOwner() const;
90+
CodeBlock* codeOriginOwner() const;
9191

9292
int stackOffset() const;
9393

‎Source/JavaScriptCore/bytecode/DeferredCompilationCallback.cpp‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,15 +33,13 @@ namespace JSC {
3333
DeferredCompilationCallback::DeferredCompilationCallback() { }
3434
DeferredCompilationCallback::~DeferredCompilationCallback() { }
3535

36-
void DeferredCompilationCallback::compilationDidComplete(CodeBlock* codeBlock, CompilationResult result)
36+
void DeferredCompilationCallback::compilationDidComplete(CodeBlock*, CodeBlock*, CompilationResult result)
3737
{
3838
dumpCompiledSourcesIfNeeded();
3939

4040
switch (result) {
4141
case CompilationFailed:
4242
case CompilationInvalidated:
43-
codeBlock->heap()->removeCodeBlock(codeBlock);
44-
break;
4543
case CompilationSuccessful:
4644
break;
4745
case CompilationDeferred:

‎Source/JavaScriptCore/bytecode/DeferredCompilationCallback.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,8 +42,8 @@ class DeferredCompilationCallback : public RefCounted<DeferredCompilationCallbac
4242
public:
4343
virtual ~DeferredCompilationCallback();
4444

45-
virtual void compilationDidBecomeReadyAsynchronously(CodeBlock*) = 0;
46-
virtual void compilationDidComplete(CodeBlock*, CompilationResult);
45+
virtual void compilationDidBecomeReadyAsynchronously(CodeBlock*, CodeBlock* profiledDFGCodeBlock) = 0;
46+
virtual void compilationDidComplete(CodeBlock*, CodeBlock* profiledDFGCodeBlock, CompilationResult);
4747

4848
Vector<DeferredSourceDump>& ensureDeferredSourceDump();
4949

‎Source/JavaScriptCore/bytecode/EvalCodeCache.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ namespace JSC {
5151
return 0;
5252
}
5353

54-
EvalExecutable* getSlow(ExecState* exec, ScriptExecutable* owner, bool inStrictContext, ThisTDZMode thisTDZMode, const String& evalSource, JSScope* scope)
54+
EvalExecutable* getSlow(ExecState* exec, JSCell* owner, bool inStrictContext, ThisTDZMode thisTDZMode, const String& evalSource, JSScope* scope)
5555
{
5656
VariableEnvironment variablesUnderTDZ;
5757
JSScope::collectVariablesUnderTDZ(scope, variablesUnderTDZ);

‎Source/JavaScriptCore/bytecode/InlineCallFrame.cpp‎

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -47,24 +47,17 @@ JSFunction* InlineCallFrame::calleeForCallFrame(ExecState* exec) const
4747

4848
CodeBlockHash InlineCallFrame::hash() const
4949
{
50-
return jsCast<FunctionExecutable*>(executable.get())->codeBlockFor(
51-
specializationKind())->hash();
50+
return baselineCodeBlock->hash();
5251
}
5352

5453
CString InlineCallFrame::hashAsStringIfPossible() const
5554
{
56-
return jsCast<FunctionExecutable*>(executable.get())->codeBlockFor(
57-
specializationKind())->hashAsStringIfPossible();
55+
return baselineCodeBlock->hashAsStringIfPossible();
5856
}
5957

6058
CString InlineCallFrame::inferredName() const
6159
{
62-
return jsCast<FunctionExecutable*>(executable.get())->inferredName().utf8();
63-
}
64-
65-
CodeBlock* InlineCallFrame::baselineCodeBlock() const
66-
{
67-
return jsCast<FunctionExecutable*>(executable.get())->baselineCodeBlockFor(specializationKind());
60+
return jsCast<FunctionExecutable*>(baselineCodeBlock->ownerExecutable())->inferredName().utf8();
6861
}
6962

7063
void InlineCallFrame::dumpBriefFunctionInformation(PrintStream& out) const
@@ -74,8 +67,8 @@ void InlineCallFrame::dumpBriefFunctionInformation(PrintStream& out) const
7467

7568
void InlineCallFrame::dumpInContext(PrintStream& out, DumpContext* context) const
7669
{
77-
out.print(briefFunctionInformation(), ":<", RawPointer(executable.get()));
78-
if (executable->isStrictMode())
70+
out.print(briefFunctionInformation(), ":<", RawPointer(baselineCodeBlock.get()));
71+
if (isStrictMode())
7972
out.print(" (StrictMode)");
8073
out.print(", bc#", directCaller.bytecodeIndex, ", ", static_cast<Kind>(kind));
8174
if (isClosureCall)

‎Source/JavaScriptCore/bytecode/InlineCallFrame.h‎

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@
2929
#include "CodeBlock.h"
3030
#include "CodeBlockHash.h"
3131
#include "CodeOrigin.h"
32-
#include "Executable.h"
3332
#include "ValueRecovery.h"
3433
#include "WriteBarrier.h"
3534
#include <wtf/BitVector.h>
@@ -42,7 +41,6 @@ namespace JSC {
4241

4342
struct InlineCallFrame;
4443
class ExecState;
45-
class ScriptExecutable;
4644
class JSFunction;
4745

4846
struct InlineCallFrame {
@@ -174,7 +172,7 @@ struct InlineCallFrame {
174172
}
175173

176174
Vector<ValueRecovery> arguments; // Includes 'this'.
177-
WriteBarrier<ScriptExecutable> executable;
175+
WriteBarrier<CodeBlock> baselineCodeBlock;
178176
ValueRecovery calleeRecovery;
179177
CodeOrigin directCaller;
180178

@@ -209,8 +207,6 @@ struct InlineCallFrame {
209207
CodeBlockHash hash() const;
210208
CString hashAsStringIfPossible() const;
211209

212-
CodeBlock* baselineCodeBlock() const;
213-
214210
void setStackOffset(signed offset)
215211
{
216212
stackOffset = offset;
@@ -220,6 +216,8 @@ struct InlineCallFrame {
220216
ptrdiff_t callerFrameOffset() const { return stackOffset * sizeof(Register) + CallFrame::callerFrameOffset(); }
221217
ptrdiff_t returnPCOffset() const { return stackOffset * sizeof(Register) + CallFrame::returnPCOffset(); }
222218

219+
bool isStrictMode() const { return baselineCodeBlock->isStrictMode(); }
220+
223221
void dumpBriefFunctionInformation(PrintStream&) const;
224222
void dump(PrintStream&) const;
225223
void dumpInContext(PrintStream&, DumpContext*) const;
@@ -231,9 +229,7 @@ struct InlineCallFrame {
231229
inline CodeBlock* baselineCodeBlockForInlineCallFrame(InlineCallFrame* inlineCallFrame)
232230
{
233231
RELEASE_ASSERT(inlineCallFrame);
234-
ScriptExecutable* executable = inlineCallFrame->executable.get();
235-
RELEASE_ASSERT(executable->structure()->classInfo() == FunctionExecutable::info());
236-
return static_cast<FunctionExecutable*>(executable)->baselineCodeBlockFor(inlineCallFrame->specializationKind());
232+
return inlineCallFrame->baselineCodeBlock.get();
237233
}
238234

239235
inline CodeBlock* baselineCodeBlockForOriginAndBaselineCodeBlock(const CodeOrigin& codeOrigin, CodeBlock* baselineCodeBlock)

0 commit comments

Comments
 (0)