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

ログチャネルのクローズ処理の中で計算量が多すぎるという疑惑 #66

Open
umegane opened this issue Nov 19, 2024 · 1 comment

Comments

@umegane
Copy link
Contributor

umegane commented Nov 19, 2024

ログチャネルのクローズ時に、次のコードが呼ばれる。1回のエポックで、数百のログチャネル
がクローズされるような状況ではO(n^2)のオーダーで、メモリオーダ、memory_order_seq_cst
でAtomic変数へのアクセスが行われる。これが、性能上問題ないか確認する必要がある。

for (const auto& e : log_channels_) {
    auto working_epoch = static_cast<epoch_id_type>(e->current_epoch_id_.load());
    if ((working_epoch - 1) < upper_limit && working_epoch != UINT64_MAX) {
        upper_limit = working_epoch - 1;
    }
    auto finished_epoch = e->finished_epoch_id_.load();
    if (max_finished_epoch < finished_epoch) {
        max_finished_epoch = finished_epoch;
    }
@t-horikawa
Copy link
Contributor

下記がvoid datastore::update_min_epoch_id(bool from_switch_epoch)のdisassemble結果(sourceとの対応有)で、問題とされているforは"===="で囲まれた部分ですが、「メモリオーダ、memory_order_seq_cstでAtomic変数へのアクセス」はどの機械語命令のことでしょうか?()内は私が補ったコメントです。
なお、std::atomic_thread_fence(std::memory_order_acq_rel); でmfence命令が入ることを期待したのですが、入っていませんでした。memory orderを保証する別のメカニズムがあるのでしょうか?(最近のCPU事情には疎い...)

000000000002b810 <_ZN9limestone3api9datastore19update_min_epoch_idEb>:
void datastore::update_min_epoch_id(bool from_switch_epoch) {  // NOLINT(readability-function-cognitive-complexity)
   2b810:       f3 0f 1e fa             endbr64
   2b814:       55                      push   %rbp
    epoch_id_type max_finished_epoch = 0;
   2b815:       31 c9                   xor    %ecx,%ecx
void datastore::update_min_epoch_id(bool from_switch_epoch) {  // NOLINT(readability-function-cognitive-complexity)
   2b817:       48 89 e5                mov    %rsp,%rbp
   2b81a:       41 57                   push   %r15
   2b81c:       41 56                   push   %r14
   2b81e:       49 89 fe                mov    %rdi,%r14
   2b821:       41 55                   push   %r13
   2b823:       41 54                   push   %r12
   2b825:       53                      push   %rbx
   2b826:       48 81 ec e8 01 00 00    sub    $0x1e8,%rsp
   2b82d:       64 48 8b 04 25 28 00    mov    %fs:0x28,%rax
   2b834:       00 00
   2b836:       48 89 45 c8             mov    %rax,-0x38(%rbp)
   2b83a:       31 c0                   xor    %eax,%eax
   2b83c:       48 8b 5f 38             mov    0x38(%rdi),%rbx
    auto upper_limit = epoch_id_switched_.load() - 1;
   2b840:       48 83 eb 01             sub    $0x1,%rbx
   2b844:       48 8b 07                mov    (%rdi),%rax
   2b847:       48 8b 7f 08             mov    0x8(%rdi),%rdi
====
    for (const auto& e : log_channels_) {
   2b84b:       48 39 f8                cmp    %rdi,%rax
   2b84e:       74 29                   je     2b879 <_ZN9limestone3api9datastore19update_min_epoch_idEb+0x69>
   2b850:       48 8b 10                mov    (%rax),%rdx
   2b853:       48 8b 52 70             mov    0x70(%rdx),%rdx
        if ((working_epoch - 1) < upper_limit) {
   2b857:       48 83 ea 01             sub    $0x1,%rdx
   2b85b:       48 39 d3                cmp    %rdx,%rbx
   2b85e:       48 0f 47 da             cmova  %rdx,%rbx (rdx: working_epoch - 1, rbx: upper_limit)
   2b862:       48 8b 10                mov    (%rax),%rdx
   2b865:       48 8b 52 78             mov    0x78(%rdx),%rdx (0x78(%rdx): e->finished_epoch_id_をrdx: finished_epochに入れる)
   2b869:       48 39 d1                cmp    %rdx,%rcx (rcx: finished_epoch、このcmpがif (max_finished_epoch < finished_epoch) に相当)
   2b86c:       48 0f 42 ca             cmovb  %rdx,%rcx
    for (const auto& e : log_channels_) {
   2b870:       48 83 c0 08             add    $0x8,%rax (次のlog_channels_エントリへ)
   2b874:       48 39 f8                cmp    %rdi,%rax
   2b877:       75 d7                   jne    2b850 <_ZN9limestone3api9datastore19update_min_epoch_idEb+0x40>
====

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

No branches or pull requests

2 participants