Skip to content

INTERNAL: Refactor the structure of additional item in mblck#756

Open
ing-eoking wants to merge 1 commit into
naver:developfrom
ing-eoking:mblck
Open

INTERNAL: Refactor the structure of additional item in mblck#756
ing-eoking wants to merge 1 commit into
naver:developfrom
ing-eoking:mblck

Conversation

@ing-eoking
Copy link
Copy Markdown
Collaborator

@ing-eoking ing-eoking commented Apr 26, 2024

🔗 Related Issue

  • jam2in/arcus-works#539

⌨️ What I did

1. memory block 에서 value_item 를 사용하도록 변경합니다.

  • Additional ritem으로 활용되는 아래의 3가지 item이 동일한 타입을 갖게 끔 하기 위함입니다.
    • hash item : value_item
    • element item : value_item
    • memory block item : mblck_node_t
  • value_item으로 변경 시 아래 사항을 고려해야 했습니다.
    • memory block의 body_len 계산
      - pool->body_len = blck_len - sizeof(void *);
      + pool->body_len = blck_len - (sizeof(void *) + sizeof(uint32_t))

2. Memory Block List 자료 구조 내에서 Head를 제외한 Memory Block을 Additional List에 넣어둡니다.

  • Additional List는 인덱스 기반(rindex)으로 다음 item에 접근할 때 활용됩니다.

🎯 To Do

  • Ubuntu에서 unit test 실패 발생 : 원인 파악중
    • 초기값으로 NULL이 들어가지 않을 수 있음
  • 메모리 부족 로직 추가
  • mblck_list 에서 body_len 제거 : value_item 내부에 len 정보를 담음
  • 관련 매크로 정리
  • 링크드리스트 조회를 위해 사용된 c->membk 제거

@ing-eoking ing-eoking marked this pull request as ready for review June 17, 2024 01:33
@ing-eoking ing-eoking marked this pull request as draft June 19, 2024 08:49
@ing-eoking ing-eoking marked this pull request as ready for review October 16, 2024 08:55
@ing-eoking
Copy link
Copy Markdown
Collaborator Author

@namsic

해당 PR도 리뷰 부탁드립니다.

Comment thread mc_util.h
Comment on lines 45 to +47
mblck_node_t *head;
mblck_node_t *tail;
value_item **addnl;
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.

  • headnext를 통해 전체 item에 접근 가능할 것인데, addnl을 두는 이유는 무엇인가요?
  • value_item array를 둔다면, tail 등 링크드리스트 관련 field를 제거할 수 있는 것이 아닌지?

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.

  1. addnl은 ritem read 시 활용하기 위하여 추가한 것 (HINFO, EINFO와 유사한 구조를 갖도록)
  2. mblck은 ritem read 외에도 활용하는 경우가 있는데 (tokenize_XXX()), 이 때 list 방식의 구현이 필요하다고 함
  3. 따라서 PR 구현에서는 하나의 데이터를 array 방식으로도 접근(ritem_set_XXX())할 수 있고,
    기존 list 방식으로도 접근(tokenize_XXX())할 수 있도록 구현한 상태

Comment thread mc_util.c
pool->head = NULL;
pool->blck_len = blck_len;
pool->body_len = blck_len - sizeof(void *);
pool->body_len = blck_len - (sizeof(void *) + sizeof(uint32_t));
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.

sizeof(void *)sizeof(uint32_t)는 각각 어떤 값의 크기인가요?

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.

old

| <----- blck_len -----> |
        | <- body_len -> |
--------------------------
| *next |      data      |

new

| <-------- blck_len --------> |
        | <--- value_item ---> |
              | <- body_len -> |
--------------------------
| *next | len |       ptr      |
  • value_item.lenpool->body_len은 결국에는 같은 값을 갖게 된다고 함

@jhpark816
Copy link
Copy Markdown
Collaborator

@ing-eoking
본 PR은 어떻게 진행되고 있나요?

@ing-eoking
Copy link
Copy Markdown
Collaborator Author

@jhpark816

오래 전에 진행한 이후로 진행되지 않았습니다.

당시 논의한 결과, 현재로서는 보다 깔끔한 프로토콜 모듈화를 위해 해당 방식으로 변경하는 방법 외에는 떠오르지 않지만,
이 방식이 좋은지 확신이 서지 않아서 당장 반영할 계획은 없다고 하셨습니다.

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