Most of the GCs while browsing the web are due to the GC timer. Currently the GC timer always does FullCollections. To reduce the impact of the GC timer on the system we should add some logic to intelligently schedule both EdenCollections and FullCollections.
Created attachment 226599 [details] straw man
Attachment 226599 [details] did not pass style-queue: ERROR: Source/WebCore/ChangeLog:8: You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible. [changelog/nonewtests] [5] Total errors found: 1 in 8 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 226600 [details] straw man 2
Comment on attachment 226600 [details] straw man 2 View in context: https://bugs.webkit.org/attachment.cgi?id=226600&action=review r=me Please test PLT before landing this. Plus a few fixups below. > Source/JavaScriptCore/heap/GCActivityCallback.cpp:-97 > if (heap->isPagedOut(startTime + pagingTimeOut)) { > - heap->activityCallback()->cancel(); EdenCollection probably should not do the isPagedOut check. I think isPagedOut touches the whole heap, so that's not great for Eden. Also, EdenCollection is more likely to find free space than FullCollection, so I think it should be less conservative about whether collection is "worth it". > Source/JavaScriptCore/heap/GCActivityCallback.cpp:112 > + heap->setShouldDoFullCollection(true); > heap->collect(); I think it would be a slightly better abstraction to pass a collection type to collect(), rather than setting a data member. Perhaps the Heap implements things internally in terms of a data member, but clients need not know that detail. > Source/JavaScriptCore/heap/Heap.cpp:281 > + , m_fullActivityCallback(GCActivityCallback::createFullTimer(this)) > + , m_edenActivityCallback(GCActivityCallback::createEdenTimer(this)) If GenGC is disabled, both pointers should point to the same callback, and it should be a full callback. That way, we can still A/B test GenGC on vs off. > Source/JavaScriptCore/heap/Heap.cpp:340 > - if (m_activityCallback) > - m_activityCallback->didAllocate(m_bytesAllocatedThisCycle + m_bytesAbandonedThisCycle); > + if (m_fullActivityCallback) > + m_fullActivityCallback->didAllocate(m_sizeAfterLastCollect + m_bytesAbandonedThisCycle); This is pretty aggro. The first abandoned object graph reports that it abandoned the whole heap! I think you want this instead: m_sizeAfterLastCollect - m_sizeAfterLastFullCollect + m_bytesAllocatedThisCycle + m_bytesAbandonedThisCycle That way, we only report our growth since the last full collect. Also: You should rename m_bytesAbandonedThisCycle to m_bytesAbandonedSinceLastFullCollect, and change it so that it only resets to 0 during a full collect. Our theory of abandoned object graphs is that they should hasten full collections, so we should not reset the abandoned counter during eden collections. For example, if I navigate between three websites, and do three eden collections, I still want to do a full collection soon.
Created attachment 227087 [details] Patch
Created attachment 227107 [details] Patch
Created attachment 227114 [details] Patch
Created attachment 227117 [details] Patch
Created attachment 227125 [details] Patch
Created attachment 227128 [details] Patch
Created attachment 227138 [details] Patch
Created attachment 227142 [details] Patch
Comment on attachment 227142 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=227142&action=review r=me > Source/JavaScriptCore/heap/EdenGCActivityCallback.cpp:57 > + return double(sizeBefore - sizeAfter) / double(sizeBefore); static_cast<double> is more our style.
Committed r165926: <http://trac.webkit.org/changeset/165926>
Re-opened since this is blocked by bug 130488
This broke linking WebCore on iOS: Undefined symbols for architecture x86_64: "__ZN3JSC22FullGCActivityCallback12doCollectionEv", referenced from: __ZTVN7WebCore29WebSafeFullGCActivityCallbackE in JSDOMWindowBase.o "__ZN3JSC22FullGCActivityCallback11gcTimeSliceEm", referenced from: __ZTVN7WebCore29WebSafeFullGCActivityCallbackE in JSDOMWindowBase.o "__ZN3JSC18GCActivityCallback11willCollectEv", referenced from: __ZTVN7WebCore29WebSafeEdenGCActivityCallbackE in JSDOMWindowBase.o __ZTVN7WebCore29WebSafeFullGCActivityCallbackE in JSDOMWindowBase.o "__ZN3JSC22FullGCActivityCallback12lastGCLengthEv", referenced from: __ZTVN7WebCore29WebSafeFullGCActivityCallbackE in JSDOMWindowBase.o "__ZN3JSC22FullGCActivityCallback9deathRateEv", referenced from: __ZTVN7WebCore29WebSafeFullGCActivityCallbackE in JSDOMWindowBase.o "_wkQueryDecoderAvailability", referenced from: -exported_symbol[s_list] command line option "__ZN3JSC18GCActivityCallback6doWorkEv", referenced from: __ZTVN7WebCore29WebSafeEdenGCActivityCallbackE in JSDOMWindowBase.o __ZTVN7WebCore29WebSafeFullGCActivityCallbackE in JSDOMWindowBase.o "__ZN3JSC22EdenGCActivityCallback12doCollectionEv", referenced from: __ZTVN7WebCore29WebSafeEdenGCActivityCallbackE in JSDOMWindowBase.o "__ZN3JSC18GCActivityCallback6cancelEv", referenced from: __ZTVN7WebCore29WebSafeEdenGCActivityCallbackE in JSDOMWindowBase.o __ZTVN7WebCore29WebSafeFullGCActivityCallbackE in JSDOMWindowBase.o "__ZN3JSC22EdenGCActivityCallback9deathRateEv", referenced from: __ZTVN7WebCore29WebSafeEdenGCActivityCallbackE in JSDOMWindowBase.o "__ZN3JSC18GCActivityCallback11didAllocateEm", referenced from: __ZTVN7WebCore29WebSafeEdenGCActivityCallbackE in JSDOMWindowBase.o __ZTVN7WebCore29WebSafeFullGCActivityCallbackE in JSDOMWindowBase.o "__ZN3JSC22EdenGCActivityCallback12lastGCLengthEv", referenced from: __ZTVN7WebCore29WebSafeEdenGCActivityCallbackE in JSDOMWindowBase.o "__ZN3JSC22EdenGCActivityCallback11gcTimeSliceEm", referenced from: __ZTVN7WebCore29WebSafeEdenGCActivityCallbackE in JSDOMWindowBase.o ld: symbol(s) not found for architecture x86_64 clang: error: linker command failed with exit code 1 (use -v to see invocation)
Committed r165940: <http://trac.webkit.org/changeset/165940>