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

issue #3978: improve st_utime's default impl. #3979

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

suzp1984
Copy link
Contributor

@suzp1984 suzp1984 commented Mar 7, 2024

try to fix #3978

Background
check #3978

Research

I referred the Android platform's solution, because I have android background, and there is a loop to handle message inside android.

https://github.com/aosp-mirror/platform_frameworks_base/blob/ff007a03c01bf936d1e961a13adff9f266d5189c/core/java/android/os/Handler.java#L701-L706C6

    public final boolean sendMessageDelayed(@NonNull Message msg, long delayMillis) {
        if (delayMillis < 0) {
            delayMillis = 0;
        }
        return sendMessageAtTime(msg, SystemClock.uptimeMillis() + delayMillis);
    }

https://github.com/aosp-mirror/platform_system_core/blob/59d9dc1f50b1ae8630ec11a431858a3cb66487b7/libutils/SystemClock.cpp#L37-L51

/*
 * native public static long uptimeMillis();
 */
int64_t uptimeMillis()
{
    return nanoseconds_to_milliseconds(uptimeNanos());
}


/*
 * public static native long uptimeNanos();
 */
int64_t uptimeNanos()
{
    return systemTime(SYSTEM_TIME_MONOTONIC);
}

https://github.com/aosp-mirror/platform_system_core/blob/59d9dc1f50b1ae8630ec11a431858a3cb66487b7/libutils/Timers.cpp#L32-L55

#if defined(__linux__)
nsecs_t systemTime(int clock) {
    checkClockId(clock);
    static constexpr clockid_t clocks[] = {CLOCK_REALTIME, CLOCK_MONOTONIC,
                                           CLOCK_PROCESS_CPUTIME_ID, CLOCK_THREAD_CPUTIME_ID,
                                           CLOCK_BOOTTIME};
    static_assert(clock_id_max == arraysize(clocks));
    timespec t = {};
    clock_gettime(clocks[clock], &t);
    return nsecs_t(t.tv_sec)*1000000000LL + t.tv_nsec;
}
#else
nsecs_t systemTime(int clock) {
    // TODO: is this ever called with anything but REALTIME on mac/windows?
    checkClockId(clock);


    // Clock support varies widely across hosts. Mac OS doesn't support
    // CLOCK_BOOTTIME (and doesn't even have clock_gettime until 10.12).
    // Windows is windows.
    timeval t = {};
    gettimeofday(&t, nullptr);
    return nsecs_t(t.tv_sec)*1000000000LL + nsecs_t(t.tv_usec)*1000LL;
}
#endif

For Linux system, we can use clock_gettime api, but it's first appeared in Mac OSX 10.12.

man clock_gettime

The requirement is to find an alternative way to get the timestamp in microsecond unit, but the clock_gettime get nanoseconds, the math formula is the nanoseconds / 1000 = microsecond. Then I check the performance of this api + math division.

I used those code to check the clock_gettime performance.

#include <sys/time.h>
#include <time.h>
#include <stdio.h>
#include <unistd.h>

int main() {
	struct timeval tv;
	struct timespec ts;
	clock_t start;
	clock_t end;
	long t;

	while (1) {
		start = clock();
		gettimeofday(&tv, NULL);
		end = clock();
		printf("gettimeofday clock is %lu\n", end - start);
		printf("gettimeofday is %lld\n", (tv.tv_sec * 1000000LL + tv.tv_usec));

		start = clock();
		clock_gettime(CLOCK_MONOTONIC, &ts);
		t = ts.tv_sec * 1000000L + ts.tv_nsec / 1000L;
		end = clock();
		printf("clock_monotonic clock is %lu\n", end - start);
		printf("clock_monotonic: seconds is %ld, nanoseconds is %ld, sum is %ld\n", ts.tv_sec, ts.tv_nsec, t);

		start = clock();
		clock_gettime(CLOCK_MONOTONIC_RAW, &ts);
		t = ts.tv_sec * 1000000L + ts.tv_nsec / 1000L;
		end = clock();
		printf("clock_monotonic_raw clock is %lu\n", end - start);
		printf("clock_monotonic_raw: nanoseconds is %ld, sum is %ld\n", ts.tv_nsec, t);

		sleep(3);
	}
	
	return 0;
}

Here is output:

env: Mac OS M2 chip.

gettimeofday clock is 11
gettimeofday is 1709775727153949
clock_monotonic clock is 2
clock_monotonic: seconds is 1525204, nanoseconds is 409453000, sum is 1525204409453
clock_monotonic_raw clock is 2
clock_monotonic_raw: nanoseconds is 770493000, sum is 1525222770493

We can see the clock_gettime is faster than gettimeofday, so there are no performance risks.

MacOS solution

clock_gettime api only available until mac os 10.12, for the mac os older than 10.12, just keep the gettimeofday.
check osx version in auto/options.sh, then add MACRO in auto/depends.sh, the MACRO is MD_OSX_HAS_NO_CLOCK_GETTIME.

CYGWIN
According to google search, it seems the clock_gettime(CLOCK_MONOTONIC) is not support well at least 10 years ago, but I didn't own an windows machine, so can't verify it. so keep win's solution.

@winlinvip winlinvip added the EnglishNative This issue is conveyed exclusively in English. label Mar 7, 2024
@suzp1984 suzp1984 force-pushed the issue/3978-refactor-st-utime branch from c3ee990 to d952b60 Compare March 7, 2024 08:38
@winlinvip
Copy link
Member

Thank you for your excellent work. Please add new tests to validate your updates in the pull request. The utest is crucial and a necessary condition for merging the pull request. Without it and automated testing, we risk introducing bugs sooner or later due to many changes over time.

@suzp1984 suzp1984 force-pushed the issue/3978-refactor-st-utime branch 2 times, most recently from 3ce21e5 to d59a1d1 Compare March 9, 2024 09:58
@suzp1984
Copy link
Contributor Author

suzp1984 commented Mar 9, 2024

Add a UT in srs root project.

@winlinvip
Copy link
Member

Failed in cygwin?

@suzp1984 suzp1984 force-pushed the issue/3978-refactor-st-utime branch 2 times, most recently from 22590ef to 1aae04f Compare March 18, 2024 09:38
@suzp1984 suzp1984 force-pushed the issue/3978-refactor-st-utime branch from 1aae04f to 5c045b4 Compare March 18, 2024 13:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
EnglishNative This issue is conveyed exclusively in English.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

change system time backward will let st-srs behavior abnormal
2 participants