From 7987dd1757a93dad8d706d533f5026cc6037fd49 Mon Sep 17 00:00:00 2001 From: Chris Down Date: Fri, 6 Jan 2017 10:48:02 +0000 Subject: [PATCH 1/7] Add test-perf script --- test/test-perf | 90 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 90 insertions(+) create mode 100755 test/test-perf diff --git a/test/test-perf b/test/test-perf new file mode 100755 index 0000000..098d396 --- /dev/null +++ b/test/test-perf @@ -0,0 +1,90 @@ +#!/bin/bash + +msg() { + printf '>>> %s\n' "$@" >&2 +} + +dir=/tmp/clipmenu.$USER +cache_file=$dir/line_cache + +log=$(mktemp) +tim=$(mktemp) +clipmenu_shim=$(mktemp) +num_files=1500 + +trap 'rm -f -- "$log" "$tim" "$clipmenu_shim"' EXIT + +if [[ $0 == /* ]]; then + location=${0%/*} +else + location=$PWD/${0#./} + location=${location%/*} +fi + +msg 'Setting up edited clipmenu' + +cat - "$location/../clipmenu" > /tmp/clipmenu << EOF +#!/bin/bash + +exec 3>&2 2> >(tee "$log" | + sed -u 's/^.*$/now/' | + date -f - +%s.%N > "$tim") +set -x + +shopt -s expand_aliases + +alias dmenu=: +alias xsel=: +alias xclip=: + +EOF + +chmod a+x /tmp/clipmenu + +if ! (( NO_RECREATE )); then + rm -rf "$dir" + mkdir -p "$dir" + + msg "Writing $num_files clipboard files" + + for (( i = 0; i <= num_files; i++ )); do + (( i % 100 )) || printf '%s... ' "$i" + + line_len=$(( (RANDOM % 10000) + 1 )) + num_lines=$(( (RANDOM % 10) + 1 )) + fn=$dir/$(LC_ALL=C date +%F-%T.%N) + data=$( + tr -dc 'a-zA-Z0-9' < /dev/urandom | + fold -w "$line_len" | + head -"$num_lines" + ) + printf '%s' "$data" > "$fn" + read -r first_line <<< "$data" + printf '%s|%s (%s lines)\n' \ + "$fn" "$first_line" "$num_lines" >> "$cache_file" + done + + printf 'done\n' +else + msg 'Not nuking/creating new clipmenu files' +fi + +msg 'Running modified clipmenu' + +time /tmp/clipmenu + +(( TIME_ONLY )) && exit 0 + +msg 'Displaying perf data' + +# modified from http://stackoverflow.com/a/20855353/945780 +paste <( + while read -r tim ;do + [ -z "$last" ] && last=${tim//.} && first=${tim//.} + crt=000000000$((${tim//.}-10#0$last)) + ctot=000000000$((${tim//.}-10#0$first)) + printf "%12.9f %12.9f\n" ${crt:0:${#crt}-9}.${crt:${#crt}-9} \ + ${ctot:0:${#ctot}-9}.${ctot:${#ctot}-9} + last=${tim//.} + done < "$tim" +) "$log" | less From e8ba60e43228311b6da89245c49944405ae36a86 Mon Sep 17 00:00:00 2001 From: Chris Down Date: Fri, 6 Jan 2017 12:03:13 +0000 Subject: [PATCH 2/7] Have clipmenud cache first lines --- clipmenu | 29 +++++------------------------ clipmenud | 36 ++++++++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 24 deletions(-) diff --git a/clipmenu b/clipmenu index 8bb79ff..90cbebf 100755 --- a/clipmenu +++ b/clipmenu @@ -5,35 +5,16 @@ shopt -s nullglob # We use this to make sure the cache files are sorted bytewise LC_COLLATE=C -# Some people copy/paste huge swathes of text that could slow down dmenu -line_length_limit=500 - declare -A selections ordered_selections=() -files=("/tmp/clipmenu.$USER/"*) - -# We can't use `for ... in` here because we need to add files to -# ordered_selections from last to first -- that is, newest to oldest. Incoming -# clipboard entries have a ISO datetime prefixed to the front to aid in this. -for (( i=${#files[@]}-1; i>=0; i-- )); do - file=${files[$i]} - - # We look for the first line matching regex /./ here because we want the - # first line that can provide reasonable context to the user. That is, if - # you have 5 leading lines of whitespace, displaying " (6 lines)" is much - # less useful than displaying "foo (6 lines)", where "foo" is the first - # line in the entry with actionable context. - first_line=$(sed -n '/./{p;q}' "$file" | cut -c1-"$line_length_limit") - lines=$(wc -l < "$file") - - if (( lines > 1 )); then - first_line+=" ($lines lines)" - fi +cache_file=/tmp/clipmenu.$USER/line_cache +# We use tac since we want newest to oldest, and we append in clipmenud +while IFS='|' read -r full_file first_line; do ordered_selections+=("$first_line") - selections[$first_line]=$file -done + selections[$first_line]=$full_file +done < <(tac "$cache_file") # It's okay to hardcode `-l 8` here as a sensible default without checking # whether `-l` is also in "$@", because the way that dmenu works allows a later diff --git a/clipmenud b/clipmenud index c6c0fb3..3274291 100755 --- a/clipmenud +++ b/clipmenud @@ -1,5 +1,36 @@ #!/bin/bash +get_first_line() { + # Args: + # - $1, the file or data + # - $2, optional, the line length limit + + data=${1?} + line_length_limit=${2-300} + + # We look for the first line matching regex /./ here because we want the + # first line that can provide reasonable context to the user. That is, if + # you have 5 leading lines of whitespace, displaying " (6 lines)" is much + # less useful than displaying "foo (6 lines)", where "foo" is the first + # line in the entry with actionable context. + awk -v limit="$line_length_limit" ' + BEGIN { printed = 0; } + + printed == 0 && NF { + $0 = substr($0, 0, limit); + printf("%s", $0); + printed = 1; + } + + END { + if (NR > 1) { + print " (" NR " lines)"; + } else { + printf("\n"); + } + }' <<< "$data" +} + hr_msg() { printf -- '\n--- %s ---\n\n' "$1" >&2 } @@ -38,6 +69,7 @@ print_debug_info() { print_debug_info 'Initialising' cache_dir=/tmp/clipmenu.$USER/ +cache_file=$cache_dir/line_cache # It's ok that this only applies to the final directory. # shellcheck disable=SC2174 @@ -96,6 +128,10 @@ while sleep "${CLIPMENUD_SLEEP:-0.5}"; do debug "Writing $data to $filename" printf '%s' "$data" > "$filename" + debug "Writing new entry to $cache_file" + printf '%s|%s\n' \ + "$filename" "$(get_first_line "$data")" >> "$cache_file" + if ! (( NO_OWN_CLIPBOARD )) && [[ $selection != primary ]]; then # Take ownership of the clipboard, in case the original application # is unable to serve the clipboard request (due to being suspended, From f0fe96955af9bb0bacea06b93752fe51c72d3024 Mon Sep 17 00:00:00 2001 From: Chris Down Date: Fri, 6 Jan 2017 12:52:34 +0000 Subject: [PATCH 3/7] perf: Don't use printf with ordered_selections printf is really, really slow with large arrays of strings. Switching to this results in about a 50% performance improvement. --- clipmenu | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/clipmenu b/clipmenu index 90cbebf..4bdffe6 100755 --- a/clipmenu +++ b/clipmenu @@ -6,13 +6,11 @@ shopt -s nullglob LC_COLLATE=C declare -A selections -ordered_selections=() cache_file=/tmp/clipmenu.$USER/line_cache # We use tac since we want newest to oldest, and we append in clipmenud while IFS='|' read -r full_file first_line; do - ordered_selections+=("$first_line") selections[$first_line]=$full_file done < <(tac "$cache_file") @@ -20,7 +18,7 @@ done < <(tac "$cache_file") # whether `-l` is also in "$@", because the way that dmenu works allows a later # argument to override an earlier one. That is, if the user passes in `-l`, our # one will be ignored. -chosen_line=$(printf '%s\n' "${ordered_selections[@]}" | uniq | dmenu -l 8 "$@") +chosen_line=$(sed 's/^[^|]\+|//' "$cache_file" | tac | uniq | dmenu -l 8 "$@") [[ $chosen_line ]] || exit 1 From 6edf3c437bb9bafb41e0e7383ff4def6a794cd13 Mon Sep 17 00:00:00 2001 From: Chris Down Date: Fri, 6 Jan 2017 14:02:47 +0000 Subject: [PATCH 4/7] perf: Add naive but performant path --- clipmenu | 35 +++++++++++++++++++++++++---------- 1 file changed, 25 insertions(+), 10 deletions(-) diff --git a/clipmenu b/clipmenu index 4bdffe6..3d0d579 100755 --- a/clipmenu +++ b/clipmenu @@ -3,17 +3,10 @@ shopt -s nullglob # We use this to make sure the cache files are sorted bytewise -LC_COLLATE=C - -declare -A selections +export LC_COLLATE=C cache_file=/tmp/clipmenu.$USER/line_cache -# We use tac since we want newest to oldest, and we append in clipmenud -while IFS='|' read -r full_file first_line; do - selections[$first_line]=$full_file -done < <(tac "$cache_file") - # It's okay to hardcode `-l 8` here as a sensible default without checking # whether `-l` is also in "$@", because the way that dmenu works allows a later # argument to override an earlier one. That is, if the user passes in `-l`, our @@ -22,10 +15,32 @@ chosen_line=$(sed 's/^[^|]\+|//' "$cache_file" | tac | uniq | dmenu -l 8 "$@") [[ $chosen_line ]] || exit 1 +# Naive, but performant path, only follow expensive path if it doesn't work +out_line=$(grep -F "|$chosen_line" "$cache_file") +out_length=$(wc -l <<< "$out_line") + +if (( out_length == 1 )); then + # Cheap path succeded + file=${out_line%%|*} +elif (( out_length > 1 )); then + # Cheap path failed + while IFS='|' read -r full_file first_line; do + if [[ $first_line == "$chosen_line" ]]; then + file=$full_file + break + fi + done <<< "$out_line" +else + # We didn't find this in cache + printf 'FATAL: %s not in cache, run clipmenu-fsck\n' "$chosen_line" >&2 + exit 2 +fi + + for selection in clipboard primary; do if type -p xsel >/dev/null 2>&1; then - xsel -i --"$selection" < "${selections[$chosen_line]}" + xsel -i --"$selection" < "$file" else - xclip -sel "$selection" < "${selections[$chosen_line]}" + xclip -sel "$selection" < "$file" fi done From 8885306970fb0e5ef16620451b9fdd7061db2de6 Mon Sep 17 00:00:00 2001 From: Chris Down Date: Fri, 6 Jan 2017 14:53:59 +0000 Subject: [PATCH 5/7] perf: Don't use date, use CRC instead We don't need to sort by date now that we record in $cache_file, which does it naturally for us. --- clipmenu | 25 +++++-------------------- clipmenud | 8 ++++---- test/test-perf | 8 ++++---- 3 files changed, 13 insertions(+), 28 deletions(-) diff --git a/clipmenu b/clipmenu index 3d0d579..5105b7b 100755 --- a/clipmenu +++ b/clipmenu @@ -2,35 +2,20 @@ shopt -s nullglob -# We use this to make sure the cache files are sorted bytewise -export LC_COLLATE=C - -cache_file=/tmp/clipmenu.$USER/line_cache +cache_dir=/tmp/clipmenu.$USER +cache_file=$cache_dir/line_cache # It's okay to hardcode `-l 8` here as a sensible default without checking # whether `-l` is also in "$@", because the way that dmenu works allows a later # argument to override an earlier one. That is, if the user passes in `-l`, our # one will be ignored. -chosen_line=$(sed 's/^[^|]\+|//' "$cache_file" | tac | uniq | dmenu -l 8 "$@") +chosen_line=$(tac "$cache_file" | uniq | dmenu -l 8 "$@") [[ $chosen_line ]] || exit 1 -# Naive, but performant path, only follow expensive path if it doesn't work -out_line=$(grep -F "|$chosen_line" "$cache_file") -out_length=$(wc -l <<< "$out_line") +file=$cache_dir/$(cksum <<< "$chosen_line") -if (( out_length == 1 )); then - # Cheap path succeded - file=${out_line%%|*} -elif (( out_length > 1 )); then - # Cheap path failed - while IFS='|' read -r full_file first_line; do - if [[ $first_line == "$chosen_line" ]]; then - file=$full_file - break - fi - done <<< "$out_line" -else +if ! [[ -f "$file" ]]; then # We didn't find this in cache printf 'FATAL: %s not in cache, run clipmenu-fsck\n' "$chosen_line" >&2 exit 2 diff --git a/clipmenud b/clipmenud index 3274291..b3d0b6b 100755 --- a/clipmenud +++ b/clipmenud @@ -120,17 +120,17 @@ while sleep "${CLIPMENUD_SLEEP:-0.5}"; do rm -- "${last_filename[$selection]}" fi - filename="$cache_dir/$(LC_ALL=C date +%F-%T.%N)" last_data[$selection]=$data last_filename[$selection]=$filename + first_line=$(get_first_line "$data") + filename="$cache_dir/$(cksum <<< "$first_line")" debug "Writing $data to $filename" printf '%s' "$data" > "$filename" - debug "Writing new entry to $cache_file" - printf '%s|%s\n' \ - "$filename" "$(get_first_line "$data")" >> "$cache_file" + debug "Writing $first_line to $cache_file" + printf '%s\n' "$first_line" >> "$cache_file" if ! (( NO_OWN_CLIPBOARD )) && [[ $selection != primary ]]; then # Take ownership of the clipboard, in case the original application diff --git a/test/test-perf b/test/test-perf index 098d396..fca39ac 100755 --- a/test/test-perf +++ b/test/test-perf @@ -52,16 +52,16 @@ if ! (( NO_RECREATE )); then line_len=$(( (RANDOM % 10000) + 1 )) num_lines=$(( (RANDOM % 10) + 1 )) - fn=$dir/$(LC_ALL=C date +%F-%T.%N) data=$( tr -dc 'a-zA-Z0-9' < /dev/urandom | fold -w "$line_len" | head -"$num_lines" ) + read -r first_line_raw <<< "$data" + printf -v first_line '%s (%s lines)\n' "$first_line_raw" "$num_lines" + printf '%s' "$first_line" >> "$cache_file" + fn=$dir/$(cksum <<< "$first_line") printf '%s' "$data" > "$fn" - read -r first_line <<< "$data" - printf '%s|%s (%s lines)\n' \ - "$fn" "$first_line" "$num_lines" >> "$cache_file" done printf 'done\n' From 1daaeda6f3aca6f9f8e5b3196314442fb74fae14 Mon Sep 17 00:00:00 2001 From: Chris Down Date: Fri, 6 Jan 2017 16:46:49 +0000 Subject: [PATCH 6/7] Remove print_debug_info, this was never very useful --- clipmenu | 1 - clipmenud | 35 ----------------------------------- 2 files changed, 36 deletions(-) diff --git a/clipmenu b/clipmenu index 5105b7b..390d2a5 100755 --- a/clipmenu +++ b/clipmenu @@ -21,7 +21,6 @@ if ! [[ -f "$file" ]]; then exit 2 fi - for selection in clipboard primary; do if type -p xsel >/dev/null 2>&1; then xsel -i --"$selection" < "$file" diff --git a/clipmenud b/clipmenud index b3d0b6b..d66ad98 100755 --- a/clipmenud +++ b/clipmenud @@ -31,43 +31,12 @@ get_first_line() { }' <<< "$data" } -hr_msg() { - printf -- '\n--- %s ---\n\n' "$1" >&2 -} - debug() { if (( DEBUG )); then printf '%s\n' "$@" >&2 fi } -print_debug_info() { - # DEBUG comes from the environment - if ! (( DEBUG )); then - return - fi - - local msg="${1?}" - - hr_msg "$msg" - - hr_msg Environment - env | LC_ALL=C sort >&2 - - cgroup_path=/proc/$$/cgroup - - if [[ -f $cgroup_path ]]; then - hr_msg cgroup - cat "$cgroup_path" >&2 - else - hr_msg 'NO CGROUP' - fi - - hr_msg 'Finished debug info' -} - -print_debug_info 'Initialising' - cache_dir=/tmp/clipmenu.$USER/ cache_file=$cache_dir/line_cache @@ -79,11 +48,8 @@ declare -A last_data declare -A last_filename while sleep "${CLIPMENUD_SLEEP:-0.5}"; do - print_debug_info 'About to run selection' for selection in clipboard primary; do - print_debug_info "About to do selection for '$selection'" - if type -p xsel >/dev/null 2>&1; then debug 'Using xsel' data=$(xsel -o --"$selection"; printf x) @@ -120,7 +86,6 @@ while sleep "${CLIPMENUD_SLEEP:-0.5}"; do rm -- "${last_filename[$selection]}" fi - last_data[$selection]=$data last_filename[$selection]=$filename From fe8ca2e3ca8ab60d3c39a5af04ddf7c4188ffd6c Mon Sep 17 00:00:00 2001 From: Chris Down Date: Fri, 6 Jan 2017 17:28:31 +0000 Subject: [PATCH 7/7] Don't mention non-existing clipmenu-fsck --- clipmenu | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clipmenu b/clipmenu index 390d2a5..5921af8 100755 --- a/clipmenu +++ b/clipmenu @@ -17,7 +17,7 @@ file=$cache_dir/$(cksum <<< "$chosen_line") if ! [[ -f "$file" ]]; then # We didn't find this in cache - printf 'FATAL: %s not in cache, run clipmenu-fsck\n' "$chosen_line" >&2 + printf 'FATAL: %s not in cache\n' "$chosen_line" >&2 exit 2 fi