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

FOGL-9183: Round of the microseconds to seconds for time difference #93

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

gnandan
Copy link
Contributor

@gnandan gnandan commented Oct 23, 2024

No description provided.

@gnandan gnandan requested a review from MarkRiddoch October 23, 2024 12:32
MarkRiddoch
MarkRiddoch previously approved these changes Oct 25, 2024
@praveen-garg
Copy link
Member

@gnandan better to change the base to 2.6.0RC if all good. /cc @ashish-jabble @Mohit04tomar

@@ -780,6 +780,13 @@ bool NotificationInstance::handleState(bool evalRet)
// Calculate time diff
timersub(&now_tv, &m_lastSentTv, &diffTimeTv);

// round up the seconds If microseconds are greater than or equal to 500000
if (diffTimeTv.tv_usec >= 500000)

Choose a reason for hiding this comment

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

It would be better if define it as const

Choose a reason for hiding this comment

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

I do not think, i fully understand the rationality of the logic here. What is the need to look into lower order precision (micros second) ? to override, high order precision ? Does our need to have time precision of 500 milli second ..?

I think it is better if we stick with the time precisions.

I see we are using gettimeofday() here are some resons we should not use it https://blog.habets.se/2010/09/gettimeofday-should-never-be-used-to-measure-time.html

Here the example code with custom precision.

    auto start = std::chrono::steady_clock::now();

    // Perform some operations
    for (int i = 0; i < 1000000; ++i);

    auto end = std::chrono::steady_clock::now();
    auto duration = std::chrono::duration_cast<std::chrono::milliseconds>(end - start);

Copy link
Contributor

@MarkRiddoch MarkRiddoch left a comment

Choose a reason for hiding this comment

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

It is very debatable as to if the issue raised is a real problem. I would be very tempted to say we do not include this and close the original Jira as not a problem.

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

Successfully merging this pull request may close these issues.

5 participants