ufs-community / ufs-srweather-app

UFS Short-Range Weather Application
Other
53 stars 114 forks source link

`create_symlink_to_file.sh` is unreasonably slow #1066

Closed mkavulich closed 3 months ago

mkavulich commented 3 months ago

Expected behavior

create_symlink_to_file.sh is meant to create a simple symlink, which should take a trivial number of compute cycles, even with additional checks for file existence that might need to be made.

Current behavior

create_symlink_to_file.sh takes a half second to more than 1 second, depending on the platform. This is a very long amount of time for such a trivial operation. Even on platforms where it takes on the order of half a second, this is still far too long for something that should take microseconds. For complicated workflow tasks that involve linking many files (such as run_fcst), this can result in more than a minute of wasted compute cycles.

Machines affected

It seems as if the bulk overhead from the create_symlink_to_file.sh runs quite slow on all platforms. The ones I have tested are:

Derecho:

+ create_symlink_to_file target=/glade/derecho/scratch/kavulich/FIRE/2024/march_updates/updated_hashes/expt_dirs/fire_test_case/field_table symlink=/glade/derecho/scratch/kavulich/FIRE/2024/march_updates/updated_hashes/expt_dirs/fire_test_case/2020081318//field_table relative=TRUE

real    0m1.563s
user    0m0.100s
sys     0m1.475s

Hera:

real    0m0.673s
user    0m0.107s
sys 0m0.561s

Orion:

real    0m0.345s
user    0m0.079s
sys     0m0.273s

Jet:

real    0m0.585s
user    0m0.078s
sys     0m0.509s

It also seems to be very inconsistent, depending on system-level factors I guess? When I re-ran a job on Orion, which was previously the fastest machine, it ended up being the second-slowest:

real    0m0.874s
user    0m0.159s
sys     0m0.668s

Steps To Reproduce

  1. Modify any test script to include the time command with a call to create_symlink_to_file.sh
  2. Observe symlink times that take 0.4-1.5 seconds to complete

Detailed Description of Fix (optional)

The biggest change that is badly needed is using the native UNIX ln function rather than the duplicate, broken ln_vrfy (see #861).

Additionally, create_symlink_to_file.sh can be simplified greatly to have simple positional arguments rather than named arguments (that require loading another function to parse), remove all of the overhead of saving/re-loading the environment, loading other unnecessary functions, and checking valid values of boolean inputs. Making all of these simplifications (diffs attached) reduced the time spent linking files by an order of magnitude:

real    0m0.062s
user    0m0.014s
sys 0m0.041s

Possible Implementation

Note that this change would require modifications of all scripts that call this function to use positional arguments rather than named arguments. But given that this increases the speed by an order of magnitude, I believe it is worth it.

diff --git a/ush/bash_utils/create_symlink_to_file.sh b/ush/bash_utils/create_symlink_to_file.sh
index 38099ff..94ee75c 100644
--- a/ush/bash_utils/create_symlink_to_file.sh
+++ b/ush/bash_utils/create_symlink_to_file.sh
@@ -16,78 +16,21 @@ function create_symlink_to_file() {
 #
 #-----------------------------------------------------------------------
 #
-# Save current shell options (in a global array).  Then set new options
-# for this script/function.
-#
-#-----------------------------------------------------------------------
-#
-  { save_shell_opts; . ${USHdir}/preamble.sh; } > /dev/null 2>&1
-#
-#-----------------------------------------------------------------------
-#
-# Get the full path to the file in which this script/function is located 
-# (scrfunc_fp), the name of that file (scrfunc_fn), and the directory in
-# which the file is located (scrfunc_dir).
-#
-#-----------------------------------------------------------------------
-#
-  local scrfunc_fp=$( $READLINK -f "${BASH_SOURCE[0]}" )
-  local scrfunc_fn=$( basename "${scrfunc_fp}" )
-  local scrfunc_dir=$( dirname "${scrfunc_fp}" )
-#
-#-----------------------------------------------------------------------
-#
-# Get the name of this function.
-#
-#-----------------------------------------------------------------------
-#
-  local func_name="${FUNCNAME[0]}"
-#
-#-----------------------------------------------------------------------
-#
 # Specify the set of valid argument names for this script/function.  Then
 # process the arguments provided to this script/function (which should
 # consist of a set of name-value pairs of the form arg1="value1", etc).
 #
 #-----------------------------------------------------------------------
 #
-  local valid_args=( \
-"target" \
-"symlink" \
-"relative" \
-  )
-  process_args valid_args "$@"
-#
-#-----------------------------------------------------------------------
-#
-# For debugging purposes, print out values of arguments passed to this
-# script.  Note that these will be printed out only if VERBOSE is set to
-# TRUE.
-#
-#-----------------------------------------------------------------------
-#
-  print_input_args valid_args
-#
-#-----------------------------------------------------------------------
-#
-# Verify that the required arguments to this function have been specified.
-# If not, print out an error message and exit.
-#
-#-----------------------------------------------------------------------
-#
-  if [ -z "${target}" ]; then
-    print_err_msg_exit "\
-The argument \"target\" specifying the target of the symbolic link that
-this function will create was not specified in the call to this function:
-  target = \"$target\""
-  fi

-  if [ -z "${symlink}" ]; then
-    print_err_msg_exit "\
-The argument \"symlink\" specifying the symbolic link that this function
-will create was not specified in the call to this function:
-  symlink = \"$symlink\""
-  fi
+if [[ $# -lt 2 ]]; then
+  usage
+  print_err_msg_exit "Function create_symlink_to_file() requires at least two arguments"
+fi
+
+target=$1
+symlink=$2
+relative=${3:-TRUE}
 #
 #-----------------------------------------------------------------------
 #
@@ -106,8 +49,6 @@ will create was not specified in the call to this function:
 #
 #-----------------------------------------------------------------------
 #
-  relative=${relative:-"TRUE"}
-
   valid_vals_relative=("TRUE" "true" "YES" "yes" "FALSE" "false" "NO" "no")
   check_var_valid_value "relative" "valid_vals_relative"
 #
@@ -148,16 +89,7 @@ not exist or is not a file:
 #
 #-----------------------------------------------------------------------
 #
-  ln_vrfy -sf ${relative_flag} "$target" "$symlink"
-#
-#-----------------------------------------------------------------------
-#
-# Restore the shell options saved at the beginning of this script/func-
-# tion.
-#
-#-----------------------------------------------------------------------
-#
-  { restore_shell_opts; } > /dev/null 2>&1
+  ln -sf ${relative_flag} "$target" "$symlink"

 }