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

Use clock_gettime instead of gettimeofday, it's more efficiency on low power system #1979

Merged
merged 1 commit into from Feb 18, 2015

Conversation

Mullin
Copy link
Contributor

@Mullin Mullin commented Jan 28, 2015

Use clock_gettime instead of gettimeofday, it's more efficiency on low power system

@Mullin Mullin changed the title Merge Use clock_gettime instead of gettimeofday, it's more effiency on low power system Jan 28, 2015
@ghost
Copy link

ghost commented Jan 28, 2015

Disregarding portability issues (I don't believe OSX has clock_gettime()), this is actually a good idea on Android; it prevents the clock jumping around if the system is set to update automatically from the network and the device switches between cell towers that aren't perfectly synchronized.

@Mullin
Copy link
Contributor Author

Mullin commented Jan 29, 2015

Yeah, i know. I just forgot to add a def for OSX. I'm a bit busy. I commit soon as possible :)

@skidau
Copy link
Contributor

skidau commented Jan 30, 2015

Please fix up the commit history.

@delroth
Copy link
Member

delroth commented Jan 30, 2015

Definitely agree about using clock_gettime and CLOCK_MONOTONIC being superior, but I really doubt this is any different in terms of efficiency. In fact, I would expect gettimeofday to be faster on x86_64 according to the VDSO implementation (gettimeofday is implemented as do_realtime + a division, while clock_gettime needs to also branch depending on the clock type).

struct timeval t;
(void)gettimeofday(&t, nullptr);
return ((u64)(t.tv_sec * 1000000 + t.tv_usec));
return ((u64)(t.tv_sec * 1000 + t.tv_usec / 1000));

This comment was marked as off-topic.

@Mullin
Copy link
Contributor Author

Mullin commented Jan 30, 2015

Hmmm.. So just defined clockgettime for ARM devices and x86 and leave gettimeofday for x86-64

@Mullin Mullin changed the title Use clock_gettime instead of gettimeofday, it's more effiency on low power system Use clock_gettime instead of gettimeofday, it's more efficiency on low power system Jan 30, 2015
@delroth
Copy link
Member

delroth commented Jan 30, 2015

Don't, nobody cares about its efficiency anyway (I can't imagine a situation where it would be the bottleneck). CLOCK_MONOTONIC has clear advantages.

@comex
Copy link
Contributor

comex commented Jan 30, 2015

FYI, you can use mach_absolute_time for this purpose on OS X (it is monotonic and fast). See:

http://shiftedbits.org/2008/10/01/mach_absolute_time-on-the-iphone/

(and you can pretty much ignore the timebase thing, it's always gonna be nanoseconds.)

@Mullin
Copy link
Contributor Author

Mullin commented Feb 17, 2015

@skidau history fixed.

@magumagu
Copy link
Contributor

@dolphin-emu-bot rebuild

@@ -237,7 +252,7 @@ double Timer::GetDoubleTime()
#ifdef _WIN32
double ms = tp.millitm / 1000.0 / 1000.0;
#else

This comment was marked as off-topic.

@Mullin
Copy link
Contributor Author

Mullin commented Feb 17, 2015

Benchmark on Armv7 :
BUST A MOVE 3000 before fix : 16-17 fps
BUST A MOVE 3000 after fix : 30-34 fps

@degasus
Copy link
Member

degasus commented Feb 17, 2015

I'd like to see those commits squashed, else LGTM

…power system

Add def for mac(They don't support clock_gettime)

Fix my mistake

Fix my mistake 2
@Mullin
Copy link
Contributor Author

Mullin commented Feb 18, 2015

Squashed :)

@degasus
Copy link
Member

degasus commented Feb 18, 2015

@dolphin-emu-bot rebuild
LGTM

degasus added a commit that referenced this pull request Feb 18, 2015
Use clock_gettime instead of gettimeofday, it's more efficiency on low power system
@degasus degasus merged commit 86226cb into dolphin-emu:master Feb 18, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
6 participants