Skip to content

Commit dfba56d

Browse files
cyx1231stcyx1231st
committed
crimson/os/seastore: simplify cache access metrics
Don't distinguish between cache_absent and load_absent. Drop cache_access_stats_t::cache_absent and unify with extent_access_stats_t. Signed-off-by: Yingxin Cheng <[email protected]>
1 parent 1e29203 commit dfba56d

File tree

7 files changed

+53
-145
lines changed

7 files changed

+53
-145
lines changed

‎src/crimson/os/seastore/backref/btree_backref_manager.cc‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,12 +36,10 @@ const get_phy_tree_root_node_ret get_phy_tree_root_node<
3636
return {true,
3737
c.cache.get_extent_viewable_by_trans(c.trans, backref_root)};
3838
} else {
39-
c.cache.account_absent_access(c.trans.get_src());
4039
return {false,
4140
Cache::get_extent_iertr::make_ready_future<CachedExtentRef>()};
4241
}
4342
} else {
44-
c.cache.account_absent_access(c.trans.get_src());
4543
return {false,
4644
Cache::get_extent_iertr::make_ready_future<CachedExtentRef>()};
4745
}

‎src/crimson/os/seastore/btree/fixed_kv_btree.h‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1503,7 +1503,6 @@ class FixedKVBtree {
15031503
return on_found(child->template cast<internal_node_t>());
15041504
});
15051505
}
1506-
c.cache.account_absent_access(c.trans.get_src());
15071506

15081507
auto child_pos = v.get_child_pos();
15091508
auto next_iter = node_iter + 1;
@@ -1575,7 +1574,6 @@ class FixedKVBtree {
15751574
return on_found(child->template cast<leaf_node_t>());
15761575
});
15771576
}
1578-
c.cache.account_absent_access(c.trans.get_src());
15791577

15801578
auto child_pos = v.get_child_pos();
15811579
auto next_iter = node_iter + 1;
@@ -2135,7 +2133,6 @@ class FixedKVBtree {
21352133
return do_merge(child->template cast<NodeType>());
21362134
});
21372135
}
2138-
c.cache.account_absent_access(c.trans.get_src());
21392136

21402137
auto child_pos = v.get_child_pos();
21412138
return get_node<NodeType>(

‎src/crimson/os/seastore/cache.cc‎

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -208,7 +208,7 @@ void Cache::register_metrics()
208208
sm::make_counter(
209209
"cache_hit",
210210
[this] {
211-
return stats.access.s.get_cache_hit();
211+
return stats.access.get_cache_hit();
212212
},
213213
sm::description("total number of cache hits")
214214
),
@@ -2417,22 +2417,20 @@ cache_stats_t Cache::get_stats(
24172417
oss << "\ncache total"
24182418
<< cache_size_stats_t{extents_index.get_bytes(), extents_index.size()};
24192419

2420-
counter_by_src_t<counter_by_extent_t<extent_access_stats_t> >
2420+
counter_by_src_t<counter_by_extent_t<cache_access_stats_t> >
24212421
_access_by_src_ext = stats.access_by_src_ext;
24222422
counter_by_src_t<cache_access_stats_t> access_by_src;
24232423
for (uint8_t _src=0; _src<TRANSACTION_TYPE_MAX; ++_src) {
24242424
auto src = static_cast<transaction_type_t>(_src);
24252425
cache_access_stats_t& trans_access = get_by_src(access_by_src, src);
2426-
trans_access.cache_absent = get_by_src(stats.cache_absent_by_src, src);
2427-
trans_access.cache_absent -= get_by_src(last_cache_absent_by_src, src);
24282426
auto& access_by_ext = get_by_src(_access_by_src_ext, src);
24292427
const auto& last_access_by_ext = get_by_src(last_access_by_src_ext, src);
24302428
for (uint8_t _ext=0; _ext<EXTENT_TYPES_MAX; ++_ext) {
24312429
auto ext = static_cast<extent_types_t>(_ext);
2432-
extent_access_stats_t& extent_access = get_by_ext(access_by_ext, ext);
2430+
cache_access_stats_t& extent_access = get_by_ext(access_by_ext, ext);
24332431
const auto& last_extent_access = get_by_ext(last_access_by_ext, ext);
24342432
extent_access.minus(last_extent_access);
2435-
trans_access.s.add(extent_access);
2433+
trans_access.add(extent_access);
24362434
}
24372435
}
24382436
oss << "\naccess: total"
@@ -2443,9 +2441,9 @@ cache_stats_t Cache::get_stats(
24432441
if (trans_access.is_empty()) {
24442442
continue;
24452443
}
2446-
extent_access_stats_t data_access;
2447-
extent_access_stats_t mdat_access;
2448-
extent_access_stats_t phys_access;
2444+
cache_access_stats_t data_access;
2445+
cache_access_stats_t mdat_access;
2446+
cache_access_stats_t phys_access;
24492447
const auto& access_by_ext = get_by_src(_access_by_src_ext, src);
24502448
for (uint8_t _ext=0; _ext<EXTENT_TYPES_MAX; ++_ext) {
24512449
auto ext = static_cast<extent_types_t>(_ext);
@@ -2461,11 +2459,11 @@ cache_stats_t Cache::get_stats(
24612459
oss << "\n " << src << ": "
24622460
<< cache_access_stats_printer_t{seconds, trans_access}
24632461
<< "\n data"
2464-
<< extent_access_stats_printer_t{seconds, data_access}
2462+
<< cache_access_stats_printer_t{seconds, data_access}
24652463
<< "\n mdat"
2466-
<< extent_access_stats_printer_t{seconds, mdat_access}
2464+
<< cache_access_stats_printer_t{seconds, mdat_access}
24672465
<< "\n phys"
2468-
<< extent_access_stats_printer_t{seconds, phys_access};
2466+
<< cache_access_stats_printer_t{seconds, phys_access};
24692467
}
24702468

24712469
INFO("{}", oss.str());

‎src/crimson/os/seastore/cache.h‎

Lines changed: 21 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -186,11 +186,6 @@ class Cache : public ExtentTransViewRetriever {
186186
return t.root;
187187
}
188188

189-
void account_absent_access(Transaction::src_t src) {
190-
++(get_by_src(stats.cache_absent_by_src, src));
191-
++stats.access.cache_absent;
192-
}
193-
194189
/**
195190
* get_extent_if_cached
196191
*
@@ -208,7 +203,7 @@ class Cache : public ExtentTransViewRetriever {
208203
const auto t_src = t.get_src();
209204
CachedExtentRef ret;
210205
auto result = t.get_extent(paddr, &ret);
211-
extent_access_stats_t& access_stats = get_by_ext(
206+
cache_access_stats_t& access_stats = get_by_ext(
212207
get_by_src(stats.access_by_src_ext, t_src),
213208
type);
214209
if (result == Transaction::get_extent_ret::RETIRED) {
@@ -220,14 +215,14 @@ class Cache : public ExtentTransViewRetriever {
220215
if (ret->is_stable()) {
221216
if (ret->is_dirty()) {
222217
++access_stats.trans_dirty;
223-
++stats.access.s.trans_dirty;
218+
++stats.access.trans_dirty;
224219
} else {
225220
++access_stats.trans_lru;
226-
++stats.access.s.trans_lru;
221+
++stats.access.trans_lru;
227222
}
228223
} else {
229224
++access_stats.trans_pending;
230-
++stats.access.s.trans_pending;
225+
++stats.access.trans_pending;
231226
}
232227

233228
if (ret->get_length() != len) {
@@ -261,7 +256,6 @@ class Cache : public ExtentTransViewRetriever {
261256
SUBDEBUGT(seastore_cache,
262257
"{} {}~0x{:x} is absent in cache",
263258
t, type, paddr, len);
264-
account_absent_access(t_src);
265259
return get_extent_if_cached_iertr::make_ready_future<CachedExtentRef>();
266260
}
267261

@@ -270,16 +264,15 @@ class Cache : public ExtentTransViewRetriever {
270264
SUBDEBUGT(seastore_cache,
271265
"{} {}~0x{:x} ~0x{:x} is absent(placeholder) in cache",
272266
t, type, paddr, len, ret->get_length());
273-
account_absent_access(t_src);
274267
return get_extent_if_cached_iertr::make_ready_future<CachedExtentRef>();
275268
}
276269

277270
if (ret->is_dirty()) {
278271
++access_stats.cache_dirty;
279-
++stats.access.s.cache_dirty;
272+
++stats.access.cache_dirty;
280273
} else {
281274
++access_stats.cache_lru;
282-
++stats.access.s.cache_lru;
275+
++stats.access.cache_lru;
283276
}
284277

285278
if (ret->get_length() != len) {
@@ -402,11 +395,11 @@ class Cache : public ExtentTransViewRetriever {
402395
// FIXME: assert(ext.is_stable_clean());
403396
assert(ext.is_stable());
404397
assert(T::TYPE == ext.get_type());
405-
extent_access_stats_t& access_stats = get_by_ext(
398+
cache_access_stats_t& access_stats = get_by_ext(
406399
get_by_src(stats.access_by_src_ext, t_src),
407400
T::TYPE);
408401
++access_stats.load_absent;
409-
++stats.access.s.load_absent;
402+
++stats.access.load_absent;
410403

411404
t.add_to_read_set(CachedExtentRef(&ext));
412405
touch_extent(ext, &t_src, t.get_cache_hint());
@@ -475,7 +468,7 @@ class Cache : public ExtentTransViewRetriever {
475468

476469
const auto t_src = t.get_src();
477470
auto ext_type = extent->get_type();
478-
extent_access_stats_t& access_stats = get_by_ext(
471+
cache_access_stats_t& access_stats = get_by_ext(
479472
get_by_src(stats.access_by_src_ext, t_src),
480473
ext_type);
481474

@@ -487,7 +480,7 @@ class Cache : public ExtentTransViewRetriever {
487480
assert(p_extent->is_pending_in_trans(t.get_trans_id()));
488481
assert(!p_extent->is_stable_writting());
489482
++access_stats.trans_pending;
490-
++stats.access.s.trans_pending;
483+
++stats.access.trans_pending;
491484
if (p_extent->is_mutable()) {
492485
assert(p_extent->is_fully_loaded());
493486
assert(!p_extent->is_pending_io());
@@ -502,27 +495,27 @@ class Cache : public ExtentTransViewRetriever {
502495
if (t.maybe_add_to_read_set(p_extent)) {
503496
if (p_extent->is_dirty()) {
504497
++access_stats.cache_dirty;
505-
++stats.access.s.cache_dirty;
498+
++stats.access.cache_dirty;
506499
} else {
507500
++access_stats.cache_lru;
508-
++stats.access.s.cache_lru;
501+
++stats.access.cache_lru;
509502
}
510503
touch_extent(*p_extent, &t_src, t.get_cache_hint());
511504
} else {
512505
if (p_extent->is_dirty()) {
513506
++access_stats.trans_dirty;
514-
++stats.access.s.trans_dirty;
507+
++stats.access.trans_dirty;
515508
} else {
516509
++access_stats.trans_lru;
517-
++stats.access.s.trans_lru;
510+
++stats.access.trans_lru;
518511
}
519512
}
520513
}
521514
} else {
522515
assert(!extent->is_stable_writting());
523516
assert(extent->is_pending_in_trans(t.get_trans_id()));
524517
++access_stats.trans_pending;
525-
++stats.access.s.trans_pending;
518+
++stats.access.trans_pending;
526519
if (extent->is_mutable()) {
527520
assert(extent->is_fully_loaded());
528521
assert(!extent->is_pending_io());
@@ -564,11 +557,11 @@ class Cache : public ExtentTransViewRetriever {
564557
t, extent->get_type(), extent->get_paddr(), extent->get_length(),
565558
partial_off, partial_len, *extent);
566559
const auto t_src = t.get_src();
567-
extent_access_stats_t& access_stats = get_by_ext(
560+
cache_access_stats_t& access_stats = get_by_ext(
568561
get_by_src(stats.access_by_src_ext, t_src),
569562
extent->get_type());
570563
++access_stats.load_present;
571-
++stats.access.s.load_present;
564+
++stats.access.load_present;
572565
return trans_intr::make_interruptible(
573566
do_read_extent_maybe_partial(
574567
std::move(extent), partial_off, partial_len, &t_src));
@@ -878,11 +871,11 @@ class Cache : public ExtentTransViewRetriever {
878871
auto f = [&t, this, t_src](CachedExtent &ext) {
879872
// FIXME: assert(ext.is_stable_clean());
880873
assert(ext.is_stable());
881-
extent_access_stats_t& access_stats = get_by_ext(
874+
cache_access_stats_t& access_stats = get_by_ext(
882875
get_by_src(stats.access_by_src_ext, t_src),
883876
ext.get_type());
884877
++access_stats.load_absent;
885-
++stats.access.s.load_absent;
878+
++stats.access.load_absent;
886879

887880
t.add_to_read_set(CachedExtentRef(&ext));
888881
touch_extent(ext, &t_src, t.get_cache_hint());
@@ -1763,7 +1756,7 @@ class Cache : public ExtentTransViewRetriever {
17631756

17641757
cache_access_stats_t access;
17651758
counter_by_src_t<uint64_t> cache_absent_by_src;
1766-
counter_by_src_t<counter_by_extent_t<extent_access_stats_t> >
1759+
counter_by_src_t<counter_by_extent_t<cache_access_stats_t> >
17671760
access_by_src_ext;
17681761

17691762
uint64_t onode_tree_depth = 0;
@@ -1800,7 +1793,7 @@ class Cache : public ExtentTransViewRetriever {
18001793
mutable rewrite_stats_t last_reclaim_rewrites;
18011794
mutable cache_access_stats_t last_access;
18021795
mutable counter_by_src_t<uint64_t> last_cache_absent_by_src;
1803-
mutable counter_by_src_t<counter_by_extent_t<extent_access_stats_t> >
1796+
mutable counter_by_src_t<counter_by_extent_t<cache_access_stats_t> >
18041797
last_access_by_src_ext;
18051798

18061799
void account_conflict(Transaction::src_t src1, Transaction::src_t src2) {

‎src/crimson/os/seastore/lba_manager/btree/btree_lba_manager.cc‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -60,12 +60,10 @@ const get_phy_tree_root_node_ret get_phy_tree_root_node<
6060
return {true,
6161
c.cache.get_extent_viewable_by_trans(c.trans, lba_root)};
6262
} else {
63-
c.cache.account_absent_access(c.trans.get_src());
6463
return {false,
6564
Cache::get_extent_iertr::make_ready_future<CachedExtentRef>()};
6665
}
6766
} else {
68-
c.cache.account_absent_access(c.trans.get_src());
6967
return {false,
7068
Cache::get_extent_iertr::make_ready_future<CachedExtentRef>()};
7169
}
@@ -103,7 +101,6 @@ BtreeLBAMapping::get_logical_extent(Transaction &t)
103101
: get_key();
104102
auto v = p.template get_child<LogicalChildNode>(ctx.trans, ctx.cache, pos, k);
105103
if (!v.has_child()) {
106-
ctx.cache.account_absent_access(ctx.trans.get_src());
107104
this->child_pos = v.get_child_pos();
108105
}
109106
return v;

‎src/crimson/os/seastore/seastore_types.cc‎

Lines changed: 15 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -1042,42 +1042,6 @@ std::ostream& operator<<(std::ostream& out, const dirty_io_stats_printer_t& p)
10421042
return out;
10431043
}
10441044

1045-
std::ostream& operator<<(std::ostream& out, const extent_access_stats_printer_t& p)
1046-
{
1047-
constexpr const char* dfmt = "{:.2f}";
1048-
double est_total_access = static_cast<double>(p.stats.get_estimated_total_access());
1049-
out << "(~";
1050-
if (est_total_access > 1000000) {
1051-
out << fmt::format(dfmt, est_total_access/1000000)
1052-
<< "M, ";
1053-
} else {
1054-
out << fmt::format(dfmt, est_total_access/1000)
1055-
<< "K, ";
1056-
}
1057-
double trans_hit = static_cast<double>(p.stats.get_trans_hit());
1058-
double cache_hit = static_cast<double>(p.stats.get_cache_hit());
1059-
double est_cache_access = static_cast<double>(p.stats.get_estimated_cache_access());
1060-
double load_absent = static_cast<double>(p.stats.load_absent);
1061-
out << "trans-hit=~"
1062-
<< fmt::format(dfmt, trans_hit/est_total_access*100)
1063-
<< "%(p"
1064-
<< fmt::format(dfmt, p.stats.trans_pending/trans_hit)
1065-
<< ",d"
1066-
<< fmt::format(dfmt, p.stats.trans_dirty/trans_hit)
1067-
<< ",l"
1068-
<< fmt::format(dfmt, p.stats.trans_lru/trans_hit)
1069-
<< "), cache-hit=~"
1070-
<< fmt::format(dfmt, cache_hit/est_cache_access*100)
1071-
<< "%(d"
1072-
<< fmt::format(dfmt, p.stats.cache_dirty/cache_hit)
1073-
<< ",l"
1074-
<< fmt::format(dfmt, p.stats.cache_lru/cache_hit)
1075-
<<"), load-present/absent="
1076-
<< fmt::format(dfmt, p.stats.load_present/load_absent)
1077-
<< ")";
1078-
return out;
1079-
}
1080-
10811045
std::ostream& operator<<(std::ostream& out, const cache_access_stats_printer_t& p)
10821046
{
10831047
constexpr const char* dfmt = "{:.2f}";
@@ -1090,28 +1054,26 @@ std::ostream& operator<<(std::ostream& out, const cache_access_stats_printer_t&
10901054
out << fmt::format(dfmt, total_access/1000)
10911055
<< "K, ";
10921056
}
1093-
double trans_hit = static_cast<double>(p.stats.s.get_trans_hit());
1094-
double cache_hit = static_cast<double>(p.stats.s.get_cache_hit());
1057+
double trans_hit = static_cast<double>(p.stats.get_trans_hit());
1058+
double cache_hit = static_cast<double>(p.stats.get_cache_hit());
10951059
double cache_access = static_cast<double>(p.stats.get_cache_access());
1096-
double load_absent = static_cast<double>(p.stats.s.load_absent);
1060+
double load_absent = static_cast<double>(p.stats.load_absent);
10971061
out << "trans-hit="
10981062
<< fmt::format(dfmt, trans_hit/total_access*100)
1099-
<< "%(p"
1100-
<< fmt::format(dfmt, p.stats.s.trans_pending/trans_hit)
1101-
<< ",d"
1102-
<< fmt::format(dfmt, p.stats.s.trans_dirty/trans_hit)
1103-
<< ",l"
1104-
<< fmt::format(dfmt, p.stats.s.trans_lru/trans_hit)
1063+
<< "%(pend"
1064+
<< fmt::format(dfmt, p.stats.trans_pending/trans_hit)
1065+
<< ",dirt"
1066+
<< fmt::format(dfmt, p.stats.trans_dirty/trans_hit)
1067+
<< ",lru"
1068+
<< fmt::format(dfmt, p.stats.trans_lru/trans_hit)
11051069
<< "), cache-hit="
11061070
<< fmt::format(dfmt, cache_hit/cache_access*100)
1107-
<< "%(d"
1108-
<< fmt::format(dfmt, p.stats.s.cache_dirty/cache_hit)
1109-
<< ",l"
1110-
<< fmt::format(dfmt, p.stats.s.cache_lru/cache_hit)
1111-
<<"), load/absent="
1112-
<< fmt::format(dfmt, load_absent/p.stats.cache_absent*100)
1113-
<< "%, load-present/absent="
1114-
<< fmt::format(dfmt, p.stats.s.load_present/load_absent)
1071+
<< "%(dirt"
1072+
<< fmt::format(dfmt, p.stats.cache_dirty/cache_hit)
1073+
<< ",lru"
1074+
<< fmt::format(dfmt, p.stats.cache_lru/cache_hit)
1075+
<<"), load-present/absent="
1076+
<< fmt::format(dfmt, p.stats.load_present/load_absent)
11151077
<< ")";
11161078
return out;
11171079
}

0 commit comments

Comments
 (0)