I tested the patch and it works well on my machine. I focused my review on src/xz/coder.c. and src/liblzma/api/lzma/container.h since you have already had some discussion on src/liblzma/common/stream_decoder_mt.c and it seemed similar in style to src/liblzma/common/stream_encoder_mt.c. Here are my thoughts from my review:
In src/xz/coder.c 1. The get_free_mem function could be renamed into something more clear. Just by reading the name, I can't tell if it returns bytes, kb, mb, etc. 2. get_free_mem returns a int32_t, but treats its returns as boolean (-1 on failure, 0 on success). I would suggest returning either bool, lzma_ret, or just int. When I see int32_t as a developer, I assume there is a reason it needs to be 32 bits. Since all relevant error codes will be much less than 32 bits, I don't see a reason to specify this as a 32 bit return. 3. get_free_mem uses a lot of magic numbers. At the top of coder.c I would define: #define ARCH_32BIT_KB_MAX 2621440 #define KB_TO_B(bytes) 1024 * bytes #define MEMINFO_BUF_SIZE 4096 #define BASE_DECIMAL 10 And use these macros throughout the get_free_mem function 4. It may be a good idea to add a TODO to include implementations of get_free_mem for other platforms like Windows and Mac 5. get_free_mem is only checked once before the decompression starts. If the system is low on memory right when it starts, it will limit the memory for the entire decompression even if memory frees up later. I am not sure what the best solution is, but periodically checking the available free memory and adjusting the threads could improve large decompression tasks performance. In src/liblzma/api/lzma/container.h 1. In the lzma_memlimit_opt, the comment has a typo "Continue with without a thread" in the brief section One question I have is whether the get_free_mem memlimit should be included in the multithreaded compression logic. It's out of the scope for this patch, but it is something worth considering. Just trying to do my part as a helper elf! Jia Tan