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

Using std::chrono::steady_clock instead of system_clock in RaftKeeper #266

Open
lzydmxy opened this issue May 16, 2024 · 3 comments · May be fixed by #352
Open

Using std::chrono::steady_clock instead of system_clock in RaftKeeper #266

lzydmxy opened this issue May 16, 2024 · 3 comments · May be fixed by #352
Labels
performance Performance promotion

Comments

@lzydmxy
Copy link
Contributor

lzydmxy commented May 16, 2024

Description

The following is flame graph of RaftKeeper instance which serving as ClickHouse metadata store. A lot of cpu time cost () on std::chrono::system::clock (because of syscall).Maybe we could optimize the std::chrono::steady_clock instead of std::chrono::steady_clock
image
image

small test

#include <iostream>
#include <chrono>

using steady_clock = std::chrono::steady_clock;
using system_clock = std::chrono::system_clock;

int main()
{
    auto t1 = steady_clock::now();
    auto t2 = steady_clock::now();
    auto t3 = steady_clock::now();
    std::cout << "chrono::steady_clock: " << (t3-t1).count() << "ns" << std::endl;


    auto t4 = steady_clock::now();
    auto t5 = system_clock::now();
    auto t6 = steady_clock::now();
    std::cout << "chrono::system_clock: " << (t6-t4).count() << "ns" << std::endl;
}

./main
chrono::steady_clock: 197ns
chrono::system_clock: 501ns

twice std::chrono::steady_clock cost 197ns
once std::chrono::steady_clock once std::chrono::system_clock cost 500ns

once std::chrono::system_clock may cost 400ns and once std::chrono::steady_clock may cost 100ns

Are you willing to submit PR?

@lzydmxy lzydmxy added the enhancement Enhancements label May 16, 2024
@JackyWoo JackyWoo changed the title Using std::chrono::steady_clock instead of std::chrono::system::clock in RaftKeeper Using std::chrono::steady_clock instead of system_clock in RaftKeeper May 17, 2024
@JackyWoo
Copy link
Contributor

JackyWoo commented May 17, 2024

hi, I make a simple test with the following code snippet:

inline size_t getCurrentTimestamp()
{
    auto now = std::chrono::steady_clock::now();
    return std::chrono::duration_cast<std::chrono::nanoseconds>(now.time_since_epoch()).count();
}

int main()
{
    size_t n = 3;
    auto t1 = getCurrentTimestamp();

    for (size_t i=0; i<n; i++)
        std::chrono::steady_clock::now();

    auto t2 = getCurrentTimestamp();

    for (size_t i=0; i<n; i++)
        std::chrono::system_clock::now();

    auto t3 = getCurrentTimestamp();

    std::cout << "steady clock: " << std::to_string(t2 - t1) << "ns" << std::endl;
    std::cout << "system clock: " << std::to_string(t3 - t2) << "ns" << std::endl;
}

I found that the result are quiet different in multiple runing:

The 1st run:

steady clock: 167ns
system clock: 208ns

The 2nd run:

steady clock: 250ns
system clock: 166ns

The 3rd run:

steady clock: 291ns
system clock: 292ns

Maybe there is something we are missing.

PS: my env is MacOS(M3) clang16

@JackyWoo
Copy link
Contributor

JackyWoo commented May 17, 2024

Different result in Ubuntu 20.4

Nearly same in multiple runnings

steady clock: 261ns
system clock: 577ns

@JackyWoo JackyWoo added performance Performance promotion and removed enhancement Enhancements labels May 22, 2024
@JackyWoo
Copy link
Contributor

JackyWoo commented Sep 6, 2024

Will be fixed by #352

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Performance promotion
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants