From c5c130e675624eef03f5bcaf848810659e205ed2 Mon Sep 17 00:00:00 2001 From: Nils Gladitz Date: Mon, 19 Apr 2021 19:37:57 +0200 Subject: cmArchiveWrite: Consolidate multiple ways to set thread count Merge use of SetFilterOption() into more abstract thread count in cmArchiveWrite constructor. libarchive defaulting of threads for threads == 0 seems to be configuration dependent. Preemptively default thread count via std::thread::hardware_concurrency(). Also allow negative values for the thread count in which case the detected hardware concurrency is also used but the given absolute thread count is used as an upper limit. --- Modules/CPack.cmake | 18 +++++--- Source/CPack/cmCPackArchiveGenerator.cxx | 39 +++++------------- Source/CPack/cmCPackArchiveGenerator.h | 2 +- Source/CPack/cmCPackDebGenerator.cxx | 5 --- Source/cmArchiveWrite.cxx | 48 ++++++++++++++-------- Source/cmArchiveWrite.h | 3 -- Tests/RunCMake/CPack/tests/THREADED_ALL/test.cmake | 2 +- 7 files changed, 56 insertions(+), 61 deletions(-) diff --git a/Modules/CPack.cmake b/Modules/CPack.cmake index 3b29ede644..373a707308 100644 --- a/Modules/CPack.cmake +++ b/Modules/CPack.cmake @@ -291,13 +291,21 @@ installers. The most commonly-used variables are: Some compression methods used by CPack generators such as Debian or Archive may take advantage of multiple CPU cores to speed up compression. - ``CPACK_THREADS`` can be set to positive integer to specify how many threads - will be used for compression. If it is set to 0, CPack will set it so that - all available CPU cores are used. + ``CPACK_THREADS`` can be set to specify how many threads will be + used for compression. + + A positive integer can be used to specify an exact desired thread count. + + When given a negative integer CPack will use the absolute value + as the upper limit but may choose a lower value based on + the available hardware concurrency. + + Given 0 CPack will try to use all available CPU cores. + By default ``CPACK_THREADS`` is set to ``1``. - Currently only ``xz`` compression *may* take advantage of multiple cores. Other - compression methods ignore this value and use only one thread. + Currently only ``xz`` compression *may* take advantage of multiple cores. + Other compression methods ignore this value and use only one thread. .. versionadded:: 3.21 diff --git a/Source/CPack/cmCPackArchiveGenerator.cxx b/Source/CPack/cmCPackArchiveGenerator.cxx index 7fd12dd4c1..d9234e6669 100644 --- a/Source/CPack/cmCPackArchiveGenerator.cxx +++ b/Source/CPack/cmCPackArchiveGenerator.cxx @@ -2,14 +2,13 @@ file Copyright.txt or https://cmake.org/licensing for details. */ #include "cmCPackArchiveGenerator.h" +#include #include #include #include #include #include -#include - #include "cmCPackComponentGroup.h" #include "cmCPackGenerator.h" #include "cmCPackLog.h" @@ -154,15 +153,9 @@ int cmCPackArchiveGenerator::addOneComponentToArchive( << (filename) << ">." << std::endl); \ return 0; \ } \ - cmArchiveWrite archive(gf, this->Compress, this->ArchiveFormat); \ + cmArchiveWrite archive(gf, this->Compress, this->ArchiveFormat, 0, \ + this->GetThreadCount()); \ do { \ - if (!this->SetArchiveOptions(&archive)) { \ - cmCPackLogger(cmCPackLog::LOG_ERROR, \ - "Problem to set archive options <" \ - << (filename) << ">, ERROR = " << (archive).GetError() \ - << std::endl); \ - return 0; \ - } \ if (!archive.Open()) { \ cmCPackLogger(cmCPackLog::LOG_ERROR, \ "Problem to open archive <" \ @@ -346,26 +339,16 @@ bool cmCPackArchiveGenerator::SupportsComponentInstallation() const return this->IsOn("CPACK_ARCHIVE_COMPONENT_INSTALL"); } -bool cmCPackArchiveGenerator::SetArchiveOptions(cmArchiveWrite* archive) +int cmCPackArchiveGenerator::GetThreadCount() const { -#if ARCHIVE_VERSION_NUMBER >= 3004000 - // Upstream fixed an issue with their integer parsing in 3.4.0 which would - // cause spurious errors to be raised from `strtoull`. - if (this->Compress == cmArchiveWrite::CompressXZ) { - const char* threads = "1"; - - // CPACK_ARCHIVE_THREADS overrides CPACK_THREADS - if (this->IsSet("CPACK_ARCHIVE_THREADS")) { - threads = this->GetOption("CPACK_ARCHIVE_THREADS"); - } else if (this->IsSet("CPACK_THREADS")) { - threads = this->GetOption("CPACK_THREADS"); - } + int threads = 1; - if (!archive->SetFilterOption("xz", "threads", threads)) { - return false; - } + // CPACK_ARCHIVE_THREADS overrides CPACK_THREADS + if (this->IsSet("CPACK_ARCHIVE_THREADS")) { + threads = std::atoi(this->GetOption("CPACK_ARCHIVE_THREADS")); + } else if (this->IsSet("CPACK_THREADS")) { + threads = std::atoi(this->GetOption("CPACK_THREADS")); } -#endif - return true; + return threads; } diff --git a/Source/CPack/cmCPackArchiveGenerator.h b/Source/CPack/cmCPackArchiveGenerator.h index 5b40013b4a..8a9bbc66b5 100644 --- a/Source/CPack/cmCPackArchiveGenerator.h +++ b/Source/CPack/cmCPackArchiveGenerator.h @@ -85,7 +85,7 @@ private: return this->OutputExtension.c_str(); } - bool SetArchiveOptions(cmArchiveWrite* archive); + int GetThreadCount() const; private: cmArchiveWrite::Compress Compress; diff --git a/Source/CPack/cmCPackDebGenerator.cxx b/Source/CPack/cmCPackDebGenerator.cxx index e7bcfaca4f..0fafd856ce 100644 --- a/Source/CPack/cmCPackDebGenerator.cxx +++ b/Source/CPack/cmCPackDebGenerator.cxx @@ -130,11 +130,6 @@ DebGenerator::DebGenerator( "Unrecognized number of threads: " << numThreads << std::endl); } - - if (this->NumThreads < 0) { - cmCPackLogger(cmCPackLog::LOG_ERROR, - "Number of threads cannot be negative" << std::endl); - } } bool DebGenerator::generate() const diff --git a/Source/cmArchiveWrite.cxx b/Source/cmArchiveWrite.cxx index b685b73af8..54b2998246 100644 --- a/Source/cmArchiveWrite.cxx +++ b/Source/cmArchiveWrite.cxx @@ -2,11 +2,16 @@ file Copyright.txt or https://cmake.org/licensing for details. */ #include "cmArchiveWrite.h" -#include +#include #include #include #include +#include #include +#include +#include + +#include #include #include @@ -144,16 +149,36 @@ cmArchiveWrite::cmArchiveWrite(std::ostream& os, Compress c, cm_archive_error_string(this->Archive)); return; } + { - char sNumThreads[8]; - snprintf(sNumThreads, sizeof(sNumThreads), "%d", numThreads); - sNumThreads[7] = '\0'; // for safety +#if ARCHIVE_VERSION_NUMBER >= 3004000 + // Upstream fixed an issue with their integer parsing in 3.4.0 + // which would cause spurious errors to be raised from `strtoull`. + + if (numThreads < 1) { + int upperLimit = (numThreads == 0) ? std::numeric_limits::max() + : std::abs(numThreads); + + numThreads = + cm::clamp(std::thread::hardware_concurrency(), 1, upperLimit); + } + +# ifdef _AIX + // FIXME: Using more than 2 threads creates an empty archive. + // Enforce this limit pending further investigation. + numThreads = std::min(numThreads, 2); +# endif + + std::string sNumThreads = std::to_string(numThreads); + if (archive_write_set_filter_option(this->Archive, "xz", "threads", - sNumThreads) != ARCHIVE_OK) { + sNumThreads.c_str()) != + ARCHIVE_OK) { this->Error = cmStrCat("archive_compressor_xz_options: ", cm_archive_error_string(this->Archive)); return; } +#endif } break; @@ -425,16 +450,3 @@ bool cmArchiveWrite::AddData(const char* file, size_t size) } return true; } - -bool cmArchiveWrite::SetFilterOption(const char* module, const char* key, - const char* value) -{ - if (archive_write_set_filter_option(this->Archive, module, key, value) != - ARCHIVE_OK) { - this->Error = "archive_write_set_filter_option: "; - this->Error += cm_archive_error_string(this->Archive); - return false; - } - - return true; -} diff --git a/Source/cmArchiveWrite.h b/Source/cmArchiveWrite.h index 34aafe905c..260bd20caa 100644 --- a/Source/cmArchiveWrite.h +++ b/Source/cmArchiveWrite.h @@ -141,9 +141,6 @@ public: this->Gname = ""; } - //! Set an option on a filter; - bool SetFilterOption(const char* module, const char* key, const char* value); - private: bool Okay() const { return this->Error.empty(); } bool AddPath(const char* path, size_t skip, const char* prefix, diff --git a/Tests/RunCMake/CPack/tests/THREADED_ALL/test.cmake b/Tests/RunCMake/CPack/tests/THREADED_ALL/test.cmake index 6f37201311..af39e5f2c9 100644 --- a/Tests/RunCMake/CPack/tests/THREADED_ALL/test.cmake +++ b/Tests/RunCMake/CPack/tests/THREADED_ALL/test.cmake @@ -1,6 +1,6 @@ install(FILES CMakeLists.txt DESTINATION foo COMPONENT test) -set(CPACK_THREADS 0) +set(CPACK_THREADS "-4") if(PACKAGING_TYPE STREQUAL "COMPONENT") set(CPACK_COMPONENTS_ALL test) -- cgit v1.2.1