Skip to content

Commit a516249

Browse files
committed
[JSC] Remove GCRequest's unused didFinishEndPhase callback
https://bugs.webkit.org/show_bug.cgi?id=325776 rdar://188787440 Reviewed by Marcus Plutowski and Tadeu Zagallo. A Collector that marks more than one Heap will take collection requests from each of them, and a request will likely need to say which Heap it comes from: to decide the scope from that Heap's state, and to let a Heap that goes away drop its own pending requests. Let's clean up an unused field first. GCRequest carried a callback for Collector::runEndPhase() to run once the request was served. Nothing has ever set it: it came with GCRequest in 188598@main (2017), for a WebCore change that never used it. It still shaped how requests are deduplicated, since GCRequest::subsumedBy() never treated a request with a callback as covered by a queued one, so any change to what a request carries would have had to keep that rule. Removing it leaves a request that is only a scope. No change in behavior. * Source/JavaScriptCore/heap/Collector.cpp: (JSC::Collector::runEndPhase): * Source/JavaScriptCore/heap/GCRequest.cpp: (JSC::GCRequest::subsumedBy const): (JSC::GCRequest::dump const): * Source/JavaScriptCore/heap/GCRequest.h: Canonical link: https://commits.webkit.org/322293@main
1 parent c572e93 commit a516249

3 files changed

Lines changed: 1 addition & 11 deletions

File tree

‎Source/JavaScriptCore/heap/Collector.cpp‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -591,9 +591,6 @@ NEVER_INLINE bool Collector::runEndPhase(GCConductor conn)
591591

592592
didFinishCollection();
593593

594-
if (m_currentRequest.didFinishEndPhase)
595-
RefPtr { m_currentRequest.didFinishEndPhase }->run();
596-
597594
if (CollectorInternal::verbose) {
598595
dataLogLn(CollectorInternal::verbose, "Heap state after GC:");
599596
m_heap.m_objectSpace.dumpBits();

‎Source/JavaScriptCore/heap/GCRequest.cpp‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,6 @@ namespace JSC {
3030

3131
bool GCRequest::subsumedBy(const GCRequest& other) const
3232
{
33-
// If we have callbacks, then there is no chance that we're subsumed by an existing request.
34-
if (didFinishEndPhase)
35-
return false;
36-
3733
if (other.scope == CollectionScope::Full)
3834
return true;
3935

@@ -53,7 +49,7 @@ bool GCRequest::subsumedBy(const GCRequest& other) const
5349

5450
void GCRequest::dump(PrintStream& out) const
5551
{
56-
out.print("{scope = ", scope, ", didFinishEndPhase = ", didFinishEndPhase ? "engaged" : "null", "}");
52+
out.print("{scope = ", scope, "}");
5753
}
5854

5955
} // namespace JSC

‎Source/JavaScriptCore/heap/GCRequest.h‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,6 @@
2727

2828
#include <JavaScriptCore/CollectionScope.h>
2929
#include <wtf/PrintStream.h>
30-
#include <wtf/RefPtr.h>
31-
#include <wtf/SharedTask.h>
3230

3331
namespace JSC {
3432

@@ -54,7 +52,6 @@ struct GCRequest {
5452
void dump(PrintStream&) const;
5553

5654
std::optional<CollectionScope> scope;
57-
RefPtr<SharedTask<void()>> didFinishEndPhase;
5855
};
5956

6057
} // namespace JSC

0 commit comments

Comments
 (0)