[PATCH] D130516: [Support] compression classes

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

Thanks for experimenting the refactoring. My gut feeling is that

- for inheritance `llvm/lib/Support/Compression.cpp` introduces quite a bit of complexity.
- BestSpeedCompression/DefaultCompression/BestSizeCompression may be kinda weird. Not all algorithms may need all of three.
- this new interface does not make parallel compression/decompression easier.

I mentioned this on https://groups.google.com/g/generic-abi/c/satyPkuMisk/m/xRqMj8M3AwAJ

> I know the paradox of choice:) Certainly that we should not add a plethora of bzip2, xz, lzo, brotli, etc to generic-abi. I don't think users are so fond of using a different format for a slight different read/write/compression ratio/memory usage need. They want to pick one format which performs well across a wide variety of workloads. (Adding a new format also introduces complexity on their build system side. They need to teach DWARF consumers they use. It's not a small undertaking.)

I think for a long time llvm/lib/Support/Compression.cpp will stay with just zlib and zstd. There is a possibility if a llvm-project use case needs an alternative (it needs very strong arguments not using zstd), it has the other approach that not using llvm/Support/Compression.h


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130516/new/

https://reviews.llvm.org/D130516