feat: add concurrent working threads option to CLI args

[ttys3]

add concurrent working threads option to CLI args

[ttys3]

close due to ThreadPoolBuildError { kind: GlobalPoolAlreadyInitialized }

spawn_logger_thread() called and which uses rayon, the the global pool already initialized there

[ttys3]

maybe, could use https://docs.rs/rayon/latest/rayon/struct.ThreadPoolBuilder.html#method.build_scoped ?

[ttys3]

https://github.com/rayon-rs/rayon/blob/main/FAQ.md#how-many-threads-will-rayon-spawn

[marcospb19]

You could call parse_and_validate_args, build the threadpool, and then call spawn_logger_thread, as parse_and_validate_args doesn't require the logger thread to be alive yet.

[ttys3]

You could call parse_and_validate_args, build the threadpool, and then call spawn_logger_thread, as parse_and_validate_args doesn't require the logger thread to be alive yet.

yes, that's a workaround.

but the problem is, I did not expect the logger and the business code share the same global rayon threadpool.

because, when I set the threads (the one I want to add) to 1, what I mean it decompress the files one by one, to reduce disk io impact.

But since the logger may require more concurrent threads, I'm unsure if this will cause problems for it. Is it okay for us to do this?

[ttys3]

maybe we could use async? that way, the async logger wouldn't have to depend on rayon crate to achieve that.

[marcospb19]

But since the logger may require more concurrent threads, I'm unsure if this will cause problems for it. Is it okay for us to do this?

Oh, I see your concern, I overlooked the case where parallelism is set to 1.

The solution is then to use thread::spawn instead of rayon::spawn in the logger thread, so we limit rayon threads just to the decompression.

Setting up an async logger wouldn't help because we'd need at least one thread in the async runtime threadpool, and that would be equivalent (but more complicated) than just having a thread::spawn thread separated for it.

[ttys3]

Oops, seems that the UI tests failed

[marcospb19]

it changed what the --help looks like and we have that on UI tests, so you need to run cargo insta test and cargo insta review, or update those snapshots manually till they match.

I know this is kinda annoying, but makes life easier in the long term.

[ttys3]

UI tests fixed

[marcospb19]

Ty, let me know if you think the default shouldn't be all cores.

[ttys3]

Ty, let me know if you think the default shouldn't be all cores.

If the compression algorithm itself is fast enough (and can effectively utilize all cores), I believe we should have all CPU cores focus on processing a single compressed document instead of dividing the CPU cores to handle multiple documents. This way, a single document can be decompressed more quickly. Meanwhile, in this scenario, if we decompress all documents concurrently, the bottleneck will not be the CPU, but rather disk IO, and threads can easily enter the D-state （the state in top or htop, mostly iowait）.

If the compression algorithm cannot efficiently utilize the CPU (for example, when decompressing with 7z and rar, despite claiming to support multithreading, they cannot fully utilize the CPU, and overall decompression speed is very slow). Then decompressing documents one by one will be relatively slow.

Therefore, I feel that both approaches (processing one task at a time vs. concurrently processing multiple compressed documents) do not have absolute pros and cons; it depends on the situation.