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

Update forkrun.bash #53

Open
wants to merge 2 commits into
base: forkrun_testing
Choose a base branch
from
Open
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
103 changes: 46 additions & 57 deletions forkrun.bash
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ forkrun() {
shopt -s extglob

# make all variables local
local tmpDir fPath outStr delimiterVal delimiterReadStr delimiterRemoveStr exitTrapStr exitTrapStr_kill nLines0 nOrder nProcs nProcsMax nBytes tTimeout coprocSrcCode outCur tmpDirRoot returnVal tmpVar t0 readBytesProg nullDelimiterProg ddQuietStr trailingNullFlag inotifyFlag lseekFlag fallocateFlag nLinesAutoFlag nQueueFlag substituteStringFlag substituteStringIDFlag nOrderFlag readBytesFlag readBytesExactFlag nullDelimiterFlag subshellRunFlag stdinRunFlag pipeReadFlag rmTmpDirFlag exportOrderFlag noFuncFlag unescapeFlag optParseFlag continueFlag doneIndicatorFlag FORCE_allowCarriageReturnsFlag ddAvailableFlag fd_continue fd_inotify fd_inotify0 fd_nAuto fd_nAuto0 fd_nOrder fd_nOrder0 fd_read fd_read0 fd_write fd_stdout fd_stdin fd_stdin0 fd_stderr pWrite pOrder pAuto pQueue pWrite_PID pNotify_PID pOrder_PID pAuto_PID pQueue_PID fd_read_pos fd_read_pos_old fd_write_pos DEBUG_FORKRUN
local -i PID0 nLines nLinesCur nLinesNew nLinesMax nRead nWait nOrder0 nBytesRead nQueue nQueueLast nQueueMin nQueueLastCount nCPU v9 kkMax kkCur kk kkProcs verboseLevel pLOAD_max pAdd
local tmpDir fPath outStr delimiterVal delimiterReadStr delimiterRemoveStr exitTrapStr exitTrapStr_kill nLines nLines0 nLinesMax nQueueMin nOrder nProcs nProcsMax nBytes tTimeout coprocSrcCode outCur tmpDirRoot returnVal tmpVar t0 readBytesProg nullDelimiterProg ddQuietStr trailingNullFlag inotifyFlag lseekFlag fallocateFlag nLinesAutoFlag nQueueFlag substituteStringFlag substituteStringIDFlag nOrderFlag readBytesFlag readBytesExactFlag nullDelimiterFlag subshellRunFlag stdinRunFlag pipeReadFlag rmTmpDirFlag exportOrderFlag noFuncFlag unescapeFlag optParseFlag continueFlag doneIndicatorFlag FORCE_allowCarriageReturnsFlag ddAvailableFlag fd_continue fd_inotify fd_inotify0 fd_nAuto fd_nAuto0 fd_nOrder fd_nOrder0 fd_read fd_read0 fd_write fd_stdout fd_stdin fd_stdin0 fd_stderr pWrite pOrder pAuto pQueue pWrite_PID pNotify_PID pOrder_PID pAuto_PID pQueue_PID fd_read_pos fd_read_pos_old fd_write_pos DEBUG_FORKRUN
local -i PID0 nLinesCur nLinesNew nRead nWait nOrder0 nBytesRead nQueue nQueueLast nQueueLastCount nCPU v9 kkMax kkCur kk kkProcs verboseLevel pLOAD_max pAdd
local -a A p_PID runCmd outHave outPrint pLOADA
local -A pMap

Expand Down Expand Up @@ -73,7 +73,7 @@ forkrun() {
continue
fi
if [[ "${nLines0}" == +([0-9])','+([0-9]) ]]; then
nLinesMax="${nLines0##*,}"
nLinesMax="$(_forkrun_getVal "${nLines0##*,}")"
nLines="${nLines0%%,*}"
else
nLines="${nLines0}"
Expand Down Expand Up @@ -307,22 +307,13 @@ forkrun() {
nBytes="${nBytes,,}"
nBytes="${nBytes//' '/}"

[[ "${nBytes}" == +([0-9])?([KkMmGgTtPp])?(i)?([Bb]),+([0-9])?(.+([0-9])) ]] && {
[[ "${nBytes}" == +([0-9])?([kmgtpezyrq])?(i)?([b]),+([0-9])?(.+([0-9])) ]] && {
tTimeout="${nBytes##*,}"
[[ "${tTimeout}" == +([0-9]).*([0-9]) ]] && { tTimeout="${tTimeout%%.*}"; ((tTimeout++)); }
nBytes="${nBytes%,*}"
}

nBytes="${nBytes%b}"
[[ "${nBytes}" == +([0-9])@([kmgtp])?(i) ]] && {
local -A nBytesParser=([k]=1 [m]=2 [g]=3 [t]=4 [p]=5)

if [[ ${nBytes: -1:1} == 'i' ]]; then
nBytes="$(( ${nBytes%[kmgtp]i} * ( 1024 ** ${nBytesParser[${nBytes: -2:1}]} ) ))"
else
nBytes="$(( ${nBytes%[kmgtp]} * ( 1000 ** ${nBytesParser[${nBytes: -1:1}]} ) ))"
fi
}
nBytes="$(_forkrun_getVal "${nBytes}")"

# make sure nBytes is only digits
[[ "${nBytes//[0-9]/}" ]] && (( ${verboseLevel} >= 0 )) && {
Expand Down Expand Up @@ -367,7 +358,7 @@ forkrun() {
else
# set batch size
{ [[ ${nLines} ]] && (( ${nLines} > 0 )) && : "${nLinesAutoFlag:=false}"; } || : "${nLinesAutoFlag:=true}"
{ [[ -z ${nLines} ]] || [[ ${nLines} == 0 ]]; } && nLines=1
{ [[ -z ${nLines} ]] || [[ ${nLines} == 0 ]]; } && nLines=1 || nLines="$(_forkrun_getVal "${nLines}")"
fi

# set number of coproc workers and (if enabled) minimim worker read queue length
Expand All @@ -379,16 +370,17 @@ forkrun() {
[[ "${nProcs}" == *','* ]] && {
: "${nQueueFlag:=true}"
nProcsMax="${nProcs#*,}"
nProcs="${nProcs%%,*}"
nProcs="$(_forkrun_getVal "${nProcs%%,*}")"
[[ "${nProcsMax}" == *','* ]] && {
nQueueMin="${nProcsMax#*,}"
nProcsMax="${nProcsMax%%,*}"
nQueueMin="$(_forkrun_getVal "${nProcsMax#*,}")"
nProcsMax="$(_forkrun_getVal "${nProcsMax%%,*}")"
}
}

: "${nQueueFlag:=false}" "${nQueueMin:=1}"

local -i nProcs="${nProcs}" nProcsMax="${nProcsMax}"
local -i nProcs="${nProcs}" nProcsMax="${nProcsMax}" nQueueMin="${nQueueMin}" nLines="${nLines}" nLinesMax="${nLinesMax}"

nCPU="$({ type -a nproc &>/dev/null && nproc; } || { type -a grep &>/dev/null && grep -cE '^processor.*: ' /proc/cpuinfo; } || { mapfile -t tmpA </proc/cpuinfo && tmpA=("${tmpA[@]//processor*/$'\034'}") && tmpA=("${tmpA[@]//!($'\034')/}") && tmpA=("${tmpA[@]//$'\034'/1}") && tmpA="${tmpA[*]}" && tmpA="${tmpA// /}" && echo ${#tmpA}; } || printf '8')";
{ [[ ${nProcs} ]] && (( ${nProcs:-0} > 0 )); } || { ${nQueueFlag} && nProcs=$(( ${nCPU} / 2 )) || nProcs=${nCPU}; }

Expand Down Expand Up @@ -1064,18 +1056,18 @@ p_PID+=(\${p{<#>}_PID})""" )"
kkProcs=${nProcs}

p_PID=()
pLOADA=()
pLOADA=()

nQueue=0
nQueueLastCount=0
nQueueLastCount=0

(( "${nQueueMin}" <= 0 )) && nQueueMin=1

: "${pLOAD_max:=9500}" "${nProcsMax:=$((2*${nCPU}))}" "${nQueueLastCountGoal:=5}"

mapfile -t pLOADA < <(_forkrun_get_load -i)

(( ${verboseLevel} > 2 )) && printf 'pLOADA = ( %s %s %s %s )\n' "${pLOADA[@]}" >&${fd_stderr}
(( ${verboseLevel} > 2 )) && printf 'pLOADA = ( %s %s %s %s )\n' "${pLOADA[@]}" >&${fd_stderr}

until [[ -f "${tmpDir}"/.quit ]] || (( ${kkProcs} >= ${nProcsMax} )); do
nQueueLast=${nQueue}
Expand All @@ -1097,14 +1089,14 @@ p_PID+=(\${p{<#>}_PID})""" )"

if (( ( ${nQueue} + ${nQueueLast} ) < ( 2 * ${nQueueMin} ) )); then

if (( ${nQueueLastCount} < ( ${nQueueLastCountGoal} * ( 1 + ( kkProcs / nCPU ) ) ) )); then
if (( ${nQueueLastCount} < ( ${nQueueLastCountGoal} * ( 1 + ( kkProcs / nCPU ) ) ) )); then
((nQueueLastCount++))
else
nQueueLastCount=0

mapfile -t pLOADA < <(_forkrun_get_load "${pLOADA[@]}")

(( ${verboseLevel} > 2 )) && printf 'pLOADA = ( %s %s %s %s )\n' "${pLOADA[@]}" >&${fd_stderr}
(( ${verboseLevel} > 2 )) && printf 'pLOADA = ( %s %s %s %s )\n' "${pLOADA[@]}" >&${fd_stderr}

(( ${pLOADA} >= ${pLOAD_max} )) || {

Expand Down Expand Up @@ -1133,7 +1125,7 @@ p_PID+=(\${p{<#>}_PID})""" )"
}
fi
else
nQueueLastCount=0
nQueueLastCount=0
fi

done
Expand Down Expand Up @@ -1804,13 +1796,13 @@ _forkrun_get_load() (
}
;;
[0-9]*)
case "${argCount}" in
0) [[ ${1} == 0 ]] && pLOAD0=1 || pLOAD0="${1}" ;;
1) cpu_ALL0="${1}" ;;
2) cpu_LOAD0="${1}" ;;
3) tALL0="${1}" ;;
esac
((argCount++))
case "${argCount}" in
0) [[ ${1} == 0 ]] && pLOAD0=1 || pLOAD0="${1}" ;;
1) cpu_ALL0="${1}" ;;
2) cpu_LOAD0="${1}" ;;
3) tALL0="${1}" ;;
esac
((argCount++))
;;
esac
shift 1
Expand All @@ -1826,10 +1818,10 @@ _forkrun_get_load() (
cpu_ALL=$(( cpu_LOAD + cpu_idle + cpu_IOwait ))

${initFlag} && {
cpu_ALL0="${cpu_ALL}"
cpu_ALL0="${cpu_ALL}"
cpu_LOAD0="${cpu_LOAD}"

( read -r -u $fd_sleep -t 0.01; ) {fd_sleep}<><(:)
( read -r -u $fd_sleep -t 0.01; ) {fd_sleep}<><(:)

read -r _ cpu_user cpu_nice cpu_system cpu_idle cpu_IOwait cpu_irq cpu_softirq cpu_steal cpu_guest cpu_guestnice </proc/stat

Expand Down Expand Up @@ -1858,32 +1850,29 @@ _forkrun_get_load() (
)

_forkrun_getVal() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue (complexity): Consider simplifying prefix handling by using a lookup table with pre-computed values instead of runtime calculations

The prefix handling can be simplified using a lookup table approach while preserving all functionality. Here's a suggested implementation:

_forkrun_getVal() {
    local -A multipliers=(
        [k]=1024  [kb]=1024  [kib]=1024
        [m]=1048576  [mb]=1048576  [mib]=1048576
        [+k]=1000  [+kb]=1000  [+kib]=1024
        [+m]=1000000  [+mb]=1000000  [+mib]=1048576
        # Add other prefixes similarly...
    )

    local nn
    for nn in "${@,,}"; do
        nn="${nn%[b]}"
        local base="${nn//[0-9]/}"
        local num="${nn//[^0-9]/}"
        echo "$((num * ${multipliers[$base]:-1}))"
    done
}

This approach:

  1. Uses a single lookup table with pre-computed values
  2. Eliminates complex string manipulations and case statements
  3. Preserves all functionality including IEC/SI prefix handling
  4. Maintains backward compatibility
  5. Makes the behavior more explicit and easier to maintain

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue (complexity): Consider simplifying the _forkrun_getVal function by using a more structured approach with clear variable naming and separation of concerns.

The _forkrun_getVal function has become unnecessarily complex. Here's a cleaner approach that maintains all functionality:

_forkrun_getVal() {
    # Handles both IEC (1024^N) and SI (1000^N) prefixes
    # Examples: 1K=1000, 1KiB=1024, +1K=1024
    local -l input  # Force lowercase
    local base multiplier prefix

    for input in "${@%%[Bb]*}"; do  # Strip trailing B/b
        input="${input// /}"  # Remove spaces

        # Extract numeric part and prefix
        local num="${input//[^0-9]/}"
        prefix="${input##*[0-9]}"

        # Determine base (1000 vs 1024)
        if [[ "$input" == *i* ]] || [[ "$input" == +* ]]; then
            base=1024
            multiplier=${pMap[${prefix%i}]:-0}  # Strip 'i' from prefix
        else
            base=1000
            multiplier=${pMap[${prefix}]:-0}
        fi

        # Calculate and output
        if ((base == 1024)); then
            printf '%s\n' "$((num << (10 * multiplier)))"
        else
            printf '%s\n' "$((num * (base ** multiplier)))"
        fi
    done
}

This version:

  1. Clearly separates IEC vs SI prefix handling
  2. Uses descriptive variable names
  3. Has a more logical flow with input normalization first
  4. Maintains all edge case handling
  5. Uses simpler pattern matching

## expands IEC and SI prefixed to get the numeric value they represent
#
# IEC prefixes (1024^N) are used by default are used to be consistent with other linux tools
# SI prefixes (1000^N) can be used by adding a '+' to the start of the number
#
# neither capatalization nor trailing -i's and -b's have any effect
# 1k = 1K = 1kb = 1KB = 1kib = 1KiB = 1024
# +1k = +1K = +1kb = +1KB = +1kib = +1KiB = 1000

local +i nn
local -AI pMap

(( ${#pMap[@]} > 0 )) || pMap=([k]=1 [m]=2 [g]=3 [t]=4 [p]=5 [e]=6 [z]=7 [y]=8 [r]=9 [q]=10)
## Expands IEC and SI prefixes to get the numeric value they represent
#
# IEC PREFIC (1024^N) is used if the prefix has a trailing '-i' (Ki/Mi/Gi). This is is the case without exception.
# SI PREFIX (1000^N) is used if the prefix is a single letter (K/M/G/...), UNLESS the number is prefaced with a '+'.
#
# NOTE: neither capatalization nor a trailing -b/-B have any effect. full word prefixes (e.g., '1 kilobyte') are not supported.
#
# PARSING EXAMPLES:
# +1k = +1K = +1kb = +1KB = 1kib = 1KiB = +1kib = +1KiB = 1024
# 1k = 1K = 1kb = 1KB = 1000

local +i -l nn

(( ${#pMap[@]} == 20 )) || local -Ag pMap=([k]=1 [m]=2 [g]=3 [t]=4 [p]=5 [e]=6 [z]=7 [y]=8 [r]=9 [q]=10 [ki]=1 [mi]=2 [gi]=3 [ti]=4 [pi]=5 [ei]=6 [zi]=7 [yi]=8 [ri]=9 [qi]=10)

for nn in "${@,,}"; do
nn="${nn%b}";
[[ "${nn:0:1}${nn: -1}" == '+i' ]] && nn="${nn:1}"
nn="${nn%i}";

case "${nn:0:1}" in
'+')
echo "$(( ${nn//[^0-9]/} * ( 1000 ** ${pMap[${nn: -1}]:-0} ) ))"
for nn in "${@%%[Bb]*}"; do
case "${nn// /}" in
*'i'|'+'*)
printf '%s\n' "$(( ${nn//[^0-9]/} << ( 10 * ${pMap[${nn##*[0-9]}]:-0} ) ))"
;;
*)
echo "$(( ${nn//[^0-9]/} << ( 10 * ${pMap[${nn: -1}]:-0} ) ))"
printf '%s\n' "$(( ${nn//[^0-9]/} * ( 1000 ** ${pMap[${nn: -1}]:-0} ) ))"
;;
esac
done
}
}