Attention is currently required from: Arthur Heymans, Julius Werner, Martin L Roth, Shelley Chen.
Wonkyu Kim has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/69753?usp=email )
Change subject: util/cbfstool: Add zstd support ......................................................................
Patch Set 3:
(1 comment)
File src/commonlib/zstd-1.5.2/lib/common/mem.h:
https://review.coreboot.org/c/coreboot/+/69753/comment/9390c124_cf9cc7f8 : PS1, Line 174: return one.c[0];
Hi, […]
|That's not really a fair comparison (because if we found that ZSTD is a lot better |than LZMA we could drop LZMA instead) Agreed. As there is no addtional code change, LZMA code is added by default and it's because we still have file which use LZMA. I think ZSTD can be replaced for LZMA with some more optimization for not for LZ4 due to compression speed.
|Also, can you clarify which compression level you used in ZSTD? (Did you play with |compression levels at all? Does it affect decompression speed?)
As I don't change code, it's default compression level in the patch.
|(BTW I don't really understand your FSP results. Are you saying that ZSTD |compression ratio is lower than LZ4 for that file? Are you sure that's not a |measurement error? The compression ratios are also very low compared to the |ramstage... I wonder if the reason for that—if those numbers are accurate—is that |a large part of the FSP is either already compressed or encrypted, and LZ4 is just |able to skip incompressible data with less overhead.)
For FSPs result show lower rate than ramstage but not lower than LZ4.
FSPs LZ4 ZSTD Raw size 204800 208896 compresed size 194449 193414 Compression ratio 5.05% 7.41%
-
Wonkyu Kim (Code Review)