reduce use of external tools in script code

[markusschuh]

As an developer with experience in shell scripting, I want to share my knowledge of advanced scripting techniques, so that this project may win from that.

I propose to introduce more modern shell scripting techniques into the devonfw-ide code - even if that comes with some disadvantages:

Pro: More execution speed, less external dependencies, Con: Bash v3 no longer sufficient, more script skills needed for developers, code maybe harder to read

I am aware, that the balance between Pro/Con isn't a clear win to make such a change, Even when the below implementation speeds up the import of specific keys from a java properties file at about 200 times, this typically is not that relevant for the user, since even the slower implementation ( about 200 ms) is quick enough for the user. In detail the default bash in macOS is an issue, since Apple sticks to bashv3 to avoid the bashv4 license. On the other side it is easy to install a bashv4 on macOS.

Nevertheless - even when this enhancement won't be accepted - I want at least increase the awareness for those different solution algorithms. If not here it may be helpful elsewhere.

$ ./compare_read_property_implementation.sh
Execute read_properties_using_external_tools 100 times:

real    0m23.697s
user    0m8.248s
sys     0m16.902s

Execute read_properties_without_external_tools 100 times:

real    0m0.133s
user    0m0.063s
sys     0m0.063s

#!/usr/bin/env bash

projectconfig=project.properties

function read_properties_using_external_tools() {
  project_path=$(grep "^path" "$projectconfig" | cut -d'=' -f2)
  project_workingsets=$(grep "^workingsets" "$projectconfig" | cut -d'=' -f2)
  project_workspace=$(grep "^workspace" "$projectconfig" | cut -d'=' -f2)
  project_git_url=$(grep "^git.url" "$projectconfig" | cut -d'=' -f2)
  project_git_branch=$(grep "^git.branch" "$projectconfig" | cut -d'=' -f2)
  project_eclipse=$(grep "^eclipse" "$projectconfig" | cut -d'=' -f2)
}

function read_properties_without_external_tools() {
  declare -A project     # associative array needs bashv4
  while IFS='=' read -r key val; do
    if [[ $key =~ ^(path|workingsets|workspace|git\.url|git\.branch|eclipse)$ ]]; then
      project[$key]="$val"
    fi
  done < "$projectconfig"
}

i=100
echo Execute 'read_properties_using_external_tools' $i times:
time while ((i--)); do
  read_properties_using_external_tools
done
echo

i=100
echo Execute 'read_properties_without_external_tools' $i times:
time while ((i--)); do
  read_properties_without_external_tools
done

content of the used sample project.properties

path=/some_path
workspace=some_workspace
workingsets="1 2 3"
git.uri=http://localhost
git.url=http://localhost
git.branch=master
git.branch=main
eclipse=test

[hohwille]

@markusschuh thanks for this feedback. I also agree that the current way we are reading properties (devon.properties or «project».properties) sucks and is slow. Your improvements to boost performance with factor 200 are very significant.

What is important to me however, is to have the pre-requisites as minimal as possible. Could we even do a trade-off and detect if bashv4 is available and then user performance-optimized code but still keep support for bashv3 (and optionally print out a log message that upgrade to bashv4 is recommended and will boost performance)?

[hohwille]

Actually looking at «project».properties this does not really matter that much. These are only parsed during setup or explicit devon ide update projects or devon ide update all. Compared to the actual project setup with maven build and eclipse import taking up to ten minutes a boost of 23 seconds is IMHO neglectable. At least if it comes with the disadvantage of increasing pre-requisites (and according support effort for stupid questions and error reports). If we can boost the reading of all the devon.properties what happens with EVERY devon command all the time, then I think it is worth some invest.

[markusschuh]

Execution speed is only one aspect, why use of external tools should be kept as small as possible. There is also a small risk of different syntax between the BSD-originating tools on macOS vs. the GNU based on other supported environments. Granted - by restricting to the base options the usage is compatible most often.

And with respect to bashv3, there is the additional issue, that use of eval is potentially risky. (E.g. add a line "date; testvar=1" to devon.properties) For that reason it is a bit sad, that only the old bashv3 in macOS forces to stick on older script code. Eval - for the used purpose of reading variables - can be replaced there, too. But this is a bit more complex, compare https://stackoverflow.com/questions/17529220/why-should-eval-be-avoided-in-bash-and-what-should-i-use-instead

Wrt to speed: The boost in this example is 23 seconds for 100 (!) iterations - so of course this is neglectable for a single run - nevertheless worth to mention it. At least, it has made you mention, that read of properties files is done on each devon run.

So let's as least speed up this a bit ( 200 ms less ) by changing

value="$(echo -n "$value" | tr -d '\r')"

with

value="${value//[$'\r']}"

But while I am testing this code replacement, I realise some other small issue with the current code: When any of the read properties files has Windows line endings, empty lines are not considered empty and the code triggers an

unset "$'\r'"

which yields

unset: `^M': not a valid identifier

I'll work on a PR for this.

[hohwille]

@markusschuh I meanwhile implemented PR #383 Can you confirm that this solves the problem and close this issue? Otherwise just comment or provide another PR...