[Rpm-maint] [rpm-software-management/rpm] Support threading for zstd compression. (#1303)

[Igor Raits]

@ignatenkobrain commented on this pull request.

LGTM with small nitpicks

>  	    goto err;
 	}
+
+	if (threads > 0)

If I read https://facebook.github.io/zstd/zstd_manual.html correctly, even if you set it to `0`, it will use single thread which makes this if unneeded.

> @@ -1100,7 +1101,12 @@ static rpmzstd rpmzstdNew(int fdno, const char *fmode)
 	    flags &= ~O_ACCMODE;
 	    flags |= O_RDWR;
 	    continue;
-	    break;
+	case 'T':

I think it would be nice to support setting it to -1, same as for lzma:

```
                if (threads == -1)
                    threads = rpmExpandNumeric("%{getncpus}");
```

> +	    if (c >= (int)'0' && c <= (int)'9')
+		threads = strtol(s-1, (char **)&s, 10);

Just curious if it would make sense to change lzopen_internal() to follow the same logic? If so, could you push a separate commit for that?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1303#pullrequestreview-446799809
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.rpm.org/pipermail/rpm-maint/attachments/20200711/56147583/attachment.html>