Skip to content

Commit fb92a01

Browse files
rphilliSkCQ
authored andcommitted
[graphite] Add a non-threaded PipelineManager
This CL introduces simplified versions of the three main parts of the threaded compilation system: GraphicsPipelineHandle - a proxy for a Pipeline/Creation Task PipelineCreationTask - a task that can be run to build a Pipeline PipelineManager - orchestrates the creation of Pipelines This CL doesn't change a lot of behavior but just adds overhead. In DrawPass::prepareResources we create GraphicsPipelineHandles from the GraphicsPipelineDescs and then, immediately, resolve them to GraphicsPipelines. If the GraphicsPipeline already exists in the cache, it is found and returned w/in the Handle. If the GraphicsPipeline doesn't exist then we will create a Task, put it in the TaskMap, blindly execute it in-line, then try to remove it from the TaskMap. This causes a bit of thread-safety danger since, if another Recording is being snapped at the same time or a Precompile is happening (with the same Pipeline), the two threads could share the same task, both race to complete it and then both attempt to remove it from the map. The race to Pipeline completion already currently exists so the main thread-safety risk is in the TaskMap's state. Bug: b/434712686 Change-Id: Id2a329e598d5e20c561838e53d88e6cd459deda6 Reviewed-on: https://skia-review.googlesource.com/c/skia/+/1046996 Reviewed-by: Thomas Smith <[email protected]> Reviewed-by: Michael Ludwig <[email protected]> Commit-Queue: Robert Phillips <[email protected]>
1 parent bfbe80b commit fb92a01

15 files changed

+496
-75
lines changed

gn/graphite.gni

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,7 @@ skia_graphite_sources = [
117117
"$_src/gpu/graphite/GraphicsPipeline.cpp",
118118
"$_src/gpu/graphite/GraphicsPipeline.h",
119119
"$_src/gpu/graphite/GraphicsPipelineDesc.h",
120+
"$_src/gpu/graphite/GraphicsPipelineHandle.h",
120121
"$_src/gpu/graphite/GraphiteResourceKey.cpp",
121122
"$_src/gpu/graphite/GraphiteResourceKey.h",
122123
"$_src/gpu/graphite/ImageFactories.cpp",
@@ -138,7 +139,10 @@ skia_graphite_sources = [
138139
"$_src/gpu/graphite/PaintParamsKey.h",
139140
"$_src/gpu/graphite/PathAtlas.cpp",
140141
"$_src/gpu/graphite/PathAtlas.h",
142+
"$_src/gpu/graphite/PipelineCreationTask.h",
141143
"$_src/gpu/graphite/PipelineData.h",
144+
"$_src/gpu/graphite/PipelineManager.cpp",
145+
"$_src/gpu/graphite/PipelineManager.h",
142146
"$_src/gpu/graphite/PrecompileContext.cpp",
143147
"$_src/gpu/graphite/PrecompileContextPriv.h",
144148
"$_src/gpu/graphite/ProxyCache.cpp",

src/gpu/graphite/BUILD.bazel

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ GRAPHITE_FILES = [
6565
"GraphicsPipeline.cpp",
6666
"GraphicsPipeline.h",
6767
"GraphicsPipelineDesc.h",
68+
"GraphicsPipelineHandle.h",
6869
"GraphiteResourceKey.cpp",
6970
"GraphiteResourceKey.h",
7071
"ImageFactories.cpp",
@@ -86,7 +87,10 @@ GRAPHITE_FILES = [
8687
"PaintParamsKey.h",
8788
"PathAtlas.cpp",
8889
"PathAtlas.h",
90+
"PipelineCreationTask.h",
8991
"PipelineData.h",
92+
"PipelineManager.cpp",
93+
"PipelineManager.h",
9094
"ProxyCache.cpp",
9195
"ProxyCache.h",
9296
"QueueManager.cpp",

src/gpu/graphite/DrawPass.cpp

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88

99
#include "src/core/SkTraceEvent.h"
1010
#include "src/gpu/graphite/Log.h"
11+
#include "src/gpu/graphite/PipelineCreationTask.h"
1112
#include "src/gpu/graphite/PipelineData.h"
1213
#include "src/gpu/graphite/Resource.h" // IWYU pragma: keep
1314
#include "src/gpu/graphite/ResourceProvider.h"
@@ -41,24 +42,30 @@ bool DrawPass::prepareResources(ResourceProvider* resourceProvider,
4142
const RenderPassDesc& renderPassDesc) {
4243
TRACE_EVENT0("skia.gpu", TRACE_FUNC);
4344

44-
const Caps* caps = resourceProvider->caps();
45-
46-
fFullPipelines.reserve(fPipelineDescs.size());
45+
fPipelineHandles.reserve(fPipelineDescs.size());
4746
for (const GraphicsPipelineDesc& pipelineDesc : fPipelineDescs) {
48-
UniqueKey pipelineKey = caps->makeGraphicsPipelineKey(pipelineDesc, renderPassDesc);
49-
auto pipeline = resourceProvider->findOrCreateGraphicsPipeline(runtimeDict.get(),
50-
pipelineKey,
51-
pipelineDesc,
52-
renderPassDesc);
47+
fPipelineHandles.push_back(
48+
resourceProvider->createGraphicsPipelineHandle(pipelineDesc,
49+
renderPassDesc,
50+
PipelineCreationFlags::kNone));
51+
resourceProvider->startPipelineCreationTask(runtimeDict, fPipelineHandles.back());
52+
}
53+
54+
// The DrawPass may be long lived on a Recording and we no longer need the GraphicPipelineDescs
55+
// once we've created pipeline handles, so we drop the storage for them here.
56+
fPipelineDescs.clear();
57+
58+
// TODO(robertphillips): move this resolvePipeline loop to addResourceRefs
59+
fFullPipelines.reserve(fPipelineHandles.size());
60+
for (const GraphicsPipelineHandle& handle : fPipelineHandles) {
61+
sk_sp<GraphicsPipeline> pipeline = resourceProvider->resolveHandle(handle);
5362
if (!pipeline) {
5463
SKGPU_LOG_W("Failed to create GraphicsPipeline for draw in RenderPass. Dropping pass!");
5564
return false;
5665
}
5766
fFullPipelines.push_back(std::move(pipeline));
5867
}
59-
// The DrawPass may be long lived on a Recording and we no longer need the GraphicPipelineDescs
60-
// once we've created pipelines, so we drop the storage for them here.
61-
fPipelineDescs.clear();
68+
fPipelineHandles.clear();
6269

6370
for (int i = 0; i < fSampledTextures.size(); ++i) {
6471
// It should not have been possible to draw an Image that has an invalid texture info

src/gpu/graphite/DrawPass.h

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
#include "include/private/base/SkTArray.h"
1414
#include "src/gpu/graphite/DrawCommands.h"
1515
#include "src/gpu/graphite/GraphicsPipelineDesc.h"
16+
#include "src/gpu/graphite/GraphicsPipelineHandle.h"
1617

1718
struct SkImageInfo;
1819

@@ -98,14 +99,17 @@ class DrawPass {
9899
std::pair<LoadOp, StoreOp> fOps;
99100
std::array<float, 4> fClearColor;
100101

101-
// The pipelines are referenced by index in BindGraphicsPipeline, but that will index into a
102-
// an array of actual GraphicsPipelines.
102+
// The pipelines are referenced by index in BindGraphicsPipeline, but that will index into
103+
// an array of actual GraphicsPipelines (i.e., fFullPipelines).
103104
skia_private::TArray<GraphicsPipelineDesc> fPipelineDescs;
104105

105106
// These resources all get instantiated during prepareResources.
106-
skia_private::TArray<sk_sp<GraphicsPipeline>> fFullPipelines;
107+
skia_private::TArray<GraphicsPipelineHandle> fPipelineHandles;
107108
skia_private::TArray<sk_sp<TextureProxy>> fSampledTextures;
108109

110+
// These get resolved (from the GraphicsPipelineHandles) in prepareResources
111+
skia_private::TArray<sk_sp<GraphicsPipeline>> fFullPipelines;
112+
109113
sk_sp<FloatStorageManager> fFloatStorageManager;
110114

111115
#if defined(SK_TRACE_GRAPHITE_PIPELINE_USE)
Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
/*
2+
* Copyright 2025 Google LLC
3+
*
4+
* Use of this source code is governed by a BSD-style license that can be
5+
* found in the LICENSE file.
6+
*/
7+
8+
#ifndef skgpu_graphite_GraphicsPipelineHandle_DEFINED
9+
#define skgpu_graphite_GraphicsPipelineHandle_DEFINED
10+
11+
#include "include/core/SkRefCnt.h"
12+
13+
#include <variant>
14+
15+
namespace skgpu::graphite {
16+
17+
class GraphicsPipeline;
18+
class PipelineCreationTask;
19+
20+
/*
21+
* The PipelineHandle holds a ref to either a Pipeline or the Task that will create the Pipeline.
22+
* In the latter case, ResourceProvider::resolveHandle can be used to wait for the Task to
23+
* complete. How this works is:
24+
* At Recorder::snap time, the DrawPass will create GraphicsPipelineHandles and will
25+
* kick off all the tasks (using ResourceProvider::kickOffTask)
26+
* Upon Context::insertRecording, all the Handles will be exchanged for GraphicsPipelines in
27+
* the DrawPass::addResourceRefs. This will also release any Tasks being held by the Handles.
28+
*
29+
* Note that the Tasks lock the generated Pipelines in the cache until they are deleted. This avoids
30+
* any race conditions where a Pipeline could be purged between when it was created on a thread
31+
* and when it was actually requested by a DrawPass.
32+
*/
33+
class GraphicsPipelineHandle {
34+
private:
35+
friend class PipelineManager; // for ctors
36+
37+
GraphicsPipelineHandle(sk_sp<PipelineCreationTask> task);
38+
39+
GraphicsPipelineHandle(sk_sp<GraphicsPipeline> pipeline);
40+
41+
std::variant<sk_sp<PipelineCreationTask>, sk_sp<GraphicsPipeline>> fTaskOrPipeline;
42+
};
43+
44+
} // namespace skgpu::graphite
45+
46+
#endif // skgpu_graphite_GraphicsPipelineHandle_DEFINED
Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
/*
2+
* Copyright 2025 Google LLC
3+
*
4+
* Use of this source code is governed by a BSD-style license that can be
5+
* found in the LICENSE file.
6+
*/
7+
8+
#ifndef skgpu_graphite_PipelineCreationTask_DEFINED
9+
#define skgpu_graphite_PipelineCreationTask_DEFINED
10+
11+
#include "include/core/SkRefCnt.h"
12+
#include "src/gpu/graphite/GraphicsPipeline.h"
13+
#include "src/gpu/graphite/GraphicsPipelineDesc.h"
14+
#include "src/gpu/graphite/RenderPassDesc.h"
15+
16+
namespace skgpu::graphite {
17+
18+
// Once completed, the PipelineTask will lock the created Pipeline in the cache (via 'fPipeline')
19+
// until the PipelineTask is deleted.
20+
// Note that this is not a task in the sense of the Task class. It is a task in the sense that
21+
// it is a unit of work possibly delegated to a thread. PipelineCreationTasks are also only
22+
// known and handled by the PipelineManager vs being added to TaskLists (as Task-derived classes
23+
// are).
24+
class PipelineCreationTask : public SkRefCnt {
25+
private:
26+
friend class PipelineManager; // for entire API and fPipeline
27+
28+
PipelineCreationTask(const UniqueKey& pipelineKey,
29+
const GraphicsPipelineDesc& graphicsPipelineDesc,
30+
const RenderPassDesc& renderPassDesc,
31+
SkEnumBitMask<PipelineCreationFlags> pipelineCreationFlags)
32+
: fPipelineKey(pipelineKey)
33+
, fGraphicsPipelineDesc(graphicsPipelineDesc)
34+
, fRenderPassDesc(renderPassDesc)
35+
, fPipelineCreationFlags(pipelineCreationFlags) {}
36+
37+
const UniqueKey fPipelineKey; // used to track this task in the PipelineManager
38+
const GraphicsPipelineDesc fGraphicsPipelineDesc;
39+
const RenderPassDesc fRenderPassDesc;
40+
const SkEnumBitMask<PipelineCreationFlags> fPipelineCreationFlags;
41+
42+
// Once completed, this task will have filled in 'fPipeline' (if compilation succeeded).
43+
// This also serves to lock the pipeline in the cache.
44+
sk_sp<GraphicsPipeline> fPipeline;
45+
46+
std::atomic<bool> fCompleted = false;
47+
};
48+
49+
} // namespace skgpu::graphite
50+
51+
#endif // skgpu_graphite_PipelineCreationTask_DEFINED

src/gpu/graphite/PipelineManager.cpp

Lines changed: 172 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,172 @@
1+
/*
2+
* Copyright 2025 Google LLC
3+
*
4+
* Use of this source code is governed by a BSD-style license that can be
5+
* found in the LICENSE file.
6+
*/
7+
8+
#include "src/gpu/graphite/PipelineManager.h"
9+
10+
#include "src/core/SkTaskGroup.h"
11+
#include "src/gpu/graphite/GraphicsPipelineDesc.h"
12+
#include "src/gpu/graphite/GraphicsPipelineHandle.h"
13+
#include "src/gpu/graphite/Log.h"
14+
#include "src/gpu/graphite/PipelineCreationTask.h"
15+
#include "src/gpu/graphite/RenderPassDesc.h"
16+
#include "src/gpu/graphite/ResourceProvider.h"
17+
#include "src/gpu/graphite/RuntimeEffectDictionary.h"
18+
19+
namespace skgpu::graphite {
20+
21+
GraphicsPipelineHandle::GraphicsPipelineHandle(sk_sp<PipelineCreationTask> task)
22+
: fTaskOrPipeline(std::move(task)) {}
23+
24+
GraphicsPipelineHandle::GraphicsPipelineHandle(sk_sp<GraphicsPipeline> pipeline)
25+
: fTaskOrPipeline(std::move(pipeline)) {}
26+
27+
PipelineManager::PipelineManager() {}
28+
PipelineManager::~PipelineManager() {}
29+
30+
const UniqueKey& PipelineManager::Traits::GetKey(const sk_sp<PipelineCreationTask>& task) {
31+
return task->fPipelineKey;
32+
}
33+
34+
uint32_t PipelineManager::Traits::Hash(const UniqueKey& pipelineKey) {
35+
return pipelineKey.hash();
36+
}
37+
38+
GraphicsPipelineHandle PipelineManager::createHandle(
39+
ResourceProvider* resourceProvider,
40+
const GraphicsPipelineDesc& pipelineDesc,
41+
const RenderPassDesc& renderPassDesc,
42+
SkEnumBitMask<PipelineCreationFlags> pipelineCreationFlags) {
43+
const Caps* caps = resourceProvider->caps();
44+
45+
UniqueKey pipelineKey = caps->makeGraphicsPipelineKey(pipelineDesc, renderPassDesc);
46+
47+
if (sk_sp<PipelineCreationTask> task = this->findTask(pipelineKey)) {
48+
// There is a task in progress to compile this Pipeline so it can't be ready yet (i.e.,
49+
// it isn't in the Pipeline Cache).
50+
return GraphicsPipelineHandle(std::move(task));
51+
}
52+
53+
sk_sp<GraphicsPipeline> pipeline = resourceProvider->findGraphicsPipeline(
54+
pipelineKey,
55+
pipelineCreationFlags);
56+
if (pipeline) {
57+
return GraphicsPipelineHandle(std::move(pipeline));
58+
}
59+
60+
sk_sp<PipelineCreationTask> task = this->findOrCreateTask(pipelineKey,
61+
pipelineDesc,
62+
renderPassDesc,
63+
pipelineCreationFlags);
64+
return GraphicsPipelineHandle(std::move(task));
65+
}
66+
67+
void PipelineManager::startPipelineCreationTask(ResourceProvider* resourceProvider,
68+
sk_sp<const RuntimeEffectDictionary> runtimeDict,
69+
const GraphicsPipelineHandle& handle) {
70+
if (std::holds_alternative<sk_sp<GraphicsPipeline>>(handle.fTaskOrPipeline)) {
71+
return;
72+
}
73+
74+
sk_sp<PipelineCreationTask> task =
75+
std::get<sk_sp<PipelineCreationTask>>(handle.fTaskOrPipeline);
76+
77+
task->fPipeline = resourceProvider->findOrCreateGraphicsPipeline(
78+
runtimeDict.get(),
79+
task->fPipelineKey,
80+
task->fGraphicsPipelineDesc,
81+
task->fRenderPassDesc,
82+
task->fPipelineCreationFlags);
83+
84+
if (!task->fPipeline) {
85+
SKGPU_LOG_W("Failed to create GraphicsPipeline!");
86+
}
87+
88+
task->fCompleted = true;
89+
90+
this->removeTask(task.get());
91+
}
92+
93+
sk_sp<GraphicsPipeline> PipelineManager::resolveHandle(const GraphicsPipelineHandle& handle) {
94+
if (std::holds_alternative<sk_sp<GraphicsPipeline>>(handle.fTaskOrPipeline)) {
95+
return std::get<sk_sp<GraphicsPipeline>>(handle.fTaskOrPipeline);
96+
}
97+
98+
// Since 'fTaskOrPipeline' doesn't hold a pipeline the pipeline must not have existed when
99+
// the handle was created so a compilation task must've been created to compile it
100+
sk_sp<PipelineCreationTask> task =
101+
std::get<sk_sp<PipelineCreationTask>>(handle.fTaskOrPipeline);
102+
103+
// For the non-threaded version of the PipelineManager, whenever a thread gets here it
104+
// will already have blindly executed the task (in DrawPass::prepareResources).
105+
SkASSERT(task->fCompleted);
106+
return task->fPipeline;
107+
}
108+
109+
#if defined(GPU_TEST_UTILS)
110+
PipelineManager::Stats PipelineManager::getStats() const {
111+
SkAutoSpinlock lock{fSpinLock};
112+
113+
return fStats;
114+
}
115+
#endif
116+
117+
sk_sp<PipelineCreationTask> PipelineManager::findTask(const UniqueKey& pipelineKey) {
118+
SkAutoSpinlock lock{fSpinLock};
119+
120+
sk_sp<PipelineCreationTask> task = fActiveTasks.findOrNull(pipelineKey);
121+
122+
#if defined(GPU_TEST_UTILS)
123+
if (task) {
124+
fStats.fNumPreemptivelyFoundTasks++;
125+
}
126+
#endif
127+
128+
return task;
129+
}
130+
131+
sk_sp<PipelineCreationTask> PipelineManager::findOrCreateTask(
132+
const UniqueKey& pipelineKey,
133+
const GraphicsPipelineDesc& pipelineDesc,
134+
const RenderPassDesc& renderPassDesc,
135+
SkEnumBitMask<PipelineCreationFlags> pipelineCreationFlags) {
136+
SkAutoSpinlock lock{fSpinLock};
137+
138+
sk_sp<PipelineCreationTask>* task = fActiveTasks.find(pipelineKey);
139+
if (task) {
140+
// There is a race in createHandle from when we first check for a task; then, failing that,
141+
// check for an existing pipeline; then, failing that, try to create a new task. Thus,
142+
// we can sometimes find our task here.
143+
#if defined(GPU_TEST_UTILS)
144+
fStats.fNumTaskCreationRaces++;
145+
#endif
146+
return *task;
147+
}
148+
149+
#if defined(GPU_TEST_UTILS)
150+
fStats.fNumTasksCreated++;
151+
#endif
152+
153+
sk_sp<PipelineCreationTask> newTask = sk_sp<PipelineCreationTask>(
154+
new PipelineCreationTask(pipelineKey,
155+
pipelineDesc,
156+
renderPassDesc,
157+
pipelineCreationFlags));
158+
fActiveTasks.set(newTask);
159+
return newTask;
160+
}
161+
162+
void PipelineManager::removeTask(PipelineCreationTask* task) {
163+
SkAutoSpinlock lock{fSpinLock};
164+
165+
// TODO(robertphillips): this guard is only necessary in the non-threaded version of
166+
// the PipelineManager
167+
if (fActiveTasks.findOrNull(task->fPipelineKey)) {
168+
fActiveTasks.remove(task->fPipelineKey);
169+
}
170+
}
171+
172+
} // namespace skgpu::graphite

0 commit comments

Comments
 (0)