Skip to content
New issue

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

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Optimize keytablelist implementation #2768

Merged
merged 4 commits into from
Oct 7, 2024
Merged

Conversation

MJY-HUST
Copy link
Contributor

What problem does this PR solve?

Issue Number: #2756

Problem Summary:

  • 设置thread_local的keytable_list长度上限为KEY_TABLE_LIST_SIZE = 10000
  • 超出KEY_TABLE_LIST_SIZE 后,返回KEY_TABLE_LIST_SIZE / 2 个 keytable给全局的free_keytables
  • 当前thread的keytable_list为空但是free_keytables不为空时,一次性从free_keytables中取出至多BORROW_FROM_GLOBLE_SIZE = 100 个keytable给keytable_list

What is changed and the side effects?

Changed:

Side effects:

  • Performance effects(性能影响):

  • Breaking backward compatibility(向后兼容性):


Check List:

  • Please make sure your changes are compilable(请确保你的更改可以通过编译).
  • When providing us with a new feature, it is best to add related tests(如果你向我们增加一个新的功能, 请添加相关测试).
  • Please follow Contributor Covenant Code of Conduct.(请遵循贡献者准则).

@wwbmmm
Copy link
Contributor

wwbmmm commented Sep 21, 2024

补充一些单测吧,感觉现有的单测没有覆盖到新增的代码

Copy link
Contributor

@chenBright chenBright left a comment

Choose a reason for hiding this comment

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

缩进改成4个空格吧

@MJY-HUST
Copy link
Contributor Author

MJY-HUST commented Sep 25, 2024

补充一些单测吧,感觉现有的单测没有覆盖到新增的代码

补充了两个单测:
场景一:启动多个bthread,获取keytable后bthread_usleep,模拟issue中的场景
场景二:启动多个bthread,一组bthread直接调用borrow_keytable获取keytable并存放到全局链表,另一组从全局链表中获取keytable并return_keytable,这些bthread理论上都不会发生切换,来模拟keytable不均衡的场景。
两个场景均检查thread local list的长度小于bthread::FLAGS_key_table_list_size

@MJY-HUST
Copy link
Contributor Author

缩进改成4个空格吧

已修改

@wwbmmm
Copy link
Contributor

wwbmmm commented Sep 26, 2024

LGTM

Copy link
Contributor

@chenBright chenBright left a comment

Choose a reason for hiding this comment

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

LGTM

@chenBright
Copy link
Contributor

@lsdh-fei 可以验证一下这个PR能否解决#2756 的问题吗?

@wwbmmm wwbmmm merged commit d94c88f into apache:master Oct 7, 2024
20 checks passed
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants