GNOME Bugzilla – Bug 157254
slow switch to task view (calling gmtime too often)
Last modified: 2005-03-15 00:05:41 UTC
Converting a plan from a different tool to planner I discovered a performance problem. I don't know what triggered this but here is how I fixed it: When switching from gantt to task view planner calls mrp_time_to_tm() very often. mrp_time_to_tm() calls gmtime() which does around 15 system calls (read / etc/timezone, read the timezone definition file). I added functions to do the time_t -> struct tm conversion manually and got a huge performance boost. With this it's possible to switch to task view on a planner xml file with 4100 lines in 5 seconds (on a PIII-850). Without the change the switch to task view took forever. I changed mrp_time_from_tm() too btw. I took the functions for the conversion from ftp://ftp.dalsemi.com/pub/ timekeeping/DS1371/CFunction.cpp but you might want to look for a GPL version (eg. glibc).
Good news. Redoing the time conversion is something I've wanted to do quite some time, but haven't had the time. If you are willing to help, that would be greatly appreciated! Right now, we even have code that tries to work around the fact that there are timezones, since we are treating all times as localtime. Getting this mess fixed would be very nice.
I forgot to say: I'm really looking into using some other time representation than a time_t, since it's only good up to 2038. Ideally we should use something that keeps the "struct tm" style info in the same representation so we don't need to convert back and forth constantly (GDate might work, not sure). So, it's probably not a good idea to put too much work into the current time_t mess. If we can find a simple implementation of gmtime and the mktime that are faster and does what we need, that could still be useful. We could also look into getting the views call mrp_time_* less too.
I agree that time_t is not the perfect time representation as too much conversion is needed. At least planner already has its own type mrptime. But there are some places outside of mrp_time.c where this isn't used "opaque" but as integer (time=0, comparison with -1). So it isn't just using "typedef struct tm mrptime", some work is needed. C++ would help but I like that planner is written in C ;-) I'm already spending too much time on project planning (working on planner belongs to this) instead of working on the planned projects so don't expect a patch from me, sorry.
How about ... http://cr.yp.to/libtai.html The library licensing should be compatible with the GNOME stuff.
It looks interesting, but also totally unmaintained and a bit weird in that it stores leap seconds on disk in a file that needs updating since it only covers up to 1998 or so. We also basically need more than the API provides, we need functions to get the weekday, add a number of hours, days, months, etc. We might have to implement somthing like that ourselves...
Created attachment 37719 [details] [review] Debugging output
Comment on attachment 37719 [details] [review] Debugging output That patch was meant for another bug.
I looked into this some more. It's definitly doable by just changing our time presentation to typedef struct { GDate date; gint hour; gint min; gint sec; } MrpTime; I've started doing this, and implementing an API that operates on this type. I've also started changing the code to use the new type instead of the old. This is going to be A LOT of work though so it will take quite some time and probably introduce a lot of bugs. I will probably commit this to a separate branch in CVS and keep working on it slowly there.
Committed.