Skip to content

[ELF] Adjust --compress-sections to support compression level #90567

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
May 1, 2024

Conversation

MaskRay
Copy link
Member

@MaskRay MaskRay commented Apr 30, 2024

zstd excels at scaling from low-ratio-very-fast to
high-ratio-pretty-slow. Some users prioritize speed and prefer disk read
speed, while others focus on achieving the highest compression ratio
possible, similar to traditional high-ratio codecs like LZMA.

Add an optional level to --compress-sections (#84855) to cater to
these diverse needs. While we initially aimed for a one-size-fits-all
approach, this no longer seems to work.
(https://richg42.blogspot.com/2015/11/the-lossless-decompression-pareto.html)

When --compress-debug-sections is used together, make
--compress-sections take precedence since --compress-sections is usually
more specific.

Remove the level distinction between -O/-O1 and -O2 for
--compress-debug-sections=zlib for a more consistent user experience.

Created using spr 1.3.5-bogner
@llvmbot
Copy link
Member

llvmbot commented Apr 30, 2024

@llvm/pr-subscribers-lld-elf

@llvm/pr-subscribers-lld

Author: Fangrui Song (MaskRay)

Changes

zstd excels at scaling from low-ratio-very-fast to
high-ratio-pretty-slow. Some users prioritize speed and prefer disk read
speed, while others focus on achieving the highest compression ratio
possible, similar to traditional high-ratio codecs like LZMA.

Add --compression-level to cater to these diverse needs. While we
initially aimed for a one-size-fits-all approach, this no longer seems
to work.
(https://richg42.blogspot.com/2015/11/the-lossless-decompression-pareto.html)

While here, remove the level distinction between -O/-O1 and -O2 for
--compress-debug-sections=zlib for a more consistent user experience.


Full diff: https://github.com/llvm/llvm-project/pull/90567.diff

6 Files Affected:

  • (modified) lld/ELF/Config.h (+1)
  • (modified) lld/ELF/Driver.cpp (+1)
  • (modified) lld/ELF/Options.td (+3)
  • (modified) lld/ELF/OutputSections.cpp (+8-8)
  • (modified) lld/docs/ld.lld.1 (+5)
  • (modified) lld/test/ELF/compressed-debug-level.test (+10-9)
diff --git a/lld/ELF/Config.h b/lld/ELF/Config.h
index 33bfa42b0fcbf0..d619fe2bd5bf48 100644
--- a/lld/ELF/Config.h
+++ b/lld/ELF/Config.h
@@ -226,6 +226,7 @@ struct Config {
   std::optional<llvm::DebugCompressionType> compressDebugSections;
   llvm::SmallVector<std::pair<llvm::GlobPattern, llvm::DebugCompressionType>, 0>
       compressSections;
+  int compressionLevel = 0;
   bool cref;
   llvm::SmallVector<std::pair<llvm::GlobPattern, uint64_t>, 0>
       deadRelocInNonAlloc;
diff --git a/lld/ELF/Driver.cpp b/lld/ELF/Driver.cpp
index a5b47f020f8726..3ef4a7a8bbfcad 100644
--- a/lld/ELF/Driver.cpp
+++ b/lld/ELF/Driver.cpp
@@ -1233,6 +1233,7 @@ static void readConfigs(opt::InputArgList &args) {
     config->compressDebugSections =
         getCompressionType(arg->getValue(), "--compress-debug-sections");
   }
+  config->compressionLevel = args::getInteger(args, OPT_compression_level, 0);
   config->cref = args.hasArg(OPT_cref);
   config->optimizeBBJumps =
       args.hasFlag(OPT_optimize_bb_jumps, OPT_no_optimize_bb_jumps, false);
diff --git a/lld/ELF/Options.td b/lld/ELF/Options.td
index 72eaf157a181cf..dba2e733af5d45 100644
--- a/lld/ELF/Options.td
+++ b/lld/ELF/Options.td
@@ -71,6 +71,9 @@ defm compress_sections: EEq<"compress-sections",
   "Compress non-SHF_ALLOC output sections matching <section-glob>">,
   MetaVarName<"<section-glob>=[none|zlib|zstd]">;
 
+defm compression_level: EEq<"compression-level", "Set the compression level">,
+  MetaVarName<"level">;
+
 defm defsym: Eq<"defsym", "Define a symbol alias">, MetaVarName<"<symbol>=<value>">;
 
 defm optimize_bb_jumps: BB<"optimize-bb-jumps",
diff --git a/lld/ELF/OutputSections.cpp b/lld/ELF/OutputSections.cpp
index 3e58ed4bda2d3c..51bd60d45d800f 100644
--- a/lld/ELF/OutputSections.cpp
+++ b/lld/ELF/OutputSections.cpp
@@ -372,13 +372,15 @@ template <class ELFT> void OutputSection::maybeCompress() {
   auto shardsOut = std::make_unique<SmallVector<uint8_t, 0>[]>(numShards);
 
 #if LLVM_ENABLE_ZSTD
-  // Use ZSTD's streaming compression API which permits parallel workers working
-  // on the stream. See http://facebook.github.io/zstd/zstd_manual.html
-  // "Streaming compression - HowTo".
+  // Use ZSTD's streaming compression API. See
+  // http://facebook.github.io/zstd/zstd_manual.html "Streaming compression -
+  // HowTo".
   if (ctype == DebugCompressionType::Zstd) {
     parallelFor(0, numShards, [&](size_t i) {
       SmallVector<uint8_t, 0> out;
       ZSTD_CCtx *cctx = ZSTD_createCCtx();
+      ZSTD_CCtx_setParameter(cctx, ZSTD_c_compressionLevel,
+                             config->compressionLevel);
       ZSTD_inBuffer zib = {shardsIn[i].data(), shardsIn[i].size(), 0};
       ZSTD_outBuffer zob = {nullptr, 0, 0};
       size_t size;
@@ -406,12 +408,10 @@ template <class ELFT> void OutputSection::maybeCompress() {
 
 #if LLVM_ENABLE_ZLIB
   // We chose 1 (Z_BEST_SPEED) as the default compression level because it is
-  // the fastest. If -O2 is given, we use level 6 to compress debug info more by
-  // ~15%. We found that level 7 to 9 doesn't make much difference (~1% more
-  // compression) while they take significant amount of time (~2x), so level 6
-  // seems enough.
+  // the fastest.
   if (ctype == DebugCompressionType::Zlib) {
-    const int level = config->optimize >= 2 ? 6 : Z_BEST_SPEED;
+    const int level =
+        config->compressionLevel ? config->compressionLevel : Z_BEST_SPEED;
 
     // Compress shards and compute Alder-32 checksums. Use Z_SYNC_FLUSH for all
     // shards but the last to flush the output to a byte boundary to be
diff --git a/lld/docs/ld.lld.1 b/lld/docs/ld.lld.1
index 3861120915e8bc..90bc80329f06bd 100644
--- a/lld/docs/ld.lld.1
+++ b/lld/docs/ld.lld.1
@@ -172,6 +172,11 @@ This is like a generalized
 Output cross reference table. If
 .Fl Map
 is specified, print to the map file.
+.It Fl -compression-level Ns = Ns level
+Set the compression level for
+.Fl -compress-debug-sections
+and
+.Fl -compress-sections.
 .It Fl -debug-names
 Generate a merged
 .Li .debug_names
diff --git a/lld/test/ELF/compressed-debug-level.test b/lld/test/ELF/compressed-debug-level.test
index ee95f126799722..e911bf5ebcb030 100644
--- a/lld/test/ELF/compressed-debug-level.test
+++ b/lld/test/ELF/compressed-debug-level.test
@@ -2,22 +2,23 @@
 
 # RUN: yaml2obj %s -o %t.o
 
+## LLD uses zlib compression of level 1 by default. Unlike previous versions,
+## -O does not change the level.
 # RUN: ld.lld %t.o -o %t.default --compress-debug-sections=zlib
 # RUN: llvm-readelf --sections %t.default | FileCheck -check-prefixes=HEADER,LEVEL1 %s
 
 # RUN: ld.lld -O0 %t.o -o %t.O0 --compress-debug-sections=zlib
-# RUN: llvm-readelf --sections %t.O0 | FileCheck -check-prefixes=HEADER,LEVEL1 %s
 # RUN: cmp %t.default %t.O0
 
-# RUN: ld.lld -O1 %t.o -o %t.O1 --compress-debug-sections=zlib
-# RUN: llvm-readelf --sections %t.O1 | FileCheck -check-prefixes=HEADER,LEVEL1 %s
-# RUN: cmp %t.default %t.O1
-
 # RUN: ld.lld -O2 %t.o -o %t.O2 --compress-debug-sections=zlib
-# RUN: llvm-readelf --sections %t.O2 | FileCheck -check-prefixes=HEADER,LEVEL6 %s
-
-## LLD uses zlib compression of level 1 when -O0, -O1 and level 6 when -O2.
-## Here we check how -O flag affects the size of compressed sections produced.
+# RUN: cmp %t.default %t.O2
+
+## --compression-level specifies the level.
+# RUN: ld.lld --compression-level=6 %t.o -o %t.6 --compress-debug-sections=zlib
+# RUN: llvm-readelf --sections %t.6 | FileCheck -check-prefixes=HEADER,LEVEL6 %s
+## zlib uses the default (6) when an invalid level is specified.
+# RUN: ld.lld --compression-level=-1 %t.o -o %t.bad --compress-debug-sections=zlib
+# RUN: cmp %t.6 %t.bad
 
 # HEADER: [Nr] Name        Type     Address  Off    Size
 # LEVEL1: [ 1] .debug_info PROGBITS 00000000 000094 00001{{[bc]}}

.
Created using spr 1.3.5-bogner
Copy link
Collaborator

@smithp35 smithp35 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No objections from me. I agree that keeping the default independent of optimisation level is the right thing to do.

One minor suggestion for the release notes.

@@ -29,6 +29,9 @@ ELF Improvements
* ``--compress-sections <section-glib>=[none|zlib|zstd]`` is added to compress
matched output sections without the ``SHF_ALLOC`` flag.
(`#84855 <https://github.com/llvm/llvm-project/pull/84855>`_)
* ``--compression-level`` is added to set the compression level
for ``--compress-debug-sections=[zlib|zstd]`` and ``--compress-sections``.
(`#90567 <https://github.com/llvm/llvm-project/pull/90567>`_)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Worth mentioning that default compression level is now independent of linker optimization level (Z_BEST_SPEED)?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thx for the suggestion. adopting

@MaskRay MaskRay changed the title [ELF] Add --compression-level [ELF] Adjust --compress-sections to support compression level Apr 30, 2024
@MaskRay
Copy link
Member Author

MaskRay commented Apr 30, 2024

No objections from me. I agree that keeping the default independent of optimisation level is the right thing to do.

One minor suggestion for the release notes.

Thanks. ISTM adjusting --compress-sections seems better (and matches zlib:1-style strings I found elsewhere).

It might be useful for --compress-debug-sections to get the optional level as well but its use case seems obsoleted by --compress-sections...

MaskRay added 2 commits April 30, 2024 12:53
Created using spr 1.3.5-bogner
@MaskRay MaskRay merged commit 6d44a1e into main May 1, 2024
@MaskRay MaskRay deleted the users/MaskRay/spr/elf-add-compression-level branch May 1, 2024 18:40
@Colibrow
Copy link
Contributor

@MaskRay Hi, I'm wondering if there need to add option for lzma which aims for the best size.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants