From 6904351d619df21b43194e1c3c8672c809ca5cf9 Mon Sep 17 00:00:00 2001 From: Matt Bauman Date: Fri, 23 Feb 2024 17:04:29 -0500 Subject: [PATCH] improve `--heap-size-hint` arg handling (#48050) Previously, `--heap-size-hint` would silently ignore many flavors of "bad" input, parsing things like "3PB" as 3 bytes. This change makes it significantly less permissive, erroring unless it can parse a number (still relying on the C `sscanf` `%Lf` format specifier there) with an optional unit (case-insensitive, either with or without the trailing `b`). Also test it. (cherry picked from commit 138aba7320c4c37b1ad6330ec46531077f8ee516) --- doc/man/julia.1 | 7 +- doc/src/manual/command-line-interface.md | 2 +- src/jloptions.c | 90 ++++++++++++++---------- test/cmdlineargs.jl | 16 +++++ 4 files changed, 73 insertions(+), 42 deletions(-) diff --git a/doc/man/julia.1 b/doc/man/julia.1 index c58b00120b7ec..afde0326e4952 100644 --- a/doc/man/julia.1 +++ b/doc/man/julia.1 @@ -228,9 +228,10 @@ fallbacks to the latest compatible BugReporting.jl if not. For more information, --bug-report=help. .TP ---heap-size-hint= -Forces garbage collection if memory usage is higher than that value. The value can be -specified in units of K, M, G, T, or % of physical memory. +--heap-size-hint= +Forces garbage collection if memory usage is higher than the given value. +The value may be specified as a number of bytes, optionally in units of +KB, MB, GB, or TB, or as a percentage of physical memory with %. .TP --compile={yes*|no|all|min} diff --git a/doc/src/manual/command-line-interface.md b/doc/src/manual/command-line-interface.md index 4faa3e8d3fc05..448964bf1ef8a 100644 --- a/doc/src/manual/command-line-interface.md +++ b/doc/src/manual/command-line-interface.md @@ -213,7 +213,7 @@ The following is a complete list of command-line switches available when launchi |`--output-incremental={yes\|no*}` |Generate an incremental output file (rather than complete)| |`--trace-compile={stderr,name}` |Print precompile statements for methods compiled during execution or save to a path| |`--image-codegen` |Force generate code in imaging mode| -|`--heap-size-hint=` |Forces garbage collection if memory usage is higher than that value. The memory hint might be specified in megabytes (e.g., 500M) or gigabytes (e.g., 1G)| +|`--heap-size-hint=` |Forces garbage collection if memory usage is higher than the given value. The value may be specified as a number of bytes, optionally in units of KB, MB, GB, or TB, or as a percentage of physical memory with %.| !!! compat "Julia 1.1" diff --git a/src/jloptions.c b/src/jloptions.c index 4b3f7209dc56d..570d021351104 100644 --- a/src/jloptions.c +++ b/src/jloptions.c @@ -8,6 +8,7 @@ #include #include + #include "julia_assert.h" #ifdef _OS_WINDOWS_ @@ -18,6 +19,15 @@ char *shlib_ext = ".dylib"; char *shlib_ext = ".so"; #endif +/* This simple hand-crafted tolower exists to avoid locale-dependent effects in + * behaviors (and utf8proc_tolower wasn't linking properly on all platforms) */ +static char ascii_tolower(char c) +{ + if ('A' <= c && c <= 'Z') + return c - 'A' + 'a'; + return c; +} + static const char system_image_path[256] = "\0" JL_SYSTEM_IMAGE_PATH; JL_DLLEXPORT const char *jl_get_default_sysimg_path(void) { @@ -187,9 +197,9 @@ static const char opts[] = " expressions. It first tries to use BugReporting.jl installed in current environment and\n" " fallbacks to the latest compatible BugReporting.jl if not. For more information, see\n" " --bug-report=help.\n\n" - - " --heap-size-hint= Forces garbage collection if memory usage is higher than that value.\n" - " The value can be specified in units of K, M, G, T, or % of physical memory.\n\n" + " --heap-size-hint= Forces garbage collection if memory usage is higher than the given value.\n" + " The value may be specified as a number of bytes, optionally in units of\n" + " KB, MB, GB, or TB, or as a percentage of physical memory with %.\n\n" ; static const char opts_hidden[] = @@ -801,43 +811,47 @@ JL_DLLEXPORT void jl_parse_opts(int *argcp, char ***argvp) break; case opt_heap_size_hint: if (optarg != NULL) { - size_t endof = strlen(optarg); long double value = 0.0; - if (sscanf(optarg, "%Lf", &value) == 1 && value > 1e-7) { - char unit = optarg[endof - 1]; - uint64_t multiplier = 1ull; - switch (unit) { - case 'k': - case 'K': - multiplier <<= 10; - break; - case 'm': - case 'M': - multiplier <<= 20; - break; - case 'g': - case 'G': - multiplier <<= 30; - break; - case 't': - case 'T': - multiplier <<= 40; - break; - case '%': - if (value > 100) - jl_errorf("julia: invalid percentage specified in --heap-size-hint"); - uint64_t mem = uv_get_total_memory(); - uint64_t cmem = uv_get_constrained_memory(); - if (cmem > 0 && cmem < mem) - mem = cmem; - multiplier = mem/100; - break; - default: - jl_errorf("julia: invalid unit specified in --heap-size-hint"); - break; - } - jl_options.heap_size_hint = (uint64_t)(value * multiplier); + char unit[4] = {0}; + int nparsed = sscanf(optarg, "%Lf%3s", &value, unit); + if (nparsed == 0 || strlen(unit) > 2 || (strlen(unit) == 2 && ascii_tolower(unit[1]) != 'b')) { + jl_errorf("julia: invalid argument to --heap-size-hint (%s)", optarg); + } + uint64_t multiplier = 1ull; + switch (ascii_tolower(unit[0])) { + case '\0': + case 'b': + break; + case 'k': + multiplier <<= 10; + break; + case 'm': + multiplier <<= 20; + break; + case 'g': + multiplier <<= 30; + break; + case 't': + multiplier <<= 40; + break; + case '%': + if (value > 100) + jl_errorf("julia: invalid percentage specified in --heap-size-hint"); + uint64_t mem = uv_get_total_memory(); + uint64_t cmem = uv_get_constrained_memory(); + if (cmem > 0 && cmem < mem) + mem = cmem; + multiplier = mem/100; + break; + default: + jl_errorf("julia: invalid argument to --heap-size-hint (%s)", optarg); + break; + } + long double sz = value * multiplier; + if (isnan(sz) || sz < 0) { + jl_errorf("julia: invalid argument to --heap-size-hint (%s)", optarg); } + jl_options.heap_size_hint = sz < UINT64_MAX ? (uint64_t)sz : UINT64_MAX; } if (jl_options.heap_size_hint == 0) jl_errorf("julia: invalid memory size specified in --heap-size-hint"); diff --git a/test/cmdlineargs.jl b/test/cmdlineargs.jl index 3af31965e63fc..498254bb26ce0 100644 --- a/test/cmdlineargs.jl +++ b/test/cmdlineargs.jl @@ -1148,3 +1148,19 @@ if Sys.islinux() && Sys.ARCH in (:i686, :x86_64) # rr is only available on these "_RR_TRACE_DIR" => temp_trace_dir); #=stderr, stdout=#)) end end + +@testset "--heap-size-hint" begin + exename = `$(Base.julia_cmd())` + @test errors_not_signals(`$exename --heap-size-hint -e "exit(0)"`) + @testset "--heap-size-hint=$str" for str in ["asdf","","0","1.2vb","b","GB","2.5GBÌ‚","1.2gb2","42gigabytes","5gig","2GiB","NaNt"] + @test errors_not_signals(`$exename --heap-size-hint=$str -e "exit(0)"`) + end + k = 1024 + m = 1024k + g = 1024m + t = 1024g + @testset "--heap-size-hint=$str" for (str, val) in [("1", 1), ("1e7", 1e7), ("2.5e7", 2.5e7), ("1MB", 1m), ("2.5g", 2.5g), ("1e4kB", 1e4k), + ("1e100", typemax(UInt64)), ("1e500g", typemax(UInt64)), ("1e-12t", 1), ("500000000b", 500000000)] + @test parse(UInt64,read(`$exename --heap-size-hint=$str -E "Base.JLOptions().heap_size_hint"`, String)) == val + end +end