[sleepy.penguin] [PATCH] Add support for Mac OS X

[sleepy.penguin] [PATCH] Add support for Mac OS X

[Lin Jen-Shin]

Since Mac OS X does not support clock_gettime, we need to wrap
around clock_gettime to make kqueue work on Mac OS X.

epoll.c does not need this wrapper as it's only on Linux,
but I guess it's fine to use the wrapper as well. Maybe
someday *BSD could also support epoll in someway?
---
 ext/sleepy_penguin/clock_gettime.c | 17 +++++++++++++++++
 ext/sleepy_penguin/clock_gettime.h | 23 +++++++++++++++++++++++
 ext/sleepy_penguin/epoll.c         |  3 ++-
 ext/sleepy_penguin/kqueue.c        |  5 +++--
 4 files changed, 45 insertions(+), 3 deletions(-)
 create mode 100644 ext/sleepy_penguin/clock_gettime.c
 create mode 100644 ext/sleepy_penguin/clock_gettime.h

diff --git a/ext/sleepy_penguin/clock_gettime.c b/ext/sleepy_penguin/clock_gettime.c
new file mode 100644
index 0000000..1f42c10
--- /dev/null
+++ b/ext/sleepy_penguin/clock_gettime.c
@@ -0,0 +1,17 @@
+#ifdef __MACH__
+/*
+ * Mac OS X does not support clock_gettime
+ * ref: http://stackoverflow.com/questions/11680461/monotonic-clock-on-osx
+ */
+#include "clock_gettime.h"
+
+void clock_gettime_mach(struct timespec *now){
+	clock_serv_t cclock;
+	mach_timespec_t mts;
+	host_get_clock_service(mach_host_self(), CALENDAR_CLOCK, &cclock);
+	clock_get_time(cclock, &mts);
+	mach_port_deallocate(mach_task_self(), cclock);
+	now->tv_sec = mts.tv_sec;
+	now->tv_nsec = mts.tv_nsec;
+}
+#endif
diff --git a/ext/sleepy_penguin/clock_gettime.h b/ext/sleepy_penguin/clock_gettime.h
new file mode 100644
index 0000000..3294176
--- /dev/null
+++ b/ext/sleepy_penguin/clock_gettime.h
@@ -0,0 +1,23 @@
+#ifndef CLOCK_GETTIME_H
+#define CLOCK_GETTIME_H
+
+#include <time.h>
+#include <sys/time.h>
+
+#ifdef __MACH__
+#include <mach/clock.h>
+#include <mach/mach.h>
+#endif
+
+#ifdef __MACH__
+/*
+ * Mac OS X does not support clock_gettime
+ * ref: http://stackoverflow.com/questions/11680461/monotonic-clock-on-osx
+ */
+void clock_gettime_mach(struct timespec*);
+#define CLOCK_GETTIME(now) clock_gettime_mach(now);
+#else
+#define CLOCK_GETTIME(now) clock_gettime(CLOCK_MONOTONIC, now);
+#endif
+
+#endif /* CLOCK_GETTIME_H */
diff --git a/ext/sleepy_penguin/epoll.c b/ext/sleepy_penguin/epoll.c
index 9010e2d..d4eedf7 100644
--- a/ext/sleepy_penguin/epoll.c
+++ b/ext/sleepy_penguin/epoll.c
@@ -1,4 +1,5 @@
 #include "sleepy_penguin.h"
+#include "clock_gettime.h"
 #ifdef HAVE_SYS_EPOLL_H
 #include <sys/epoll.h>
 #include <unistd.h>
@@ -14,7 +15,7 @@ static uint64_t now_ms(void)
 {
 	struct timespec now;
 
-	clock_gettime(CLOCK_MONOTONIC, &now);
+	CLOCK_GETTIME(&now);
 
 	return now.tv_sec * 1000 + (now.tv_nsec + 500000) / 1000000;
 }
diff --git a/ext/sleepy_penguin/kqueue.c b/ext/sleepy_penguin/kqueue.c
index 59c3dae..d9e3f16 100644
--- a/ext/sleepy_penguin/kqueue.c
+++ b/ext/sleepy_penguin/kqueue.c
@@ -1,4 +1,5 @@
 #include "sleepy_penguin.h"
+#include "clock_gettime.h"
 #ifdef HAVE_SYS_EVENT_H
 #include <sys/types.h>
 #include <sys/event.h>
@@ -196,7 +197,7 @@ kevent_resume_p(struct timespec *expire_at, struct kq_per_thread *kpt)
 	if (kpt->ts == NULL)
 		return 1;
 
-	clock_gettime(CLOCK_MONOTONIC, &now);
+	CLOCK_GETTIME(&now);
 	if (now.tv_sec > expire_at->tv_sec)
 		return 0;
 	if (now.tv_sec == expire_at->tv_sec && now.tv_nsec > expire_at->tv_nsec)
@@ -221,7 +222,7 @@ static VALUE do_kevent(struct kq_per_thread *kpt)
 	struct timespec expire_at;
 
 	if (kpt->ts) {
-		clock_gettime(CLOCK_MONOTONIC, &expire_at);
+		CLOCK_GETTIME(&expire_at);
 
 		expire_at.tv_sec += kpt->ts->tv_sec;
 		expire_at.tv_nsec += kpt->ts->tv_nsec;
-- 
1.8.4

[sleepy.penguin] Re: [PATCH] Add support for Mac OS X

[Lin Jen-Shin (godfat)]

On Mon, Aug 26, 2013 at 4:46 PM, Lin Jen-Shin <godfat@godfat.org> wrote:
> Since Mac OS X does not support clock_gettime, we need to wrap
> around clock_gettime to make kqueue work on Mac OS X.
>
> epoll.c does not need this wrapper as it's only on Linux,
> but I guess it's fine to use the wrapper as well. Maybe
> someday *BSD could also support epoll in someway?
[...]

Unfortunately, this looks like an intrusive patch that makes
calling clock_gettime not as straight as before (in terms of
code readability).

Feel free to not merge this then. I could totally understand it.

Re: [sleepy.penguin] [PATCH] Add support for Mac OS X

[Eric Wong]

Lin Jen-Shin <godfat@godfat.org> wrote:
> Since Mac OS X does not support clock_gettime, we need to wrap
> around clock_gettime to make kqueue work on Mac OS X.
> 
> epoll.c does not need this wrapper as it's only on Linux,
> but I guess it's fine to use the wrapper as well. Maybe
> someday *BSD could also support epoll in someway?

I can accept something along these lines if it only uses
generic/portable calls instead of system-specific code/ifdefs.

This way it can be easier to test/review and reusable to other platforms
which may not have proper clock_gettime

> @@ -0,0 +1,17 @@
> +#ifdef __MACH__

Use have_func() checks instead.

Actually, clogger already has a fallback for clock_gettime which works
for all platforms (but is less "correct")

	git clone git://bogomips.org/clogger.git

You can probably just copy code from that.

I can't accept using interfaces which only work on rare/non-Free systems.

> +void clock_gettime_mach(struct timespec *now){
> +	clock_serv_t cclock;
> +	mach_timespec_t mts;
> +	host_get_clock_service(mach_host_self(), CALENDAR_CLOCK, &cclock);
> +	clock_get_time(cclock, &mts);
> +	mach_port_deallocate(mach_task_self(), cclock);
> +	now->tv_sec = mts.tv_sec;
> +	now->tv_nsec = mts.tv_nsec;

clogger just uses gettimeofday(), but that's usually good enough.

Also, Ruby trunk recently got Process.clock_gettime; so someday we can
just fall back to rb_funcall.

Anyways thanks for bringing this to my attention.  If you want, please
send me a patch based on what's in clogger (and that fallback in clogger
is probably buggy, too, so an extra set of eyes would be appreciated :)

Re: [sleepy.penguin] [PATCH] Add support for Mac OS X

[Lin Jen-Shin (godfat)]

On Mon, Aug 26, 2013 at 5:12 PM, Eric Wong <normalperson@yhbt.net> wrote:
> Lin Jen-Shin <godfat@godfat.org> wrote:
[...]
>> @@ -0,0 +1,17 @@
>> +#ifdef __MACH__
>
> Use have_func() checks instead.
>
> Actually, clogger already has a fallback for clock_gettime which works
> for all platforms (but is less "correct")
>
>         git clone git://bogomips.org/clogger.git
>
> You can probably just copy code from that.
>
> I can't accept using interfaces which only work on rare/non-Free systems.

I'll take a look later, thanks!

>> +void clock_gettime_mach(struct timespec *now){
>> +     clock_serv_t cclock;
>> +     mach_timespec_t mts;
>> +     host_get_clock_service(mach_host_self(), CALENDAR_CLOCK, &cclock);
>> +     clock_get_time(cclock, &mts);
>> +     mach_port_deallocate(mach_task_self(), cclock);
>> +     now->tv_sec = mts.tv_sec;
>> +     now->tv_nsec = mts.tv_nsec;
>
> clogger just uses gettimeofday(), but that's usually good enough.
>
> Also, Ruby trunk recently got Process.clock_gettime; so someday we can
> just fall back to rb_funcall.
>
> Anyways thanks for bringing this to my attention.  If you want, please
> send me a patch based on what's in clogger (and that fallback in clogger
> is probably buggy, too, so an extra set of eyes would be appreciated :)

Thanks for reviewing and suggesting, too. I'll work on the patch,
but only after the one for rainbows with eventmachine with threads :)
Maybe this weekend, or next week...