From 746181cb33776075576a7c693229b253b4aeb605 Mon Sep 17 00:00:00 2001 From: Edward Hennis Date: Tue, 8 Dec 2020 12:46:12 -0500 Subject: [PATCH] Analyze and document levelization: * Markdown explanation of what levelization is, the intended levels, as well as the process used to determine dependencies * Shell script finds all dependencies, groups them, and finds cyclic dependencies and maps out non-cyclic dependencies. * Github job to run the script and fail if anything changes. Should catch introduction of new dependencies and new problems. Will also detect changes if problems or dependencies are removed. --- .github/workflows/levelization.yml | 18 ++ .gitignore | 6 + Builds/levelization/README.md | 109 +++++++++++ Builds/levelization/levelization.sh | 127 +++++++++++++ Builds/levelization/results/loops.txt | 54 ++++++ Builds/levelization/results/ordering.txt | 219 +++++++++++++++++++++++ 6 files changed, 533 insertions(+) create mode 100644 .github/workflows/levelization.yml create mode 100644 Builds/levelization/README.md create mode 100755 Builds/levelization/levelization.sh create mode 100644 Builds/levelization/results/loops.txt create mode 100644 Builds/levelization/results/ordering.txt diff --git a/.github/workflows/levelization.yml b/.github/workflows/levelization.yml new file mode 100644 index 000000000..3d2eed84d --- /dev/null +++ b/.github/workflows/levelization.yml @@ -0,0 +1,18 @@ +name: levelization + +on: [push, pull_request] + +jobs: + check: + runs-on: ubuntu-18.04 + env: + CLANG_VERSION: 10 + steps: + - uses: actions/checkout@v2 + - name: Check levelization + run: Builds/levelization/levelization.sh + - name: Check for differences + run: git diff --exit-code + # If this workflow fails, and you have improved levelization, run + # Builds/levelization/levelization.sh, and commit the changes to + # loops.txt. diff --git a/.gitignore b/.gitignore index f6ed188fb..23d896b8e 100644 --- a/.gitignore +++ b/.gitignore @@ -37,6 +37,12 @@ Release/*.* *.gcda *.gcov +# Levelization checking +Builds/levelization/results/rawincludes.txt +Builds/levelization/results/paths.txt +Builds/levelization/results/includes/ +Builds/levelization/results/includedby/ + # Ignore tmp directory. tmp diff --git a/Builds/levelization/README.md b/Builds/levelization/README.md new file mode 100644 index 000000000..147fb56b4 --- /dev/null +++ b/Builds/levelization/README.md @@ -0,0 +1,109 @@ +# Levelization + +Levelization is the term used to describe efforts to prevent rippled from +having or creating cyclic dependencies. + +rippled code is organized into directories under `src/rippled` (and +`src/test`) representing modules. The modules are intended to be +organized into "tiers" or "levels" such that a module from one level can +only include code from lower levels. Additionally, a module +in one level should never include code in an `impl` folder of any level +other than it's own. + +Unfortunately, over time, enforcement of levelization has been +inconsistent, so the current state of the code doesn't necessarily +reflect these rules. Whenever possible, developers should refactor any +levelization violations they find (by moving files or individual +classes). At the very least, don't make things worse. + +The table below summarizes the _desired_ division of modules. The levels +are numbered from the bottom up with the lower level, lower numbered, +more independent modules listed first, and the higher level, higher +numbered modules with more dependencies listed later. + +| Level / Tier | Module(s) | +|--------------|-----------------------------------------------| +| 01 | ripple/beast ripple/unity +| 02 | ripple/basics +| 03 | ripple/json ripple/crypto +| 04 | ripple/protocol +| 05 | ripple/core ripple/conditions ripple/consensus ripple/resource ripple/server +| 06 | ripple/peerfinder ripple/ledger ripple/nodestore ripple/net +| 07 | ripple/shamap ripple/overlay +| 08 | ripple/app +| 09 | ripple/rpc +| 10 | test/jtx test/beast test/csf +| 11 | test/unit_test +| 12 | test/crypto test/conditions test/json test/resource test/shamap test/peerfinder test/basics test/overlay +| 13 | test +| 14 | test/net test/protocol test/ledger test/consensus test/core test/server test/nodestore +| 15 | test/rpc test/app + +(Note that `test` levelization is *much* less important and *much* less +strictly enforced than `ripple` levelization, other than the requirement +that `test` code should *never* be included in `ripple` code.) + +## Validation + +The [levelization.sh](levelization.sh) script takes no parameters, +reads no environment variables, and can be run from any directory, +as long as it is in the expected location in the rippled repo. +It can be run at any time from within a checked out repo, and will +do an analysis of all the `#include`s in +the rippled source. The only caveat is that it runs much slower +under Windows than in Linux. It hasn't yet been tested under MacOS. +It generates many files of [results](results): + +* `rawincludes.txt`: The raw dump of the `#includes` +* `paths.txt`: A second dump grouping the source module + to the destination module, deduped, and with frequency counts. +* `includes/`: A directory where each file represents a module and + contains a list of modules and counts that the module _includes_. +* `includedby/`: Similar to `includes/`, but the other way around. Each + file represents a module and contains a list of modules and counts + that _include_ the module. +* [`loops.txt`](results/loops.txt): A list of direct loops detected + between modules as they actually exist, as opposed to how they are + desired as described above. In a perfect repo, this file will be + empty. + This file is committed to the repo, and is used by the [levelization + Github workflow](../../.github/workflows/levelization.yml) to validate + that nothing changed. +* [`ordering.txt`](results/ordering.txt): A list showing relationships + between modules where there are no loops as they actually exist, as + opposed to how they are desired as described above. + This file is committed to the repo, and is used by the [levelization + Github workflow](../../.github/workflows/levelization.yml) to validate + that nothing changed. +* [`levelization.yml`](../../.github/workflows/levelization.yml) + Github Actions workflow to test that levelization loops haven't + changed. Unfortunately, if changes are detected, it can't tell if + they are improvements or not, so if you have resolved any issues or + done anything else to improve levelization, run `levelization.sh`, + and commit the updated results. + +The `loops.txt` and `ordering.txt` files relate the modules +using comparison signs, which indicate the number of times each +module is included in the other. + +* `A > B` means that A should probably be at a higher level than B, + because B is included in A significantly more than A is included in B. + These results can be included in both `loops.txt` and `ordering.txt`. + Because `ordering.txt`only includes relationships where B is not + included in A at all, it will only include these types of results. +* `A ~= B` means that A and B are included in each other a different + number of times, but the values are so close that the script can't + definitively say that one should be above the other. These results + will only be included in `loops.txt`. +* `A == B` means that A and B include each other the same number of + times, so the script has no clue which should be higher. These results + will only be included in `loops.txt`. + +The committed files hide the detailed values intentionally, to +prevent false alarms and merging issues, and because it's easy to +get those details locally. + +1. Run `levelization.sh` +2. Grep the modules in `paths.txt`. + * For example, if a cycle is found `A ~= B`, simply `grep -w + A Builds/levelization/results/paths.txt | grep -w B` diff --git a/Builds/levelization/levelization.sh b/Builds/levelization/levelization.sh new file mode 100755 index 000000000..34487f746 --- /dev/null +++ b/Builds/levelization/levelization.sh @@ -0,0 +1,127 @@ +#!/bin/bash + +# Usage: levelization.sh +# This script takes no parameters, reads no environment variables, +# and can be run from any directory, as long as it is in the expected +# location in the repo. + +pushd $( dirname $0 ) + +if [ -v PS1 ] +then + # if the shell is interactive, clean up any flotsam before analyzing + git clean -ix +fi + +rm -rfv results +mkdir results +includes="$( pwd )/results/rawincludes.txt" +pushd ../.. +echo Raw includes: +grep -r '#include.*/.*\.h' src/ripple/ src/test/ | \ + grep -v boost | tee ${includes} +popd +pushd results + +oldifs=${IFS} +IFS=: +mkdir includes +mkdir includedby +echo Build levelization paths +exec 3< ${includes} # open rawincludes.txt for input +while read -r -u 3 file include +do + level=$( echo ${file} | cut -d/ -f 2,3 ) + # If the "level" indicates a file, cut off the filename + if [[ "${level##*.}" != "${level}" ]] + then + # Use the "toplevel" label as a workaround for `sort` + # inconsistencies between different utility versions + level="$( dirname ${level} )/toplevel" + fi + level=$( echo ${level} | tr '/' '.' ) + + includelevel=$( echo ${include} | sed 's/.*["<]//; s/[">].*//' | \ + cut -d/ -f 1,2 ) + if [[ "${includelevel##*.}" != "${includelevel}" ]] + then + # Use the "toplevel" label as a workaround for `sort` + # inconsistencies between different utility versions + includelevel="$( dirname ${includelevel} )/toplevel" + fi + includelevel=$( echo ${includelevel} | tr '/' '.' ) + + if [[ "$level" != "$includelevel" ]] + then + echo $level $includelevel | tee -a paths.txt + fi +done +echo Sort and dedup paths +sort -ds paths.txt | uniq -c | tee sortedpaths.txt +mv sortedpaths.txt paths.txt +exec 3>&- #close fd 3 +IFS=${oldifs} +unset oldifs + +echo Split into flat-file database +exec 4&- #close fd 4 + +loops="$( pwd )/loops.txt" +ordering="$( pwd )/ordering.txt" +pushd includes +echo Search for loops +# Redirect stdout to a file +exec 4>&1 +exec 1>"${loops}" +for source in * +do + if [[ -f "$source" ]] + then + exec 5<"${source}" # open for input + while read -r -u 5 include includefreq + do + if [[ -f $include ]] + then + if grep -q -w $source $include + then + if grep -q -w "Loop: $include $source" "${loops}" + then + continue + fi + sourcefreq=$( grep -w $source $include | cut -d\ -f2 ) + echo "Loop: $source $include" + # If the counts are close, indicate that the two modules are + # on the same level, though they shouldn't be + if [[ $(( $includefreq - $sourcefreq )) -gt 3 ]] + then + echo -e " $source > $include\n" + elif [[ $(( $sourcefreq - $includefreq )) -gt 3 ]] + then + echo -e " $include > $source\n" + elif [[ $sourcefreq -eq $includefreq ]] + then + echo -e " $include == $source\n" + else + echo -e " $include ~= $source\n" + fi + else + echo "$source > $include" >> "${ordering}" + fi + fi + done + exec 5>&- #close fd 5 + fi +done +exec 1>&4 #close fd 1 +exec 4>&- #close fd 4 +cat "${ordering}" +cat "${loops}" +popd +popd +popd diff --git a/Builds/levelization/results/loops.txt b/Builds/levelization/results/loops.txt new file mode 100644 index 000000000..2afe4f51c --- /dev/null +++ b/Builds/levelization/results/loops.txt @@ -0,0 +1,54 @@ +Loop: ripple.app ripple.core + ripple.app > ripple.core + +Loop: ripple.app ripple.ledger + ripple.app > ripple.ledger + +Loop: ripple.app ripple.net + ripple.app > ripple.net + +Loop: ripple.app ripple.nodestore + ripple.app > ripple.nodestore + +Loop: ripple.app ripple.overlay + ripple.overlay ~= ripple.app + +Loop: ripple.app ripple.rpc + ripple.rpc > ripple.app + +Loop: ripple.app ripple.shamap + ripple.app > ripple.shamap + +Loop: ripple.basics ripple.core + ripple.core > ripple.basics + +Loop: ripple.basics ripple.json + ripple.json ~= ripple.basics + +Loop: ripple.basics ripple.protocol + ripple.protocol > ripple.basics + +Loop: ripple.basics ripple.rpc + ripple.rpc > ripple.basics + +Loop: ripple.core ripple.net + ripple.net > ripple.core + +Loop: ripple.crypto ripple.protocol + ripple.protocol > ripple.crypto + +Loop: ripple.net ripple.rpc + ripple.rpc > ripple.net + +Loop: ripple.nodestore ripple.overlay + ripple.overlay == ripple.nodestore + +Loop: ripple.overlay ripple.rpc + ripple.rpc ~= ripple.overlay + +Loop: test.jtx test.toplevel + test.toplevel > test.jtx + +Loop: test.jtx test.unit_test + test.unit_test == test.jtx + diff --git a/Builds/levelization/results/ordering.txt b/Builds/levelization/results/ordering.txt new file mode 100644 index 000000000..87501a599 --- /dev/null +++ b/Builds/levelization/results/ordering.txt @@ -0,0 +1,219 @@ +ripple.app > ripple.basics +ripple.app > ripple.beast +ripple.app > ripple.conditions +ripple.app > ripple.consensus +ripple.app > ripple.crypto +ripple.app > ripple.json +ripple.app > ripple.protocol +ripple.app > ripple.resource +ripple.app > test.unit_test +ripple.basics > ripple.beast +ripple.conditions > ripple.basics +ripple.conditions > ripple.protocol +ripple.consensus > ripple.basics +ripple.consensus > ripple.beast +ripple.consensus > ripple.json +ripple.consensus > ripple.protocol +ripple.core > ripple.beast +ripple.core > ripple.json +ripple.core > ripple.protocol +ripple.crypto > ripple.basics +ripple.json > ripple.beast +ripple.ledger > ripple.basics +ripple.ledger > ripple.beast +ripple.ledger > ripple.core +ripple.ledger > ripple.json +ripple.ledger > ripple.protocol +ripple.net > ripple.basics +ripple.net > ripple.beast +ripple.net > ripple.json +ripple.net > ripple.protocol +ripple.net > ripple.resource +ripple.nodestore > ripple.basics +ripple.nodestore > ripple.beast +ripple.nodestore > ripple.core +ripple.nodestore > ripple.protocol +ripple.nodestore > ripple.unity +ripple.overlay > ripple.basics +ripple.overlay > ripple.beast +ripple.overlay > ripple.core +ripple.overlay > ripple.json +ripple.overlay > ripple.peerfinder +ripple.overlay > ripple.protocol +ripple.overlay > ripple.resource +ripple.overlay > ripple.server +ripple.peerfinder > ripple.basics +ripple.peerfinder > ripple.beast +ripple.peerfinder > ripple.core +ripple.peerfinder > ripple.protocol +ripple.protocol > ripple.beast +ripple.protocol > ripple.json +ripple.resource > ripple.basics +ripple.resource > ripple.beast +ripple.resource > ripple.json +ripple.resource > ripple.protocol +ripple.rpc > ripple.beast +ripple.rpc > ripple.core +ripple.rpc > ripple.crypto +ripple.rpc > ripple.json +ripple.rpc > ripple.ledger +ripple.rpc > ripple.nodestore +ripple.rpc > ripple.protocol +ripple.rpc > ripple.resource +ripple.rpc > ripple.server +ripple.rpc > ripple.shamap +ripple.server > ripple.basics +ripple.server > ripple.beast +ripple.server > ripple.crypto +ripple.server > ripple.json +ripple.server > ripple.protocol +ripple.shamap > ripple.basics +ripple.shamap > ripple.beast +ripple.shamap > ripple.crypto +ripple.shamap > ripple.nodestore +ripple.shamap > ripple.protocol +test.app > ripple.app +test.app > ripple.basics +test.app > ripple.beast +test.app > ripple.core +test.app > ripple.json +test.app > ripple.ledger +test.app > ripple.overlay +test.app > ripple.protocol +test.app > ripple.resource +test.app > ripple.rpc +test.app > test.jtx +test.app > test.rpc +test.app > test.toplevel +test.app > test.unit_test +test.basics > ripple.basics +test.basics > ripple.beast +test.basics > ripple.json +test.basics > ripple.protocol +test.basics > ripple.rpc +test.basics > test.jtx +test.basics > test.unit_test +test.beast > ripple.basics +test.beast > ripple.beast +test.conditions > ripple.basics +test.conditions > ripple.beast +test.conditions > ripple.conditions +test.consensus > ripple.app +test.consensus > ripple.basics +test.consensus > ripple.beast +test.consensus > ripple.consensus +test.consensus > ripple.ledger +test.consensus > ripple.rpc +test.consensus > test.csf +test.consensus > test.toplevel +test.consensus > test.unit_test +test.core > ripple.basics +test.core > ripple.beast +test.core > ripple.core +test.core > ripple.crypto +test.core > ripple.json +test.core > ripple.server +test.core > test.jtx +test.core > test.toplevel +test.core > test.unit_test +test.crypto > ripple.beast +test.crypto > ripple.crypto +test.csf > ripple.basics +test.csf > ripple.beast +test.csf > ripple.consensus +test.csf > ripple.json +test.csf > ripple.protocol +test.json > ripple.beast +test.json > ripple.json +test.json > test.jtx +test.jtx > ripple.app +test.jtx > ripple.basics +test.jtx > ripple.beast +test.jtx > ripple.consensus +test.jtx > ripple.core +test.jtx > ripple.json +test.jtx > ripple.ledger +test.jtx > ripple.net +test.jtx > ripple.protocol +test.jtx > ripple.server +test.ledger > ripple.app +test.ledger > ripple.basics +test.ledger > ripple.beast +test.ledger > ripple.core +test.ledger > ripple.ledger +test.ledger > ripple.protocol +test.ledger > test.jtx +test.ledger > test.toplevel +test.net > ripple.net +test.net > test.jtx +test.net > test.toplevel +test.net > test.unit_test +test.nodestore > ripple.app +test.nodestore > ripple.basics +test.nodestore > ripple.beast +test.nodestore > ripple.core +test.nodestore > ripple.nodestore +test.nodestore > ripple.unity +test.nodestore > test.jtx +test.nodestore > test.toplevel +test.nodestore > test.unit_test +test.overlay > ripple.app +test.overlay > ripple.basics +test.overlay > ripple.beast +test.overlay > ripple.core +test.overlay > ripple.overlay +test.overlay > ripple.protocol +test.overlay > ripple.shamap +test.overlay > test.jtx +test.overlay > test.unit_test +test.peerfinder > ripple.basics +test.peerfinder > ripple.beast +test.peerfinder > ripple.core +test.peerfinder > ripple.peerfinder +test.peerfinder > ripple.protocol +test.peerfinder > test.beast +test.peerfinder > test.unit_test +test.protocol > ripple.basics +test.protocol > ripple.beast +test.protocol > ripple.crypto +test.protocol > ripple.json +test.protocol > ripple.protocol +test.protocol > test.toplevel +test.resource > ripple.basics +test.resource > ripple.beast +test.resource > ripple.resource +test.resource > test.unit_test +test.rpc > ripple.app +test.rpc > ripple.basics +test.rpc > ripple.beast +test.rpc > ripple.core +test.rpc > ripple.json +test.rpc > ripple.net +test.rpc > ripple.nodestore +test.rpc > ripple.overlay +test.rpc > ripple.protocol +test.rpc > ripple.resource +test.rpc > ripple.rpc +test.rpc > test.jtx +test.rpc > test.nodestore +test.rpc > test.toplevel +test.server > ripple.app +test.server > ripple.basics +test.server > ripple.beast +test.server > ripple.core +test.server > ripple.json +test.server > ripple.rpc +test.server > ripple.server +test.server > test.jtx +test.server > test.toplevel +test.server > test.unit_test +test.shamap > ripple.basics +test.shamap > ripple.beast +test.shamap > ripple.nodestore +test.shamap > ripple.protocol +test.shamap > ripple.shamap +test.shamap > test.unit_test +test.toplevel > ripple.json +test.toplevel > test.csf +test.unit_test > ripple.basics +test.unit_test > ripple.beast