Changeset 181060 in webkit


Ignore:
Timestamp:
Mar 4, 2015 6:19:14 PM (9 years ago)
Author:
akling@apple.com
Message:

GC should compute stack bounds and dump registers at the earliest opportunity.
<​https://webkit.org/b/142310>
<rdar://problem/20045624>

Reviewed by Geoffrey Garen.

Make Heap::collect() a wrapper function around a collectImpl() where the work is actually done.
The wrapper function that grabs a snapshot of the current stack boundaries and register values
on entry, and sanitizes the stack on exit.

This is a speculative fix for what appears to be overly conservative behavior in the garbage
collector following r178364 which caused a measurable regression in memory usage on Membuster.
The theory being that we were putting pointers to dead things on the stack before scanning it,
and by doing that ended up marking things that we'd otherwise discover to be garbage.

  • heap/Heap.cpp:

(JSC::Heap::markRoots):
(JSC::Heap::gatherStackRoots):
(JSC::Heap::collect):
(JSC::Heap::collectImpl):

  • heap/Heap.h:
  • heap/MachineStackMarker.cpp:

(JSC::MachineThreads::gatherFromCurrentThread):
(JSC::MachineThreads::gatherConservativeRoots):

  • heap/MachineStackMarker.h:
Location:
trunk/Source/JavaScriptCore
Files:
5 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/JavaScriptCore/ChangeLog

    r181059 r181060  
     12015-03-04  Andreas Kling  <akling@apple.com>
     2
     3        GC should compute stack bounds and dump registers at the earliest opportunity.
     4        <https://webkit.org/b/142310>
     5        <rdar://problem/20045624>
     6
     7        Reviewed by Geoffrey Garen.
     8
     9        Make Heap::collect() a wrapper function around a collectImpl() where the work is actually done.
     10        The wrapper function that grabs a snapshot of the current stack boundaries and register values
     11        on entry, and sanitizes the stack on exit.
     12
     13        This is a speculative fix for what appears to be overly conservative behavior in the garbage
     14        collector following r178364 which caused a measurable regression in memory usage on Membuster.
     15        The theory being that we were putting pointers to dead things on the stack before scanning it,
     16        and by doing that ended up marking things that we'd otherwise discover to be garbage.
     17
     18        * heap/Heap.cpp:
     19        (JSC::Heap::markRoots):
     20        (JSC::Heap::gatherStackRoots):
     21        (JSC::Heap::collect):
     22        (JSC::Heap::collectImpl):
     23        * heap/Heap.h:
     24        * heap/MachineStackMarker.cpp:
     25        (JSC::MachineThreads::gatherFromCurrentThread):
     26        (JSC::MachineThreads::gatherConservativeRoots):
     27        * heap/MachineStackMarker.h:
     28
    1292015-03-04  Debarshi Ray  <debarshir@gnome.org>
    230

  • trunk/Source/JavaScriptCore/heap/Heap.cpp

    r181010 r181060  
    514514}
    515515
    516 void Heap::markRoots(double gcStartTime)
     516void Heap::markRoots(double gcStartTime, void* stackOrigin, void* stackTop, MachineThreads::RegisterState& calleeSavedRegisters)
    517517{
    518518    SamplingRegion samplingRegion("Garbage Collection: Marking");
    … …  
    535535    // We gather conservative roots before clearing mark bits because conservative
    536536    // gathering uses the mark bits to determine whether a reference is valid.
    537     void* dummy;
    538     ALLOCATE_AND_GET_REGISTER_STATE(registers);
    539537    ConservativeRoots conservativeRoots(&m_objectSpace.blocks(), &m_storageSpace);
    540     gatherStackRoots(conservativeRoots, &dummy, registers);
     538    gatherStackRoots(conservativeRoots, stackOrigin, stackTop, calleeSavedRegisters);
    541539    gatherJSStackRoots(conservativeRoots);
    542540    gatherScratchBufferRoots(conservativeRoots);
    543 
    544     sanitizeStackForVM(m_vm);
    545541
    546542    clearLivenessData();
    … …  
    599595}
    600596
    601 void Heap::gatherStackRoots(ConservativeRoots& roots, void** dummy, MachineThreads::RegisterState& registers)
     597void Heap::gatherStackRoots(ConservativeRoots& roots, void* stackOrigin, void* stackTop, MachineThreads::RegisterState& calleeSavedRegisters)
    602598{
    603599    GCPHASE(GatherStackRoots);
    604600    m_jitStubRoutines.clearMarks();
    605     m_machineThreads.gatherConservativeRoots(roots, m_jitStubRoutines, m_codeBlocks, dummy, registers);
     601    m_machineThreads.gatherConservativeRoots(roots, m_jitStubRoutines, m_codeBlocks, stackOrigin, stackTop, calleeSavedRegisters);
    606602}
    607603
    … …  
    10041000static double minute = 60.0;
    10051001
    1006 void Heap::collect(HeapOperation collectionType)
     1002NEVER_INLINE void Heap::collect(HeapOperation collectionType)
     1003{
     1004    void* stackTop;
     1005    ALLOCATE_AND_GET_REGISTER_STATE(registers);
     1006
     1007    collectImpl(collectionType, wtfThreadData().stack().origin(), &stackTop, registers);
     1008
     1009    sanitizeStackForVM(m_vm);
     1010}
     1011
     1012NEVER_INLINE void Heap::collectImpl(HeapOperation collectionType, void* stackOrigin, void* stackTop, MachineThreads::RegisterState& calleeSavedRegisters)
    10071013{
    10081014#if ENABLE(ALLOCATION_LOGGING)
    … …  
    10491055    flushWriteBarrierBuffer();
    10501056
    1051     markRoots(gcStartTime);
     1057    markRoots(gcStartTime, stackOrigin, stackTop, calleeSavedRegisters);
    10521058
    10531059    if (m_verifier) {

  • trunk/Source/JavaScriptCore/heap/Heap.h

    r181019 r181060  
    274274    JS_EXPORT_PRIVATE void reportExtraMemoryCostSlowCase(size_t);
    275275
     276    void collectImpl(HeapOperation, void* stackOrigin, void* stackTop, MachineThreads::RegisterState&);
     277
    276278    void suspendCompilerThreads();
    277279    void willStartCollection(HeapOperation collectionType);
    … …  
    281283    void stopAllocation();
    282284
    283     void markRoots(double gcStartTime);
    284     void gatherStackRoots(ConservativeRoots&, void** dummy, MachineThreads::RegisterState& registers);
     285    void markRoots(double gcStartTime, void* stackOrigin, void* stackTop, MachineThreads::RegisterState&);
     286    void gatherStackRoots(ConservativeRoots&, void* stackOrigin, void* stackTop, MachineThreads::RegisterState&);
    285287    void gatherJSStackRoots(ConservativeRoots&);
    286288    void gatherScratchBufferRoots(ConservativeRoots&);

  • trunk/Source/JavaScriptCore/heap/MachineStackMarker.cpp

    r180716 r181060  
    277277}
    278278   
    279 void MachineThreads::gatherFromCurrentThread(ConservativeRoots& conservativeRoots, JITStubRoutineSet& jitStubRoutines, CodeBlockSet& codeBlocks, void* stackCurrent, RegisterState& registers)
    280 {
    281     void* registersBegin = &registers;
    282     void* registersEnd = reinterpret_cast<void*>(roundUpToMultipleOf<sizeof(void*)>(reinterpret_cast<uintptr_t>(&registers + 1)));
     279void MachineThreads::gatherFromCurrentThread(ConservativeRoots& conservativeRoots, JITStubRoutineSet& jitStubRoutines, CodeBlockSet& codeBlocks, void* stackOrigin, void* stackTop, RegisterState& calleeSavedRegisters)
     280{
     281    void* registersBegin = &calleeSavedRegisters;
     282    void* registersEnd = reinterpret_cast<void*>(roundUpToMultipleOf<sizeof(void*)>(reinterpret_cast<uintptr_t>(&calleeSavedRegisters + 1)));
    283283    conservativeRoots.add(registersBegin, registersEnd, jitStubRoutines, codeBlocks);
    284284
    285     void* stackBegin = stackCurrent;
    286     void* stackEnd = wtfThreadData().stack().origin();
    287     conservativeRoots.add(stackBegin, stackEnd, jitStubRoutines, codeBlocks);
     285    conservativeRoots.add(stackTop, stackOrigin, jitStubRoutines, codeBlocks);
    288286}
    289287
    … …  
    615613}
    616614
    617 void MachineThreads::gatherConservativeRoots(ConservativeRoots& conservativeRoots, JITStubRoutineSet& jitStubRoutines, CodeBlockSet& codeBlocks, void* stackCurrent, RegisterState& currentThreadRegisters)
    618 {
    619     gatherFromCurrentThread(conservativeRoots, jitStubRoutines, codeBlocks, stackCurrent, currentThreadRegisters);
     615void MachineThreads::gatherConservativeRoots(ConservativeRoots& conservativeRoots, JITStubRoutineSet& jitStubRoutines, CodeBlockSet& codeBlocks, void* stackOrigin, void* stackTop, RegisterState& calleeSavedRegisters)
     616{
     617    gatherFromCurrentThread(conservativeRoots, jitStubRoutines, codeBlocks, stackOrigin, stackTop, calleeSavedRegisters);
    620618
    621619    size_t size;

  • trunk/Source/JavaScriptCore/heap/MachineStackMarker.h

    r180716 r181060  
    4343        ~MachineThreads();
    4444
    45         void gatherConservativeRoots(ConservativeRoots&, JITStubRoutineSet&, CodeBlockSet&, void* stackCurrent, RegisterState& registers);
     45        void gatherConservativeRoots(ConservativeRoots&, JITStubRoutineSet&, CodeBlockSet&, void* stackOrigin, void* stackTop, RegisterState& calleeSavedRegisters);
    4646
    4747        JS_EXPORT_PRIVATE void addCurrentThread(); // Only needs to be called by clients that can use the same heap from multiple threads.
    … …  
    5050        class Thread;
    5151
    52         void gatherFromCurrentThread(ConservativeRoots&, JITStubRoutineSet&, CodeBlockSet&, void* stackCurrent, RegisterState& registers);
     52        void gatherFromCurrentThread(ConservativeRoots&, JITStubRoutineSet&, CodeBlockSet&, void* stackOrigin, void* stackTop, RegisterState& calleeSavedRegisters);
    5353
    5454        void tryCopyOtherThreadStack(Thread*, void*, size_t capacity, size_t*);
Note: See TracChangeset for help on using the changeset viewer.