Skip to content

Commit 8380311

Browse files
kungasckungasc
committed
cr fixes round 1
1 parent e1703a0 commit 8380311

File tree

4 files changed

+102
-87
lines changed

4 files changed

+102
-87
lines changed

‎ydb/core/tablet_flat/shared_cache_s3fifo.h‎

Lines changed: 34 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,16 @@ namespace NKikimr::NCache {
99

1010
// TODO: remove template args and make some page base class
1111

12+
enum class ES3FIFOPageLocation {
13+
None,
14+
SmallQueue,
15+
MainQueue
16+
};
17+
1218
template <typename TPageKey
1319
, typename TPageKeyHash
1420
, typename TPageKeyEqual>
15-
class TTS3FIFOGhostPageQueue {
21+
class TS3FIFOGhostPageQueue {
1622
struct TGhostPage {
1723
TPageKey Key;
1824
ui64 Size; // zero size is tombstone
@@ -36,7 +42,7 @@ class TTS3FIFOGhostPageQueue {
3642
};
3743

3844
public:
39-
TTS3FIFOGhostPageQueue(ui64 limit)
45+
TS3FIFOGhostPageQueue(ui64 limit)
4046
: Limit(limit)
4147
{}
4248

@@ -50,6 +56,7 @@ class TTS3FIFOGhostPageQueue {
5056
if (Y_UNLIKELY(!GhostsSet.emplace(ghost).second)) {
5157
GhostsQueue.pop_back();
5258
Y_DEBUG_ABORT_S("Duplicated " << key.ToString() << " page");
59+
return;
5360
}
5461

5562
Size += ghost->Size;
@@ -60,7 +67,7 @@ class TTS3FIFOGhostPageQueue {
6067
bool Erase(const TPageKey& key, ui64 size) {
6168
TGhostPage key_(key, size);
6269
if (auto it = GhostsSet.find(&key_); it != GhostsSet.end()) {
63-
TGhostPage* ghost = const_cast<TGhostPage*>(*it);
70+
TGhostPage* ghost = *it;
6471
Y_DEBUG_ABORT_UNLESS(ghost->Size == size);
6572
Y_ABORT_UNLESS(Size >= ghost->Size);
6673
Size -= ghost->Size;
@@ -102,7 +109,8 @@ class TTS3FIFOGhostPageQueue {
102109
if (ghost->Size) { // isn't deleted
103110
Y_ABORT_UNLESS(Size >= ghost->Size);
104111
Size -= ghost->Size;
105-
Y_ABORT_UNLESS(GhostsSet.erase(ghost));
112+
bool erased = GhostsSet.erase(ghost);
113+
Y_ABORT_UNLESS(erased);
106114
}
107115
GhostsQueue.pop_front();
108116
}
@@ -123,12 +131,6 @@ template <typename TPage
123131
, typename TPageFrequency
124132
>
125133
class TS3FIFOCache : public ICacheCache<TPage> {
126-
enum class ELocation {
127-
None,
128-
SmallQueue,
129-
MainQueue
130-
};
131-
132134
struct TLimit {
133135
ui64 SmallQueueLimit;
134136
ui64 MainQueueLimit;
@@ -140,20 +142,20 @@ class TS3FIFOCache : public ICacheCache<TPage> {
140142
};
141143

142144
struct TQueue {
143-
TQueue(ELocation location)
145+
TQueue(ES3FIFOPageLocation location)
144146
: Location(location)
145147
{}
146148

147-
ELocation Location;
149+
ES3FIFOPageLocation Location;
148150
TIntrusiveList<TPage> Queue;
149151
ui64 Size = 0;
150152
};
151153

152154
public:
153155
TS3FIFOCache(ui64 limit)
154156
: Limit(limit)
155-
, SmallQueue(ELocation::SmallQueue)
156-
, MainQueue(ELocation::MainQueue)
157+
, SmallQueue(ES3FIFOPageLocation::SmallQueue)
158+
, MainQueue(ES3FIFOPageLocation::MainQueue)
157159
, GhostQueue(limit)
158160
{}
159161

@@ -174,30 +176,30 @@ class TS3FIFOCache : public ICacheCache<TPage> {
174176
}
175177

176178
TIntrusiveList<TPage> Touch(TPage* page) override {
177-
const ELocation location = GetLocation(page);
179+
const ES3FIFOPageLocation location = GetLocation(page);
178180
switch (location) {
179-
case ELocation::SmallQueue:
180-
case ELocation::MainQueue: {
181+
case ES3FIFOPageLocation::SmallQueue:
182+
case ES3FIFOPageLocation::MainQueue: {
181183
TouchFast(page);
182184
return {};
183185
}
184-
case ELocation::None:
186+
case ES3FIFOPageLocation::None:
185187
return Insert(page);
186188
default:
187189
Y_ABORT("Unknown page location");
188190
}
189191
}
190192

191193
void Erase(TPage* page) override {
192-
const ELocation location = GetLocation(page);
194+
const ES3FIFOPageLocation location = GetLocation(page);
193195
switch (location) {
194-
case ELocation::None:
196+
case ES3FIFOPageLocation::None:
195197
EraseGhost(page);
196198
break;
197-
case ELocation::SmallQueue:
199+
case ES3FIFOPageLocation::SmallQueue:
198200
Erase(SmallQueue, page);
199201
break;
200-
case ELocation::MainQueue:
202+
case ES3FIFOPageLocation::MainQueue:
201203
Erase(MainQueue, page);
202204
break;
203205
default:
@@ -267,7 +269,7 @@ class TS3FIFOCache : public ICacheCache<TPage> {
267269
}
268270

269271
void TouchFast(TPage* page) {
270-
Y_DEBUG_ABORT_UNLESS(GetLocation(page) != ELocation::None);
272+
Y_DEBUG_ABORT_UNLESS(GetLocation(page) != ES3FIFOPageLocation::None);
271273

272274
ui32 frequency = GetFrequency(page);
273275
if (frequency < 3) {
@@ -276,7 +278,7 @@ class TS3FIFOCache : public ICacheCache<TPage> {
276278
}
277279

278280
TIntrusiveList<TPage> Insert(TPage* page) {
279-
Y_DEBUG_ABORT_UNLESS(GetLocation(page) == ELocation::None);
281+
Y_DEBUG_ABORT_UNLESS(GetLocation(page) == ES3FIFOPageLocation::None);
280282

281283
Push(EraseGhost(page) ? MainQueue : SmallQueue, page);
282284
SetFrequency(page, 0);
@@ -296,13 +298,13 @@ class TS3FIFOCache : public ICacheCache<TPage> {
296298

297299
TPage* page = queue.Queue.PopFront();
298300
queue.Size -= GetSize(page);
299-
SetLocation(page, ELocation::None);
301+
SetLocation(page, ES3FIFOPageLocation::None);
300302

301303
return page;
302304
}
303305

304306
void Push(TQueue& queue, TPage* page) {
305-
Y_ABORT_UNLESS(GetLocation(page) == ELocation::None);
307+
Y_ABORT_UNLESS(GetLocation(page) == ES3FIFOPageLocation::None);
306308

307309
queue.Queue.PushBack(page);
308310
queue.Size += GetSize(page);
@@ -315,7 +317,7 @@ class TS3FIFOCache : public ICacheCache<TPage> {
315317

316318
page->Unlink();
317319
queue.Size -= GetSize(page);
318-
SetLocation(page, ELocation::None);
320+
SetLocation(page, ES3FIFOPageLocation::None);
319321
}
320322

321323
void AddGhost(const TPage* page) {
@@ -334,12 +336,12 @@ class TS3FIFOCache : public ICacheCache<TPage> {
334336
return TPageSize::Get(page);
335337
}
336338

337-
ELocation GetLocation(const TPage* page) const {
338-
return static_cast<ELocation>(TPageLocation::Get(page));
339+
ES3FIFOPageLocation GetLocation(const TPage* page) const {
340+
return TPageLocation::Get(page);
339341
}
340342

341-
void SetLocation(TPage* page, ELocation location) const {
342-
TPageLocation::Set(page, static_cast<ui32>(location));
343+
void SetLocation(TPage* page, ES3FIFOPageLocation location) const {
344+
TPageLocation::Set(page, location);
343345
}
344346

345347
ui32 GetFrequency(const TPage* page) const {
@@ -354,7 +356,7 @@ class TS3FIFOCache : public ICacheCache<TPage> {
354356
TLimit Limit;
355357
TQueue SmallQueue;
356358
TQueue MainQueue;
357-
TTS3FIFOGhostPageQueue<TPageKey, TPageKeyHash, TPageKeyEqual> GhostQueue;
359+
TS3FIFOGhostPageQueue<TPageKey, TPageKeyHash, TPageKeyEqual> GhostQueue;
358360

359361
};
360362

‎ydb/core/tablet_flat/shared_cache_s3fifo_ut.cpp‎

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -52,22 +52,23 @@ namespace {
5252
};
5353

5454
struct TPageLocation {
55-
static ui32 Get(const TPage *page) {
56-
return page->CacheFlags1;
55+
static ES3FIFOPageLocation Get(const TPage *page) {
56+
return static_cast<ES3FIFOPageLocation>(page->CacheFlags1);
5757
}
58-
static void Set(TPage *x, ui32 flags) {
59-
Y_ABORT_UNLESS(flags < (1 << 4));
60-
x->CacheFlags1 = flags;
58+
static void Set(TPage *x, ES3FIFOPageLocation location) {
59+
ui32 generation_ = static_cast<ui32>(location);
60+
Y_ABORT_UNLESS(generation_ < (1 << 4));
61+
x->CacheFlags1 = generation_;
6162
}
6263
};
6364

6465
struct TPageFrequency {
6566
static ui32 Get(const TPage *page) {
6667
return page->CacheFlags2;
6768
}
68-
static void Set(TPage *page, ui32 flags) {
69-
Y_ABORT_UNLESS(flags < (1 << 4));
70-
page->CacheFlags2 = flags;
69+
static void Set(TPage *x, ui32 frequency) {
70+
Y_ABORT_UNLESS(frequency < (1 << 4));
71+
x->CacheFlags2 = frequency;
7172
}
7273
};
7374

@@ -76,7 +77,7 @@ namespace {
7677
Y_UNIT_TEST_SUITE(TS3FIFOGhostQueue) {
7778

7879
Y_UNIT_TEST(Add) {
79-
TTS3FIFOGhostPageQueue<TPageKey, TPageKeyHash, TPageKeyEqual> queue(100);
80+
TS3FIFOGhostPageQueue<TPageKey, TPageKeyHash, TPageKeyEqual> queue(100);
8081
UNIT_ASSERT_VALUES_EQUAL(queue.Dump(), "");
8182

8283
queue.Add(1, 10);
@@ -96,7 +97,7 @@ Y_UNIT_TEST_SUITE(TS3FIFOGhostQueue) {
9697
}
9798

9899
Y_UNIT_TEST(Erase) {
99-
TTS3FIFOGhostPageQueue<TPageKey, TPageKeyHash, TPageKeyEqual> queue(100);
100+
TS3FIFOGhostPageQueue<TPageKey, TPageKeyHash, TPageKeyEqual> queue(100);
100101
UNIT_ASSERT_VALUES_EQUAL(queue.Dump(), "");
101102

102103
queue.Add(1, 10);
@@ -118,7 +119,7 @@ Y_UNIT_TEST_SUITE(TS3FIFOGhostQueue) {
118119
}
119120

120121
Y_UNIT_TEST(Erase_Add) {
121-
TTS3FIFOGhostPageQueue<TPageKey, TPageKeyHash, TPageKeyEqual> queue(100);
122+
TS3FIFOGhostPageQueue<TPageKey, TPageKeyHash, TPageKeyEqual> queue(100);
122123
UNIT_ASSERT_VALUES_EQUAL(queue.Dump(), "");
123124

124125
queue.Add(1, 10);
@@ -137,15 +138,15 @@ Y_UNIT_TEST_SUITE(TS3FIFOGhostQueue) {
137138
}
138139

139140
Y_UNIT_TEST(Add_Big) {
140-
TTS3FIFOGhostPageQueue<TPageKey, TPageKeyHash, TPageKeyEqual> queue(100);
141+
TS3FIFOGhostPageQueue<TPageKey, TPageKeyHash, TPageKeyEqual> queue(100);
141142
UNIT_ASSERT_VALUES_EQUAL(queue.Dump(), "");
142143

143144
queue.Add(1, 101);
144145
UNIT_ASSERT_VALUES_EQUAL(queue.Dump(), "");
145146
}
146147

147148
Y_UNIT_TEST(UpdateLimit) {
148-
TTS3FIFOGhostPageQueue<TPageKey, TPageKeyHash, TPageKeyEqual> queue(100);
149+
TS3FIFOGhostPageQueue<TPageKey, TPageKeyHash, TPageKeyEqual> queue(100);
149150
UNIT_ASSERT_VALUES_EQUAL(queue.Dump(), "");
150151

151152
queue.Add(1, 10);

‎ydb/core/tablet_flat/shared_sausagecache.cpp‎

Lines changed: 24 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -146,23 +146,35 @@ class TSharedPageCache : public TActorBootstrapped<TSharedPageCache> {
146146
}
147147
};
148148

149-
struct TCacheFlags1 {
150-
static ui32 Get(const TPage *page) {
151-
return page->CacheFlags1;
149+
struct TS3FIFOPageLocation {
150+
static ES3FIFOPageLocation Get(const TPage *page) {
151+
return static_cast<ES3FIFOPageLocation>(page->CacheFlags1);
152152
}
153-
static void Set(TPage *x, ui32 flags) {
154-
Y_ABORT_UNLESS(flags < (1 << 4));
155-
x->CacheFlags1 = flags;
153+
static void Set(TPage *x, ES3FIFOPageLocation location) {
154+
ui32 location_ = static_cast<ui32>(location);
155+
Y_ABORT_UNLESS(location_ < (1 << 4));
156+
x->CacheFlags1 = location_;
156157
}
157158
};
158159

159-
struct TCacheFlags2 {
160+
struct TS3FIFOPageFrequency {
160161
static ui32 Get(const TPage *page) {
161162
return page->CacheFlags2;
162163
}
163-
static void Set(TPage *page, ui32 flags) {
164-
Y_ABORT_UNLESS(flags < (1 << 4));
165-
page->CacheFlags2 = flags;
164+
static void Set(TPage *x, ui32 frequency) {
165+
Y_ABORT_UNLESS(frequency < (1 << 4));
166+
x->CacheFlags2 = frequency;
167+
}
168+
};
169+
170+
struct TCacheCacheGeneration {
171+
static ECacheCacheGeneration Get(const TPage *page) {
172+
return static_cast<ECacheCacheGeneration>(page->CacheFlags1);
173+
}
174+
static void Set(TPage *x, ECacheCacheGeneration generation) {
175+
ui32 generation_ = static_cast<ui32>(generation);
176+
Y_ABORT_UNLESS(generation_ < (1 << 4));
177+
x->CacheFlags1 = generation_;
166178
}
167179
};
168180
};
@@ -256,11 +268,11 @@ class TSharedPageCache : public TActorBootstrapped<TSharedPageCache> {
256268

257269
switch (Config->ReplacementPolicy) {
258270
case NKikimrSharedCache::S3FIFO:
259-
return MakeHolder<TS3FIFOCache<TPage, TPage::TKey, TPage::TKeyHash, TPage::TKeyEqual, TPage::TSize, TPage::TCacheFlags1, TPage::TCacheFlags2>>(1);
271+
return MakeHolder<TS3FIFOCache<TPage, TPage::TKey, TPage::TKeyHash, TPage::TKeyEqual, TPage::TSize, TPage::TS3FIFOPageLocation, TPage::TS3FIFOPageFrequency>>(1);
260272
case NKikimrSharedCache::ThreeLeveledLRU:
261273
default: {
262274
TCacheCacheConfig cacheCacheConfig(1, Config->Counters->FreshBytes, Config->Counters->StagingBytes, Config->Counters->WarmBytes);
263-
return MakeHolder<TCacheCache<TPage, TPage::TSize, TPage::TCacheFlags1>>(std::move(cacheCacheConfig));
275+
return MakeHolder<TCacheCache<TPage, TPage::TSize, TPage::TCacheCacheGeneration>>(std::move(cacheCacheConfig));
264276
}
265277
}
266278
}

0 commit comments

Comments
 (0)