Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Heap::writeBarrier shouldn't be static
https://bugs.webkit.org/show_bug.cgi?id=127807

Reviewed by Geoffrey Garen.

Currently it looks up the Heap in which to fire the write barrier by using
the cell passed to it. Almost every call site already has a reference to the
VM or the Heap itself. It seems wasteful to look it up all over again.

Source/JavaScriptCore:

* GNUmakefile.list.am:
* JavaScriptCore.vcxproj/JavaScriptCore.vcxproj:
* JavaScriptCore.vcxproj/JavaScriptCore.vcxproj.filters:
* JavaScriptCore.xcodeproj/project.pbxproj:
* heap/CopyWriteBarrier.h:
(JSC::CopyWriteBarrier::set):
* heap/Heap.cpp:
(JSC::Heap::writeBarrier):
* heap/Heap.h:
(JSC::Heap::writeBarrier):
* jit/JITOperations.cpp:
* jit/JITWriteBarrier.h:
(JSC::JITWriteBarrierBase::set):
* llint/LLIntSlowPaths.cpp:
(JSC::LLInt::llint_write_barrier_slow):
* runtime/Arguments.h:
* runtime/JSWeakMap.cpp:
* runtime/MapData.cpp:
(JSC::MapData::ensureSpaceForAppend):
* runtime/PropertyTable.cpp:
(JSC::PropertyTable::PropertyTable):
* runtime/Structure.h:
* runtime/WriteBarrier.h:
* runtime/WriteBarrierInlines.h: Added.

Source/WebCore:

* bindings/js/JSEventListener.cpp:
(WebCore::JSEventListener::JSEventListener):
* bindings/js/JSEventListener.h:
(WebCore::JSEventListener::jsFunction):


Canonical link: https://commits.webkit.org/146325@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@163542 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
Mark Hahnenberg committed Feb 6, 2014
1 parent d684805 commit a0e2438
Show file tree
Hide file tree
Showing 21 changed files with 154 additions and 44 deletions.
36 changes: 36 additions & 0 deletions Source/JavaScriptCore/ChangeLog
@@ -1,3 +1,39 @@
2014-02-05 Mark Hahnenberg <mhahnenberg@apple.com>

Heap::writeBarrier shouldn't be static
https://bugs.webkit.org/show_bug.cgi?id=127807

Reviewed by Geoffrey Garen.

Currently it looks up the Heap in which to fire the write barrier by using
the cell passed to it. Almost every call site already has a reference to the
VM or the Heap itself. It seems wasteful to look it up all over again.

* GNUmakefile.list.am:
* JavaScriptCore.vcxproj/JavaScriptCore.vcxproj:
* JavaScriptCore.vcxproj/JavaScriptCore.vcxproj.filters:
* JavaScriptCore.xcodeproj/project.pbxproj:
* heap/CopyWriteBarrier.h:
(JSC::CopyWriteBarrier::set):
* heap/Heap.cpp:
(JSC::Heap::writeBarrier):
* heap/Heap.h:
(JSC::Heap::writeBarrier):
* jit/JITOperations.cpp:
* jit/JITWriteBarrier.h:
(JSC::JITWriteBarrierBase::set):
* llint/LLIntSlowPaths.cpp:
(JSC::LLInt::llint_write_barrier_slow):
* runtime/Arguments.h:
* runtime/JSWeakMap.cpp:
* runtime/MapData.cpp:
(JSC::MapData::ensureSpaceForAppend):
* runtime/PropertyTable.cpp:
(JSC::PropertyTable::PropertyTable):
* runtime/Structure.h:
* runtime/WriteBarrier.h:
* runtime/WriteBarrierInlines.h: Added.

2014-02-04 Filip Pizlo <fpizlo@apple.com>

Make FTL OSR entry something we only try after we've already compiled the function with the FTL and it still got stuck in a loop after that without ever returning like a sensible function oughta have
Expand Down
1 change: 1 addition & 0 deletions Source/JavaScriptCore/GNUmakefile.list.am
Expand Up @@ -1221,6 +1221,7 @@ javascriptcore_sources += \
Source/JavaScriptCore/runtime/WeakMapPrototype.h \
Source/JavaScriptCore/runtime/WeakRandom.h \
Source/JavaScriptCore/runtime/WriteBarrier.h \
Source/JavaScriptCore/runtime/WriteBarrierInlines.h \
Source/JavaScriptCore/tools/CodeProfile.cpp \
Source/JavaScriptCore/tools/CodeProfile.h \
Source/JavaScriptCore/tools/CodeProfiling.cpp \
Expand Down
Expand Up @@ -1388,6 +1388,7 @@
<ClInclude Include="..\runtime\WeakMapPrototype.h" />
<ClInclude Include="..\runtime\WeakRandom.h" />
<ClInclude Include="..\runtime\WriteBarrier.h" />
<ClInclude Include="..\runtime\WriteBarrierInlines.h" />
<ClInclude Include="..\tools\CodeProfile.h" />
<ClInclude Include="..\tools\CodeProfiling.h" />
<ClInclude Include="..\tools\ProfileTreeNode.h" />
Expand Down
Expand Up @@ -2438,6 +2438,9 @@
<ClInclude Include="..\runtime\WriteBarrier.h">
<Filter>runtime</Filter>
</ClInclude>
<ClInclude Include="..\runtime\WriteBarrierInlines.h">
<Filter>runtime</Filter>
</ClInclude>
<ClInclude Include="..\tools\CodeProfile.h">
<Filter>tools</Filter>
</ClInclude>
Expand Down
Expand Up @@ -1333,6 +1333,7 @@
C2981FDD17BAFF4400A3BC98 /* DFGDesiredWriteBarriers.h in Headers */ = {isa = PBXBuildFile; fileRef = C2981FDB17BAFF4400A3BC98 /* DFGDesiredWriteBarriers.h */; settings = {ATTRIBUTES = (Private, ); }; };
C29ECB031804D0ED00D2CBB4 /* CurrentThisInsideBlockGetterTest.mm in Sources */ = {isa = PBXBuildFile; fileRef = C29ECB011804D0ED00D2CBB4 /* CurrentThisInsideBlockGetterTest.mm */; };
C2A7F688160432D400F76B98 /* JSDestructibleObject.h in Headers */ = {isa = PBXBuildFile; fileRef = C2A7F687160432D400F76B98 /* JSDestructibleObject.h */; settings = {ATTRIBUTES = (Private, ); }; };
C2B6D75318A33793004A9301 /* WriteBarrierInlines.h in Headers */ = {isa = PBXBuildFile; fileRef = C2B6D75218A33793004A9301 /* WriteBarrierInlines.h */; settings = {ATTRIBUTES = (Private, ); }; };
C2B916C214DA014E00CBAC86 /* MarkedAllocator.h in Headers */ = {isa = PBXBuildFile; fileRef = C2B916C114DA014E00CBAC86 /* MarkedAllocator.h */; settings = {ATTRIBUTES = (Private, ); }; };
C2B916C514DA040C00CBAC86 /* MarkedAllocator.cpp in Sources */ = {isa = PBXBuildFile; fileRef = C2B916C414DA040C00CBAC86 /* MarkedAllocator.cpp */; };
C2C0F7CD17BBFC5B00464FE4 /* DFGDesiredTransitions.cpp in Sources */ = {isa = PBXBuildFile; fileRef = C2C0F7CB17BBFC5B00464FE4 /* DFGDesiredTransitions.cpp */; };
Expand Down Expand Up @@ -2765,6 +2766,7 @@
C29ECB011804D0ED00D2CBB4 /* CurrentThisInsideBlockGetterTest.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; name = CurrentThisInsideBlockGetterTest.mm; path = API/tests/CurrentThisInsideBlockGetterTest.mm; sourceTree = "<group>"; };
C29ECB021804D0ED00D2CBB4 /* CurrentThisInsideBlockGetterTest.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = CurrentThisInsideBlockGetterTest.h; path = API/tests/CurrentThisInsideBlockGetterTest.h; sourceTree = "<group>"; };
C2A7F687160432D400F76B98 /* JSDestructibleObject.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = JSDestructibleObject.h; sourceTree = "<group>"; };
C2B6D75218A33793004A9301 /* WriteBarrierInlines.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = WriteBarrierInlines.h; sourceTree = "<group>"; };
C2B916C114DA014E00CBAC86 /* MarkedAllocator.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = MarkedAllocator.h; sourceTree = "<group>"; };
C2B916C414DA040C00CBAC86 /* MarkedAllocator.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = MarkedAllocator.cpp; sourceTree = "<group>"; };
C2C0F7CB17BBFC5B00464FE4 /* DFGDesiredTransitions.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = DFGDesiredTransitions.cpp; path = dfg/DFGDesiredTransitions.cpp; sourceTree = "<group>"; };
Expand Down Expand Up @@ -3624,6 +3626,7 @@
7EF6E0BB0EB7A1EC0079AFAF /* runtime */ = {
isa = PBXGroup;
children = (
C2B6D75218A33793004A9301 /* WriteBarrierInlines.h */,
BCF605110E203EF800B9A64D /* ArgList.cpp */,
BCF605120E203EF800B9A64D /* ArgList.h */,
BC257DE50E1F51C50016B6C9 /* Arguments.cpp */,
Expand Down Expand Up @@ -4641,6 +4644,7 @@
65C0285D1717966800351E35 /* ARMv7DOpcode.h in Headers */,
2A68295B1875F80500B6C3E2 /* CopyWriteBarrier.h in Headers */,
2A4EC90C1860D6C20094F782 /* WriteBarrierBuffer.h in Headers */,
C2B6D75318A33793004A9301 /* WriteBarrierInlines.h in Headers */,
FE5932A8183C5A2600A1ECCC /* VMEntryScope.h in Headers */,
A532439318569709002ED692 /* CodeGeneratorInspectorStrings.py in Headers */,
A532439218569709002ED692 /* CodeGeneratorInspector.py in Headers */,
Expand Down
4 changes: 2 additions & 2 deletions Source/JavaScriptCore/heap/CopyWriteBarrier.h
Expand Up @@ -68,10 +68,10 @@ class CopyWriteBarrier {
return get();
}

void set(VM&, const JSCell* owner, T* value)
void set(VM& vm, const JSCell* owner, T* value)
{
this->m_value = value;
Heap::writeBarrier(owner);
vm.heap.writeBarrier(owner);
}

void setWithoutWriteBarrier(T* value)
Expand Down
3 changes: 1 addition & 2 deletions Source/JavaScriptCore/heap/Heap.cpp
Expand Up @@ -1087,8 +1087,7 @@ void Heap::writeBarrier(const JSCell* from)
ASSERT_GC_OBJECT_LOOKS_VALID(const_cast<JSCell*>(from));
if (!from || !isMarked(from))
return;
Heap* heap = Heap::heap(from);
heap->addToRememberedSet(from);
addToRememberedSet(from);
#else
UNUSED_PARAM(from);
#endif
Expand Down
8 changes: 4 additions & 4 deletions Source/JavaScriptCore/heap/Heap.h
Expand Up @@ -102,9 +102,9 @@ namespace JSC {
return MarkedBlock::blockFor(cell)->isRemembered(cell);
}
static bool isWriteBarrierEnabled();
JS_EXPORT_PRIVATE static void writeBarrier(const JSCell*);
static void writeBarrier(const JSCell*, JSValue);
static void writeBarrier(const JSCell*, JSCell*);
JS_EXPORT_PRIVATE void writeBarrier(const JSCell*);
void writeBarrier(const JSCell*, JSValue);
void writeBarrier(const JSCell*, JSCell*);

WriteBarrierBuffer& writeBarrierBuffer() { return m_writeBarrierBuffer; }
void flushWriteBarrierBuffer(JSCell*);
Expand Down Expand Up @@ -401,7 +401,7 @@ namespace JSC {
return;
if (!to || isMarked(to))
return;
Heap::heap(from)->addToRememberedSet(from);
addToRememberedSet(from);
}

inline void Heap::writeBarrier(const JSCell* from, JSValue to)
Expand Down
4 changes: 2 additions & 2 deletions Source/JavaScriptCore/jit/JITOperations.cpp
Expand Up @@ -1750,7 +1750,7 @@ void JIT_OPERATION operationOSRWriteBarrier(ExecState* exec, JSCell* cell)
{
VM* vm = &exec->vm();
NativeCallFrameTracer tracer(vm, exec);
exec->heap()->writeBarrier(cell);
vm->heap.writeBarrier(cell);
}

// NB: We don't include the value as part of the barrier because the write barrier elision
Expand All @@ -1760,7 +1760,7 @@ void JIT_OPERATION operationUnconditionalWriteBarrier(ExecState* exec, JSCell* c
{
VM* vm = &exec->vm();
NativeCallFrameTracer tracer(vm, exec);
Heap::writeBarrier(cell);
vm->heap.writeBarrier(cell);
}

void JIT_OPERATION operationInitGlobalConst(ExecState* exec, Instruction* pc)
Expand Down
4 changes: 2 additions & 2 deletions Source/JavaScriptCore/jit/JITWriteBarrier.h
Expand Up @@ -77,9 +77,9 @@ class JITWriteBarrierBase {
{
}

void set(VM&, CodeLocationDataLabelPtr location, JSCell* owner, JSCell* value)
void set(VM& vm, CodeLocationDataLabelPtr location, JSCell* owner, JSCell* value)
{
Heap::writeBarrier(owner, value);
vm.heap.writeBarrier(owner, value);
m_location = location;
ASSERT(((!!m_location) && m_location.executableAddress() != JITWriteBarrierFlag) || (location.executableAddress() == m_location.executableAddress()));
MacroAssembler::repatchPointer(m_location, value);
Expand Down
5 changes: 3 additions & 2 deletions Source/JavaScriptCore/llint/LLIntSlowPaths.cpp
Expand Up @@ -1436,9 +1436,10 @@ extern "C" SlowPathReturnType llint_stack_check_at_vm_entry(VM* vm, Register* ne
}
#endif

extern "C" void llint_write_barrier_slow(ExecState*, JSCell* cell)
extern "C" void llint_write_barrier_slow(ExecState* exec, JSCell* cell)
{
Heap::writeBarrier(cell);
VM& vm = exec->vm();
vm.heap.writeBarrier(cell);
}

} } // namespace JSC::LLInt
Expand Down
1 change: 1 addition & 0 deletions Source/JavaScriptCore/runtime/Arguments.h
Expand Up @@ -31,6 +31,7 @@
#include "JSGlobalObject.h"
#include "Interpreter.h"
#include "ObjectConstructor.h"
#include "WriteBarrierInlines.h"
#include <wtf/StdLibExtras.h>

namespace JSC {
Expand Down
1 change: 1 addition & 0 deletions Source/JavaScriptCore/runtime/JSWeakMap.cpp
Expand Up @@ -29,6 +29,7 @@
#include "JSCJSValueInlines.h"
#include "SlotVisitorInlines.h"
#include "WeakMapData.h"
#include "WriteBarrierInlines.h"

namespace JSC {

Expand Down
2 changes: 1 addition & 1 deletion Source/JavaScriptCore/runtime/MapData.cpp
Expand Up @@ -208,7 +208,7 @@ CheckedBoolean MapData::ensureSpaceForAppend(CallFrame* callFrame)
replaceAndPackBackingStore(newEntries, requiredSize);
else
replaceBackingStore(newEntries, requiredSize);
Heap::writeBarrier(this);
callFrame->heap()->writeBarrier(this);
return true;
}

Expand Down
4 changes: 2 additions & 2 deletions Source/JavaScriptCore/runtime/PropertyTable.cpp
Expand Up @@ -84,7 +84,7 @@ PropertyTable::PropertyTable(VM& vm, JSCell* owner, const PropertyTable& other)
iterator end = this->end();
for (iterator iter = begin(); iter != end; ++iter) {
iter->key->ref();
Heap::writeBarrier(owner, iter->specificValue.get());
vm.heap.writeBarrier(owner, iter->specificValue.get());
}

// Copy the m_deletedOffsets vector.
Expand All @@ -109,7 +109,7 @@ PropertyTable::PropertyTable(VM& vm, JSCell* owner, unsigned initialCapacity, co
ASSERT(canInsert());
reinsert(*iter);
iter->key->ref();
Heap::writeBarrier(owner, iter->specificValue.get());
vm.heap.writeBarrier(owner, iter->specificValue.get());
}

// Copy the m_deletedOffsets vector.
Expand Down
1 change: 1 addition & 0 deletions Source/JavaScriptCore/runtime/Structure.h
Expand Up @@ -42,6 +42,7 @@
#include "JSTypeInfo.h"
#include "Watchpoint.h"
#include "Weak.h"
#include "WriteBarrierInlines.h"
#include <wtf/CompilationThread.h>
#include <wtf/PassRefPtr.h>
#include <wtf/PrintStream.h>
Expand Down
29 changes: 4 additions & 25 deletions Source/JavaScriptCore/runtime/WriteBarrier.h
Expand Up @@ -71,13 +71,7 @@ template<class T> inline void validateCell(T)
// We have a separate base class with no constructors for use in Unions.
template <typename T> class WriteBarrierBase {
public:
void set(VM& vm, const JSCell* owner, T* value)
{
ASSERT(value);
ASSERT(!Options::enableConcurrentJIT() || !isCompilationThread());
validateCell(value);
setEarlyValue(vm, owner, value);
}
void set(VM&, const JSCell* owner, T* value);

// This is meant to be used like operator=, but is called copyFrom instead, in
// order to kindly inform the C++ compiler that its advice is not appreciated.
Expand All @@ -86,20 +80,11 @@ template <typename T> class WriteBarrierBase {
m_cell = other.m_cell;
}

void setMayBeNull(VM& vm, const JSCell* owner, T* value)
{
if (value)
validateCell(value);
setEarlyValue(vm, owner, value);
}
void setMayBeNull(VM&, const JSCell* owner, T* value);

// Should only be used by JSCell during early initialisation
// when some basic types aren't yet completely instantiated
void setEarlyValue(VM&, const JSCell* owner, T* value)
{
this->m_cell = reinterpret_cast<JSCell*>(value);
Heap::writeBarrier(owner, this->m_cell);
}
void setEarlyValue(VM&, const JSCell* owner, T* value);

T* get() const
{
Expand Down Expand Up @@ -151,13 +136,7 @@ template <typename T> class WriteBarrierBase {

template <> class WriteBarrierBase<Unknown> {
public:
void set(VM&, const JSCell* owner, JSValue value)
{
ASSERT(!Options::enableConcurrentJIT() || !isCompilationThread());
m_value = JSValue::encode(value);
Heap::writeBarrier(owner, value);
}

void set(VM&, const JSCell* owner, JSValue);
void setWithoutWriteBarrier(JSValue value)
{
m_value = JSValue::encode(value);
Expand Down
67 changes: 67 additions & 0 deletions Source/JavaScriptCore/runtime/WriteBarrierInlines.h
@@ -0,0 +1,67 @@
/*
* Copyright (C) 2014 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
* are met:
* 1. Redistributions of source code must retain the above copyright
* notice, this list of conditions and the following disclaimer.
* 2. Redistributions in binary form must reproduce the above copyright
* notice, this list of conditions and the following disclaimer in the
* documentation and/or other materials provided with the distribution.
*
* THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS''
* AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
* THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
* PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR ITS CONTRIBUTORS
* BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
* CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
* SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
* INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
* CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
* ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF
* THE POSSIBILITY OF SUCH DAMAGE.
*/

#ifndef WriteBarrierInlines_h
#define WriteBarrierInlines_h

#include "VM.h"
#include "WriteBarrier.h"

namespace JSC {

template <typename T>
inline void WriteBarrierBase<T>::set(VM& vm, const JSCell* owner, T* value)
{
ASSERT(value);
ASSERT(!Options::enableConcurrentJIT() || !isCompilationThread());
validateCell(value);
setEarlyValue(vm, owner, value);
}

template <typename T>
inline void WriteBarrierBase<T>::setMayBeNull(VM& vm, const JSCell* owner, T* value)
{
if (value)
validateCell(value);
setEarlyValue(vm, owner, value);
}

template <typename T>
inline void WriteBarrierBase<T>::setEarlyValue(VM& vm, const JSCell* owner, T* value)
{
this->m_cell = reinterpret_cast<JSCell*>(value);
vm.heap.writeBarrier(owner, this->m_cell);
}

inline void WriteBarrierBase<Unknown>::set(VM& vm, const JSCell* owner, JSValue value)
{
ASSERT(!Options::enableConcurrentJIT() || !isCompilationThread());
m_value = JSValue::encode(value);
vm.heap.writeBarrier(owner, value);
}

} // namespace JSC

#endif // WriteBarrierInlines_h
16 changes: 16 additions & 0 deletions Source/WebCore/ChangeLog
@@ -1,3 +1,19 @@
2014-02-05 Mark Hahnenberg <mhahnenberg@apple.com>

Heap::writeBarrier shouldn't be static
https://bugs.webkit.org/show_bug.cgi?id=127807

Reviewed by Geoffrey Garen.

Currently it looks up the Heap in which to fire the write barrier by using
the cell passed to it. Almost every call site already has a reference to the
VM or the Heap itself. It seems wasteful to look it up all over again.

* bindings/js/JSEventListener.cpp:
(WebCore::JSEventListener::JSEventListener):
* bindings/js/JSEventListener.h:
(WebCore::JSEventListener::jsFunction):

2014-02-06 Brady Eidson <beidson@apple.com>

IDB: storage/indexeddb/mozilla/clear.html fails
Expand Down
2 changes: 1 addition & 1 deletion Source/WebCore/bindings/js/JSEventListener.cpp
Expand Up @@ -47,7 +47,7 @@ JSEventListener::JSEventListener(JSObject* function, JSObject* wrapper, bool isA
, m_isolatedWorld(&isolatedWorld)
{
if (wrapper) {
JSC::Heap::writeBarrier(wrapper, function);
JSC::Heap::heap(wrapper)->writeBarrier(wrapper, function);
m_jsFunction = JSC::Weak<JSC::JSObject>(function);
} else
ASSERT(!function);
Expand Down
2 changes: 1 addition & 1 deletion Source/WebCore/bindings/js/JSEventListener.h
Expand Up @@ -84,7 +84,7 @@ namespace WebCore {

if (!m_jsFunction) {
JSC::JSObject* function = initializeJSFunction(scriptExecutionContext);
JSC::Heap::writeBarrier(m_wrapper.get(), function);
JSC::Heap::heap(m_wrapper.get())->writeBarrier(m_wrapper.get(), function);
m_jsFunction = JSC::Weak<JSC::JSObject>(function);
}

Expand Down

0 comments on commit a0e2438

Please sign in to comment.