Skip to content
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

sbcl: read memory from envvar #303891

Merged
merged 2 commits into from
May 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions pkgs/development/compilers/sbcl/default.nix
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,15 @@ stdenv.mkDerivation (self: rec {
# have it block a release.
"futex-wait.test.sh"
];
patches = [
# Support the NIX_SBCL_DYNAMIC_SPACE_SIZE envvar. Upstream SBCL didn’t want
# to include this (see
# "https://sourceforge.net/p/sbcl/mailman/sbcl-devel/thread/2cf20df7-01d0-44f2-8551-0df01fe55f1a%400brg.net/"),
# but for Nix envvars are sufficiently useful that it’s worth maintaining
# this functionality downstream.
./dynamic-space-size-envvar-feature.patch
./dynamic-space-size-envvar-tests.patch
];
postPatch = lib.optionalString (self.disabledTestFiles != [ ]) ''
(cd tests ; rm -f ${lib.concatStringsSep " " self.disabledTestFiles})
''
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
From ac15f9f7c75c1fb5767514e64b609e2a75e6fe9d Mon Sep 17 00:00:00 2001
From: Hraban Luyat <hraban@0brg.net>
Date: Sat, 13 Apr 2024 14:04:57 -0400
Subject: [PATCH] feat: NIX_SBCL_DYNAMIC_SPACE_SIZE envvar

Read SBCL dynamic space size configuration from env if available.
---
src/runtime/runtime.c | 25 +++++++++++++++++++++++++
1 file changed, 25 insertions(+)

diff --git a/src/runtime/runtime.c b/src/runtime/runtime.c
index 274687c8f..970caa8f4 100644
--- a/src/runtime/runtime.c
+++ b/src/runtime/runtime.c
@@ -422,6 +422,29 @@ static int is_memsize_arg(char *argv[], int argi, int argc, int *merge_core_page
return 0;
}

+/**
+ * Read memory options from the environment, if present.
+ *
+ * Memory settings are read in the following priority:
+ *
+ * 1. command line arguments
+ * 2. environment variable
+ * 3. embedded options in core
+ * 4. default
+ */
+static void
+read_memsize_from_env(void) {
+ const char *val = getenv("NIX_SBCL_DYNAMIC_SPACE_SIZE");
+ // The distinction is blurry between setting an envvar to the empty string and
+ // unsetting it entirely. Depending on the calling environment it can even be
+ // tricky to properly unset an envvar in the first place. An empty envvar is
+ // practically always intended to just mean “unset”, so let’s interpret it
+ // that way.
+ if (val != NULL && (strcmp(val, "") != 0)) {
+ dynamic_space_size = parse_size_arg(val, "NIX_SBCL_DYNAMIC_SPACE_SIZE");
+ }
+}
+
static struct cmdline_options
parse_argv(struct memsize_options memsize_options,
int argc, char *argv[], char *envp[], char *core)
@@ -462,6 +485,7 @@ parse_argv(struct memsize_options memsize_options,
dynamic_space_size = memsize_options.dynamic_space_size;
thread_control_stack_size = memsize_options.thread_control_stack_size;
dynamic_values_bytes = memsize_options.thread_tls_bytes;
+ read_memsize_from_env();
int stop_parsing = 0; // have we seen '--'
int output_index = 1;

@@ -488,6 +512,7 @@ parse_argv(struct memsize_options memsize_options,
}
sbcl_argv[output_index] = 0;
} else {
+ read_memsize_from_env();
bool end_runtime_options = 0;
/* Parse our any of the command-line options that we handle from C,
* stopping at the first one that we don't, and leave the rest */
--
2.44.0

104 changes: 104 additions & 0 deletions pkgs/development/compilers/sbcl/dynamic-space-size-envvar-tests.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
From 9d4a886a8a76ea8be51bcf754cefacdf30986f46 Mon Sep 17 00:00:00 2001
From: Hraban Luyat <hraban@0brg.net>
Date: Sat, 13 Apr 2024 15:39:58 -0400
Subject: [PATCH 2/2] test: dynamic space size envvar and precedence

---
tests/memory-args.test.sh | 22 ++++++++++++++++++++++
tests/save7.test.sh | 37 ++++++++++++++++++++++++++++++++-----
2 files changed, 54 insertions(+), 5 deletions(-)
create mode 100755 tests/memory-args.test.sh

diff --git a/tests/memory-args.test.sh b/tests/memory-args.test.sh
new file mode 100755
index 000000000..72ef0cc79
--- /dev/null
+++ b/tests/memory-args.test.sh
@@ -0,0 +1,22 @@
+#!/bin/sh
+
+. ./subr.sh
+
+use_test_subdirectory
+
+set -e
+
+# Allow slight shrinkage if heap relocation has to adjust for alignment
+NIX_SBCL_DYNAMIC_SPACE_SIZE=234mb run_sbcl_with_args --script <<EOF
+(assert (<= 0 (- (* 234 1024 1024) (sb-ext:dynamic-space-size)) 65536))
+EOF
+
+NIX_SBCL_DYNAMIC_SPACE_SIZE=555mb run_sbcl_with_args --dynamic-space-size 234mb --script <<EOF
+(assert (<= 0 (- (* 234 1024 1024) (sb-ext:dynamic-space-size)) 65536))
+EOF
+
+run_sbcl_with_args --dynamic-space-size 234mb --script <<EOF
+(assert (<= 0 (- (* 234 1024 1024) (sb-ext:dynamic-space-size)) 65536))
+EOF
+
+exit $EXIT_TEST_WIN
diff --git a/tests/save7.test.sh b/tests/save7.test.sh
index f9225543b..3c35e7b31 100644
--- a/tests/save7.test.sh
+++ b/tests/save7.test.sh
@@ -59,9 +59,9 @@ run_sbcl_with_core "$tmpcore" --noinform --control-stack-size 640KB \
(assert (eql (extern-alien "dynamic_values_bytes" (unsigned 32))
(* 5000 sb-vm:n-word-bytes)))
; allow slight shrinkage if heap relocation has to adjust for alignment
- (defun dynamic-space-size-good-p ()
- (<= 0 (- (* 260 1048576) (dynamic-space-size)) 65536))
- (assert (dynamic-space-size-good-p))
+ (defun dynamic-space-size-good-p (expected-mb)
+ (<= 0 (- (* expected-mb 1024 1024) (dynamic-space-size)) 65536))
+ (assert (dynamic-space-size-good-p 260))
(save-lisp-and-die "${tmpcore}2" :executable t :save-runtime-options t)
EOF
chmod u+x "${tmpcore}2"
@@ -70,15 +70,42 @@ echo "::: INFO: prepared test core"
(when (and (eql (extern-alien "thread_control_stack_size" unsigned) (* 640 1024))
(eql (extern-alien "dynamic_values_bytes" (unsigned 32))
(* 5000 sb-vm:n-word-bytes))
- (dynamic-space-size-good-p))
+ (dynamic-space-size-good-p 260))
(exit :code 42))
EOF
status=$?
-rm "$tmpcore" "${tmpcore}2"
if [ $status -ne 42 ]; then
echo "re-saved executable used wrong memory size options"
exit 1
fi
echo "::: Success"

+echo "::: Running :DYNAMIC-SPACE-SIZE-ENV"
+NIX_SBCL_DYNAMIC_SPACE_SIZE=432MB ./"${tmpcore}2" --no-userinit --no-sysinit --noprint <<EOF
+ (when (dynamic-space-size-good-p 432)
+ (exit :code 42))
+EOF
+status=$?
+if [ $status -ne 42 ]; then
+ echo "re-saved executable should have prioritized memory specification from env"
+ exit 1
+fi
+echo "::: Success"
+
+echo "::: Running :DYNAMIC-SPACE-SIZE-PRECEDENCE"
+NIX_SBCL_DYNAMIC_SPACE_SIZE=432MB ./"${tmpcore}2" --dynamic-space-size 333MB \
+ --no-userinit --no-sysinit --noprint <<EOF
+ (when (dynamic-space-size-good-p 333))
+ (exit :code 42))
+EOF
+status=$?
+rm "$tmpcore" "${tmpcore}2"
+if [ $status -ne 42 ]; then
+ echo "re-saved executable should have prioritized memory specification from arg"
+ exit 1
+fi
+echo "::: Success"
+
+
+
exit $EXIT_TEST_WIN
--
2.44.0

Loading