-
Notifications
You must be signed in to change notification settings - Fork 333
feat: Replace deprecated time crate with std::time #131
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
feat: Replace deprecated time crate with std::time #131
Conversation
|
Thanks for the PR. The reason we used this crate is that libc and Do you know if there's another alternative? Does the API from the current |
|
Hey, thanks for the quick response. Tbh I've been really surprised about the windows in appveyor, but it did really pick up the cross-platform issue. I'll update this xD |
2bd322d to
5080644
Compare
|
Hi, @qmonnet, I took a look and saw no direct way to do this in time v0.3. I assume this should also work in cross-platforms. It now uses std::time::Instant to capture a reference time on the first call and returns elapsed nanoseconds from that point. TL;DR time intervals should be the same, which should work the same for eBPF programs since they typically just measure elapsed time between events anyway. Let me know ;) |
5080644 to
28a065d
Compare
qmonnet
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not, we can do this I suppose.
Could you please also do the following?:
-
Update the
timecrate dependency version in the Cargo.toml -
Update the comment for the bpf_time_getns helper, it's not from boot time (it hasn't been since we switched to
timeanyway 🤦) -
Rename (including in Cargo.toml) and change the
uptimeexample (examples/uptime.rs). One suggestion, if you feel like: instead of reusing the existingbpf_time_getnshelper, we could use re-implement a separate helper in that file, that returns the current time, and print it (and rename the example todate). If you don't want to spend too much time on this one I can understand, I can follow up with it. Note: running the JIT for the examples currently doesn't work on my setup, I had to change it to use the interpreter instead.
The v0.2 time crate is deprecated and time::precise_time_ns() has been removed in newer versions. This causes build failures with modern Rust toolchains. This replaces that function with std::time::Instant, which provides cross-platform monotonic time functionality using only the standard library. Signed-off-by: Daniel Mellado <[email protected]>
28a065d to
992cd61
Compare
|
Hi @qmonnet, yeah, JIT also crashed for me. I kept it as it was later but I'm wondering if it'd make sense to just change it. Thanks! |
|
Thanks for the update. I'll try to take a look in the next few days, sorry for the delay. |
|
@qmonnet could you PTAL? Thanks! |
qmonnet
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apologies for the delay - Looks OK, let's go for it, thank you!
|
FYI I just tagged and published v0.4.0, which includes your changes. |
Awesome, thanks! I was about to ask for that ;) |
The v0.2 time crate is deprecated and time::precise_time_ns() has been removed in newer versions. This causes build failures with modern Rust toolchains.
This replaces that function with std::time::Instant, which provides cross-platform monotonic time functionality using only the standard library. The bpf_time_getns() helper now returns elapsed nanoseconds since the first call, and the documentation has been updated to reflect this behavior.
The uptime.rs example has been replaced with a new date.rs example. This new example defines a helper that returns the current Unix timestamp and prints the current date and time.
Signed-off-by: Daniel Mellado [email protected]