Re: [PATCH] pack-objects: Add runtime detection of online CPU's

From: Brandon Casey <casey@nrlssc.navy.mil>
Date: Feb 12, 2008, 15:44 UTC
Message-ID: <47B1BEC6.6080906@nrlssc.navy.mil>
In-Reply-To: <47B156CD.1010209@op5.se>

Andreas Ericsson wrote:

Show 10 quoted lines

> @@ -1861,7 +1858,7 @@ static int git_pack_config(const char *k, const
> char *v)
>     }
>     if (!strcmp(k, "pack.threads")) {
>         delta_search_threads = git_config_int(k, v);
> -        if (delta_search_threads < 1)
> +        if (delta_search_threads < 0)
>             die("invalid number of threads specified (%d)",
>                 delta_search_threads);
> #ifndef THREADED_DELTA_SEARCH

	if (delta_search_threads != 1)
		warning("no threads support, ignoring %s", k);

I changed this to '!= 1' since that is the only time the user gets what they asked for when THREADED_DELTA_SEARCH is not enabled. If the user requested nthreads == ncpus by setting delta_search_threads = 0, I think we should let the user know that thread support is not enabled, and we are ignoring their request.

Show 7 quoted lines

> @@ -2076,6 +2073,9 @@ int cmd_pack_objects(int argc, const char **argv,
> const char *prefix)
>     if (!pack_compression_seen && core_compression_seen)
>         pack_compression_level = core_compression_level;
> 
> +    if (!delta_search_threads)    /* --threads=0 means autodetect */
> +        delta_search_threads = online_cpus();

This is in the wrong place. It should be _after_ command line arguments are processed to handle --threads=0

Show 15 quoted lines

> +
>     progress = isatty(2);
>     for (i = 1; i < argc; i++) {
>         const char *arg = argv[i];
> @@ -2130,7 +2130,8 @@ int cmd_pack_objects(int argc, const char **argv,
> const char *prefix)
>         if (!prefixcmp(arg, "--threads=")) {
>             char *end;
>             delta_search_threads = strtoul(arg+10, &end, 0);
> -            if (!arg[10] || *end || delta_search_threads < 1)
> +
> +            if (!arg[10] || *end || delta_search_threads < 0)
>                 usage(pack_usage);
> #ifndef THREADED_DELTA_SEARCH
>             if (delta_search_threads > 1)

Same comment as above about warning when delta_search_threads != 1.

-brandon