From c96fc9d23024b2ad070b1b0b39f66aa6ffa90d8e Mon Sep 17 00:00:00 2001 From: Brad Chamberlain Date: Fri, 20 Dec 2024 17:30:07 -0800 Subject: [PATCH 1/5] Raise number of threads used by GASNet by default + add warning if exceeded --- Signed-off-by: Brad Chamberlain --- runtime/src/tasks/qthreads/tasks-qthreads.c | 12 +++++++++++- third-party/gasnet/Makefile | 2 +- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/runtime/src/tasks/qthreads/tasks-qthreads.c b/runtime/src/tasks/qthreads/tasks-qthreads.c index 02cb8cb6ef32..568a21718103 100644 --- a/runtime/src/tasks/qthreads/tasks-qthreads.c +++ b/runtime/src/tasks/qthreads/tasks-qthreads.c @@ -502,7 +502,17 @@ static void setupAvailableParallelism(int32_t maxThreads) { if (hwpar > 0) { // Limit the parallelism to the maximum imposed by the comm layer. if (0 < maxThreads && maxThreads < hwpar) { - hwpar = maxThreads; + if (chpl_nodeID == 0) { + char msg[1024]; + snprintf(msg, sizeof(msg), + "The CHPL_COMM setting is limiting the number of threads " + "to %d, rather than the hardware's preference of %d. " + "If using CHPL_COMM=gasnet, set 'GASNET_MAX_THREADS' to " + "a higher number in your environment to address this.\n", + maxThreads, hwpar); + chpl_warning(msg, 0, 0); + } + hwpar = maxThreads; } // diff --git a/third-party/gasnet/Makefile b/third-party/gasnet/Makefile index fecfda6fc03d..9d0aec0fa542 100644 --- a/third-party/gasnet/Makefile +++ b/third-party/gasnet/Makefile @@ -135,7 +135,7 @@ gasnet-config: FORCE mkdir -p $(GASNET_BUILD_DIR) cd $(GASNET_SUBDIR) && ./Bootstrap -T $(XTRA_CONFIG_COMMAND) - $(ENSURE_GASNET_COMPATIBLE_COMPILER) cd $(GASNET_BUILD_DIR) && $(CHPL_GASNET_ENV_VARS) $(GASNET_SUBDIR)/$(CHPL_GASNET_CFG_SCRIPT) --prefix=$(GASNET_INSTALL_DIR) $(CHPL_GASNET_CFG_OPTIONS) --disable-seq --enable-par --disable-parsync $(CHPL_GASNET_CFG_OPTIONS) + $(ENSURE_GASNET_COMPATIBLE_COMPILER) cd $(GASNET_BUILD_DIR) && $(CHPL_GASNET_ENV_VARS) $(GASNET_SUBDIR)/$(CHPL_GASNET_CFG_SCRIPT) --prefix=$(GASNET_INSTALL_DIR) --disable-seq --enable-par --disable-parsync --with-max-pthreads-per-node=65535 $(CHPL_GASNET_CFG_OPTIONS) gasnet-build: FORCE $(ENSURE_GASNET_COMPATIBLE_COMPILER) cd $(GASNET_BUILD_DIR) && $(MAKE) all From 4b28e4cc709006a7a99bcae8be62cb9dca50dd2d Mon Sep 17 00:00:00 2001 From: Brad Chamberlain Date: Wed, 15 Jan 2025 14:04:16 -0800 Subject: [PATCH 2/5] Improve Makefile by marking 'FORCE' as a phony target Borrowed from @bonachea's PR in #25414 which contained this along with the other Makefile change I made in this PR. --- Signed-off-by: Brad Chamberlain --- third-party/gasnet/Makefile | 2 ++ 1 file changed, 2 insertions(+) diff --git a/third-party/gasnet/Makefile b/third-party/gasnet/Makefile index 9d0aec0fa542..27f9a59b24a1 100644 --- a/third-party/gasnet/Makefile +++ b/third-party/gasnet/Makefile @@ -158,4 +158,6 @@ gasnet: gasnet-config gasnet-build gasnet-install FORCE: +.PHONY: FORCE + .NOTPARALLEL: From d44edd2a8559f719ffa5789260a4f147bc6d66ee Mon Sep 17 00:00:00 2001 From: Brad Chamberlain Date: Wed, 15 Jan 2025 16:29:25 -0800 Subject: [PATCH 3/5] Specialize error message for GASNet; Fix query of GASNet #threads This does two things: * specializes the error message I just added to only print the GASNet-specific information if CHPL_COMM is GASNet once Jade reminded me of how easy it was * fixes an issue that I noticed in testing where the value being returned was the hard limit on the number of threads that GASNet is configured to use rather than the soft limit that it's using in practice. With Dan's help, I updated this to the API that we should've been using all along and put in some bounds-checking code that is unlikely to ever matter (b/c we configure the cap at well within an int(32)'s value and b/c it's hard to imagine a user setting it higher than max(int(32)). --- Signed-off-by: Brad Chamberlain --- runtime/src/comm/gasnet/comm-gasnet-ex.c | 10 +++++++++- runtime/src/tasks/qthreads/tasks-qthreads.c | 9 +++++---- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/runtime/src/comm/gasnet/comm-gasnet-ex.c b/runtime/src/comm/gasnet/comm-gasnet-ex.c index 88e9ff99b10b..7c83ba9e30e7 100644 --- a/runtime/src/comm/gasnet/comm-gasnet-ex.c +++ b/runtime/src/comm/gasnet/comm-gasnet-ex.c @@ -714,7 +714,15 @@ int chpl_comm_addr_gettable(c_nodeid_t node, void* start, size_t len) int32_t chpl_comm_getMaxThreads(void) { - return GASNETI_MAX_THREADS-1; + uint64_t maxGasnetThreads = gex_System_QueryMaxThreads(); + if (maxGasnetThreads > INT32_MAX) { + if (chpl_nodeID == 0) { + chpl_warning("GASNet's maximum number of threads exceeds Chapel's " + "'int32_t' value, so capping it at INT32_MAX.", 0, 0); + } + maxGasnetThreads = INT32_MAX; + } + return maxGasnetThreads; } // diff --git a/runtime/src/tasks/qthreads/tasks-qthreads.c b/runtime/src/tasks/qthreads/tasks-qthreads.c index 568a21718103..6f05e0c3ae50 100644 --- a/runtime/src/tasks/qthreads/tasks-qthreads.c +++ b/runtime/src/tasks/qthreads/tasks-qthreads.c @@ -506,10 +506,11 @@ static void setupAvailableParallelism(int32_t maxThreads) { char msg[1024]; snprintf(msg, sizeof(msg), "The CHPL_COMM setting is limiting the number of threads " - "to %d, rather than the hardware's preference of %d. " - "If using CHPL_COMM=gasnet, set 'GASNET_MAX_THREADS' to " - "a higher number in your environment to address this.\n", - maxThreads, hwpar); + "to %d, rather than the hardware's preference of %d.%s", + maxThreads, hwpar, + (strcmp(CHPL_COMM, "gasnet") ? "" : + " To increase this limit, set 'GASNET_MAX_THREADS' to " + "a larger value in your environment.")); chpl_warning(msg, 0, 0); } hwpar = maxThreads; From 465e17357708cf6480563abb21961f13ae078bae Mon Sep 17 00:00:00 2001 From: Brad Chamberlain Date: Thu, 16 Jan 2025 15:19:49 -0800 Subject: [PATCH 4/5] Reserve a thread for the original thread (thanks to Dan for pointing this out) --- Signed-off-by: Brad Chamberlain --- runtime/src/comm/gasnet/comm-gasnet-ex.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/runtime/src/comm/gasnet/comm-gasnet-ex.c b/runtime/src/comm/gasnet/comm-gasnet-ex.c index 7c83ba9e30e7..85058435e710 100644 --- a/runtime/src/comm/gasnet/comm-gasnet-ex.c +++ b/runtime/src/comm/gasnet/comm-gasnet-ex.c @@ -722,7 +722,7 @@ int32_t chpl_comm_getMaxThreads(void) { } maxGasnetThreads = INT32_MAX; } - return maxGasnetThreads; + return maxGasnetThreads - 1; // reserve one thread for the primordial thread } // From ee208d37830121eadb4d34bda369bb29747f631a Mon Sep 17 00:00:00 2001 From: Brad Chamberlain Date: Thu, 16 Jan 2025 16:08:13 -0800 Subject: [PATCH 5/5] Update existing test designed to test tasking thread limits This test formerly tried to exceed our thread limits by setting the number of threads to 300, but that limit is now below the GASNet default soft limit of 1024, so here, I bumped the number of threads up to 2000 to exceed that limit again, and keep it testing the same things. --- Signed-off-by: Brad Chamberlain --- .../taskPool/figueroa/ManyThreads.comm-gasnet.tasks-fifo.good | 2 +- .../figueroa/ManyThreads.comm-gasnet.tasks-qthreads.good | 2 +- test/parallel/taskPool/figueroa/ManyThreads.execenv | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/test/parallel/taskPool/figueroa/ManyThreads.comm-gasnet.tasks-fifo.good b/test/parallel/taskPool/figueroa/ManyThreads.comm-gasnet.tasks-fifo.good index 6c4568669d9e..5563d86a2266 100644 --- a/test/parallel/taskPool/figueroa/ManyThreads.comm-gasnet.tasks-fifo.good +++ b/test/parallel/taskPool/figueroa/ManyThreads.comm-gasnet.tasks-fifo.good @@ -1,3 +1,3 @@ total is 4501500 -warning: CHPL_RT_NUM_THREADS_PER_LOCALE = 300 is too large; limit is NNNN +warning: CHPL_RT_NUM_THREADS_PER_LOCALE = 2000 is too large; limit is NNNN warning: Setting number of threads in CHPL_TASKS=fifo can lead to deadlock diff --git a/test/parallel/taskPool/figueroa/ManyThreads.comm-gasnet.tasks-qthreads.good b/test/parallel/taskPool/figueroa/ManyThreads.comm-gasnet.tasks-qthreads.good index 36cfd841f92a..5a3db7e073e1 100644 --- a/test/parallel/taskPool/figueroa/ManyThreads.comm-gasnet.tasks-qthreads.good +++ b/test/parallel/taskPool/figueroa/ManyThreads.comm-gasnet.tasks-qthreads.good @@ -1,3 +1,3 @@ total is 4501500 -warning: CHPL_RT_NUM_THREADS_PER_LOCALE = 300 is too large; limit is NNNN +warning: CHPL_RT_NUM_THREADS_PER_LOCALE = 2000 is too large; limit is NNNN warning: QTHREADS: Reduced numThreadsPerLocale=NNNN to NNNN to prevent oversubscription of the system. diff --git a/test/parallel/taskPool/figueroa/ManyThreads.execenv b/test/parallel/taskPool/figueroa/ManyThreads.execenv index 224f30ea399d..53593e2c4ef1 100644 --- a/test/parallel/taskPool/figueroa/ManyThreads.execenv +++ b/test/parallel/taskPool/figueroa/ManyThreads.execenv @@ -1,4 +1,4 @@ -CHPL_RT_NUM_THREADS_PER_LOCALE=300 +CHPL_RT_NUM_THREADS_PER_LOCALE=2000 CHPL_RT_NUM_THREADS_PER_LOCALE_QUIET=no CHPL_RT_CALL_STACK_SIZE=256K QT_GUARD_PAGES=false