Skip to content

Commit ef74125

Browse files
gapisbackgapisback
committed
(#604) Cache page-size in local variables to reduce multiple lookups.
In debug-build test runs, profiling shows interfaces like clockcache_page_size(), clockcache_config_page_size() bubbling up to the top of 'perf top' output. This commit replaces multiple calls to lookup functions that retrieve the page-size by caching the page-size once per function where it's used multiple times. The affected interfaces are: btree_page_size(), clockcache_page_size(), cache_page_size(), cache_config_page_size(), trunk_page_size() and a few similar ones. These changes add up to saving few seconds of test-execution (out of few mins of run-time) in debug-build mode, esp for BTree-related tests.
1 parent 3e259ca commit ef74125

File tree

7 files changed

+67
-75
lines changed

7 files changed

+67
-75
lines changed

‎src/btree_private.h‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -223,11 +223,11 @@ btree_get_leaf_entry(const btree_config *cfg,
223223
*/
224224
debug_assert(diff_ptr(hdr, &hdr->offsets[hdr->num_entries])
225225
<= hdr->offsets[k]);
226-
debug_assert(hdr->offsets[k] + sizeof(leaf_entry) <= btree_page_size(cfg));
226+
debug_code(uint64 bt_page_size = btree_page_size(cfg));
227+
debug_assert(hdr->offsets[k] + sizeof(leaf_entry) <= bt_page_size);
227228
leaf_entry *entry =
228229
(leaf_entry *)const_pointer_byte_offset(hdr, hdr->offsets[k]);
229-
debug_assert(hdr->offsets[k] + sizeof_leaf_entry(entry)
230-
<= btree_page_size(cfg));
230+
debug_assert(hdr->offsets[k] + sizeof_leaf_entry(entry) <= bt_page_size);
231231
return entry;
232232
}
233233

@@ -270,27 +270,27 @@ btree_get_index_entry(const btree_config *cfg,
270270
*/
271271
debug_assert(diff_ptr(hdr, &hdr->offsets[hdr->num_entries])
272272
<= hdr->offsets[k]);
273-
debug_assert(hdr->offsets[k] + sizeof(index_entry) <= btree_page_size(cfg),
273+
debug_code(uint64 bt_page_size = btree_page_size(cfg));
274+
debug_assert((hdr->offsets[k] + sizeof(index_entry) <= bt_page_size),
274275
"k=%d, offsets[k]=%d, sizeof(index_entry)=%lu"
275276
", btree_page_size=%lu.",
276277
k,
277278
hdr->offsets[k],
278279
sizeof(index_entry),
279-
btree_page_size(cfg));
280+
bt_page_size);
280281

281282
index_entry *entry =
282283
(index_entry *)const_pointer_byte_offset(hdr, hdr->offsets[k]);
283284

284285
/* Now ensure that the entire entry fits in the page. */
285-
debug_assert(hdr->offsets[k] + sizeof_index_entry(entry)
286-
<= btree_page_size(cfg),
286+
debug_assert((hdr->offsets[k] + sizeof_index_entry(entry) <= bt_page_size),
287287
"Offsets entry at index k=%d does not fit in the page."
288288
" offsets[k]=%d, sizeof_index_entry()=%lu"
289289
", btree_page_size=%lu.",
290290
k,
291291
hdr->offsets[k],
292292
sizeof_index_entry(entry),
293-
btree_page_size(cfg));
293+
bt_page_size);
294294
return entry;
295295
}
296296

‎src/clockcache.c‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1335,6 +1335,8 @@ clockcache_batch_start_writeback(clockcache *cc, uint64 batch, bool32 is_urgent)
13351335
start_entry_no,
13361336
end_entry_no - 1);
13371337

1338+
uint64 page_size = clockcache_page_size(cc);
1339+
13381340
allocator_config *allocator_cfg = allocator_get_config(cc->al);
13391341
// Iterate through the entries in the batch and try to write out the extents.
13401342
for (entry_no = start_entry_no; entry_no < end_entry_no; entry_no++) {
@@ -1348,7 +1350,7 @@ clockcache_batch_start_writeback(clockcache *cc, uint64 batch, bool32 is_urgent)
13481350
first_addr = entry->page.disk_addr;
13491351
// walk backwards through extent to find first cleanable entry
13501352
do {
1351-
first_addr -= clockcache_page_size(cc);
1353+
first_addr -= page_size;
13521354
if (allocator_config_pages_share_extent(
13531355
allocator_cfg, first_addr, addr))
13541356
next_entry_no = clockcache_lookup(cc, first_addr);
@@ -1357,11 +1359,11 @@ clockcache_batch_start_writeback(clockcache *cc, uint64 batch, bool32 is_urgent)
13571359
} while (
13581360
next_entry_no != CC_UNMAPPED_ENTRY
13591361
&& clockcache_try_set_writeback(cc, next_entry_no, is_urgent));
1360-
first_addr += clockcache_page_size(cc);
1362+
first_addr += page_size;
13611363
end_addr = entry->page.disk_addr;
13621364
// walk forwards through extent to find last cleanable entry
13631365
do {
1364-
end_addr += clockcache_page_size(cc);
1366+
end_addr += page_size;
13651367
if (allocator_config_pages_share_extent(
13661368
allocator_cfg, end_addr, addr))
13671369
next_entry_no = clockcache_lookup(cc, end_addr);
@@ -2097,7 +2099,7 @@ clockcache_get_internal(clockcache *cc, // IN
20972099
page_type type, // IN
20982100
page_handle **page) // OUT
20992101
{
2100-
debug_only uint64 page_size = clockcache_page_size(cc);
2102+
uint64 page_size = clockcache_page_size(cc);
21012103
debug_assert(
21022104
((addr % page_size) == 0), "addr=%lu, page_size=%lu\n", addr, page_size);
21032105
uint32 entry_number = CC_UNMAPPED_ENTRY;
@@ -2230,7 +2232,7 @@ clockcache_get_internal(clockcache *cc, // IN
22302232
start = platform_get_timestamp();
22312233
}
22322234

2233-
status = io_read(cc->io, entry->page.data, clockcache_page_size(cc), addr);
2235+
status = io_read(cc->io, entry->page.data, page_size, addr);
22342236
platform_assert_status_ok(status);
22352237

22362238
if (cc->cfg->use_stats) {
@@ -2856,6 +2858,7 @@ clockcache_prefetch_callback(void *metadata,
28562858
platform_assert(count > 0);
28572859
platform_assert(count <= cc->cfg->pages_per_extent);
28582860

2861+
debug_code(uint64 page_size = clockcache_page_size(cc));
28592862
for (uint64 page_off = 0; page_off < count; page_off++) {
28602863
uint32 entry_no =
28612864
clockcache_data_to_entry_number(cc, (char *)iovec[page_off].iov_base);
@@ -2872,7 +2875,7 @@ clockcache_prefetch_callback(void *metadata,
28722875
debug_code(int64 addr = entry->page.disk_addr);
28732876
debug_assert(addr != CC_UNMAPPED_ADDR);
28742877
debug_assert(last_addr == CC_UNMAPPED_ADDR
2875-
|| addr == last_addr + clockcache_page_size(cc));
2878+
|| addr == last_addr + page_size);
28762879
debug_code(last_addr = addr);
28772880
debug_assert(entry_no == clockcache_lookup(cc, addr));
28782881
}

‎src/mini_allocator.c‎

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -377,18 +377,18 @@ mini_keyed_append_entry(mini_allocator *mini,
377377
uint64 extent_addr,
378378
key start_key)
379379
{
380+
uint64 page_size = cache_page_size(mini->cc);
380381
debug_assert(mini->keyed);
381382
debug_assert(batch < mini->num_batches);
382383
debug_assert(!key_is_null(start_key));
383384
debug_assert(extent_addr != 0);
384385
debug_assert(extent_addr == TERMINAL_EXTENT_ADDR
385-
|| extent_addr % cache_page_size(mini->cc) == 0);
386+
|| (extent_addr % page_size) == 0);
386387

387388
mini_meta_hdr *hdr = (mini_meta_hdr *)meta_page->data;
388389

389-
if (!entry_fits_in_page(cache_page_size(mini->cc),
390-
hdr->pos,
391-
keyed_meta_entry_required_capacity(start_key)))
390+
if (!entry_fits_in_page(
391+
page_size, hdr->pos, keyed_meta_entry_required_capacity(start_key)))
392392
{
393393
return FALSE;
394394
}
@@ -409,15 +409,14 @@ mini_unkeyed_append_entry(mini_allocator *mini,
409409
page_handle *meta_page,
410410
uint64 extent_addr)
411411
{
412+
uint64 page_size = cache_page_size(mini->cc);
412413
debug_assert(!mini->keyed);
413414
debug_assert(extent_addr != 0);
414-
debug_assert(extent_addr % cache_page_size(mini->cc) == 0);
415+
debug_assert((extent_addr % page_size) == 0);
415416

416417
mini_meta_hdr *hdr = (mini_meta_hdr *)meta_page->data;
417418

418-
if (!entry_fits_in_page(
419-
cache_page_size(mini->cc), hdr->pos, sizeof(unkeyed_meta_entry)))
420-
{
419+
if (!entry_fits_in_page(page_size, hdr->pos, sizeof(unkeyed_meta_entry))) {
421420
return FALSE;
422421
}
423422

‎src/routing_filter.c‎

Lines changed: 16 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -173,17 +173,14 @@ routing_get_header(cache *cc,
173173
uint64 index,
174174
page_handle **filter_page)
175175
{
176-
uint64 addrs_per_page =
177-
cache_config_page_size(cfg->cache_cfg) / sizeof(uint64);
176+
uint64 page_size = cache_config_page_size(cfg->cache_cfg);
177+
uint64 addrs_per_page = page_size / sizeof(uint64);
178178
debug_assert(index / addrs_per_page < 32);
179-
uint64 index_addr =
180-
filter_addr
181-
+ cache_config_page_size(cfg->cache_cfg) * (index / addrs_per_page);
179+
uint64 index_addr = filter_addr + page_size * (index / addrs_per_page);
182180
page_handle *index_page = cache_get(cc, index_addr, TRUE, PAGE_TYPE_FILTER);
183181
uint64 hdr_raw_addr = ((uint64 *)index_page->data)[index % addrs_per_page];
184182
platform_assert(hdr_raw_addr != 0);
185-
uint64 header_addr =
186-
hdr_raw_addr - (hdr_raw_addr % cache_config_page_size(cfg->cache_cfg));
183+
uint64 header_addr = hdr_raw_addr - (hdr_raw_addr % page_size);
187184
*filter_page = cache_get(cc, header_addr, TRUE, PAGE_TYPE_FILTER);
188185
uint64 header_off = hdr_raw_addr - header_addr;
189186
routing_hdr *hdr = (routing_hdr *)((*filter_page)->data + header_off);
@@ -1004,6 +1001,7 @@ routing_filter_lookup_async(cache *cc,
10041001

10051002
debug_assert(key_is_user_key(target));
10061003

1004+
uint64 page_size = cache_config_page_size(cfg->cache_cfg);
10071005
do {
10081006
switch (ctxt->state) {
10091007
case routing_async_state_start:
@@ -1031,11 +1029,9 @@ routing_filter_lookup_async(cache *cc,
10311029
index_remainder_and_value_size);
10321030
ctxt->remainder = fp & remainder_mask;
10331031

1034-
uint64 addrs_per_page =
1035-
cache_config_page_size(cfg->cache_cfg) / sizeof(uint64);
1036-
ctxt->page_addr = filter->addr
1037-
+ cache_config_page_size(cfg->cache_cfg)
1038-
* (ctxt->index / addrs_per_page);
1032+
uint64 addrs_per_page = (page_size / sizeof(uint64));
1033+
ctxt->page_addr =
1034+
filter->addr + page_size * (ctxt->index / addrs_per_page);
10391035
routing_async_set_state(ctxt, routing_async_state_get_index);
10401036
// fallthrough;
10411037
}
@@ -1096,13 +1092,11 @@ routing_filter_lookup_async(cache *cc,
10961092
if (ctxt->was_async) {
10971093
cache_async_done(cc, PAGE_TYPE_FILTER, cache_ctxt);
10981094
}
1099-
uint64 *index_arr = ((uint64 *)cache_ctxt->page->data);
1100-
uint64 addrs_per_page =
1101-
cache_config_page_size(cfg->cache_cfg) / sizeof(uint64);
1102-
ctxt->header_addr = index_arr[ctxt->index % addrs_per_page];
1095+
uint64 *index_arr = ((uint64 *)cache_ctxt->page->data);
1096+
uint64 addrs_per_page = (page_size / sizeof(uint64));
1097+
ctxt->header_addr = index_arr[ctxt->index % addrs_per_page];
11031098
ctxt->page_addr =
1104-
ctxt->header_addr
1105-
- (ctxt->header_addr % cache_config_page_size(cfg->cache_cfg));
1099+
ctxt->header_addr - (ctxt->header_addr % page_size);
11061100
cache_unget(cc, cache_ctxt->page);
11071101
routing_async_set_state(ctxt, routing_async_state_get_filter);
11081102
break;
@@ -1117,8 +1111,7 @@ routing_filter_lookup_async(cache *cc,
11171111
}
11181112
routing_hdr *hdr =
11191113
(routing_hdr *)(cache_ctxt->page->data
1120-
+ (ctxt->header_addr
1121-
% cache_config_page_size(cfg->cache_cfg)));
1114+
+ (ctxt->header_addr % page_size));
11221115
uint64 encoding_size =
11231116
(hdr->num_remainders + cfg->index_size - 1) / 8 + 4;
11241117
uint64 header_length = encoding_size + sizeof(routing_hdr);
@@ -1276,12 +1269,10 @@ routing_filter_print_index(cache *cc,
12761269
platform_default_log("*** filter_addr: %lu\n", filter_addr);
12771270
platform_default_log("------------------------------------------------------"
12781271
"--------------------------\n");
1272+
uint64 page_size = cache_config_page_size(cfg->cache_cfg);
12791273
for (i = 0; i < num_indices; i++) {
1280-
uint64 addrs_per_page =
1281-
cache_config_page_size(cfg->cache_cfg) / sizeof(uint64);
1282-
uint64 index_addr =
1283-
filter_addr
1284-
+ cache_config_page_size(cfg->cache_cfg) * (i / addrs_per_page);
1274+
uint64 addrs_per_page = (page_size / sizeof(uint64));
1275+
uint64 index_addr = filter_addr + (page_size * (i / addrs_per_page));
12851276
page_handle *index_page =
12861277
cache_get(cc, index_addr, TRUE, PAGE_TYPE_FILTER);
12871278
platform_default_log("index 0x%lx: %lu\n",

‎src/trunk.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5862,6 +5862,7 @@ trunk_split_leaf(trunk_handle *spl,
58625862
uint16 bundle_no = trunk_leaf_rebundle_all_branches(
58635863
spl, leaf, leaf0_num_tuples, leaf0_kv_bytes, FALSE);
58645864

5865+
uint64 page_size = trunk_page_size(&spl->cfg);
58655866
for (uint16 leaf_no = 0; leaf_no < num_leaves; leaf_no++) {
58665867
/*
58675868
* 4. Create new leaf, adjust min/max keys and other metadata
@@ -5883,8 +5884,7 @@ trunk_split_leaf(trunk_handle *spl,
58835884
trunk_alloc(spl->cc, &spl->mini, 0, &new_leaf);
58845885

58855886
// copy leaf to new leaf
5886-
memmove(
5887-
new_leaf.page->data, leaf->page->data, trunk_page_size(&spl->cfg));
5887+
memmove(new_leaf.page->data, leaf->page->data, page_size);
58885888
} else {
58895889
// just going to edit the min/max keys, etc. of original leaf
58905890
new_leaf = *leaf;

‎tests/unit/btree_stress_test.c‎

Lines changed: 20 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -340,10 +340,11 @@ insert_tests(cache *cc,
340340
uint64 generation;
341341
bool32 was_unique;
342342

343-
int keybuf_size = btree_page_size(cfg);
344-
int msgbuf_size = btree_page_size(cfg);
345-
uint8 *keybuf = TYPED_MANUAL_MALLOC(hid, keybuf, keybuf_size);
346-
uint8 *msgbuf = TYPED_MANUAL_MALLOC(hid, msgbuf, msgbuf_size);
343+
uint64 bt_page_size = btree_page_size(cfg);
344+
int keybuf_size = bt_page_size;
345+
int msgbuf_size = bt_page_size;
346+
uint8 *keybuf = TYPED_MANUAL_MALLOC(hid, keybuf, keybuf_size);
347+
uint8 *msgbuf = TYPED_MANUAL_MALLOC(hid, msgbuf, msgbuf_size);
347348

348349
for (uint64 i = start; i < end; i++) {
349350
if (!SUCCESS(btree_insert(cc,
@@ -409,9 +410,10 @@ query_tests(cache *cc,
409410
uint64 root_addr,
410411
int nkvs)
411412
{
412-
uint8 *keybuf = TYPED_MANUAL_MALLOC(hid, keybuf, btree_page_size(cfg));
413-
uint8 *msgbuf = TYPED_MANUAL_MALLOC(hid, msgbuf, btree_page_size(cfg));
414-
memset(msgbuf, 0, btree_page_size(cfg));
413+
uint64 bt_page_size = btree_page_size(cfg);
414+
uint8 *keybuf = TYPED_MANUAL_MALLOC(hid, keybuf, bt_page_size);
415+
uint8 *msgbuf = TYPED_MANUAL_MALLOC(hid, msgbuf, bt_page_size);
416+
memset(msgbuf, 0, bt_page_size);
415417

416418
merge_accumulator result;
417419
merge_accumulator_init(&result, hid);
@@ -421,11 +423,11 @@ query_tests(cache *cc,
421423
cfg,
422424
root_addr,
423425
type,
424-
gen_key(cfg, i, keybuf, btree_page_size(cfg)),
426+
gen_key(cfg, i, keybuf, bt_page_size),
425427
&result);
426428
if (!btree_found(&result)
427429
|| message_lex_cmp(merge_accumulator_to_message(&result),
428-
gen_msg(cfg, i, msgbuf, btree_page_size(cfg))))
430+
gen_msg(cfg, i, msgbuf, bt_page_size)))
429431
{
430432
ASSERT_TRUE(FALSE, "Failure on lookup %lu\n", i);
431433
}
@@ -444,11 +446,12 @@ iterator_test(platform_heap_id hid,
444446
iterator *iter,
445447
bool32 forwards)
446448
{
447-
uint64 seen = 0;
448-
uint8 *prevbuf = TYPED_MANUAL_MALLOC(hid, prevbuf, btree_page_size(cfg));
449-
key prev = NULL_KEY;
450-
uint8 *keybuf = TYPED_MANUAL_MALLOC(hid, keybuf, btree_page_size(cfg));
451-
uint8 *msgbuf = TYPED_MANUAL_MALLOC(hid, msgbuf, btree_page_size(cfg));
449+
uint64 seen = 0;
450+
uint64 bt_page_size = btree_page_size(cfg);
451+
uint8 *prevbuf = TYPED_MANUAL_MALLOC(hid, prevbuf, bt_page_size);
452+
key prev = NULL_KEY;
453+
uint8 *keybuf = TYPED_MANUAL_MALLOC(hid, keybuf, bt_page_size);
454+
uint8 *msgbuf = TYPED_MANUAL_MALLOC(hid, msgbuf, bt_page_size);
452455

453456
while (iterator_can_curr(iter)) {
454457
key curr_key;
@@ -459,12 +462,11 @@ iterator_test(platform_heap_id hid,
459462
ASSERT_TRUE(k < nkvs);
460463

461464
int rc = 0;
462-
rc = data_key_compare(cfg->data_cfg,
463-
curr_key,
464-
gen_key(cfg, k, keybuf, btree_page_size(cfg)));
465+
rc = data_key_compare(
466+
cfg->data_cfg, curr_key, gen_key(cfg, k, keybuf, bt_page_size));
465467
ASSERT_EQUAL(0, rc);
466468

467-
rc = message_lex_cmp(msg, gen_msg(cfg, k, msgbuf, btree_page_size(cfg)));
469+
rc = message_lex_cmp(msg, gen_msg(cfg, k, msgbuf, bt_page_size));
468470
ASSERT_EQUAL(0, rc);
469471

470472
if (forwards) {

‎tests/unit/btree_test.c‎

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -302,13 +302,11 @@ leaf_hdr_search_tests(btree_config *cfg, platform_heap_id hid)
302302
static int
303303
index_hdr_tests(btree_config *cfg, btree_scratch *scratch, platform_heap_id hid)
304304
{
305-
306305
char *index_buffer =
307306
TYPED_MANUAL_MALLOC(hid, index_buffer, btree_page_size(cfg));
308307
btree_hdr *hdr = (btree_hdr *)index_buffer;
309308
int nkvs = 100;
310309

311-
312310
bool32 rv = FALSE;
313311
int cmp_rv = 0;
314312

@@ -406,18 +404,17 @@ leaf_split_tests(btree_config *cfg,
406404
int nkvs,
407405
platform_heap_id hid)
408406
{
409-
char *leaf_buffer =
410-
TYPED_MANUAL_MALLOC(hid, leaf_buffer, btree_page_size(cfg));
411-
char *msg_buffer =
412-
TYPED_MANUAL_MALLOC(hid, msg_buffer, btree_page_size(cfg));
407+
uint64 bt_page_size = btree_page_size(cfg);
408+
char *leaf_buffer = TYPED_MANUAL_MALLOC(hid, leaf_buffer, bt_page_size);
409+
char *msg_buffer = TYPED_MANUAL_MALLOC(hid, msg_buffer, bt_page_size);
413410

414-
memset(msg_buffer, 0, btree_page_size(cfg));
411+
memset(msg_buffer, 0, bt_page_size);
415412

416413
btree_hdr *hdr = (btree_hdr *)leaf_buffer;
417414

418415
btree_init_hdr(cfg, hdr);
419416

420-
int msgsize = btree_page_size(cfg) / (nkvs + 1);
417+
int msgsize = bt_page_size / (nkvs + 1);
421418
message msg =
422419
message_create(MESSAGE_TYPE_INSERT, slice_create(msgsize, msg_buffer));
423420
message bigger_msg = message_create(

0 commit comments

Comments
 (0)