Changeset 203370 in webkit


Ignore:
Timestamp:
Jul 18, 2016 1:31:20 PM (8 years ago)
Author:
fpizlo@apple.com
Message:

Source/JavaScriptCore:
Repeatedly creating and destroying workers that enqueue DFG plans can outpace the DFG worklist, which then causes VM shutdown to stall, which then causes memory growth
​https://bugs.webkit.org/show_bug.cgi?id=159754

Reviewed by Geoffrey Garen.

If you create and destroy workers at a high rate and those workers enqueue some DFG plans
that are still not compiled at the time that the worker is closed, then the closed workers
end up stalling in VM::~VM waiting for the DFG worklist thread to finish those plans. Since
we don't actually cancel the plans, it's easy to create a situation where the workers
outpace the DFG worklist, especially if you create many workers at a time and each one
finishes just after enqueueing those plans.

The solution is to allow VM::~VM to remove plans from the DFG worklist that are related to
that VM but aren't currently being worked on. That turns out to be an easy change.

I have a test that repros this, but it's quite long-running. I call it workers/bomb.html. We
may want to exclude it from test runs because of how long it takes.

  • dfg/DFGWorklist.cpp:

(JSC::DFG::Worklist::removeDeadPlans):
(JSC::DFG::Worklist::removeNonCompilingPlansForVM):
(JSC::DFG::Worklist::queueLength):
(JSC::DFG::Worklist::runThread):

  • dfg/DFGWorklist.h:
  • runtime/VM.cpp:

(JSC::VM::~VM):

LayoutTests:
Repeatedly creating and destroying workers that enqueue DFG plans can outpace the DFG worklist, which then causes VM shutdown to stall, which then causes a memory growth
​https://bugs.webkit.org/show_bug.cgi?id=159754

Reviewed by Geoffrey Garen.

Adds two tests that create a lot of workers that do sophisticated things. These are
long-running tests so we may want to skip them. It's OK if we end up only running them
manually occasionally.

  • workers: Added.
  • workers/bomb.html: Added.
  • workers/bomb-expected.txt: Added.
  • workers/bomb-with-v8.html: Added.
  • workers/tests: Added.
  • workers/tests/3d-cube.js: Added.
  • workers/tests/3d-morph.js: Added.
  • workers/tests/3d-raytrace.js: Added.
  • workers/tests/access-binary-trees.js: Added.
  • workers/tests/access-fannkuch.js: Added.
  • workers/tests/access-nbody.js: Added.
  • workers/tests/access-nsieve.js: Added.
  • workers/tests/bitops-3bit-bits-in-byte.js: Added.
  • workers/tests/bitops-bits-in-byte.js: Added.
  • workers/tests/bitops-bitwise-and.js: Added.
  • workers/tests/bitops-nsieve-bits.js: Added.
  • workers/tests/controlflow-recursive.js: Added.
  • workers/tests/crypto-aes.js: Added.
  • workers/tests/crypto-md5.js: Added.
  • workers/tests/crypto-sha1.js: Added.
  • workers/tests/date-format-tofte.js: Added.
  • workers/tests/date-format-xparb.js: Added.
  • workers/tests/math-cordic.js: Added.
  • workers/tests/math-partial-sums.js: Added.
  • workers/tests/math-spectral-norm.js: Added.
  • workers/tests/regexp-dna.js: Added.
  • workers/tests/string-base64.js: Added.
  • workers/tests/string-fasta.js: Added.
  • workers/tests/string-tagcloud.js: Added.
  • workers/tests/string-unpack-code.js: Added.
  • workers/tests/string-validate-input.js: Added.
  • workers/tests/v8-crypto.js: Added.
  • workers/tests/v8-deltablue.js: Added.
  • workers/tests/v8-earley-boyer.js: Added.
  • workers/tests/v8-raytrace.js: Added.
  • workers/tests/v8-regexp.js: Added.
  • workers/tests/v8-richards.js: Added.
  • workers/tests/v8-splay.js: Added.
Location:
trunk
Files:
38 added
8 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r203369 r203370  
     12016-07-18  Filip Pizlo  <fpizlo@apple.com>
     2
     3        Repeatedly creating and destroying workers that enqueue DFG plans can outpace the DFG worklist, which then causes VM shutdown to stall, which then causes a memory growth
     4        https://bugs.webkit.org/show_bug.cgi?id=159754
     5
     6        Reviewed by Geoffrey Garen.
     7       
     8        Adds two tests that create a lot of workers that do sophisticated things. These are
     9        long-running tests so we may want to skip them. It's OK if we end up only running them
     10        manually occasionally.
     11
     12        * workers: Added.
     13        * workers/bomb.html: Added.
     14        * workers/bomb-expected.txt: Added.
     15        * workers/bomb-with-v8.html: Added.
     16        * workers/tests: Added.
     17        * workers/tests/3d-cube.js: Added.
     18        * workers/tests/3d-morph.js: Added.
     19        * workers/tests/3d-raytrace.js: Added.
     20        * workers/tests/access-binary-trees.js: Added.
     21        * workers/tests/access-fannkuch.js: Added.
     22        * workers/tests/access-nbody.js: Added.
     23        * workers/tests/access-nsieve.js: Added.
     24        * workers/tests/bitops-3bit-bits-in-byte.js: Added.
     25        * workers/tests/bitops-bits-in-byte.js: Added.
     26        * workers/tests/bitops-bitwise-and.js: Added.
     27        * workers/tests/bitops-nsieve-bits.js: Added.
     28        * workers/tests/controlflow-recursive.js: Added.
     29        * workers/tests/crypto-aes.js: Added.
     30        * workers/tests/crypto-md5.js: Added.
     31        * workers/tests/crypto-sha1.js: Added.
     32        * workers/tests/date-format-tofte.js: Added.
     33        * workers/tests/date-format-xparb.js: Added.
     34        * workers/tests/math-cordic.js: Added.
     35        * workers/tests/math-partial-sums.js: Added.
     36        * workers/tests/math-spectral-norm.js: Added.
     37        * workers/tests/regexp-dna.js: Added.
     38        * workers/tests/string-base64.js: Added.
     39        * workers/tests/string-fasta.js: Added.
     40        * workers/tests/string-tagcloud.js: Added.
     41        * workers/tests/string-unpack-code.js: Added.
     42        * workers/tests/string-validate-input.js: Added.
     43        * workers/tests/v8-crypto.js: Added.
     44        * workers/tests/v8-deltablue.js: Added.
     45        * workers/tests/v8-earley-boyer.js: Added.
     46        * workers/tests/v8-raytrace.js: Added.
     47        * workers/tests/v8-regexp.js: Added.
     48        * workers/tests/v8-richards.js: Added.
     49        * workers/tests/v8-splay.js: Added.
     50
    1512016-07-18  Ryan Haddad  <ryanhaddad@apple.com>
    252

  • trunk/LayoutTests/TestExpectations

    r203330 r203370  
    10041004webkit.org/b/159678 http/tests/preload/single_download_preload_runner.html [ Timeout ]
    10051005
     1006# This test is way too slow for debug.
     1007[ Debug ] workers/bomb.html [ Skip ]
     1008# This test is just way too slow.
     1009workers/bomb-with-v8.html [ Skip ]
     1010
    10061011# WebCryptoAPI tests, skip for unimplemented features. webkit.org/b/159638
    10071012imported/w3c/WebCryptoAPI/encrypt_decrypt/test_aes_cbc.html [ Skip ]

  • trunk/Source/JavaScriptCore/ChangeLog

    r203368 r203370  
     12016-07-18  Filip Pizlo  <fpizlo@apple.com>
     2
     3        Repeatedly creating and destroying workers that enqueue DFG plans can outpace the DFG worklist, which then causes VM shutdown to stall, which then causes memory growth
     4        https://bugs.webkit.org/show_bug.cgi?id=159754
     5
     6        Reviewed by Geoffrey Garen.
     7       
     8        If you create and destroy workers at a high rate and those workers enqueue some DFG plans
     9        that are still not compiled at the time that the worker is closed, then the closed workers
     10        end up stalling in VM::~VM waiting for the DFG worklist thread to finish those plans. Since
     11        we don't actually cancel the plans, it's easy to create a situation where the workers
     12        outpace the DFG worklist, especially if you create many workers at a time and each one
     13        finishes just after enqueueing those plans.
     14       
     15        The solution is to allow VM::~VM to remove plans from the DFG worklist that are related to
     16        that VM but aren't currently being worked on. That turns out to be an easy change.
     17       
     18        I have a test that repros this, but it's quite long-running. I call it workers/bomb.html. We
     19        may want to exclude it from test runs because of how long it takes.
     20
     21        * dfg/DFGWorklist.cpp:
     22        (JSC::DFG::Worklist::removeDeadPlans):
     23        (JSC::DFG::Worklist::removeNonCompilingPlansForVM):
     24        (JSC::DFG::Worklist::queueLength):
     25        (JSC::DFG::Worklist::runThread):
     26        * dfg/DFGWorklist.h:
     27        * runtime/VM.cpp:
     28        (JSC::VM::~VM):
     29
    1302016-07-17  Filip Pizlo  <fpizlo@apple.com>
    231

  • trunk/Source/JavaScriptCore/dfg/DFGPlan.cpp

    r203356 r203370  
    550550}
    551551
    552 void Plan::notifyCompiled()
    553 {
    554     stage = Compiled;
    555 }
    556 
    557552void Plan::notifyReady()
    558553{

  • trunk/Source/JavaScriptCore/dfg/DFGPlan.h

    r203356 r203370  
    6767   
    6868    void notifyCompiling();
    69     void notifyCompiled();
    7069    void notifyReady();
    7170   
    … …  
    113112    Vector<unsigned> tierUpAndOSREnterBytecodes;
    114113
    115     enum Stage { Preparing, Compiling, Compiled, Ready, Cancelled };
     114    enum Stage { Preparing, Compiling, Ready, Cancelled };
    116115    Stage stage;
    117116

  • trunk/Source/JavaScriptCore/dfg/DFGWorklist.cpp

    r200933 r203370  
    285285                if (m_readyPlans[i]->stage != Plan::Cancelled)
    286286                    continue;
    287                 m_readyPlans[i] = m_readyPlans.last();
     287                m_readyPlans[i--] = m_readyPlans.last();
    288288                m_readyPlans.removeLast();
    289289            }
    … …  
    303303        safepoint->cancel();
    304304    }
     305}
     306
     307void Worklist::removeNonCompilingPlansForVM(VM& vm)
     308{
     309    LockHolder locker(m_lock);
     310    HashSet<CompilationKey> deadPlanKeys;
     311    Vector<RefPtr<Plan>> deadPlans;
     312    for (auto& entry : m_plans) {
     313        Plan* plan = entry.value.get();
     314        if (plan->vm != &vm)
     315            continue;
     316        if (plan->stage == Plan::Compiling)
     317            continue;
     318        deadPlanKeys.add(plan->key());
     319        deadPlans.append(plan);
     320    }
     321    for (CompilationKey key : deadPlanKeys)
     322        m_plans.remove(key);
     323    Deque<RefPtr<Plan>> newQueue;
     324    while (!m_queue.isEmpty()) {
     325        RefPtr<Plan> plan = m_queue.takeFirst();
     326        if (!deadPlanKeys.contains(plan->key()))
     327            newQueue.append(WTFMove(plan));
     328    }
     329    m_queue = WTFMove(newQueue);
     330    m_readyPlans.removeAllMatching(
     331        [&] (RefPtr<Plan>& plan) -> bool {
     332            return deadPlanKeys.contains(plan->key());
     333        });
     334    for (auto& plan : deadPlans)
     335        plan->cancel();
    305336}
    306337
    … …  
    342373           
    343374            plan = m_queue.takeFirst();
    344             if (plan)
     375            if (plan) {
     376                RELEASE_ASSERT(plan->stage == Plan::Preparing);
    345377                m_numberOfActiveThreads++;
     378            }
    346379        }
    347380       
    … …  
    376409                    continue;
    377410                }
    378                 plan->notifyCompiled();
     411               
     412                plan->notifyReady();
     413               
     414                if (Options::verboseCompilationQueue()) {
     415                    dump(locker, WTF::dataFile());
     416                    dataLog(": Compiled ", plan->key(), " asynchronously\n");
     417                }
     418               
     419                m_readyPlans.append(plan);
     420               
     421                m_planCompiled.notifyAll();
     422                m_numberOfActiveThreads--;
    379423            }
    380424            RELEASE_ASSERT(!plan->vm->heap.isCollecting());
    381         }
    382 
    383         {
    384             LockHolder locker(m_lock);
    385            
    386             // We could have been cancelled between releasing rightToRun and acquiring m_lock.
    387             // This would mean that we might be in the middle of GC right now.
    388             if (plan->stage == Plan::Cancelled) {
    389                 m_numberOfActiveThreads--;
    390                 continue;
    391             }
    392            
    393             plan->notifyReady();
    394            
    395             if (Options::verboseCompilationQueue()) {
    396                 dump(locker, WTF::dataFile());
    397                 dataLog(": Compiled ", plan->key(), " asynchronously\n");
    398             }
    399            
    400             m_readyPlans.append(plan);
    401            
    402             m_planCompiled.notifyAll();
    403             m_numberOfActiveThreads--;
    404425        }
    405426    }

  • trunk/Source/JavaScriptCore/dfg/DFGWorklist.h

    r190827 r203370  
    11/*
    2  * Copyright (C) 2013, 2014 Apple Inc. All rights reserved.
     2 * Copyright (C) 2013, 2014, 2016 Apple Inc. All rights reserved.
    33 *
    44 * Redistribution and use in source and binary forms, with or without
    … …  
    7777    void removeDeadPlans(VM&);
    7878   
     79    void removeNonCompilingPlansForVM(VM&);
     80   
    7981    void dump(PrintStream&) const;
    8082   

  • trunk/Source/JavaScriptCore/runtime/VM.cpp

    r203142 r203370  
    350350    for (unsigned i = DFG::numberOfWorklists(); i--;) {
    351351        if (DFG::Worklist* worklist = DFG::worklistForIndexOrNull(i)) {
     352            worklist->removeNonCompilingPlansForVM(*this);
    352353            worklist->waitUntilAllPlansForVMAreReady(*this);
    353354            worklist->removeAllReadyPlansForVM(*this);
Note: See TracChangeset for help on using the changeset viewer.