Patch to_s timeout#631
Conversation
There was a problem hiding this comment.
Let's please use clock_gettime with CLOCK_MONOTONIC here.
There was a problem hiding this comment.
@vmg we can probably just #ifdef this but clock_gettime doesn't exist on OSX until Sierra.
For now we could either stick with gettimeofday on OSX or mach_absolute_time? It may not work exactly the same as clock_gettime, but I think it's monotonic?
There was a problem hiding this comment.
FWIW, any time functions in libgit2 use clock_gettime(CLOCK_MONOTONIC... when it's available and fall back to gettimeofday. We never use mach_absolute_time, so that might be a precedent for you.
There was a problem hiding this comment.
@ethomson hah I actually just noticed https://github.com/libgit2/libgit2/blob/20302aa43738a972e0bd2e2ee6ae479208427b31/src/util.h#L565-L581
|
Looks pretty good. Let's switch to a monotonic clock please. :) |
| return (double)time * scaling_factor / 1.0E9; | ||
| } | ||
|
|
||
| #elif defined(AMIGA) |
|
Looking good @mclark. As you can see, we can drop a lot of libgit2's original code because we support fewer platforms. That's good news! Please do that and ping me back so I can audit the remaining code against libgit2's original blame -- as it is an OSS project, we cannot magically relicense it from GPLv2 to MIT without asking the contributors. Unless the contributors are @carlosmn and @ethomson. In that case we can, because we pay them money to abuse their copyright. 👌 |
|
thanks @brianmario for catching that one! C is weird! 😛 @vmg All concerns save the legal ones have been addressed. I can rebase this to clean it up if you like before we proceed. |
4 participants
When calculating very large patches, there is the potential for the call to hang for a while without any feedback to the caller.
Potentially this could manifest itself in one of two places:
This patch addresses the latter and gives us a mechanism to bail early when the diff output is taking too long.
I'm not sure if there is a clear path to addressing a timeout in the former case, but I am open to suggestions and feedback on both ideas.
Thanks so much to @brianmario for 🍐 ing with me on this one!