Skip to content

INTERNAL: Unify elem lifecycle state management with status field#969

Merged
jhpark816 merged 1 commit intonaver:developfrom
zhy2on:internal/elem-status-field-unification
Apr 10, 2026
Merged

INTERNAL: Unify elem lifecycle state management with status field#969
jhpark816 merged 1 commit intonaver:developfrom
zhy2on:internal/elem-status-field-unification

Conversation

@zhy2on
Copy link
Copy Markdown

@zhy2on zhy2on commented Apr 9, 2026

🔗 Related Issue

⌨️ What I did

  • list/set/map elem 구조체에 status 필드를 추가하고, next 포인터에 ADDR_MEANS_UNLINKED = 1을 저장하여 unlink 상태를 나타내던 센티넬 포인터 패턴을 제거했습니다.
  • btree에서만 사용하던 BTREE_ITEM_STATUS_USED/UNLINK/FREE define을 ELEM_STATUS_USED/UNLINK/FREE로 이름을 바꿔 item_base.h로 이동하여 4개의 컬렉션이 동일하게 사용하도록 했습니다.
  • list_elem_item의 경우 기존에 패딩용 dummy 필드가 명시적으로 정의되어 있었습니다. 때문에 새로 status 필드를 추가하는 것이 아닌, dummy 필드를 status로 교체하여 구조체 크기 변화 없이 필드를 추가했습니다.
  • btree의 기존 BTREE_ITEM_STATUS_* 사용 코드는 공용 ELEM_STATUS_*를 사용하도록 교체했으며, 동작 변화는 없습니다.

@jhpark816 jhpark816 requested a review from namsic April 9, 2026 08:23
Comment thread engines/default/coll_btree.c Outdated
Comment on lines +309 to +310
if (elem->refcount == 0 && elem->status == ELEM_STATUS_UNLINK) {
elem->status = ELEM_STATUS_FREE;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

기존 구현을 보면

  • BTREE_ITEM_STATUS_FREE 설정한 직후에는 항상 do_btree_elem_free 호출하였고,
  • 센티넬 포인터 패턴에서도 free 상태 없이 unlink 상태만 관리하고 있었습니다.

변경된 구현에서도 기존 구현 패턴을 따라 ELEM_STATUS_FREE 설정 직후에 do_coll_elem_free 호출하고 있는데요.

상단의 /* assert(elem->status != ELEM_STATUS_FREE); */ 주석 정도를 제외하면
실제로 ELEM_STATUS_FREE 값을 사용/비교하는 구현이 없고,
elem->status 조회했을 때 ELEM_STATUS_FREE가 읽히는 경우도 없을 것 같습니다.

만약 ELEM_STATUS_FREE 없이 USED / UNLINK 상태만으로 구현한다고 했을 때 문제가 생길만한 부분이 있나요?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

아래 2개 status만 두는 것이 명확한 방법이라는 것이죠?

  • ELEM_STATUS_LINKED
  • ELEM_STATUS_UNLINKED

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

아래 2개 status만 두는 것이 명확한 방법이라는 것이죠?

예. 만약 지금 구현 그대로 ELEM_STATUS_FREE만 제거했을 때 동작에 차이가 없다고 하면,
실제 동작에 아무런 영향도 주지 않는 값을 유지/관리하고 있던 것이므로 제거하는 것이 낫다고 봅니다.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

만약 ELEM_STATUS_FREE 없이 USED / UNLINK 상태만으로 구현한다고 했을 때 문제가 생길만한 부분이 있나요?

말씀하신 것처럼 이미 주석 처리된 부분 외에는 ELEM_STATUS_FREE 사용이 없었습니다.

주석 부분도 특별한 기능을 위해서가 아닌, 디버깅 용도로 사용했던 흔적으로 보입니다.

제거해도 영향이 없을 것으로 보이며 단순화하는 게 맞을 것 같습니다.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

OK. 단순화하시죠.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

반영하였습니다.

Add status field to list/set/map elem items and replace the
ADDR_MEANS_UNLINKED sentinel pointer pattern with ELEM_STATUS_LINKED/
UNLINKED, consistent with btree. The shared defines are placed in
item_base.h since all four collections use identical lifecycle states.
@zhy2on zhy2on force-pushed the internal/elem-status-field-unification branch from 81076b9 to 774fe56 Compare April 9, 2026 23:42
@jhpark816 jhpark816 merged commit b5b0337 into naver:develop Apr 10, 2026
1 check passed
@zhy2on zhy2on deleted the internal/elem-status-field-unification branch April 10, 2026 07:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants