{"thread":{"id":"54709","subject":"[PATCH] move sleep_millisec to git-compat-util.h","startedAt":"2020-11-24T19:10:17Z","lastAt":"2020-11-25T23:12:19Z","messageCount":6,"participants":["Han-Wen Nienhuys via GitGitGadget","Junio C Hamano","Randall S. Becker"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"410692","messageId":"pull.922.git.git.1606245012068.gitgitgadget@gmail.com","threadId":"54709","inReplyTo":null,"subject":"[PATCH] move sleep_millisec to git-compat-util.h","fromName":"Han-Wen Nienhuys via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-11-24T19:10:11Z","receivedAt":"2020-11-24T19:10:17Z","isPatch":true,"sender":{"key":"hanwen@google.com","avatar":"https://avatars.githubusercontent.com/u/31547?v=4"},"body":"From: Han-Wen Nienhuys \n\nThe sleep function is defined in wrapper.c, so it makes more sense to be a in\nsystem compatibility header.\n\nSigned-off-by: Han-Wen Nienhuys \n---\n move sleep_millisec to git-compat-util.h\n \n The sleep function is defined in wrapper.c, so it makes more sense to be\n a in system compatibility header.\n \n Signed-off-by: Han-Wen Nienhuys hanwen@google.com [hanwen@google.com]\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-922%2Fhanwen%2Fsleep-header-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-922/hanwen/sleep-header-v1\nPull-Request: https://github.com/git/git/pull/922\n\n cache.h | 1 -\n git-compat-util.h | 2 ++\n 2 files changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/cache.h b/cache.h\nindex c0072d43b1..e986cf4ea9 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1960,7 +1960,6 @@ int stat_validity_check(struct stat_validity *sv, const char *path);\n void stat_validity_update(struct stat_validity *sv, int fd);\n \n int versioncmp(const char *s1, const char *s2);\n-void sleep_millisec(int millisec);\n \n /*\n * Create a directory and (if share is nonzero) adjust its permissions\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex adfea06897..7d509c5022 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -1354,4 +1354,6 @@ static inline void *container_of_or_null_offset(void *ptr, size_t offset)\n \t((uintptr_t)&(ptr)->member - (uintptr_t)(ptr))\n #endif /* !__GNUC__ */\n \n+void sleep_millisec(int millisec);\n+\n #endif\n\nbase-commit: faefdd61ec7c7f6f3c8c9907891465ac9a2a1475\n-- \ngitgitgadget\n"},{"id":"410718","messageId":"xmqq8saqifu3.fsf@gitster.c.googlers.com","threadId":"54709","inReplyTo":"pull.922.git.git.1606245012068.gitgitgadget@gmail.com","subject":"Re: [PATCH] move sleep_millisec to git-compat-util.h","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-11-24T21:51:16Z","receivedAt":"2020-11-24T21:51:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Han-Wen Nienhuys via GitGitGadget\" writes:\n\n> From: Han-Wen Nienhuys \n>\n> The sleep function is defined in wrapper.c, so it makes more sense to be a in\n> system compatibility header.\n>\n> Signed-off-by: Han-Wen Nienhuys \n> ---\n> move sleep_millisec to git-compat-util.h\n\nMakes sense.\n"},{"id":"410724","messageId":"011801d6c2ae$dcdbaad0$96930070$@nexbridge.com","threadId":"54709","inReplyTo":"xmqq8saqifu3.fsf@gitster.c.googlers.com","subject":"RE: [PATCH] move sleep_millisec to git-compat-util.h","fromName":"Randall S. Becker","fromEmail":"rsbecker@nexbridge.com","sentAt":"2020-11-24T22:12:08Z","receivedAt":"2020-11-24T22:12:44Z","isPatch":true,"sender":{"key":"randall.becker@nexbridge.ca","avatar":"https://avatars.githubusercontent.com/u/28956764?v=4"},"body":"On November 24, 2020 4:51 PM, Junio C Hamano wrote:\n> To: Han-Wen Nienhuys via GitGitGadget \n> Cc: git@vger.kernel.org; Han-Wen Nienhuys ; Han-\n> Wen Nienhuys \n> Subject: Re: [PATCH] move sleep_millisec to git-compat-util.h\n> \n> \"Han-Wen Nienhuys via GitGitGadget\" writes:\n> \n> > From: Han-Wen Nienhuys \n> >\n> > The sleep function is defined in wrapper.c, so it makes more sense to\n> > be a in system compatibility header.\n> >\n> > Signed-off-by: Han-Wen Nienhuys \n> > ---\n> > move sleep_millisec to git-compat-util.h\n> \n> Makes sense.\n\nI have a platform fix that I'd like to apply once this makes it into the\nmain code. The sleep_millisec uses poll(), which is rather heavy-weight on\nthe NonStop platform. We have a much more efficient sleep function available\n(with microsecond resolution), which would be more useful unless there is a\npoll side-effect on which git depends. Would this be acceptable? I could\npush this at any time really.\n\nindex bcda41e374..972ecd67bf 100644\n--- a/wrapper.c\n+++ b/wrapper.c\n@@ -4,6 +4,10 @@\n #include \"cache.h\"\n #include \"config.h\"\n\n+#ifdef __TANDEM\n+#include /* for PROCESS_DELAY_ */\n+#endif\n+\n static int memory_limit_check(size_t size, int gentle)\n {\n static size_t limit = 0;\n@@ -650,7 +654,11 @@ void write_file(const char *path, const char *fmt, ...)\n\n void sleep_millisec(int millisec)\n {\n+#ifdef __TANDEM\n+ PROCESS_DELAY_(millisec * 1000LL);\n+#else\n poll(NULL, 0, millisec);\n+#endif\n }\n\n int xgethostname(char *buf, size_t len)\n\nRegards,\nRandall\n\n"},{"id":"410749","messageId":"xmqqy2iqf7uf.fsf@gitster.c.googlers.com","threadId":"54709","inReplyTo":"011801d6c2ae$dcdbaad0$96930070$@nexbridge.com","subject":"Re: [PATCH] move sleep_millisec to git-compat-util.h","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-11-25T03:12:08Z","receivedAt":"2020-11-25T03:12:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Randall S. Becker\" writes:\n\n> I have a platform fix that I'd like to apply once this makes it into the\n> main code. The sleep_millisec uses poll(), which is rather heavy-weight on\n> the NonStop platform. We have a much more efficient sleep function available\n> (with microsecond resolution), which would be more useful unless there is a\n> poll side-effect on which git depends. Would this be acceptable? I could\n> push this at any time really.\n\nWe strongly prefer not to contaminate generic part of source with\nplatform specific conditional compilation.\n\nIf this were a much larger helper function, it might make sense to\nperform the compat/*.c dance, but in this case:\n\n * [PATCH 1/2] that implements sleep_millisec() in git-compat-util.h\n as a static inline function; and then\n\n * [PATCH 2/2] that does the equivalent of your patch below, but in\n git-compat-util.h\n\nmight be the cleanest.\n\n> index bcda41e374..972ecd67bf 100644\n> --- a/wrapper.c\n> +++ b/wrapper.c\n> @@ -4,6 +4,10 @@\n> #include \"cache.h\"\n> #include \"config.h\"\n>\n> +#ifdef __TANDEM\n> +#include /* for PROCESS_DELAY_ */\n> +#endif\n> +\n> static int memory_limit_check(size_t size, int gentle)\n> {\n> static size_t limit = 0;\n> @@ -650,7 +654,11 @@ void write_file(const char *path, const char *fmt, ...)\n>\n> void sleep_millisec(int millisec)\n> {\n> +#ifdef __TANDEM\n> + PROCESS_DELAY_(millisec * 1000LL);\n> +#else\n> poll(NULL, 0, millisec);\n> +#endif\n> }\n>\n> int xgethostname(char *buf, size_t len)\n>\n> Regards,\n> Randall\n"},{"id":"410805","messageId":"018301d6c371$37bf6820$a73e3860$@nexbridge.com","threadId":"54709","inReplyTo":"xmqqy2iqf7uf.fsf@gitster.c.googlers.com","subject":"RE: [PATCH] move sleep_millisec to git-compat-util.h","fromName":"Randall S. Becker","fromEmail":"rsbecker@nexbridge.com","sentAt":"2020-11-25T21:23:22Z","receivedAt":"2020-11-25T21:23:47Z","isPatch":true,"sender":{"key":"randall.becker@nexbridge.ca","avatar":"https://avatars.githubusercontent.com/u/28956764?v=4"},"body":"On November 24, 2020 10:12 PM, Junio C Hamano wrote:\n> \"Randall S. Becker\" writes:\n> > I have a platform fix that I'd like to apply once this makes it into\n> > the main code. The sleep_millisec uses poll(), which is rather\n> > heavy-weight on the NonStop platform. We have a much more efficient\n> > sleep function available (with microsecond resolution), which would be\n> > more useful unless there is a poll side-effect on which git depends.\n> > Would this be acceptable? I could push this at any time really.\n> \n> We strongly prefer not to contaminate generic part of source with platform\n> specific conditional compilation.\n> \n> If this were a much larger helper function, it might make sense to perform\n> the compat/*.c dance, but in this case:\n> \n> * [PATCH 1/2] that implements sleep_millisec() in git-compat-util.h\n> as a static inline function; and then\n> \n> * [PATCH 2/2] that does the equivalent of your patch below, but in\n> git-compat-util.h\n> \n> might be the cleanest.\n> \n> > index bcda41e374..972ecd67bf 100644\n> > --- a/wrapper.c\n> > +++ b/wrapper.c\n> > @@ -4,6 +4,10 @@\n> > #include \"cache.h\"\n> > #include \"config.h\"\n> >\n> > +#ifdef __TANDEM\n> > +#include /* for PROCESS_DELAY_ */ #endif\n> > +\n> > static int memory_limit_check(size_t size, int gentle) {\n> > static size_t limit = 0;\n> > @@ -650,7 +654,11 @@ void write_file(const char *path, const char\n> > *fmt, ...)\n> >\n> > void sleep_millisec(int millisec)\n> > {\n> > +#ifdef __TANDEM\n> > + PROCESS_DELAY_(millisec * 1000LL); #else\n> > poll(NULL, 0, millisec);\n> > +#endif\n> > }\n> >\n> > int xgethostname(char *buf, size_t len)\n\nI just chatted with my team and we think that sticking with the existing\npoll call is probably better in the long term. We are planning to try to get\ngit to work using PUT threads - but that is a longer project for a whole\nslew of reasons. It involves pulling all or most of the FLOSS stuff out, or\nmaking that configurable to use FLOSS when not using threads and not using\nFLOSS when using threads. It might be useful, however, to use usleep()\ninstead of poll(NULL) when threading is used on most platforms as it is a\nmore effective way of context switching between threads than select().\n\nRegards,\nRandall\n\n"},{"id":"410828","messageId":"xmqqy2ipav5k.fsf@gitster.c.googlers.com","threadId":"54709","inReplyTo":"018301d6c371$37bf6820$a73e3860$@nexbridge.com","subject":"Re: [PATCH] move sleep_millisec to git-compat-util.h","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-11-25T23:12:07Z","receivedAt":"2020-11-25T23:12:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Randall S. Becker\" writes:\n\n> I just chatted with my team and we think that sticking with the existing\n> poll call is probably better in the long term. We are planning to try to get\n> git to work using PUT threads - but that is a longer project for a whole\n> slew of reasons. It involves pulling all or most of the FLOSS stuff out, or\n> making that configurable to use FLOSS when not using threads and not using\n> FLOSS when using threads. It might be useful, however, to use usleep()\n> instead of poll(NULL) when threading is used on most platforms as it is a\n> more effective way of context switching between threads than select().\n\nYeah, if usleep() is portable enough, that would probably be the\nideal direction to go.\n\n"}]}