Brandon Casey wrote:
Allow pack.threads config option and --threads command line option to
accept '0' as an argument and set the number of created threads equal
to the number of online processors in this case.
Signed-off-by: Brandon Casey <casey@xxxxxxxxxxxxxxx>
---
I was preparing this patch when I saw your email. I looked up your
the old email you were talking about. Your function is better since
it is cross platform.
When you redo your patch, you may want to adopt one aspect of this
one. I used a setting of zero to imply "set number of threads to
number of cpus". This allows the user to specifically set pack.threads
in the config file to zero with the above mentioned meaning, or to
override a setting in the config file from the command line with
--threads=0. This is rather than having to delete the option from the
config file.
That make sense. Perhaps even go so far as to allow 'auto' as a
keyword would be nifty.
+#ifdef THREADED_DELTA_SEARCH
+ if (!delta_search_threads) {
+#if defined _SC_NPROCESSORS_ONLN
+ delta_search_threads = sysconf(_SC_NPROCESSORS_ONLN);
+#elif defined _SC_NPROC_ONLN
+ delta_search_threads = sysconf(_SC_NPROC_ONLN);
+#endif
+ if (delta_search_threads == -1)
+ perror("Could not detect number of processors");
+ if (delta_search_threads <= 0)
+ delta_search_threads = 1;
+ }
+#endif
+
But this is not so good. For one thing you've dropped windows support
entirely. The last comment on my own patch was that get_num_active_cpus()
should live in a file of its own. You've taken one step back from that
and not even kept it in its own function.
I think perhaps it's time to introduce thread-compat.[ch] to deal with
thread-related cross-platform things like this.
I'll recook my patch and send it in a few minutes, using your suggestions
and Nicolas combined.
--
Andreas Ericsson andreas.ericsson@xxxxxx
OP5 AB www.op5.se
Tel: +46 8-230225 Fax: +46 8-230231
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
