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
19 changes: 13 additions & 6 deletions engines/default/coll_set.c
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,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;
}

Expand Down Expand Up @@ -263,12 +262,10 @@ static void do_set_node_unlink(set_meta_info *info,
assert(node->hcnt[hidx] == 0);
}
}
assert(fcnt == node->tot_elem_cnt);
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;
}

Expand Down Expand Up @@ -324,7 +321,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];
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

delete ํ•˜๋Š” ๊ฒฝ์šฐ์™€ ์œ ์‚ฌํ•˜๊ฒŒ
do_set_elem_traverse_insert() ๊ฐ™์€ ํ•จ์ˆ˜๋กœ ์žฌ๊ท€์ ์œผ๋กœ child node๋ฅผ ์ฐพ์•„๊ฐ€์„œ
linkํ•˜๋Š” ๊ตฌํ˜„์„ ์ƒ๊ฐํ•ด ๋ณผ ์ˆ˜ ์žˆ์„ ๊ฒƒ์ž…๋‹ˆ๋‹ค.
์ด๋ ‡๊ฒŒ ๊ตฌํ˜„ํ•  ํ•„์š”๋Š” ์—†๊ฒ ์ฃ ?


info->ccnt++;

Expand Down Expand Up @@ -400,6 +404,7 @@ static ENGINE_ERROR_CODE do_set_elem_traverse_delete(set_meta_info *info, set_ha
child_node->tot_elem_cnt < (SET_MAX_HASHCHAIN_SIZE/2)) {
do_set_node_unlink(info, node, hidx);
}
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.

์•„๋ž˜ ํ˜•ํƒœ๋กœ ๋ณ€๊ฒฝํ•˜์‹œ์ฃ .
tot_elem_cnt ๋ณ€๊ฒฝ ์Šคํƒ€์ผ์„ ๋™์ผํ•˜๊ฒŒ ๋งž์ถ”๊ณ ์ž ํ•ฉ๋‹ˆ๋‹ค.

tot_elem_cnt -= 1;

}
} else {
ret = ENGINE_ELEM_ENOENT;
Expand Down Expand Up @@ -497,13 +502,15 @@ 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)) {
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
2 changes: 1 addition & 1 deletion engines/default/item_base.h
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +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;
uint32_t tot_elem_cnt;
uint16_t tot_hash_cnt;
Copy link
Collaborator

Choose a reason for hiding this comment

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

unit32_t tot_elem_cnt ๋Š” child node์— ์žˆ๋Š” elements ์ˆ˜๋ฅผ ํฌํ•จํ•˜๋Š” ๊ฒƒ์œผ๋กœ ๋ณ€๊ฒฝํ•œ๋‹ค๋ฉด,
uint16_t tot_hash_cnt ๋Š” ์ œ๊ฑฐํ•˜๋Š” ๊ฒƒ์ด ๋‚˜์„ ๊ฒƒ ๊ฐ™์Šต๋‹ˆ๋‹ค. ๊ฒ€ํ†  ํ›„ ์กฐ์น˜ ๋ฐ”๋ž๋‹ˆ๋‹ค.

tot_hash_cnt๊ฐ€ ์ด์šฉ๋˜๋Š” 2๊ฐ€์ง€ ๊ฒฝ์šฐ๊ฐ€ ์žˆ์Šต๋‹ˆ๋‹ค.

์ฒซ์งธ, ์•„๋ž˜ ๊ฒฝ์šฐ์—์„œ tot_elem_cnt == 0์ด๋ฉด tot_hash_cnt == 0์ž„์„ ์•Œ ์ˆ˜ ์žˆ์œผ๋ฏ€๋กœ
tot_hash_cnt == 0 ์กฐ๊ฑด์„ ์ œ๊ฑฐํ•ด๋„ ๋ฉ๋‹ˆ๋‹ค.

            if (info->root->tot_hash_cnt == 0 && info->root->tot_elem_cnt == 0) {
                do_set_node_unlink(info, NULL, 0);
            }

๋‘˜์จฐ, ์•„๋ž˜ ๊ฒฝ์šฐ์—์„œ tot_hash_cnt ํ•„๋“œ๋ฅผ ์ œ๊ฑฐํ•˜๋Š” ๋Œ€์‹ ์—
ํ•„์š”ํ•˜๋ฉด get_child_node_count(node) ํ•จ์ˆ˜๋ฅผ ๋‘์–ด ๊ณ„์‚ฐํ•˜๊ฒŒ ์ฒ˜๋ฆฌํ•  ์ˆ˜ ์žˆ์Šต๋‹ˆ๋‹ค.

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

=> 

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

๊ทธ๋ฆฌ๊ณ  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];
Expand Down
Loading