INTERNAL: Extract htree layer functions from map/set elem insert and update by zhy2on · Pull Request #981 · naver/arcus-memcached

zhy2on · 2026-06-16T04:44:50Z

🔗 Related Issue

https://github.com/jam2in/arcus-works/issues/844#issuecomment-4278095307

⌨️ What I did

hash tree 모듈 추출을 위한 사전 작업으로, do_set_elem_insert와 do_map_elem_insert의 hash tree 조작 로직을 do_htree_elem_insert, do_htree_elem_replace로 분리했습니다.
hash tree layer는 node 탐색, 중복 체크, node split, chain 연결, elem->linked 갱신, hval 세팅을 담당하고, collection layer에는 info->ccnt 갱신, space accounting, CLOG, elem alloc/free를 유지했습니다.
node 생성 시 발생하는 space는 do_htree_elem_insert가 space_increased로 올려주고, collection layer에서 일괄 처리합니다.
set은 sticky limit check → overflow check → htree_elem_insert 순서로 처리합니다.
map은 do_map_elem_find로 먼저 탐색하고, 찾은 경우 replace_if_exist=true일 때만 해당 position에 htree_elem_replace를 수행하며, 찾지 못한 경우 set과 동일한 insert 경로를 탑니다.
현재 do_map_elem_find는 elem을 찾았을 때만 position을 채워서 반환하기 때문에, 찾지 못한 경우 do_htree_elem_insert 내부에서 position을 재탐색합니다. 추후 do_map_elem_find가 찾지 못했을 때도 마지막 탐색 위치를 채워서 반환하도록 변경하면, 해당 position을 htree_elem_add()에 바로 활용할 수 있습니다.

jhpark816

리뷰 완료

jhpark816 · 2026-06-16T07:25:47Z

+            return ENGINE_ENOMEM;
+        }
+        *space_increased += slabs_space_size(sizeof(set_hash_node));
+        do_htree_node_link(info, NULL, 0, r_node);


node 할당, 연결을 마친 후에 output 인자에 space 증가분 설정하는 순서로 변경하시죠.

do_htree_node_link(info, NULL, 0, r_node); *space_increased += slabs_space_size(sizeof(set_hash_node));

반영했습니다.

jhpark816 · 2026-06-16T07:26:33Z

-        do_set_node_link(info, node, hidx, n_node);
-
+        *space_increased += slabs_space_size(sizeof(set_hash_node));
+        do_htree_node_link(info, node, hidx, n_node);


여기도 순서를 변경하고,
기존대로 empty line도 유지하면 나을 것 같습니다.

반영했습니다

jhpark816 · 2026-06-16T07:29:05Z

+        size_t stotal = slabs_space_size(do_set_elem_ntotal(elem)) + space_increased;
+        do_coll_space_incr((coll_meta_info *)info, ITEM_TYPE_SET, stotal);
+    }
+


1개 commit으로 map elem insert도 한꺼번에 수정하시죠.

jhpark816 · 2026-06-16T07:55:36Z

-    /* insert the element */
-    ret = do_set_elem_link(info, elem);
+    size_t space_increased = 0;
+    ret = do_set_elem_insert(info, elem, &space_increased);


htree_elem_insert 여야 합니다.
먼저 unit test 한 후에 PR 올려주세요.

jhpark816

일부 리뷰

jhpark816 · 2026-06-16T08:27:35Z

+        size_t old_stotal = slabs_space_size(do_map_elem_ntotal(elem));
+        size_t new_stotal = slabs_space_size(do_map_elem_ntotal(new_elem));
+
+        CLOG_MAP_ELEM_INSERT(info, elem, new_elem);


코드 순서를 변경하는 것이 좋겠습니다.

CLOG_MAP_ELEM_INSERT(info, elem, new_elem); size_t old_stotal = slabs_space_size(do_map_elem_ntotal(elem)); size_t new_stotal = slabs_space_size(do_map_elem_ntotal(new_elem));

반영했습니다.

jhpark816 · 2026-06-16T08:37:32Z

+            size_t old_stotal = slabs_space_size(do_map_elem_ntotal(find));
+            size_t new_stotal = slabs_space_size(do_map_elem_ntotal(elem));
+
+            CLOG_MAP_ELEM_INSERT(info, find, elem);


CLOG 기록을 앞으로 옮기죠.

반영했습니다.

jhpark816 · 2026-06-16T08:45:08Z

+    return elem;
+}
+
+static void do_htree_elem_replace(map_meta_info *info,


old_elem을 리턴하는 것이 좋은 모양인 것 같습니다.

반영했습니다.

jhpark816 · 2026-06-16T08:48:43Z


        /* replace the element */
-        do_map_elem_replace(info, &pinfo, new_elem);
+        do_htree_elem_replace(info, &pinfo, new_elem);


do_htree_elem_replace() 함수가 old_elem 리턴한다면, 아래 코드가 가능할 것 같습니다.

map_elem_item *old_elem = do_htree_elem_replace(info, &pinfo, new_elem); assert(old_elem == elem);

그리고, 그 아래에서는 old_elem을 사용하면 되고요.

do_htree_elem_replace() 호출하는 다른 코드도 확인해 보세요.

사용처 모두 일괄 적용했습니다.

jhpark816

일부 리뷰 의견만 남기니, 참고 바랍니다.

Offline으로 최종 수정 방안을 논의합시다.

jhpark816 · 2026-06-16T08:57:41Z

-        if (do_item_sticky_overflowed())
-            return ENGINE_ENOMEM;
+        pinfo->node = r_node;
+        pinfo->hidx = MAP_GET_HASHIDX(elem->hval, 0);


좋은 코드가 아닙니다.

기존대로 node, hidx 변수를 사용합시다.

반영하였습니다.

jhpark816 · 2026-06-16T11:39:29Z

+
+            if (old_elem->refcount == 0) {
+                do_map_elem_free(old_elem);
+            }


위의 코드는 아래 함수로 포장하여 사용하면 나을 것 같습니다.

do_map_elem_replace()

반영했습니다.

jhpark816 · 2026-06-16T12:04:45Z

-        map_hash_node *r_node = do_map_node_alloc(0);
-        if (r_node == NULL) {
-            return ENGINE_ENOMEM;
+    find = do_map_elem_find(info, elem->data, elem->nfield, &pinfo);


아래 2 함수에서 각각 hval를 따로 계산합니다.

do_map_elem_find()

do_htree_elem_replace()

기존 element 이용하여 find 하는 경우가 있다면,
elem->hval을 먼저 설정한 후에,
hval, data, nfield를 넘겨서 find 하는 것이 나을 것 같습니다.
그러면, do_htree_elem_replace()에서 elem->hval 설정하는 코드는 없어도 됩니다.

hval 계산이 hash tree layer에 있어야할 것 같아 비효율이 있더라도 그냥 두었었는데,

다시 생각해보니 collection layer에서 한 번 세팅된 값을 재사용하게 하는 게 맞는 것 같습니다.

do_htree_elem_find에 hval 인자를 받도록 변경하고, do_htree_elem_insert, do_htree_elem_add, do_htree_elem_replace 모두 외부에서 hval을 세팅하고 넘겨주는 것으로 변경하였습니다.

jhpark816 · 2026-06-16T12:36:21Z

-            return ENGINE_SUCCESS;
-        } else {
-            return ENGINE_ELEM_EEXISTS;
+    if (pinfo->node == NULL) {


info->root == NULL 조건이 나은 것 같고,
root node 할당 받은 후 element 바로 add 하는 코드가 나은 것 같습니다.

if (info->root == NULL) { map_hash_node *node = do_map_node_alloc(0); if (node == NULL) { return ENGINE_ENOMEM; } do_map_node_link(info, NULL, 0, node); *space_increased = slabs_space_size(sizeof(map_hash_node)); hidx = MAP_GET_HASHIDX(elem->hval, node->hdepth); do_htree_elem_link(node, hidx, elem); return ENGINE_SUCCESS; }

위에서 do_htree_elem_link()는 아래 코드를 가져야 합니다.

elem->linked++; elem->next = node->htab[hidx]; node->htab[hidx] = elem; node->hcnt[hidx] += 1; node->tot_elem_cnt += 1;

do_htree_elem_link()를

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

위와 같이 추출하였습니다.

root node 할당 후엔 chain full 검사 없이 바로 add하고 SUCCESS 리턴하도록 변경하였습니다.

jhpark816 · 2026-06-16T12:47:10Z

-static ENGINE_ERROR_CODE do_map_elem_link(map_meta_info *info, map_elem_item *elem,
-                                          const bool replace_if_exist, bool *replaced)
+static ENGINE_ERROR_CODE do_htree_elem_add(map_meta_info *info, map_elem_item *elem,
+                                           map_prev_info *pinfo, size_t *space_increased)


find가 NULL인 경우에도 사용할 것이므로,
map_prev_info 보다는 map_elem_posi 용어가 나을 것 같은 데요.
용어 수정은 차후에 하는 것으로 하시죠.

추후 htree_elem_pos로 변경할 계획이었습니다.
용어는 더 고민해보도록 하겠습니다.

jhpark816

리뷰 완료

jhpark816 · 2026-06-17T04:21:09Z

+                                           map_prev_info *pinfo, size_t *space_increased)
+{
+    if (info->root == NULL) {
+        assert(pinfo->node == NULL);


pinfo가 전혀 사용되지 않을 것이므로,
위의 assert 문은 없어도 됩니다.

반영했습니다.

jhpark816 · 2026-06-17T04:24:51Z

-        return ENGINE_EOVERFLOW;
-    }
+    map_hash_node *node = pinfo->node;
+    int hidx = pinfo->hidx;


node, hidx 변수 선언을 위에 두고, info->root == NULL인 경우도 같이 사용하면 될 것 같습니다.

반영했습니다.

jhpark816 · 2026-06-17T04:36:29Z

-    new_stotal = slabs_space_size(do_map_elem_ntotal(new_elem));
-
-    CLOG_MAP_ELEM_INSERT(info, old_elem, new_elem);
+    new_elem->hval = old_elem->hval;


hval 값은 이미 설정된 상태에서 이 함수가 호출되면 좋겠습니다.

그리고, new_elem이 old_elem과 동일한 hval을 가진다는 것 자체가 오류인 것 같습니다.
두 element가 동일한 value를 가진다면 동일한 hval을 가지는 것이 맞지만,
서로 다른 value를 가지는 상황에서 동일한 hval 설정하는 것은 문제가 있어 보입니다.

zhy2on · 2026-06-17T08:42:54Z

`do_map_elem_replace` 재사용

do_map_elem_update 내부에서 do_map_elem_replace를 사용하도록 변경했습니다.

기존의 경우 do_map_elem_replace는 무조건 elem을 할당한 후에 호출이 되기 때문에
sticky limit check → replace가 한 흐름이었는데,

do_map_elem_update에서는 먼저 in-place update(같은 elem을 재활용)를 할 수 있나 확인 후, 할 수 없다면
sticky limit check → elem alloc → replace 로 진행되고 있었습니다.

때문에 replace 부분만 do_map_elem_replace 함수로 추출하고, sticky limit check, *replaced = true 등은 호출자로 이동하였습니다. 그리고 이를 do_map_elem_update에서 재사용하도록 했습니다.

insert/replace 경로의 처리 순서 통일 (CLOG → ccnt → space → free)

CLOG → (info->ccnt++ : elem 생성 시에만) → space accounting → (elem free : replace 시에 old_elem free가 필요할 때)
로 순서를 일관되게 통일하였습니다.

elem_find pinfo 초기화 제거

htree_elem_add에서 info->root == NULL일 시 pinfo를 사용하지 않고 직접 root에 연결을 하게 되었기 때문에, 불필요해진 do_**_elem_find 함수의 info->root == NULL일 시의 pinfo 초기화를 제거했습니다.

hidx, node 변수 선언 위치 통일

hidx, node 변수 선언 위치를 통일했습니다.

`do_htree_elem_link` 내부로 node split 로직 통합

do_htree_elem_link 내부로 node split 로직을 통합했습니다.
elem->linked++ 위치도 상위로 이동했습니다.

jhpark816

일부 리뷰

jhpark816 · 2026-06-17T08:51:55Z


        /* replace the element */
-        do_map_elem_replace(info, &pinfo, new_elem);
+        do_map_elem_replace(info, &pinfo, elem, new_elem);


현재 코드를 보니, elem 인자는 넘기지 않는 것이 나아 보입니다.

elem 인자를 제거하고 하나의 커밋으로 합쳤습니다.

jhpark816 · 2026-06-18T01:46:23Z

그리고, 최종 1개 commit으로 합쳐 주세요.

jhpark816

1개 commit으로 만들어 주세요.

…update

zhy2on requested a review from jhpark816 June 16, 2026 04:44

zhy2on assigned zhy2on and unassigned zhy2on Jun 16, 2026

zhy2on force-pushed the internal/map-set-insert branch from 0e6431b to 56a4acf Compare June 16, 2026 05:48

zhy2on marked this pull request as draft June 16, 2026 06:24

zhy2on force-pushed the internal/map-set-insert branch from 56a4acf to 7eb71fc Compare June 16, 2026 06:55

zhy2on marked this pull request as ready for review June 16, 2026 06:59

zhy2on marked this pull request as draft June 16, 2026 07:01

zhy2on force-pushed the internal/map-set-insert branch 2 times, most recently from 8049c74 to dc79e45 Compare June 16, 2026 07:05

zhy2on changed the title ~~INTERNAL: Extract htree_elem_insert/replace from map/set elem insert~~ INTERNAL: Refactor set elem insert to use do_htree_elem_insert Jun 16, 2026

zhy2on marked this pull request as ready for review June 16, 2026 07:07

jhpark816 reviewed Jun 16, 2026

View reviewed changes

zhy2on changed the title ~~INTERNAL: Refactor set elem insert to use do_htree_elem_insert~~ INTERNAL: Extract htree_elem_insert/replace from map/set elem insert Jun 16, 2026

zhy2on force-pushed the internal/map-set-insert branch from dc79e45 to 4467243 Compare June 16, 2026 07:50

jhpark816 reviewed Jun 16, 2026

View reviewed changes

zhy2on force-pushed the internal/map-set-insert branch from 4467243 to 5218d32 Compare June 16, 2026 07:56

jhpark816 reviewed Jun 16, 2026

View reviewed changes

zhy2on force-pushed the internal/map-set-insert branch from bd1a874 to 2ec5c60 Compare June 17, 2026 04:31

jhpark816 reviewed Jun 17, 2026

View reviewed changes

zhy2on force-pushed the internal/map-set-insert branch from 41c9c87 to 2c15144 Compare June 17, 2026 08:25

zhy2on requested a review from jhpark816 June 17, 2026 08:43

jhpark816 reviewed Jun 17, 2026

View reviewed changes

jhpark816 reviewed Jun 18, 2026

View reviewed changes

zhy2on force-pushed the internal/map-set-insert branch from eda9a77 to 323e9f8 Compare June 18, 2026 05:12

INTERNAL: Extract htree layer functions from map/set elem insert and …

f3c66ed

…update

zhy2on force-pushed the internal/map-set-insert branch from 323e9f8 to f3c66ed Compare June 18, 2026 05:16

zhy2on changed the title ~~INTERNAL: Extract htree_elem_insert/replace from map/set elem insert~~ INTERNAL: Extract htree layer functions from map/set elem insert and update Jun 18, 2026

zhy2on requested a review from jhpark816 June 18, 2026 05:21

jhpark816 approved these changes Jun 18, 2026

View reviewed changes

jhpark816 merged commit bb24ba6 into naver:develop Jun 18, 2026
1 check passed

zhy2on deleted the internal/map-set-insert branch June 18, 2026 05:39

Conversation

zhy2on commented Jun 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔗 Related Issue

⌨️ What I did

Uh oh!

jhpark816 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jhpark816 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jhpark816 Jun 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jhpark816 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zhy2on Jun 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jhpark816 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zhy2on commented Jun 16, 2026 •

edited

Loading

jhpark816 Jun 16, 2026 •

edited

Loading

zhy2on Jun 17, 2026 •

edited

Loading

`do_map_elem_replace` 재사용

`do_htree_elem_link` 내부로 node split 로직 통합