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

maint: Replace usage of folly in clock.hpp #1448

Merged
merged 2 commits into from
Apr 25, 2024
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 2 additions & 11 deletions cpp/arcticdb/util/clock.hpp
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is is possible to check if the resolution of the clock is the same?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Both have nanoseconds resolution seemingly calling clock_gettime.

Copy link
Collaborator Author

@jjerphan jjerphan Mar 22, 2024

Choose a reason for hiding this comment

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

folly::chrono::clock_gettime_ns's definition uses the vDSO on Linux (calling the vDSO is not considered a good practice (see the "Description" section of vdso(7)'s man page). Alternatively, it calls clock_gettime.

std::chrono::system_clock::now has nanosecond precision. and also calls clock_gettime with CLOCK_REALTIME.

The only drawback is that coarse_nanos_since_epoch who used to return coarse precision timestamp (see CLOCK_REALTIME_COARSE) now returns an accurate timestamp. Must we refine it or not?

Original file line number Diff line number Diff line change
Expand Up @@ -9,29 +9,20 @@

#include <arcticdb/entity/types.hpp>

#include <folly/ClockGettimeWrappers.h>
#include <folly/portability/Time.h>
#ifndef CLOCK_REALTIME_COARSE
#include <chrono>
#endif

namespace arcticdb::util {

class SysClock {
public:
static entity::timestamp nanos_since_epoch() {
return (*folly::chrono::clock_gettime_ns)(CLOCK_REALTIME);
}
#if defined(_WIN32) || defined(__APPLE__)
static entity::timestamp coarse_nanos_since_epoch() {
auto t = std::chrono::system_clock::now();
return std::chrono::duration_cast<std::chrono::nanoseconds>(t.time_since_epoch()).count();
}
#else
static entity::timestamp coarse_nanos_since_epoch() {
return (*folly::chrono::clock_gettime_ns)(CLOCK_REALTIME_COARSE);
auto t = std::chrono::system_clock::now();
return std::chrono::duration_cast<std::chrono::nanoseconds>(t.time_since_epoch()).count();
}
#endif
};


Expand Down
Loading