Diff
Modified: trunk/JSTests/ChangeLog (228365 => 228366)
--- trunk/JSTests/ChangeLog 2018-02-10 15:44:55 UTC (rev 228365)
+++ trunk/JSTests/ChangeLog 2018-02-10 23:49:54 UTC (rev 228366)
@@ -1,3 +1,15 @@
+2018-02-09 Filip Pizlo <fpi...@apple.com>
+
+ Don't waste memory for error.stack
+ https://bugs.webkit.org/show_bug.cgi?id=182656
+
+ Reviewed by Saam Barati.
+
+ Tests the policy.
+
+ * stress/gc-error-stack.js: Added. Shows that the GC forgets frames now.
+ * stress/no-gc-error-stack.js: Added. Shows that the GC won't forget things if you ask for the stack.
+
2018-02-08 Yusuke Suzuki <utatane....@gmail.com>
[JSC] Update Test262 to Feb 9 version
Added: trunk/JSTests/stress/gc-error-stack.js (0 => 228366)
--- trunk/JSTests/stress/gc-error-stack.js (rev 0)
+++ trunk/JSTests/stress/gc-error-stack.js 2018-02-10 23:49:54 UTC (rev 228366)
@@ -0,0 +1,18 @@
+"use strict";
+
+var errors = [];
+
+for (let i = 0; i < 1000; ++i)
+ eval("(function foo" + i + "() { errors.push(new Error()); })();");
+
+gc();
+
+let didGCAtLeastOne = false;
+for (let error of errors) {
+ if (!error.stack.startsWith("foo"))
+ didGCAtLeastOne = true;
+}
+
+if (!didGCAtLeastOne)
+ throw new Error("Bad result: didGCAtLeastOne = " + didGCAtLeastOne);
+
Added: trunk/JSTests/stress/no-gc-error-stack.js (0 => 228366)
--- trunk/JSTests/stress/no-gc-error-stack.js (rev 0)
+++ trunk/JSTests/stress/no-gc-error-stack.js 2018-02-10 23:49:54 UTC (rev 228366)
@@ -0,0 +1,22 @@
+//@ runDefault
+
+"use strict";
+
+var errors = [];
+
+for (let i = 0; i < 1000; ++i)
+ eval("(function foo" + i + "() { errors.push(new Error()); })();");
+
+for (let error of errors)
+ error.stack;
+
+gc();
+
+for (let error of errors) {
+ if (!error.stack.startsWith("foo")) {
+ print("ERROR: Stack does not begin with foo: " + error.stack);
+ throw new Error("fail");
+ }
+}
+
+
Modified: trunk/Source/_javascript_Core/ChangeLog (228365 => 228366)
--- trunk/Source/_javascript_Core/ChangeLog 2018-02-10 15:44:55 UTC (rev 228365)
+++ trunk/Source/_javascript_Core/ChangeLog 2018-02-10 23:49:54 UTC (rev 228366)
@@ -1,3 +1,44 @@
+2018-02-09 Filip Pizlo <fpi...@apple.com>
+
+ Don't waste memory for error.stack
+ https://bugs.webkit.org/show_bug.cgi?id=182656
+
+ Reviewed by Saam Barati.
+
+ This makes the StackFrames in ErrorInstance and Exception weak. We simply forget their
+ contents if we GC.
+
+ This isn't going to happen under normal operation since your callees and code blocks will
+ still be alive when you ask for .stack.
+
+ Bug 182650 tracks improving this so that it's not lossy. For now, I think it's worth it,
+ since it is likely to recover 3-5 MB on membuster.
+
+ * heap/Heap.cpp:
+ (JSC::Heap::finalizeUnconditionalFinalizers):
+ * runtime/ErrorInstance.cpp:
+ (JSC::ErrorInstance::visitChildren):
+ (JSC::ErrorInstance::finalizeUnconditionally):
+ * runtime/ErrorInstance.h:
+ (JSC::ErrorInstance::subspaceFor):
+ * runtime/Exception.cpp:
+ (JSC::Exception::visitChildren):
+ (JSC::Exception::finalizeUnconditionally):
+ * runtime/Exception.h:
+ (JSC::Exception::valueOffset): Deleted.
+ (JSC::Exception::value const): Deleted.
+ (JSC::Exception::stack const): Deleted.
+ (JSC::Exception::didNotifyInspectorOfThrow const): Deleted.
+ (JSC::Exception::setDidNotifyInspectorOfThrow): Deleted.
+ * runtime/StackFrame.cpp:
+ (JSC::StackFrame::isFinalizationCandidate):
+ (JSC::StackFrame::finalizeUnconditionally):
+ (JSC::StackFrame::visitChildren): Deleted.
+ * runtime/StackFrame.h:
+ * runtime/VM.cpp:
+ (JSC::VM::VM):
+ * runtime/VM.h:
+
2018-02-09 Carlos Alberto Lopez Perez <clo...@igalia.com>
Fix build on ARMv7 traditional JSCOnly bot after r228306
Modified: trunk/Source/_javascript_Core/heap/Heap.cpp (228365 => 228366)
--- trunk/Source/_javascript_Core/heap/Heap.cpp 2018-02-10 15:44:55 UTC (rev 228365)
+++ trunk/Source/_javascript_Core/heap/Heap.cpp 2018-02-10 23:49:54 UTC (rev 228366)
@@ -579,6 +579,8 @@
void Heap::finalizeUnconditionalFinalizers()
{
+ finalizeMarkedUnconditionalFinalizers<ErrorInstance>(vm()->errorInstancesWithFinalizers);
+ finalizeMarkedUnconditionalFinalizers<Exception>(vm()->exceptionsWithFinalizers);
finalizeMarkedUnconditionalFinalizers<InferredType>(vm()->inferredTypesWithFinalizers);
finalizeMarkedUnconditionalFinalizers<InferredValue>(vm()->inferredValuesWithFinalizers);
vm()->forEachCodeBlockSpace(
Modified: trunk/Source/_javascript_Core/runtime/ErrorInstance.cpp (228365 => 228366)
--- trunk/Source/_javascript_Core/runtime/ErrorInstance.cpp 2018-02-10 15:44:55 UTC (rev 228365)
+++ trunk/Source/_javascript_Core/runtime/ErrorInstance.cpp 2018-02-10 23:49:54 UTC (rev 228366)
@@ -1,6 +1,6 @@
/*
* Copyright (C) 1999-2000 Harri Porten (por...@kde.org)
- * Copyright (C) 2003-2017 Apple Inc. All rights reserved.
+ * Copyright (C) 2003-2018 Apple Inc. All rights reserved.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
@@ -23,6 +23,7 @@
#include "CodeBlock.h"
#include "InlineCallFrame.h"
+#include "IsoCellSetInlines.h"
#include "JSScope.h"
#include "JSCInlines.h"
#include "ParseInt.h"
@@ -233,15 +234,35 @@
ASSERT_GC_OBJECT_INHERITS(thisObject, info());
Base::visitChildren(thisObject, visitor);
+ bool isFinalizationCandidate = false;
{
auto locker = holdLock(thisObject->cellLock());
if (thisObject->m_stackTrace) {
- for (StackFrame& frame : *thisObject->m_stackTrace)
- frame.visitChildren(visitor);
+ for (StackFrame& frame : *thisObject->m_stackTrace) {
+ if (frame.isFinalizationCandidate()) {
+ isFinalizationCandidate = true;
+ break;
+ }
+ }
}
}
+ if (isFinalizationCandidate)
+ visitor.vm().errorInstancesWithFinalizers.add(thisObject);
}
+void ErrorInstance::finalizeUnconditionally(VM& vm)
+{
+ {
+ auto locker = holdLock(cellLock());
+ if (m_stackTrace) {
+ for (StackFrame& frame : *m_stackTrace)
+ frame.finalizeUnconditionally(vm);
+ }
+ }
+
+ vm.errorInstancesWithFinalizers.remove(this);
+}
+
bool ErrorInstance::getOwnPropertySlot(JSObject* object, ExecState* exec, PropertyName propertyName, PropertySlot& slot)
{
VM& vm = exec->vm();
Modified: trunk/Source/_javascript_Core/runtime/ErrorInstance.h (228365 => 228366)
--- trunk/Source/_javascript_Core/runtime/ErrorInstance.h 2018-02-10 15:44:55 UTC (rev 228365)
+++ trunk/Source/_javascript_Core/runtime/ErrorInstance.h 2018-02-10 23:49:54 UTC (rev 228366)
@@ -1,6 +1,6 @@
/*
* Copyright (C) 1999-2000 Harri Porten (por...@kde.org)
- * Copyright (C) 2008-2017 Apple Inc. All rights reserved.
+ * Copyright (C) 2008-2018 Apple Inc. All rights reserved.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
@@ -26,8 +26,17 @@
namespace JSC {
+// FIXME: This should be final, but isn't because of bizarre (and mostly wrong) things done in
+// WebAssembly.
+// https://bugs.webkit.org/show_bug.cgi?id=182649
class ErrorInstance : public JSDestructibleObject {
public:
+ template<typename CellType>
+ static IsoSubspace* subspaceFor(VM& vm)
+ {
+ return &vm.errorInstanceSpace;
+ }
+
typedef JSDestructibleObject Base;
const static unsigned StructureFlags = Base::StructureFlags | OverridesGetOwnPropertySlot | OverridesGetPropertyNames;
@@ -68,6 +77,8 @@
JS_EXPORT_PRIVATE String sanitizedToString(ExecState*);
Vector<StackFrame>* stackTrace() { return m_stackTrace.get(); }
+
+ void finalizeUnconditionally(VM&);
bool materializeErrorInfoIfNeeded(VM&);
bool materializeErrorInfoIfNeeded(VM&, PropertyName);
Modified: trunk/Source/_javascript_Core/runtime/Exception.cpp (228365 => 228366)
--- trunk/Source/_javascript_Core/runtime/Exception.cpp 2018-02-10 15:44:55 UTC (rev 228365)
+++ trunk/Source/_javascript_Core/runtime/Exception.cpp 2018-02-10 23:49:54 UTC (rev 228366)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2015-2017 Apple Inc. All rights reserved.
+ * Copyright (C) 2015-2018 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -27,6 +27,7 @@
#include "Exception.h"
#include "Interpreter.h"
+#include "IsoCellSetInlines.h"
#include "JSCInlines.h"
namespace JSC {
@@ -57,11 +58,26 @@
ASSERT_GC_OBJECT_INHERITS(thisObject, info());
Base::visitChildren(thisObject, visitor);
+ bool isFinalizationCandidate = false;
visitor.append(thisObject->m_value);
- for (StackFrame& frame : thisObject->m_stack)
- frame.visitChildren(visitor);
+ for (StackFrame& frame : thisObject->m_stack) {
+ if (frame.isFinalizationCandidate()) {
+ isFinalizationCandidate = true;
+ break;
+ }
+ }
+ if (isFinalizationCandidate)
+ visitor.vm().exceptionsWithFinalizers.add(thisObject);
}
+void Exception::finalizeUnconditionally(VM& vm)
+{
+ for (StackFrame& frame : m_stack)
+ frame.finalizeUnconditionally(vm);
+
+ vm.exceptionsWithFinalizers.remove(this);
+}
+
Exception::Exception(VM& vm)
: Base(vm, vm.exceptionStructure.get())
{
Modified: trunk/Source/_javascript_Core/runtime/Exception.h (228365 => 228366)
--- trunk/Source/_javascript_Core/runtime/Exception.h 2018-02-10 15:44:55 UTC (rev 228365)
+++ trunk/Source/_javascript_Core/runtime/Exception.h 2018-02-10 23:49:54 UTC (rev 228366)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2015-2017 Apple Inc. All rights reserved.
+ * Copyright (C) 2015-2018 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -31,8 +31,14 @@
namespace JSC {
-class Exception : public JSDestructibleObject {
+class Exception final : public JSDestructibleObject {
public:
+ template<typename CellType>
+ static IsoSubspace* subspaceFor(VM& vm)
+ {
+ return &vm.exceptionSpace;
+ }
+
typedef JSDestructibleObject Base;
static const unsigned StructureFlags = StructureIsImmortal | Base::StructureFlags;
@@ -60,6 +66,8 @@
void setDidNotifyInspectorOfThrow() { m_didNotifyInspectorOfThrow = true; }
~Exception();
+
+ void finalizeUnconditionally(VM&);
private:
Exception(VM&);
Modified: trunk/Source/_javascript_Core/runtime/StackFrame.cpp (228365 => 228366)
--- trunk/Source/_javascript_Core/runtime/StackFrame.cpp 2018-02-10 15:44:55 UTC (rev 228365)
+++ trunk/Source/_javascript_Core/runtime/StackFrame.cpp 2018-02-10 23:49:54 UTC (rev 228366)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2016-2017 Apple Inc. All rights reserved.
+ * Copyright (C) 2016-2018 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -142,13 +142,28 @@
return traceBuild.toString().impl();
}
-void StackFrame::visitChildren(SlotVisitor& visitor)
+bool StackFrame::isFinalizationCandidate()
{
- if (m_callee)
- visitor.append(m_callee);
- if (m_codeBlock)
- visitor.append(m_codeBlock);
+ if (m_callee && !Heap::isMarked(m_callee.get()))
+ return true;
+ if (m_codeBlock && !Heap::isMarked(m_codeBlock.get()))
+ return true;
+ return false;
}
+void StackFrame::finalizeUnconditionally(VM&)
+{
+ // FIXME: We should do something smarter. For example, if this happens, we could stringify the
+ // whole stack trace. The main shortcoming is that that requires doing operations that are not
+ // currently legal during finalization. We could make this work by giving JSC a proper "second
+ // chance" finalizer infrastructure. Or maybe there's an even easier way.
+ // https://bugs.webkit.org/show_bug.cgi?id=182650
+
+ if (m_callee && !Heap::isMarked(m_callee.get()))
+ m_callee.clear();
+ if (m_codeBlock && !Heap::isMarked(m_codeBlock.get()))
+ m_codeBlock.clear();
+}
+
} // namespace JSC
Modified: trunk/Source/_javascript_Core/runtime/StackFrame.h (228365 => 228366)
--- trunk/Source/_javascript_Core/runtime/StackFrame.h 2018-02-10 15:44:55 UTC (rev 228365)
+++ trunk/Source/_javascript_Core/runtime/StackFrame.h 2018-02-10 23:49:54 UTC (rev 228366)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2016-2017 Apple Inc. All rights reserved.
+ * Copyright (C) 2016-2018 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -56,7 +56,8 @@
return m_bytecodeOffset;
}
- void visitChildren(SlotVisitor&);
+ bool isFinalizationCandidate();
+ void finalizeUnconditionally(VM&);
private:
WriteBarrier<JSCell> m_callee { };
Modified: trunk/Source/_javascript_Core/runtime/VM.cpp (228365 => 228366)
--- trunk/Source/_javascript_Core/runtime/VM.cpp 2018-02-10 15:44:55 UTC (rev 228365)
+++ trunk/Source/_javascript_Core/runtime/VM.cpp 2018-02-10 23:49:54 UTC (rev 228366)
@@ -251,6 +251,8 @@
, webAssemblyCodeBlockSpace("JSWebAssemblyCodeBlockSpace", heap, webAssemblyCodeBlockHeapCellType.get(), fastMallocAllocator.get())
#endif
, directEvalExecutableSpace ISO_SUBSPACE_INIT(heap, destructibleCellHeapCellType.get(), DirectEvalExecutable)
+ , errorInstanceSpace ISO_SUBSPACE_INIT(heap, destructibleObjectHeapCellType.get(), ErrorInstance)
+ , exceptionSpace ISO_SUBSPACE_INIT(heap, destructibleObjectHeapCellType.get(), Exception)
, executableToCodeBlockEdgeSpace ISO_SUBSPACE_INIT(heap, cellHeapCellType.get(), ExecutableToCodeBlockEdge)
, functionExecutableSpace ISO_SUBSPACE_INIT(heap, destructibleCellHeapCellType.get(), FunctionExecutable)
, indirectEvalExecutableSpace ISO_SUBSPACE_INIT(heap, destructibleCellHeapCellType.get(), IndirectEvalExecutable)
@@ -264,6 +266,8 @@
, structureSpace ISO_SUBSPACE_INIT(heap, destructibleCellHeapCellType.get(), Structure)
, weakSetSpace ISO_SUBSPACE_INIT(heap, destructibleObjectHeapCellType.get(), JSWeakSet)
, weakMapSpace ISO_SUBSPACE_INIT(heap, destructibleObjectHeapCellType.get(), JSWeakMap)
+ , errorInstancesWithFinalizers(errorInstanceSpace)
+ , exceptionsWithFinalizers(exceptionSpace)
, executableToCodeBlockEdgesWithConstraints(executableToCodeBlockEdgeSpace)
, executableToCodeBlockEdgesWithFinalizers(executableToCodeBlockEdgeSpace)
, inferredTypesWithFinalizers(inferredTypeSpace)
Modified: trunk/Source/_javascript_Core/runtime/VM.h (228365 => 228366)
--- trunk/Source/_javascript_Core/runtime/VM.h 2018-02-10 15:44:55 UTC (rev 228365)
+++ trunk/Source/_javascript_Core/runtime/VM.h 2018-02-10 23:49:54 UTC (rev 228366)
@@ -339,6 +339,8 @@
#endif
IsoSubspace directEvalExecutableSpace;
+ IsoSubspace errorInstanceSpace;
+ IsoSubspace exceptionSpace;
IsoSubspace executableToCodeBlockEdgeSpace;
IsoSubspace functionExecutableSpace;
IsoSubspace indirectEvalExecutableSpace;
@@ -353,6 +355,8 @@
IsoSubspace weakSetSpace;
IsoSubspace weakMapSpace;
+ IsoCellSet errorInstancesWithFinalizers;
+ IsoCellSet exceptionsWithFinalizers;
IsoCellSet executableToCodeBlockEdgesWithConstraints;
IsoCellSet executableToCodeBlockEdgesWithFinalizers;
IsoCellSet inferredTypesWithFinalizers;
