Skip to content

Commit 5c7bb32

Browse files
johnstiles-googleSkia Commit-Bot
authored andcommitted
Reland "Create a basic IRNode pooling system."
This is a reland of e16eca9 This fixes the no-op (iOS) implementation of CreatePoolOnThread. Original change's description: > Create a basic IRNode pooling system. > > Allocations are redirected by overriding `operator new` and `operator > delete` on the IRNode class. This allows us to use our existing > `unique_ptr` and `make_unique` calls as-is. The Pool class is simple; > it holds a fixed number of nodes and recycles them as they are returned. > > A fixed pool size of 2000 nodes was chosen. That is large enough to hold > the contents of `sksl_large` during compilation, but it can be > overflowed by very large shaders, or if multiple programs are converted > at the same time. Exhausting the pool is not a problem; if this happens, > additional nodes will be allocated via the system allocator as usual. > More elaborate schemes are possible but might not add a lot of value. > > Thread safety is accomplished by placing the pool in a `thread_local` > static during a Program's creation and destruction; the pool is freed > when the program is destroyed. One important consequence of this > strategy is that a program must free every node that it allocated during > its creation, or else the node will be leaked. In debug, leaking a node > will be detected and causes a DEBUGFAIL. In release, the pool will be > freed despite having a live node in it, and if that node is later freed, > that pointer will be passed to the system `free` (which is likely to > cause a crash). > > In this CL, iOS does not support pooling, since support for > `thread_local` was only added on iOS 9. This is fixed in the followup > CL, http://review.skia.org/328837, which uses pthread keys on iOS. > > Nanobench shows ~15% improvement: > (last week) http://screen/5CNBhTaZApcDA8h > (today) http://screen/8ti5Rymvf6LUs8i > > Change-Id: I559de73606ee1be54e5eae7f82129dc928a63e3c > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/326876 > Commit-Queue: John Stiles <johnstiles@google.com> > Reviewed-by: Ethan Nicholas <ethannicholas@google.com> > Auto-Submit: John Stiles <johnstiles@google.com> Change-Id: I8623a574a7e92332ff00b83982497863c8953929 Reviewed-on: https://skia-review.googlesource.com/c/skia/+/329171 Commit-Queue: John Stiles <johnstiles@google.com> Commit-Queue: Brian Osman <brianosman@google.com> Auto-Submit: John Stiles <johnstiles@google.com> Reviewed-by: Brian Osman <brianosman@google.com>
1 parent f15a598 commit 5c7bb32

File tree

6 files changed

+284
-13
lines changed

6 files changed

+284
-13
lines changed

gn/sksl.gni

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,8 @@ skia_sksl_sources = [
3838
"$_src/sksl/SkSLMemoryLayout.h",
3939
"$_src/sksl/SkSLParser.cpp",
4040
"$_src/sksl/SkSLParser.h",
41+
"$_src/sksl/SkSLPool.cpp",
42+
"$_src/sksl/SkSLPool.h",
4143
"$_src/sksl/SkSLPosition.h",
4244
"$_src/sksl/SkSLRehydrator.cpp",
4345
"$_src/sksl/SkSLRehydrator.h",

src/sksl/SkSLCompiler.cpp

Lines changed: 22 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1552,24 +1552,33 @@ std::unique_ptr<Program> Compiler::convertProgram(
15521552

15531553
const ParsedModule& baseModule = this->moduleForProgramKind(kind);
15541554

1555+
// Enable node pooling while converting and optimizing the program for a performance boost.
1556+
// The Program will take ownership of the pool.
1557+
std::unique_ptr<Pool> pool = Pool::CreatePoolOnThread(2000);
15551558
IRGenerator::IRBundle ir =
15561559
fIRGenerator->convertProgram(kind, &settings, baseModule, /*isBuiltinCode=*/false,
15571560
textPtr->c_str(), textPtr->size(), externalValues);
1558-
auto result = std::make_unique<Program>(kind,
1559-
std::move(textPtr),
1560-
settings,
1561-
fContext,
1562-
std::move(ir.fElements),
1563-
std::move(ir.fModifiers),
1564-
std::move(ir.fSymbolTable),
1565-
ir.fInputs);
1561+
auto program = std::make_unique<Program>(kind,
1562+
std::move(textPtr),
1563+
settings,
1564+
fContext,
1565+
std::move(ir.fElements),
1566+
std::move(ir.fModifiers),
1567+
std::move(ir.fSymbolTable),
1568+
std::move(pool),
1569+
ir.fInputs);
1570+
bool success = false;
15661571
if (fErrorCount) {
1567-
return nullptr;
1568-
}
1569-
if (settings.fOptimize && !this->optimize(*result)) {
1570-
return nullptr;
1572+
// Do not return programs that failed to compile.
1573+
} else if (settings.fOptimize && !this->optimize(*program)) {
1574+
// Do not return programs that failed to optimize.
1575+
} else {
1576+
// We have a successful program!
1577+
success = true;
15711578
}
1572-
return result;
1579+
1580+
program->fPool->detachFromThread();
1581+
return success ? std::move(program) : nullptr;
15731582
}
15741583

15751584
bool Compiler::optimize(Program& program) {

src/sksl/SkSLPool.cpp

Lines changed: 178 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,178 @@
1+
/*
2+
* Copyright 2020 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/sksl/SkSLPool.h"
9+
10+
#include "src/sksl/ir/SkSLIRNode.h"
11+
12+
#define VLOG(...) // printf(__VA_ARGS__)
13+
14+
namespace SkSL {
15+
16+
#if defined(SK_BUILD_FOR_IOS) && \
17+
(!defined(__IPHONE_9_0) || __IPHONE_OS_VERSION_MIN_REQUIRED < __IPHONE_9_0)
18+
19+
// iOS did not support for C++11 `thread_local` variables until iOS 9.
20+
// Pooling is not supported here; we allocate all nodes directly.
21+
struct PoolData {};
22+
23+
Pool::~Pool() {}
24+
std::unique_ptr<Pool> Pool::CreatePoolOnThread(int nodesInPool) {
25+
auto pool = std::unique_ptr<Pool>(new Pool);
26+
pool->fData = nullptr;
27+
return pool;
28+
}
29+
void Pool::detachFromThread() {}
30+
void Pool::attachToThread() {}
31+
void* Pool::AllocIRNode() { return ::operator new(sizeof(IRNode)); }
32+
void Pool::FreeIRNode(void* node) { ::operator delete(node); }
33+
34+
#else // !defined(SK_BUILD_FOR_IOS)...
35+
36+
namespace { struct IRNodeData {
37+
union {
38+
uint8_t fBuffer[sizeof(IRNode)];
39+
IRNodeData* fFreeListNext;
40+
};
41+
}; }
42+
43+
struct PoolData {
44+
// This holds the first free node in the pool. It will be null when the pool is exhausted.
45+
IRNodeData* fFreeListHead = fNodes;
46+
47+
// This points to end of our pooled data, and implies the number of nodes.
48+
IRNodeData* fNodesEnd = nullptr;
49+
50+
// Our pooled data lives here. (We allocate lots of nodes here, not just one.)
51+
IRNodeData fNodes[1];
52+
53+
// Accessors.
54+
ptrdiff_t nodeCount() { return fNodesEnd - fNodes; }
55+
56+
ptrdiff_t nodeIndex(IRNodeData* node) {
57+
SkASSERT(node >= fNodes);
58+
SkASSERT(node < fNodesEnd);
59+
return node - fNodes;
60+
}
61+
};
62+
63+
static thread_local PoolData* sPoolData = nullptr;
64+
65+
static PoolData* create_pool_data(int nodesInPool) {
66+
// Create a PoolData structure with extra space at the end for additional IRNode data.
67+
int numExtraIRNodes = nodesInPool - 1;
68+
PoolData* poolData = static_cast<PoolData*>(malloc(sizeof(PoolData) +
69+
(sizeof(IRNodeData) * numExtraIRNodes)));
70+
71+
// Initialize each pool node as a free node. The free nodes form a singly-linked list, each
72+
// pointing to the next free node in sequence.
73+
for (int index = 0; index < nodesInPool - 1; ++index) {
74+
poolData->fNodes[index].fFreeListNext = &poolData->fNodes[index + 1];
75+
}
76+
poolData->fNodes[nodesInPool - 1].fFreeListNext = nullptr;
77+
poolData->fNodesEnd = &poolData->fNodes[nodesInPool];
78+
79+
return poolData;
80+
}
81+
82+
Pool::~Pool() {
83+
if (sPoolData == fData) {
84+
SkDEBUGFAIL("SkSL pool is being destroyed while it is still attached to the thread");
85+
sPoolData = nullptr;
86+
}
87+
88+
// In debug mode, report any leaked nodes.
89+
#ifdef SK_DEBUG
90+
ptrdiff_t nodeCount = fData->nodeCount();
91+
std::vector<bool> freed(nodeCount);
92+
for (IRNodeData* node = fData->fFreeListHead; node; node = node->fFreeListNext) {
93+
ptrdiff_t nodeIndex = fData->nodeIndex(node);
94+
freed[nodeIndex] = true;
95+
}
96+
bool foundLeaks = false;
97+
for (int index = 0; index < nodeCount; ++index) {
98+
if (!freed[index]) {
99+
IRNode* leak = reinterpret_cast<IRNode*>(fData->fNodes[index].fBuffer);
100+
SkDebugf("Node %d leaked: %s\n", index, leak->description().c_str());
101+
foundLeaks = true;
102+
}
103+
}
104+
if (foundLeaks) {
105+
SkDEBUGFAIL("leaking SkSL pool nodes; if they are later freed, this will likely be fatal");
106+
}
107+
#endif
108+
109+
VLOG("DELETE Pool:0x%016llX\n", (uint64_t)fData);
110+
free(fData);
111+
}
112+
113+
std::unique_ptr<Pool> Pool::CreatePoolOnThread(int nodesInPool) {
114+
auto pool = std::unique_ptr<Pool>(new Pool);
115+
pool->fData = create_pool_data(nodesInPool);
116+
pool->fData->fFreeListHead = &pool->fData->fNodes[0];
117+
VLOG("CREATE Pool:0x%016llX\n", (uint64_t)pool->fData);
118+
pool->attachToThread();
119+
return pool;
120+
}
121+
122+
void Pool::detachFromThread() {
123+
VLOG("DETACH Pool:0x%016llX\n", (uint64_t)sPoolData);
124+
SkASSERT(sPoolData != nullptr);
125+
sPoolData = nullptr;
126+
}
127+
128+
void Pool::attachToThread() {
129+
VLOG("ATTACH Pool:0x%016llX\n", (uint64_t)fData);
130+
SkASSERT(sPoolData == nullptr);
131+
sPoolData = fData;
132+
}
133+
134+
void* Pool::AllocIRNode() {
135+
// Is a pool attached?
136+
if (sPoolData) {
137+
// Does the pool contain a free node?
138+
IRNodeData* node = sPoolData->fFreeListHead;
139+
if (node) {
140+
// Yes. Take a node from the freelist.
141+
sPoolData->fFreeListHead = node->fFreeListNext;
142+
VLOG("ALLOC Pool:0x%016llX Index:%04d 0x%016llX\n",
143+
(uint64_t)sPoolData, (int)(node - &sPoolData->fNodes[0]), (uint64_t)node);
144+
return node->fBuffer;
145+
}
146+
}
147+
148+
// The pool is detached or full; allocate nodes using malloc.
149+
void* ptr = ::operator new(sizeof(IRNode));
150+
VLOG("ALLOC Pool:0x%016llX Index:____ malloc 0x%016llX\n",
151+
(uint64_t)sPoolData, (uint64_t)ptr);
152+
return ptr;
153+
}
154+
155+
void Pool::FreeIRNode(void* node_v) {
156+
// Is a pool attached?
157+
if (sPoolData) {
158+
// Did this node come from our pool?
159+
auto* node = static_cast<IRNodeData*>(node_v);
160+
if (node >= &sPoolData->fNodes[0] && node < sPoolData->fNodesEnd) {
161+
// Yes. Push it back onto the freelist.
162+
VLOG("FREE Pool:0x%016llX Index:%04d 0x%016llX\n",
163+
(uint64_t)sPoolData, (int)(node - &sPoolData->fNodes[0]), (uint64_t)node);
164+
node->fFreeListNext = sPoolData->fFreeListHead;
165+
sPoolData->fFreeListHead = node;
166+
return;
167+
}
168+
}
169+
170+
// No pool is attached or the node was malloced; it must be freed.
171+
VLOG("FREE Pool:0x%016llX Index:____ free 0x%016llX\n",
172+
(uint64_t)sPoolData, (uint64_t)node_v);
173+
::operator delete(node_v);
174+
}
175+
176+
#endif // !defined(SK_BUILD_FOR_IOS)...
177+
178+
} // namespace SkSL

src/sksl/SkSLPool.h

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
/*
2+
* Copyright 2020 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 SKSL_POOL
9+
#define SKSL_POOL
10+
11+
#include <memory>
12+
13+
namespace SkSL {
14+
15+
class IRNode;
16+
struct PoolData;
17+
18+
class Pool {
19+
public:
20+
~Pool();
21+
22+
// Creates a pool to store newly-created IRNodes during program creation and attaches it to the
23+
// current thread. When your program is complete, call pool->detachFromThread() to transfer
24+
// ownership of those nodes. Before destroying any of the program's nodes, reattach the pool via
25+
// pool->attachToThread(). It is an error to call CreatePoolOnThread if a pool is already
26+
// attached to the current thread.
27+
static std::unique_ptr<Pool> CreatePoolOnThread(int nodesInPool);
28+
29+
// Once a pool has been created and the ephemeral work has completed, detach it from its thread.
30+
// It is an error to call this while no pool is attached.
31+
void detachFromThread();
32+
33+
// Reattaches a pool to the current thread. It is an error to call this while a pool is already
34+
// attached.
35+
void attachToThread();
36+
37+
// Retrieves a node from the thread pool. If the pool is exhausted, this will allocate a node.
38+
static void* AllocIRNode();
39+
40+
// Releases a node that was created by AllocIRNode. This will return it to the pool, or free it,
41+
// as appropriate. Make sure to free all nodes, since some of them may be real allocations.
42+
static void FreeIRNode(void* node_v);
43+
44+
private:
45+
Pool() = default; // use CreatePoolOnThread to make a pool
46+
PoolData* fData = nullptr;
47+
};
48+
49+
} // namespace SkSL
50+
51+
#endif

src/sksl/ir/SkSLIRNode.h

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
#include "src/sksl/SkSLASTNode.h"
1313
#include "src/sksl/SkSLLexer.h"
1414
#include "src/sksl/SkSLModifiersPool.h"
15+
#include "src/sksl/SkSLPool.h"
1516
#include "src/sksl/SkSLString.h"
1617

1718
#include <algorithm>
@@ -64,6 +65,20 @@ class IRNode {
6465
// purposes
6566
int fOffset;
6667

68+
// Override operator new and delete to allow us to control allocation behavior.
69+
static void* operator new(const size_t size) {
70+
// TODO: once all IRNodes hold their data in fData, everything should come out of the pool,
71+
// and this check should become an assertion.
72+
if (size == sizeof(IRNode)) {
73+
return Pool::AllocIRNode();
74+
}
75+
return ::operator new(size);
76+
}
77+
78+
static void operator delete(void* ptr) {
79+
Pool::FreeIRNode(ptr);
80+
}
81+
6782
protected:
6883
struct BlockData {
6984
std::shared_ptr<SymbolTable> fSymbolTable;

src/sksl/ir/SkSLProgram.h

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
namespace SkSL {
3333

3434
class Context;
35+
class Pool;
3536

3637
/**
3738
* Represents a fully-digested program, ready for code generation.
@@ -165,16 +166,30 @@ struct Program {
165166
std::vector<std::unique_ptr<ProgramElement>> elements,
166167
std::unique_ptr<ModifiersPool> modifiers,
167168
std::shared_ptr<SymbolTable> symbols,
169+
std::unique_ptr<Pool> pool,
168170
Inputs inputs)
169171
: fKind(kind)
170172
, fSource(std::move(source))
171173
, fSettings(settings)
172174
, fContext(context)
173175
, fSymbols(symbols)
176+
, fPool(std::move(pool))
174177
, fInputs(inputs)
175178
, fElements(std::move(elements))
176179
, fModifiers(std::move(modifiers)) {}
177180

181+
~Program() {
182+
// Some or all of the program elements are in the pool. To free them safely, we must attach
183+
// the pool before destroying any program elements. (Otherwise, we may accidentally call
184+
// delete on a pooled node.)
185+
fPool->attachToThread();
186+
fElements.clear();
187+
fContext.reset();
188+
fSymbols.reset();
189+
fModifiers.reset();
190+
fPool->detachFromThread();
191+
}
192+
178193
const std::vector<std::unique_ptr<ProgramElement>>& elements() const { return fElements; }
179194

180195
Kind fKind;
@@ -184,6 +199,7 @@ struct Program {
184199
// it's important to keep fElements defined after (and thus destroyed before) fSymbols,
185200
// because destroying elements can modify reference counts in symbols
186201
std::shared_ptr<SymbolTable> fSymbols;
202+
std::unique_ptr<Pool> fPool;
187203
Inputs fInputs;
188204

189205
private:

0 commit comments

Comments
 (0)