diff --git a/autotest/cpp/test_cpl.cpp b/autotest/cpp/test_cpl.cpp index 90e2c33445ad..ba43804e23b4 100644 --- a/autotest/cpp/test_cpl.cpp +++ b/autotest/cpp/test_cpl.cpp @@ -1777,6 +1777,16 @@ TEST_F(test_cpl, CPLParseMemorySize) EXPECT_GT(nValue, 100 * 1024 * 1024); EXPECT_TRUE(bUnitSpecified); + result = CPLParseMemorySize("0", &nValue, &bUnitSpecified); + EXPECT_EQ(result, CE_None); + EXPECT_EQ(nValue, 0); + EXPECT_FALSE(bUnitSpecified); + + result = CPLParseMemorySize("0MB", &nValue, &bUnitSpecified); + EXPECT_EQ(result, CE_None); + EXPECT_EQ(nValue, 0); + EXPECT_TRUE(bUnitSpecified); + result = CPLParseMemorySize(" 802 ", &nValue, &bUnitSpecified); EXPECT_EQ(result, CE_None); EXPECT_EQ(nValue, 802); diff --git a/autotest/gcore/vsifile.py b/autotest/gcore/vsifile.py index 25d610133971..12b69d9eaca7 100755 --- a/autotest/gcore/vsifile.py +++ b/autotest/gcore/vsifile.py @@ -237,16 +237,16 @@ def test_vsifile_4(): # Test vsicache -@pytest.mark.parametrize("cache_size", ("0", "65536", None)) -def test_vsifile_5(cache_size): +@pytest.mark.parametrize("cache_size", ("0", "64kb", None)) +def test_vsifile_5(tmp_path, cache_size): - fp = gdal.VSIFOpenL("tmp/vsifile_5.bin", "wb") + fp = gdal.VSIFOpenL(tmp_path / "vsifile_5.bin", "wb") ref_data = "".join(["%08X" % i for i in range(5 * 32768)]) gdal.VSIFWriteL(ref_data, 1, len(ref_data), fp) gdal.VSIFCloseL(fp) with gdal.config_options({"VSI_CACHE": "YES", "VSI_CACHE_SIZE": cache_size}): - fp = gdal.VSIFOpenL("tmp/vsifile_5.bin", "rb") + fp = gdal.VSIFOpenL(tmp_path / "vsifile_5.bin", "rb") gdal.VSIFSeekL(fp, 50000, 0) if gdal.VSIFTellL(fp) != 50000: @@ -277,8 +277,6 @@ def test_vsifile_5(cache_size): gdal.VSIFCloseL(fp) - gdal.Unlink("tmp/vsifile_5.bin") - ############################################################################### # Test vsicache an read errors (https://github.com/qgis/QGIS/issues/45293) diff --git a/doc/source/user/configoptions.rst b/doc/source/user/configoptions.rst index 425d0292371a..3c9c4b7d8eaa 100644 --- a/doc/source/user/configoptions.rst +++ b/doc/source/user/configoptions.rst @@ -222,7 +222,10 @@ Performance and caching :program:`gdalwarp`. If its value is small (less than 100000), it is assumed to be measured in megabytes, otherwise in bytes. Alternatively, the value can be set to "X%" to mean X% - of the usable physical RAM. Note that this value is only consulted the first + of the usable physical RAM. + Since GDAL 3.11, the value of :config:`GDAL_CACHEMAX` may specify the + units directly (e.g., "500MB", "2GB"). + Note that this value is only consulted the first time the cache size is requested. To change this value programmatically during operation of the program it is better to use :cpp:func:`GDALSetCacheMax` (always in bytes) or or @@ -318,6 +321,9 @@ Performance and caching Set the size of the VSI cache. Be wary of large values for ``VSI_CACHE_SIZE`` when opening VRT datasources containing many source rasters, as this is a per-file cache. + Since GDAL 3.11, the value of ``VSI_CACHE_SIZE`` may be specified using + memory units (e.g., "25 MB"). + Driver management ^^^^^^^^^^^^^^^^^ @@ -421,7 +427,7 @@ General options option. - .. config:: CPL_VSIL_DEFLATE_CHUNK_SIZE - :default: 1 M + :default: 1M - .. config:: GDAL_DISABLE_CPLLOCALEC :choices: YES, NO @@ -610,7 +616,8 @@ Networking options :since: 2.3 Size of global least-recently-used (LRU) cache shared among all downloaded - content. + content. Value is assumed to represent bytes unless memory units are + specified (since GDAL 3.11). - .. config:: CPL_VSIL_CURL_USE_HEAD :choices: YES, NO @@ -658,6 +665,9 @@ Networking options :choices: :since: 2.3 + Value is assumed to represent bytes unless memory units are + specified (since GDAL 3.11). + - .. config:: GDAL_INGESTED_BYTES_AT_OPEN :since: 2.3 diff --git a/gcore/gdalrasterblock.cpp b/gcore/gdalrasterblock.cpp index cbb0415e6f44..ad9d73ed31cf 100644 --- a/gcore/gdalrasterblock.cpp +++ b/gcore/gdalrasterblock.cpp @@ -207,11 +207,12 @@ int CPL_STDCALL GDALGetCacheMax() * Gets the maximum amount of memory available to the GDALRasterBlock * caching system for caching GDAL read/write imagery. * - * The first type this function is called, it will read the GDAL_CACHEMAX + * The first time this function is called, it will read the GDAL_CACHEMAX * configuration option to initialize the maximum cache memory. * Starting with GDAL 2.1, the value can be expressed as x% of the usable - * physical RAM (which may potentially be used by other processes). Otherwise - * it is expected to be a value in MB. + * physical RAM (which may potentially be used by other processes). Starting + * with GDAL 3.11, the value can include units of memory. If not units are + * provided the value is assumed to be in MB. * * @return maximum in bytes. * @@ -232,60 +233,29 @@ GIntBig CPL_STDCALL GDALGetCacheMax64() CPLTestBool(CPLGetConfigOption("GDAL_DEBUG_BLOCK_CACHE", "NO")); const char *pszCacheMax = CPLGetConfigOption("GDAL_CACHEMAX", "5%"); - GIntBig nNewCacheMax; - if (strchr(pszCacheMax, '%') != nullptr) + bool bUnitSpecified = false; + + if (CPLParseMemorySize(pszCacheMax, &nNewCacheMax, + &bUnitSpecified) != CE_None) { - GIntBig nUsablePhysicalRAM = CPLGetUsablePhysicalRAM(); - if (nUsablePhysicalRAM > 0) + CPLError(CE_Failure, CPLE_NotSupported, + "Invalid value for GDAL_CACHEMAX. " + "Using default value."); + if (CPLParseMemorySize("5%", &nNewCacheMax, &bUnitSpecified) != + CE_None) { - // For some reason, coverity pretends that this will overflow. - // "Multiply operation overflows on operands - // static_cast( nUsablePhysicalRAM ) and - // CPLAtof(pszCacheMax). Example values for operands: CPLAtof( - // pszCacheMax ) = 2251799813685248, - // static_cast(nUsablePhysicalRAM) = - // -9223372036854775808." coverity[overflow,tainted_data] - double dfCacheMax = - static_cast(nUsablePhysicalRAM) * - CPLAtof(pszCacheMax) / 100.0; - if (dfCacheMax >= 0 && dfCacheMax < 1e15) - nNewCacheMax = static_cast(dfCacheMax); - else - nNewCacheMax = nCacheMax; - } - else - { - CPLDebug("GDAL", "Cannot determine usable physical RAM."); + // This means that usable physical RAM could not be determined. nNewCacheMax = nCacheMax; } } - else + + if (!bUnitSpecified && nNewCacheMax < 100000) { - nNewCacheMax = CPLAtoGIntBig(pszCacheMax); - if (nNewCacheMax < 100000) - { - if (nNewCacheMax < 0) - { - CPLError(CE_Failure, CPLE_NotSupported, - "Invalid value for GDAL_CACHEMAX. " - "Using default value."); - GIntBig nUsablePhysicalRAM = CPLGetUsablePhysicalRAM(); - if (nUsablePhysicalRAM) - nNewCacheMax = nUsablePhysicalRAM / 20; - else - { - CPLDebug("GDAL", - "Cannot determine usable physical RAM."); - nNewCacheMax = nCacheMax; - } - } - else - { - nNewCacheMax *= 1024 * 1024; - } - } + // Assume MB + nNewCacheMax *= (1024 * 1024); } + nCacheMax = nNewCacheMax; CPLDebug("GDAL", "GDAL_CACHEMAX = " CPL_FRMT_GIB " MB", nCacheMax / (1024 * 1024)); diff --git a/port/cpl_string.cpp b/port/cpl_string.cpp index 1462f5157d88..f67ea095ce9d 100644 --- a/port/cpl_string.cpp +++ b/port/cpl_string.cpp @@ -1779,10 +1779,10 @@ CPLErr CPLParseMemorySize(const char *pszValue, GIntBig *pnValue, return CE_Failure; } - if (value <= 0 || !std::isfinite(value)) + if (value < 0 || !std::isfinite(value)) { CPLError(CE_Failure, CPLE_IllegalArg, - "Memory size must be a positive number."); + "Memory size must be a positive number or zero."); return CE_Failure; } @@ -1800,6 +1800,12 @@ CPLErr CPLParseMemorySize(const char *pszValue, GIntBig *pnValue, return CE_Failure; } auto bytes = CPLGetUsablePhysicalRAM(); + if (bytes == 0) + { + CPLError(CE_Failure, CPLE_NotSupported, + "Cannot determine usable physical RAM"); + return CE_Failure; + } value *= static_cast(bytes / 100); unit = c; } @@ -1842,7 +1848,10 @@ CPLErr CPLParseMemorySize(const char *pszValue, GIntBig *pnValue, } *pnValue = static_cast(value); - *pbUnitSpecified = (unit != nullptr); + if (pbUnitSpecified) + { + *pbUnitSpecified = (unit != nullptr); + } return CE_None; } diff --git a/port/cpl_vsil_cache.cpp b/port/cpl_vsil_cache.cpp index 64b061285c6f..3e7083584205 100644 --- a/port/cpl_vsil_cache.cpp +++ b/port/cpl_vsil_cache.cpp @@ -112,13 +112,30 @@ class VSICachedFile final : public VSIVirtualHandle static size_t GetCacheMax(size_t nCacheSize) { - return nCacheSize ? nCacheSize - : static_cast(std::min( - static_cast( - std::numeric_limits::max() / 2), - CPLScanUIntBig(CPLGetConfigOption("VSI_CACHE_SIZE", - "25000000"), - 40))); + if (nCacheSize) + { + return nCacheSize; + } + + const char *pszCacheSize = CPLGetConfigOption("VSI_CACHE_SIZE", "25000000"); + GIntBig nMemorySize; + bool bUnitSpecified; + if (CPLParseMemorySize(pszCacheSize, &nMemorySize, &bUnitSpecified) != + CE_None) + { + CPLError( + CE_Failure, CPLE_IllegalArg, + "Failed to parse value of VSI_CACHE_SIZE. Using default of 25MB"); + nMemorySize = 25000000; + } + else if (static_cast(nMemorySize) > + std::numeric_limits::max() / 2) + { + nMemorySize = + static_cast(std::numeric_limits::max() / 2); + } + + return static_cast(nMemorySize); } /************************************************************************/ diff --git a/port/cpl_vsil_curl.cpp b/port/cpl_vsil_curl.cpp index 633b1636c707..66934f5fa40c 100644 --- a/port/cpl_vsil_curl.cpp +++ b/port/cpl_vsil_curl.cpp @@ -137,31 +137,63 @@ static void VSICURLReadGlobalEnvVariables() Initializer() { constexpr int DOWNLOAD_CHUNK_SIZE_DEFAULT = 16384; + const char *pszChunkSize = + CPLGetConfigOption("CPL_VSIL_CURL_CHUNK_SIZE", nullptr); + GIntBig nChunkSize = DOWNLOAD_CHUNK_SIZE_DEFAULT; + + if (pszChunkSize) + { + if (CPLParseMemorySize(pszChunkSize, &nChunkSize, nullptr) != + CE_None) + { + CPLError( + CE_Warning, CPLE_AppDefined, + "Could not parse value for CPL_VSIL_CURL_CHUNK_SIZE. " + "Using default value of %d instead.", + DOWNLOAD_CHUNK_SIZE_DEFAULT); + } + } - DOWNLOAD_CHUNK_SIZE_DO_NOT_USE_DIRECTLY = atoi(CPLGetConfigOption( - "CPL_VSIL_CURL_CHUNK_SIZE", - CPLSPrintf("%d", DOWNLOAD_CHUNK_SIZE_DEFAULT))); constexpr int MIN_CHUNK_SIZE = 1024; constexpr int MAX_CHUNK_SIZE = 10 * 1024 * 1024; - if (DOWNLOAD_CHUNK_SIZE_DO_NOT_USE_DIRECTLY < MIN_CHUNK_SIZE || - DOWNLOAD_CHUNK_SIZE_DO_NOT_USE_DIRECTLY > MAX_CHUNK_SIZE) + if (nChunkSize < MIN_CHUNK_SIZE || nChunkSize > MAX_CHUNK_SIZE) { - DOWNLOAD_CHUNK_SIZE_DO_NOT_USE_DIRECTLY = - DOWNLOAD_CHUNK_SIZE_DEFAULT; + nChunkSize = DOWNLOAD_CHUNK_SIZE_DEFAULT; CPLError(CE_Warning, CPLE_AppDefined, "Invalid value for CPL_VSIL_CURL_CHUNK_SIZE. " "Allowed range is [%d, %d]. " "Using CPL_VSIL_CURL_CHUNK_SIZE=%d instead", MIN_CHUNK_SIZE, MAX_CHUNK_SIZE, - DOWNLOAD_CHUNK_SIZE_DO_NOT_USE_DIRECTLY); + DOWNLOAD_CHUNK_SIZE_DEFAULT); } + DOWNLOAD_CHUNK_SIZE_DO_NOT_USE_DIRECTLY = + static_cast(nChunkSize); constexpr int N_MAX_REGIONS_DEFAULT = 1000; constexpr int CACHE_SIZE_DEFAULT = N_MAX_REGIONS_DEFAULT * DOWNLOAD_CHUNK_SIZE_DEFAULT; - GIntBig nCacheSize = CPLAtoGIntBig( - CPLGetConfigOption("CPL_VSIL_CURL_CACHE_SIZE", - CPLSPrintf("%d", CACHE_SIZE_DEFAULT))); + + const char *pszCacheSize = + CPLGetConfigOption("CPL_VSIL_CURL_CACHE_SIZE", nullptr); + GIntBig nCacheSize = CACHE_SIZE_DEFAULT; + + if (pszCacheSize) + { + if (CPLParseMemorySize(pszCacheSize, &nCacheSize, nullptr) != + CE_None) + { + CPLError( + CE_Warning, CPLE_AppDefined, + "Could not parse value for CPL_VSIL_CURL_CACHE_SIZE. " + "Using default value of " CPL_FRMT_GIB " instead.", + nCacheSize); + } + } + else + { + nCacheSize = CACHE_SIZE_DEFAULT; + } + const auto nMaxRAM = CPLGetUsablePhysicalRAM(); const auto nMinVal = DOWNLOAD_CHUNK_SIZE_DO_NOT_USE_DIRECTLY; auto nMaxVal = static_cast(INT_MAX) *