Hey,

This is the first time I send in patches to a project. In fact, it's the first time I send a bug report at all. Please don't bite me. ;) I can't find any way to make attachments for my patches, but I hope I can add them after I send this in. Oh, and my OS is GNU/Linux, not just Linux.

I found a missing feature in glib. The ability to offset the start time in GTimer. While working on that I found some other things that could be done better. Oh, and one bug too.

First, let's start with the bug. A simple bug fix. On Windows the "is not null"-check would never happen.

----
g_get_current_time.patch
* glib/gmain.c (g_get_current_time):
Put g_return_if_fail (result != NULL) outside preprocessor conditional.
----

Now, something that could be done better. You are currently using a real time clock for things where you should use a monotonic clock, if available. An example is GTimer. In fact, glib does not even provide an interface to a monotonic clock, but only a real time clock (g_get_current_time). Why is not a real time clock good for a timer? Because it will break the calculations of elapsed time if the real time clock is adjusted. A monotonic clock instead keeps track of time from an unspecified start point (but usually when the computer started) that cannot be adjusted. This patch adds two functions. g_get_monotonic_time and g_has_monotonic_clock, as well as a new struct GTimeSpec, which is like GTimeVal, except with a nsec (nanosecond) field instead of a usec (microsecond) one. Like GTimeVal, it has a Unix equivalent; timespec. We should make sure that the library uses the correct function as needed. The general rule is that if real time is not needed, monotonic should be used.

----
g_get_monotonic_time.patch
* glib/gtypes.h:
Added type GTimeSpec and struct _GTimeSpec.

* glib/gmain.c (g_get_current_time):
Modified documentation to point to correct use of g_get_monotonic_time().

* glib/gmain.c (g_get_monotonic_time):
* glib/gmain.h (g_get_monotonic_time):
New function.

* glib/gmain.c (g_has_monotonic_clock):
* glib/gmain.h (g_has_monotonic_clock):
New function.

* docs/reference/glib/tmpl/date.sgml:
Document GTimeSpec, g_get_monotonic_time() and g_has_monotonic_clock().
----

It is still missing autotools rules to make librt be compiled in one some systems (e.g. GNU/Linux). Also missing runtime check if MONOTONIC_CLOCK really is available (like how it is done in gthread/gthread-posix.c). I didn't want to make the check inside the function since it would have to be checked in every call. On the other hand, I didn't find any place to put the init code for a static variable (which could be set to either CLOCK_MONOTONIC or CLOCK_REALTIME) as done in gthread/gthread-posix.c. These things needs to be fixed before the patch is complete, and I need help with them.

Logically, with a new nanosecond resolution structure we also want to get real time with nanosecond resolution. This next patch adds g_get_current_time_ns() which does exactly that. It depends on the GTimeSpec structure in the previous patch (not included in this one too!).

----
* glib/gmain.c (g_get_current_time_ns):
* glib/gmain.h (g_get_current_time_ns):
New function.

* docs/reference/glib/tmpl/date.sgml:
Document g_get_current_time_ns().
----

The next patch makes GTimer use g_get_monotonic_time instead of g_thread_gettime. I'm not sure, but maybe we should also even store start and end as GTimeSpec? I think the whole question is whether it will break compatibility if we change the _GTimer structure. But storing start and end as GTimeSpec would mean less operations in total (because as it is now it converts it twice, once to gint64 every time we start and stop and then to gdouble at gtimer_elapsed()).

----
gtimer_monotonic.patch
* glib/gtimer.c (GETTIME): Uses g_get_monotonic_time() instead.
----

On another note. g_thread_gettime is a mess. It can't be decided if it returns a real time or a monotonic one. Most does real time, one (posix) does monotonic, if available. Windows 32 implementation (at least) is also duplicated (once in glib/gthread.c and another time in gthread/gthread-win32.c). Also, all implementations in the gthread folder never seems to be built, but only distributed. I really hope the function g_thread_gettime is there only for compatibility (luckily it's not in the documentation).

If we include g_get_current_time_ns we could make g_thread_gettime use that instead. g_get_current_time_ns already uses all the implementations of gthread_gettime (I think).

Because of the uncertainty in the g_thread_gettime function (f.e. whether it is monotonic or real time) we should check in other files that might use the function if they use it correctly. In fact, I think nothing should use this function at all (unless they need a real time clock with potential nanosecond precision, in which case they could use g_get_current_time_ns if we include it). gfilemonitor.c seems to use it, but I'm not sure if it's supposed to get a real time or a monotonic one. Depending on what, it should either call g_get_current_time[-ns] or g_get_monotonic_time instead. gtimer.c also uses it, but I have already fixed that in my previous patch.

Now the last patch. The one I actually did all this for.

----
gtimer_offset.patch
* glib/gtimer.c (g_timer_offset):
* glib/gtimer.h (g_timer_offset):
New function.

* docs/reference/glib/tmpl/date.sgml:
Document g_timer_offset().
----

It adds another function to GTimer that allows you to adjust the start and end time. This is useful if one wants to reset the timer and offset with a calculated delay. That way the delays (which could be several tens of milliseconds in heavy event streams in GTK) won't stack. I noticed this problem when implementing an animation redraw in the idle event (timeout was useles anyway). The frames per second would drop when I for example moved and resized the window a lot. If I made the fps limit less strict it would still drop, however it could drop to something higher than it did with a more strict fps limit. E.g. during a lot of events: 60 FPS limit -> ~40 FPS, 240 FPS limit -> ~160 FPS. When I implemented my own timer with a way to offset the delay it kept the FPS limit at all times. This patch basically makes that possible for GTimer too.


One last thing. I think the part (from date.sgml)

"GLib doesn't contain any time-manipulation functions; however, there
is a #GTime typedef and a #GTimeVal struct which represents a more 
precise time (with microseconds). You can request the current time as 
a #GTimeVal with g_get_current_time()."

is a bit outdated. GTime is deprecated (misspelt "deprected" in the docs), and
we probably need to mention GTimeSpec and g_get_current_time_ms as well (or
instead), if we add them in. But maybe we could even just remove the whole
paragraph?

Thank you for your time. And great software!

/Patrik