Created attachment 59959 [details] Patch It would be nice to have the GC run a few seconds after the last allocation. We can approximate this by having a timer start running at the beginning of allocation, and resetting the timer whenever the heap is reset. If there are a number of allocations happening, then the timer will always get reset before it times out and the GC is called. If there are very few allocations happening then the timer fire and we will collect the garbage.
Attachment 59959 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1 WebCore/bindings/js/WebCoreJSClientData.h:87: Missing space after , [whitespace/comma] [3] JavaScriptCore/runtime/Collector.cpp:982: Declaration has space between type name and * in Heap *heap [whitespace/declaration] [3] JavaScriptCore/runtime/Collector.cpp:1001: An else should appear on the same line as the preceding } [whitespace/newline] [4] JavaScriptCore/runtime/Collector.h:85: Missing space after , [whitespace/comma] [3] JavaScriptCore/runtime/Collector.h:161: Missing space after , [whitespace/comma] [3] Total errors found: 5 in 8 files If any of these errors are false positives, please file a bug against check-webkit-style.
Attachment 59959 [details] did not build on qt: Build output: http://webkit-commit-queue.appspot.com/results/3287968
Created attachment 59962 [details] Patch Should fix build problems on non macintosh platforms.
Attachment 59959 [details] did not build on gtk: Build output: http://webkit-commit-queue.appspot.com/results/3311983
Putting CF code directly in to Heap is pretty nasty. I also do not think there is any guarantee that the main thread is spinning a CF runloop from JavaScriptCore’s point of view. I also don’t think OS(DARWIN) is the right condition to use since Darwin proper doesn’t include CoreFoundation.
Attachment 59962 [details] did not build on qt: Build output: http://webkit-commit-queue.appspot.com/results/3323917
Attachment 59959 [details] did not build on win: Build output: http://webkit-commit-queue.appspot.com/results/3291940
Attachment 59962 [details] did not build on gtk: Build output: http://webkit-commit-queue.appspot.com/results/3304939
Created attachment 59978 [details] Patch This should fix compilation issues on platforms that do not have ENABLE(JSC_MULTIPLE_THREADS). It also changes OS(DARWIN) to PLATFORM(CF) to check for core foundation.
Attachment 59962 [details] did not build on win: Build output: http://webkit-commit-queue.appspot.com/results/3301990
Attachment 59978 [details] did not build on win: Build output: http://webkit-commit-queue.appspot.com/results/3283931
Created attachment 60050 [details] Patch (callback in separate file)
Attachment 60050 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1 JavaScriptCore/runtime/GCActivityCallback.h:29: #ifndef header guard has wrong style, please use: GCActivityCallback_h [build/header_guard] [5] Total errors found: 1 in 14 files If any of these errors are false positives, please file a bug against check-webkit-style.
+ runtime/GCActivityCallback.cpp + runtime/GCActivityCallbackCF.cpp Project files should only use one or the other. This also means that you can remove the #if(!CF) from the non-CF file. + : m_callback(defaultGCActivityCallback) + , m_callbackData(0) Rename to m_activityCallback* + typedef void (*GCCallback)(Heap*, void*); Move to CGActivityCallback.h, and rename to GCActivityCallback. + void setGCCallback(GCCallback, void*); setActivityCallback +void defaultGCActivityCallback(Heap* heap, void*) +{ + static CFRunLoopTimerRef timer; + static CFRunLoopTimerContext ctx; Not thread safe. Might invoke timer after heap has been destroyed. Leaks one timer per heap. Maybe change this callback to a callback object with a proper constructor and destructor and a virtual operator(), or add a createData and destroyData callback.
Created attachment 61335 [details] Patch
Attachment 61335 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1 JavaScriptCore/runtime/Collector.h:137: Use 0 instead of NULL. [readability/null] [5] Total errors found: 1 in 19 files If any of these errors are false positives, please file a bug against check-webkit-style.
Attachment 61335 [details] did not build on qt: Build output: http://webkit-commit-queue.appspot.com/results/3539002
Attachment 61335 [details] did not build on gtk: Build output: http://webkit-commit-queue.appspot.com/results/3486273
Created attachment 61373 [details] Patch
Attachment 61373 [details] did not build on qt: Build output: http://webkit-commit-queue.appspot.com/results/3484283
Attachment 61373 [details] did not build on gtk: Build output: http://webkit-commit-queue.appspot.com/results/3484289
Created attachment 61382 [details] Patch
Created attachment 61411 [details] Patch (with smart pointers)
Comment on attachment 61411 [details] Patch (with smart pointers) I think the callback invocation should be inside reset(), so that the timer is active even after events like collections due to extra cost. It would be better not to rely on calling out to WebCore just to achieve a cross-platform timer implementation. I would nix the WebCore code, and, as you suggested, add a FIXME explaining how other platforms can get the right behavior by porting the WebCore timer code to WTF. The canonical name we use for platform-speicifc data structure is Platform*. In this case, DefaultGCActivityCallbackPlatformData. Please post SunSpider results for this change.
Created attachment 62343 [details] Patch moved where the callback gets called from, verified no regression on sunspider, + other changes suggested by Geoff.
Comment on attachment 62343 [details] Patch Rejecting patch 62343 from commit-queue. Failed to run "[u'/Users/eseidel/Projects/CommitQueue/WebKitTools/Scripts/svn-apply', u'--reviewer', u'Oliver Hunt', u'--force']" exit_code: 1 Last 500 characters of output: ts to file JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj.rej patching file JavaScriptCore/jit/JITPropertyAccess.cpp patching file JavaScriptCore/runtime/Collector.cpp patching file JavaScriptCore/runtime/Collector.h patching file JavaScriptCore/runtime/GCActivityCallback.cpp patching file JavaScriptCore/runtime/GCActivityCallback.h patching file JavaScriptCore/runtime/GCActivityCallbackCF.cpp patching file JavaScriptCore/runtime/JSLock.cpp patching file JavaScriptCore/runtime/JSLock.h Full output: http://queues.webkit.org/results/3618235
Committed revision 64585.