Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weโ€™ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

INTERNAL: Change tot_elem_cnt to subtree element count in set #817

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 18 additions & 18 deletions engines/default/coll_set.c
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,6 @@ static set_hash_node *do_set_node_alloc(uint8_t hash_depth, const void *cookie)

node->refcount = 0;
node->hdepth = hash_depth;
node->tot_hash_cnt = 0;
node->tot_elem_cnt = 0;
memset(node->hcnt, 0, SET_HASHTAB_SIZE*sizeof(uint16_t));
memset(node->htab, 0, SET_HASHTAB_SIZE*sizeof(void*));
Expand Down Expand Up @@ -219,8 +218,6 @@ static void do_set_node_link(set_meta_info *info,

par_node->htab[par_hidx] = node;
par_node->hcnt[par_hidx] = -1; /* child hash node */
par_node->tot_elem_cnt -= num_found;
par_node->tot_hash_cnt += 1;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

์ˆ˜์ •ํ•˜๋Š” ๊น€์—, num_found, num_elems ๋ณ€์ˆ˜๋ฅผ ๋ชจ๋‘ ์ œ๊ฑฐํ•ด๋„ ๋  ๊ฒƒ ๊ฐ™์Šต๋‹ˆ๋‹ค.

        while (par_node->htab[par_hidx] != NULL) {
            set_elem_item *elem = par_node->htab[par_hidx];
            par_node->htab[par_hidx] = elem->next;

            int hidx = SET_GET_HASHIDX(elem->hval, node->hdepth);
            elem->next = node->htab[hidx];
            node->htab[hidx] = elem;
            node->hcnt[hidx] += 1;
            node->tot_elem_cnt += 1;
        }
        assert(node->tot_elem_cnt == par_node->hcnt[par_hidx]);
        par_node->htab[par_hidx] = node;
        par_node->hcnt[par_hidx] = -1; /* child hash node */

}

if (1) { /* apply memory space */
Expand All @@ -237,7 +234,6 @@ static void do_set_node_unlink(set_meta_info *info,
if (par_node == NULL) {
node = info->root;
info->root = NULL;
assert(node->tot_hash_cnt == 0);
assert(node->tot_elem_cnt == 0);
} else {
assert(par_node->hcnt[par_hidx] == -1); /* child hash node */
Expand All @@ -246,7 +242,6 @@ static void do_set_node_unlink(set_meta_info *info,
int hidx, fcnt = 0;

node = (set_hash_node *)par_node->htab[par_hidx];
assert(node->tot_hash_cnt == 0);

for (hidx = 0; hidx < SET_HASHTAB_SIZE; hidx++) {
assert(node->hcnt[hidx] >= 0);
Expand All @@ -263,13 +258,10 @@ static void do_set_node_unlink(set_meta_info *info,
assert(node->hcnt[hidx] == 0);
}
}
assert(fcnt == node->tot_elem_cnt);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

์—ฌ๊ธฐ assert ๋ฌธ์€ ๊ทธ๋Œ€๋กœ ๋‘ก์‹œ๋‹ค.

node->tot_elem_cnt = 0;

par_node->htab[par_hidx] = head;
par_node->hcnt[par_hidx] = fcnt;
par_node->tot_elem_cnt += fcnt;
par_node->tot_hash_cnt -= 1;
}

if (info->stotal > 0) { /* apply memory space */
Expand Down Expand Up @@ -324,7 +316,14 @@ static ENGINE_ERROR_CODE do_set_elem_link(set_meta_info *info, set_elem_item *el
elem->next = node->htab[hidx];
node->htab[hidx] = elem;
node->hcnt[hidx] += 1;
node->tot_elem_cnt += 1;
node = info->root;
while (node != NULL) {
node->tot_elem_cnt += 1;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do_set_node_link(), do_set_node_unlink(), do_set_elem_link() 3๊ฐœ ํ•จ์ˆ˜๋Š” ์ˆ˜์ •ํ•˜๋Š”๋ฐ,
do_set_elem_unlink()๋Š” ๊ธฐ์กด ๊ตฌํ˜„์„ ๊ทธ๋Œ€๋กœ ์‚ฌ์šฉํ•˜๊ณ  ์žˆ์Šต๋‹ˆ๋‹ค.

  1. do_set_elem_traverse_XXX()์—์„œ ์ฒ˜๋ฆฌํ•˜๊ธฐ ๋•Œ๋ฌธ์ธ๊ฐ€์š”?
  2. ๊ธฐ์กด ๊ตฌํ˜„์—์„œ๋Š” XXX_link()์™€ XXX_unlink()์—์„œ tot_elem_cnt ๊ฐ’์„ ๋ณ€๊ฒฝํ•˜๊ณ ,
    traverse ํ•จ์ˆ˜์—์„œ๋Š” ๋น„๊ต ์—ฐ์‚ฐ๋งŒ ์ˆ˜ํ–‰ํ–ˆ์Šต๋‹ˆ๋‹ค.
    ๊ธฐ์กด์ฒ˜๋Ÿผ ๋ฐ˜๋Œ€ ์ด๋ฆ„์„ ๊ฐ–๋Š” ํ•จ์ˆ˜์˜ ๋™์ž‘์ด ์„œ๋กœ ์ง์„ ์ด๋ฃจ๋„๋ก ๊ตฌํ˜„ํ•˜๊ธฐ๋Š” ์–ด๋ ค์šด๊ฐ€์š”?

Copy link
Author

@jeesup0103 jeesup0103 Jan 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. ๋„ค. ํ•ด๋‹น ๋…ธ๋“œ์˜ ๋ถ€๋ชจ ๋…ธ๋“œ๋“ค์˜ tot_elem_cnt ๊ฐ’์€ do_set_elem_traverse_XXX()์—์„œ ์ฒ˜๋ฆฌ ๋ฉ๋‹ˆ๋‹ค.
  2. ์–ด๋ ต์ง€ ์•Š์„ ๊ฒƒ ๊ฐ™์Šต๋‹ˆ๋‹ค.
    do_set_elem_unlink() ์—์„œ do_set_elem_link()์™€ ๊ฐ™์ด ํ•ด์‹œ๊ฐ’์„ ๊ธฐ์ค€์œผ๋กœ while๋ฌธ์„ ๋ฐ˜๋ณตํ•ด์„œ ๋ถ€๋ชจ ๋…ธ๋“œ๋“ค์˜ tot_elem_cnt ๊ฐ’์„ ์—…๋ฐ์ดํŠธ ํ•  ์ˆ˜ ์žˆ์Šต๋‹ˆ๋‹ค.
    ๊ทธ๋Ÿฌ๋ฉด ํ•˜๋‚˜์˜ element๋ฅผ ์‚ญ์ œ ํ• ๋•Œ๋งˆ๋‹ค ๋ถ€๋ชจ ๋…ธ๋“œ๋“ค์„ ์—…๋ฐ์ดํŠธ ํ•ด์•ผํ•˜๋Š” ๋น„์šฉ์€ ๋ฐœ์ƒํ•ฉ๋‹ˆ๋‹ค. (ํ˜„์žฌ PR์—์„œ๋Š” ๋…ธ๋“œ๋‹น ํ•œ๋ฒˆ ์—…๋ฐ์ดํŠธ)
    while (node != NULL) {
        node->tot_elem_cnt -= 1;
        hidx = SET_GET_HASHIDX(elem->hval, node->hdepth);
        if (node->hcnt[hidx] >= 0)
            break;
        node = node->htab[hidx];
    }

hidx = SET_GET_HASHIDX(elem->hval, node->hdepth);
if (node->hcnt[hidx] >= 0)
break;
node = node->htab[hidx];
}
jhpark816 marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

์•„๋ž˜ ์ฝ”๋“œ์™€ ๊ฐ™์ด ํ˜„์žฌ ๋…ธ๋“œ์—์„œ tot_elem_cnt ์ฆ๊ฐ€์‹œํ‚ค๋Š” ์ฝ”๋“œ์™€
์‚ผ์œ„ ๋…ธ๋“œ๋“ค์—์„œ tot_elem_cnt ์ฆ๊ฐ€์‹œํ‚ค๋Š” ์ฝ”๋“œ๊ฐ€ ๊ตฌ๋ถ„๋˜๋ฉด ์ข‹๊ฒ ์Šต๋‹ˆ๋‹ค.
do_set_elem_unlink()์™€์˜ ์ฐจ์ด๊ฐ€ ์ž˜ ๋“œ๋Ÿฌ๋‚˜๊ฒŒ ํ•˜๋ ค๋Š” ์˜๋„์ž…๋‹ˆ๋‹ค.

    elem->next = node->htab[hidx];
    node->htab[hidx] = elem;
    node->hcnt[hidx] += 1;
    node->tot_elem_cnt += 1;
    
    set_hash_node *par_node = info->root;
    while (par_node != node) {
        par_node->tot_elem_cnt += 1;
        hidx = SET_GET_HASHIDX(elem->hval, par_node->hdepth);
        assert(par_node->hcnt[hidx] == -1);
        par_node = par_node->htab[hidx];
    }
    info->ccnt++;


info->ccnt++;

Expand Down Expand Up @@ -396,10 +395,10 @@ static ENGINE_ERROR_CODE do_set_elem_traverse_delete(set_meta_info *info, set_ha
set_hash_node *child_node = node->htab[hidx];
ret = do_set_elem_traverse_delete(info, child_node, hval, val, vlen);
if (ret == ENGINE_SUCCESS) {
if (child_node->tot_hash_cnt == 0 &&
child_node->tot_elem_cnt < (SET_MAX_HASHCHAIN_SIZE/2)) {
if (child_node->tot_elem_cnt < (SET_MAX_HASHCHAIN_SIZE/2)) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

child_node->tot_elem_cnt < (SET_MAX_HASHCHAIN_SIZE/2) ์กฐ๊ฑด ๋งŒ์œผ๋กœ
child_node๊ฐ€ leaf node์ธ์ง€๋ฅผ 100% ๋ณด์žฅํ•  ์ˆ˜ ์žˆ๋Š” ์ง€๊ฐ€ ์•„์ง ์˜๋ฌธ์ธ๋ฐ์š”.
์–ด๋–ค๊ฐ€์š”?

์กฐ๊ธˆ์ด๋ผ๋„ ํ™•์‹ ์ด ์„œ์ง€ ์•Š๋Š”๋‹ค๋ฉด, ์•„๋ž˜์™€ ๊ฐ™์ด is_leaf_node() ์ถ”๊ฐ€ํ•˜๋ฉด ์ข‹๊ฒ ์Šต๋‹ˆ๋‹ค.

            if (child_node->tot_elem_cnt < (SET_MAX_HASHCHAIN_SIZE/2)
                && is_leaf_node(child_node)) {
                do_set_node_unlink(info, node, hidx);
            }

do_set_node_unlink(info, node, hidx);
}
node->tot_elem_cnt -= 1;
}
} else {
ret = ENGINE_ELEM_ENOENT;
Expand Down Expand Up @@ -431,7 +430,7 @@ static ENGINE_ERROR_CODE do_set_elem_delete_with_value(set_meta_info *info,
int hval = genhash_string_hash(val, vlen);
ret = do_set_elem_traverse_delete(info, info->root, hval, val, vlen);
if (ret == ENGINE_SUCCESS) {
if (info->root->tot_hash_cnt == 0 && info->root->tot_elem_cnt == 0) {
if (info->root->tot_elem_cnt == 0) {
do_set_node_unlink(info, NULL, 0);
}
}
Expand Down Expand Up @@ -497,13 +496,14 @@ static int do_set_elem_traverse_dfs(set_meta_info *info, set_hash_node *node,
if (node->hcnt[hidx] == -1) {
set_hash_node *child_node = (set_hash_node *)node->htab[hidx];
int rcnt = (count > 0 ? (count - fcnt) : 0);
fcnt += do_set_elem_traverse_dfs(info, child_node, rcnt, delete,
(elem_array==NULL ? NULL : &elem_array[fcnt]));
int ecnt = do_set_elem_traverse_dfs(info, child_node, rcnt, delete,
(elem_array==NULL ? NULL : &elem_array[fcnt]));
fcnt += ecnt;
if (delete) {
if (child_node->tot_hash_cnt == 0 &&
child_node->tot_elem_cnt < (SET_MAX_HASHCHAIN_SIZE/2)) {
if (child_node->tot_elem_cnt < (SET_MAX_HASHCHAIN_SIZE/2)) {
do_set_node_unlink(info, node, hidx);
}
node->tot_elem_cnt -= ecnt;
}
} else if (node->hcnt[hidx] > 0) {
set_elem_item *elem = node->htab[hidx];
Expand Down Expand Up @@ -551,7 +551,7 @@ static uint32_t do_set_elem_delete(set_meta_info *info, const uint32_t count,
uint32_t fcnt = 0;
if (info->root != NULL) {
fcnt = do_set_elem_traverse_dfs(info, info->root, count, true, NULL);
if (info->root->tot_hash_cnt == 0 && info->root->tot_elem_cnt == 0) {
if (info->root->tot_elem_cnt == 0) {
do_set_node_unlink(info, NULL, 0);
}
}
Expand All @@ -569,7 +569,7 @@ static uint32_t do_set_elem_get(set_meta_info *info,
CLOG_ELEM_DELETE_BEGIN((coll_meta_info*)info, count, ELEM_DELETE_NORMAL);
}
fcnt = do_set_elem_traverse_dfs(info, info->root, count, delete, elem_array);
if (delete && info->root->tot_hash_cnt == 0 && info->root->tot_elem_cnt == 0) {
if (delete && info->root->tot_elem_cnt == 0) {
do_set_node_unlink(info, NULL, 0);
}
if (delete) {
Expand Down
3 changes: 1 addition & 2 deletions engines/default/item_base.h
Original file line number Diff line number Diff line change
Expand Up @@ -236,8 +236,7 @@ typedef struct _set_hash_node {
uint16_t refcount;
uint8_t slabs_clsid; /* which slab class we're in */
uint8_t hdepth;
uint16_t tot_elem_cnt;
uint16_t tot_hash_cnt;
uint32_t tot_elem_cnt;
int16_t hcnt[SET_HASHTAB_SIZE];
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jeesup0103
ํ•˜์œ„ node๋ฅผ ๊ฐ€์ง€๋Š” ๊ฒฝ์šฐ์— hcnt[i] = -1 ์„ค์ •์„ ๊ทธ๋Œ€๋กœ ์œ ์ง€ํ•˜๊ณ  ์žˆ๋„ค์š”.

๊ฐ hash bucket์— ์žˆ๋Š” element ๊ฐœ์ˆ˜๋Š” hcnt[i] ๊ฐ’์„ ํ™•์ธํ•  ์ˆ˜ ์žˆ๊ณ ,
-1์ด๋ผ๋ฉด child node ์ ‘๊ทผํ•˜์—ฌ tot_elem_cnt ํ™•์ธํ•œ๋‹ค๋Š” ๊ฒƒ์ด์ฃ .

hcnt[i]์—์„œ ํ•˜์œ„ node์— ์žˆ๋Š” element ๊ฐœ์ˆ˜๋ฅผ ์œ ์ง€ํ•˜๋Š” ๋ฐฉ์•ˆ๋„ ์ƒ๊ฐํ•ด ๋ณผ ์ˆ˜ ์žˆ๋Š” ๋ฐ,
์ด ๋ฐฉ์‹์„ ํƒํ•˜์ง€ ์•Š์€ ์ด์œ ๋Š” ๋ฌด์—‡์ธ๊ฐ€์š”?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jhpark816
hcnt[hidx] ๊ฐ’์„ ๋ณ€๊ฒฝํ•˜์ง€ ์•Š์€ ์ด์œ ๋Š” ํ•ด๋‹น ๋ฒ„ํ‚ท์˜ ์ƒํƒœ๋ฅผ ๋ฐ”๋กœ ํ™•์ธํ•  ์ˆ˜ ์žˆ๋„๋ก ์œ ์ง€ํ•˜๊ธฐ ์œ„ํ•ด์„œ์ž…๋‹ˆ๋‹ค.
DFS ํƒ์ƒ‰ ๊ณผ์ •์—์„œ hcnt[hidx]๋ฅผ ํ†ตํ•ด ํ•ด๋‹น ๋ฒ„ํ‚ท์ด ์ถ”๊ฐ€ ๋…ธ๋“œ์ธ์ง€ ๋˜๋Š” element chain์ธ์ง€ ๋น ๋ฅด๊ฒŒ ํŒ๋‹จํ•  ์ˆ˜ ์žˆ์Šต๋‹ˆ๋‹ค.

๊ทธ๋ฆฌ๊ณ , hcnt[hidx] ๊ฐ’๋„ ํ•˜์œ„ ๋…ธ๋“œ์˜ element ๊ฐœ์ˆ˜๋กœ ๋ณ€๊ฒฝํ•œ๋‹ค๋ฉด, ์ด๋Š” tot_elem_cnt์™€ ์ค‘๋ณต ์ •๋ณด๋ฅผ ์ €์žฅํ•˜๋Š” ํ˜•ํƒœ๊ฐ€ ๋ฉ๋‹ˆ๋‹ค.
๊ฒฐ๊ตญ, node->hcnt[hidx]์™€ node->htab[hidx]->tot_elem_cnt๋Š” ๊ฐ™์€ ๊ฐ’์„ ๊ฐ€์ง€๊ฒŒ ๋˜๊ธฐ ๋•Œ๋ฌธ์— tot_elem_cnt๋งŒ ๋ณ€๊ฒฝํ•˜๋Š” ๊ฒƒ์ด ๋” ํšจ์œจ์ ์ด๋ผ๊ณ  ์ƒ๊ฐํ–ˆ์Šต๋‹ˆ๋‹ค.

void *htab[SET_HASHTAB_SIZE];
} set_hash_node;
Expand Down
Loading