Move std::function from JSFunction into NativeStdFunctionCell to correctly destroy...
authorutatane.tea@gmail.com <utatane.tea@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 28 Aug 2015 21:35:39 +0000 (21:35 +0000)
committerutatane.tea@gmail.com <utatane.tea@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 28 Aug 2015 21:35:39 +0000 (21:35 +0000)
commit8caa15795af42fa8e6e615713cabb91a743b8f3e
tree936e4cc237aefbb589fb5a501ca8c0c7dbfc5efb
parent450d6e7472ee1e39b39ccf082b3460ee3ba6a905
Move std::function from JSFunction into NativeStdFunctionCell to correctly destroy the heap allocated std::function
https://bugs.webkit.org/show_bug.cgi?id=148262

Reviewed by Filip Pizlo.

Source/JavaScriptCore:

std::function is heap allocated value. So if this is held in the JSCell, the cell should be destructible.
Before this patch, it is held in the JSStdFunction. JSStdFunction is the derived class from the JSFunction,
and they are not destructible. So it leaked the memory.

This patch extracts std::function field from the JSStdFunction to the NativeStdFunctionCell. NativeStdFunctionCell
is responsible for destructing the held std::function.
Instead of moving std::function to the ExecutableBase, we move it to the newly created NativeStdFunctionCell cell.
The reason is the following.

- Each NativeExecutable (in 64_32 JIT environment) has the trampolines to call given host functions.
  And the address of the host function is directly embedded on the JIT-compiled trampoline code.
- To suppress the overuse of the executable memory (which is used to generate the trampoline), NativeExecutable
  is cached. The host function address is used as the key to look up the cached executable from the table.
- In all the JSStdFunction, we use the same host function that immediately calls the each std::function.
- As a result, without any change, all the JSStdFunction hit the same cached NativeExecutable even if the held
  std::function is different.
- To solve it, if we put the std::function in the NativeExecutable, we need to add this std::function
  identity (like address) to the cache key, because the address of the stub host function (that calls the
  std::function) is the same in the all JSStdFunction.
- But since the std::function will be allocated in the heap, this address is always different. So caching has
  no effect.
- If we do not cache the NativeExecutable that holds the std::function, each time when creating the JSStdFunction,
  we need to regenerate the completely same trampolines (since it just calls the same host function stub that
  calls the std::function).

And this patch drops JSArrowFunction::destroy because (1) JSArrowFunction is not destructible and (2) it no longer
holds any fields that require destructions.

* CMakeLists.txt:
* JavaScriptCore.vcxproj/JavaScriptCore.vcxproj:
* JavaScriptCore.vcxproj/JavaScriptCore.vcxproj.filters:
* JavaScriptCore.xcodeproj/project.pbxproj:
* jsc.cpp:
(runWithScripts):
* runtime/JSArrowFunction.cpp:
(JSC::JSArrowFunction::destroy): Deleted.
* runtime/JSArrowFunction.h:
* runtime/JSFunction.cpp:
(JSC::JSFunction::lookUpOrCreateNativeExecutable):
(JSC::JSFunction::create):
(JSC::getNativeExecutable): Deleted.
(JSC::JSStdFunction::JSStdFunction): Deleted.
(JSC::runStdFunction): Deleted.
* runtime/JSFunction.h:
* runtime/JSGlobalObject.cpp:
(JSC::JSGlobalObject::init):
(JSC::JSGlobalObject::visitChildren):
* runtime/JSGlobalObject.h:
(JSC::JSGlobalObject::nativeStdFunctionStructure):
* runtime/JSNativeStdFunction.cpp: Added.
(JSC::JSNativeStdFunction::JSNativeStdFunction):
(JSC::JSNativeStdFunction::visitChildren):
(JSC::JSNativeStdFunction::finishCreation):
(JSC::runStdFunction):
(JSC::JSNativeStdFunction::create):
* runtime/JSNativeStdFunction.h: Copied from Source/JavaScriptCore/runtime/JSArrowFunction.h.
(JSC::JSNativeStdFunction::createStructure):
(JSC::JSNativeStdFunction::nativeStdFunctionCell):
* runtime/NativeStdFunctionCell.cpp: Added.
(JSC::NativeStdFunctionCell::create):
(JSC::NativeStdFunctionCell::NativeStdFunctionCell):
(JSC::NativeStdFunctionCell::destroy):
* runtime/NativeStdFunctionCell.h: Added.
(JSC::NativeStdFunctionCell::createStructure):
(JSC::NativeStdFunctionCell::function):
* runtime/VM.cpp:
(JSC::VM::VM):
* runtime/VM.h:

Source/WebCore:

No behavior change.

Change JSFunction::create to JSNativeStdFunction::create to explicitly create the JSNativeStdFunction with the C++ lambda.

* ForwardingHeaders/runtime/JSNativeStdFunction.h: Added.
* bindings/js/ReadableJSStream.cpp:
(WebCore::createStartResultFulfilledFunction):
(WebCore::createPullResultFulfilledFunction):
(WebCore::createCancelResultFulfilledFunction):
(WebCore::createCancelResultRejectedFunction):
(WebCore::ReadableJSStream::ReadableJSStream):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@189124 268f45cc-cd09-0410-ab3c-d52691b4dbfc
21 files changed:
Source/JavaScriptCore/CMakeLists.txt
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/JavaScriptCore.vcxproj/JavaScriptCore.vcxproj
Source/JavaScriptCore/JavaScriptCore.vcxproj/JavaScriptCore.vcxproj.filters
Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj
Source/JavaScriptCore/jsc.cpp
Source/JavaScriptCore/runtime/JSArrowFunction.cpp
Source/JavaScriptCore/runtime/JSArrowFunction.h
Source/JavaScriptCore/runtime/JSFunction.cpp
Source/JavaScriptCore/runtime/JSFunction.h
Source/JavaScriptCore/runtime/JSGlobalObject.cpp
Source/JavaScriptCore/runtime/JSGlobalObject.h
Source/JavaScriptCore/runtime/JSNativeStdFunction.cpp [new file with mode: 0644]
Source/JavaScriptCore/runtime/JSNativeStdFunction.h [new file with mode: 0644]
Source/JavaScriptCore/runtime/NativeStdFunctionCell.cpp [new file with mode: 0644]
Source/JavaScriptCore/runtime/NativeStdFunctionCell.h [new file with mode: 0644]
Source/JavaScriptCore/runtime/VM.cpp
Source/JavaScriptCore/runtime/VM.h
Source/WebCore/ChangeLog
Source/WebCore/ForwardingHeaders/runtime/JSNativeStdFunction.h [new file with mode: 0644]
Source/WebCore/bindings/js/ReadableJSStream.cpp