From d465d9fba33a323703a2bf40c499d74d0f017091 Mon Sep 17 00:00:00 2001 From: Vratko Polak Date: Fri, 19 Oct 2018 12:21:47 +0200 Subject: [PATCH] Per patch: multiple BMRR calls This is to reduce sensitivity on testbed unstable performance. Also add topo_cleanupbefore every pybot run to avoid issues with VPP uninstall. TRACE prints are left there to simplify investigation of false positives if/when they happen. Change-Id: I9b0cdcfbbe4aa0735a0596746ac32c9e93af0897 Signed-off-by: Vratko Polak --- resources/libraries/bash/entry/per_patch_perf.sh | 34 ++++-- resources/libraries/bash/function/common.sh | 22 +++- resources/libraries/bash/function/per_patch.sh | 137 ++++++++++++----------- resources/tools/scripts/compare_perpatch.py | 81 +++++++++----- 4 files changed, 167 insertions(+), 107 deletions(-) diff --git a/resources/libraries/bash/entry/per_patch_perf.sh b/resources/libraries/bash/entry/per_patch_perf.sh index 9fb266d106..abb1001652 100644 --- a/resources/libraries/bash/entry/per_patch_perf.sh +++ b/resources/libraries/bash/entry/per_patch_perf.sh @@ -24,7 +24,7 @@ set -exuo pipefail # + Everything needed to build VPP is already installed locally. # Consequences: # + At the end, VPP repo has parent commit checked out and built. -# + Directories build_root, dpdk and csit are reset during the run. +# + Directories build_root, build and csit are reset during the run. # + The following directories (relative to VPP repo) are (re)created: # ++ csit_new, csit_parent, build_new, build_parent, # ++ archive, csit/archive, csit_download_dir. @@ -44,9 +44,10 @@ set_perpatch_vpp_dir || die build_vpp_ubuntu_amd64 "NEW" || die prepare_build_parent || die build_vpp_ubuntu_amd64 "PARENT" || die -prepare_test_new || die +prepare_test || die ## Replace previous 4 lines with this to speed up testing. -#download_builds "REPLACE_WITH_URL" || die +#download_builds "https://jenkins.fd.io/sandbox/job/vpp-csit-verify-perf-master-2n-skx/2/artifact/*zip*/archive.zip" || die +initialize_csit_dirs || die get_test_tag_string || die get_test_code "${1-}" || die set_perpatch_dut || die @@ -55,15 +56,24 @@ activate_virtualenv "${VPP_DIR}" || die reserve_testbed || die select_tags || die compose_pybot_arguments || die -check_download_dir || die -run_pybot "10" || die -copy_archives || die -die_on_pybot_error || die -prepare_test_parent || die -check_download_dir || die -run_pybot "10" || die +iterations=8 +for ((iter=0; iter "${out_file}" || { @@ -204,10 +236,9 @@ function prepare_build_parent () { mkdir -p "build_new" || die "Directory creation failed." mv "build-root"/*".deb" "build_new"/ || die "Move operation failed." # The previous build could have left some incompatible leftovers, - # e.g. DPDK artifacts of different version. - # "make -C dpdk clean" does not actually remove such .deb file. + # e.g. DPDK artifacts of different version (in build/external). # Also, there usually is a copy of dpdk artifact in build-root. - git clean -dffx "dpdk"/ "build-root"/ || die "Git clean operation failed." + git clean -dffx "build"/ "build-root"/ || die "Git clean operation failed." # Finally, check out the parent commit. git checkout HEAD~ || die "Git checkout operation failed." # Display any other leftovers. @@ -215,74 +246,48 @@ function prepare_build_parent () { } -function prepare_test_new () { +function prepare_test () { set -exuo pipefail # Variables read: # - VPP_DIR - Path to existing directory, parent of accessed directories. - # - DOWNLOAD_DIR - Path to directory where Robot takes builds to test from. - # - ARCHIVE_DIR - Path to where robot result files are created in. # Directories read: # - build-root - Existing directory with built VPP artifacts (also DPDK). # Directories updated: - # - build_parent - Old directory removed, build-root moved to become this. - # - ${DOWNLOAD_DIR} - Old content removed, files from build_new copied here. - # - csit_new - Currently a symlink to to archive robot results on failure. + # - build_parent - Old directory removed, build-root debs moved here. # Functions called: # - die - Print to stderr and exit, defined in common.sh - cd "${VPP_DIR}" || die "Change directory operationf failed." - rm -rf "build_parent" "csit_new" "${DOWNLOAD_DIR}"/* || die "Remove failed." + cd "${VPP_DIR}" || die "Change directory operation failed." + rm -rf "build_parent" || die "Remove failed." mkdir -p "build_parent" || die "Directory creation operation failed." mv "build-root"/*".deb" "build_parent"/ || die "Move operation failed." - cp "build_new"/*".deb" "${DOWNLOAD_DIR}" || die "Copy operation failed." - # Create symlinks so that if job fails on robot, results can be archived. - ln -s "${ARCHIVE_DIR}" "csit_new" || die "Symbolic link creation failed." } -function prepare_test_parent () { +function select_build () { set -exuo pipefail + # Arguments: + # - ${1} - Path to directory to copy VPP artifacts from. Required. # Variables read: - # - VPP_DIR - Path to existing directory, parent of accessed directories. - # - CSIT_DIR - Path to existing root of local CSIT git repository. - # - ARCHIVE_DIR and DOWNLOAD_DIR - Paths to directories to update. + # - DOWNLOAD_DIR - Path to directory where Robot takes builds to test from. + # - VPP_DIR - Path to existing directory, root for relative paths. # Directories read: - # - build_parent - Build artifacts (to test next) are copied from here. + # - ${1} - Existing directory with built new VPP artifacts (and DPDK). # Directories updated: - # - csit_new - Deleted, then recreated and latest robot results copied here. - # - ${CSIT_DIR} - Subjected to git reset and git clean. - # - ${ARCHIVE_DIR} - Created if not existing (if deleted by git clean). - # - ${DOWNLOAD_DIR} - Created after git clean, parent build copied here. - # - csit_parent - Currently a symlink to csit/ to archive robot results. + # - ${DOWNLOAD_DIR} - Old content removed, .deb files from ${1} copied here. # Functions called: # - die - Print to stderr and exit, defined in common.sh - # - parse_bmrr_results - See definition in this file. cd "${VPP_DIR}" || die "Change directory operation failed." - rm -rf "csit_new" "csit_parent" || die "Remove operation failed." - mkdir -p "csit_new" || die "Create directory operation failed." - for filename in "output.xml" "log.html" "report.html"; do - mv "${ARCHIVE_DIR}/${filename}" "csit_new/${filename}" || { - die "Move operation of '${filename}' failed." - } - done - parse_bmrr_results "csit_new" || { - die "The function should have died on error." - } - - pushd "${CSIT_DIR}" || die "Change directory operation failed." - git reset --hard HEAD || die "Git reset operation failed." - git clean -dffx || die "Git clean operation failed." - popd || die "Change directory operation failed." - mkdir -p "${ARCHIVE_DIR}" "${DOWNLOAD_DIR}" || die "Dir creation failed." - - cp "build_parent"/*".deb" "${DOWNLOAD_DIR}"/ || die "Copy failed." - # Create symlinks so that if job fails on robot, results can be archived. - ln -s "${ARCHIVE_DIR}" "csit_parent" || die "Symlink creation failed." + source_dir="$(readlink -e "$1")" + rm -rf "${DOWNLOAD_DIR}"/* || die "Cleanup of download dir failed." + cp "${source_dir}"/*".deb" "${DOWNLOAD_DIR}" || die "Copy operation failed." + # TODO: Is there a nice way to create symlinks, + # so that if job fails on robot, results can be archived? } diff --git a/resources/tools/scripts/compare_perpatch.py b/resources/tools/scripts/compare_perpatch.py index cc9ffd8992..1f8a1cf892 100644 --- a/resources/tools/scripts/compare_perpatch.py +++ b/resources/tools/scripts/compare_perpatch.py @@ -14,7 +14,7 @@ """Script for determining whether per-patch perf test votes -1. This script assumes there exist two text files with processed BMRR results, -located at hardcoded relative paths, having several lines +located at hardcoded relative paths (subdirs thereof), having several lines of json-parseable lists of float values, corresponding to testcase results. This script then uses jumpavg library to determine whether there was a regression, progression or no change for each testcase. @@ -38,32 +38,62 @@ def hack(value_list): :rtype: list of float """ tmp = sorted(value_list) - quarter = len(tmp) / 4 - ret = tmp[quarter:-quarter] + eight = len(tmp) / 8 + ret = tmp[3*eight:-eight] return ret -parent_lines = list() -new_lines = list() -with open("csit_parent/results.txt") as parent_file: - parent_lines = parent_file.readlines() -with open("csit_new/results.txt") as new_file: - new_lines = new_file.readlines() -if len(parent_lines) != len(new_lines): - print "Number of passed tests does not match!" - sys.exit(1) +iteration = -1 +parent_iterations = list() +new_iterations = list() +num_tests = None +while 1: + iteration += 1 + parent_lines = list() + new_lines = list() + filename = "csit_parent/{iter}/results.txt".format(iter=iteration) + try: + with open(filename) as parent_file: + parent_lines = parent_file.readlines() + except IOError: + break + num_lines = len(parent_lines) + filename = "csit_new/{iter}/results.txt".format(iter=iteration) + with open(filename) as new_file: + new_lines = new_file.readlines() + if num_lines != len(new_lines): + print "Number of tests does not match within iteration", iteration + sys.exit(1) + if num_tests is None: + num_tests = num_lines + elif num_tests != num_lines: + print "Number of tests does not match previous at iteration", iteration + sys.exit(1) + parent_iterations.append(parent_lines) + new_iterations.append(new_lines) classifier = BitCountingClassifier() -num_tests = len(parent_lines) exit_code = 0 -for index in range(num_tests): - parent_values = hack(json.loads(parent_lines[index])) - new_values = hack(json.loads(new_lines[index])) +for test_index in range(num_tests): + val_max = 1.0 + parent_values = list() + new_values = list() + for iteration_index in range(len(parent_iterations)): + parent_values.extend( + json.loads(parent_iterations[iteration_index][test_index])) + new_values.extend( + json.loads(new_iterations[iteration_index][test_index])) + print "TRACE pre-hack parent: {p}".format(p=parent_values) + print "TRACE pre-hack new: {n}".format(n=new_values) + parent_values = hack(parent_values) + new_values = hack(new_values) parent_max = BitCountingMetadataFactory.find_max_value(parent_values) new_max = BitCountingMetadataFactory.find_max_value(new_values) - cmax = max(parent_max, new_max) - factory = BitCountingMetadataFactory(cmax) + val_max = max(val_max, parent_max, new_max) + factory = BitCountingMetadataFactory(val_max) parent_stats = factory.from_data(parent_values) - factory = BitCountingMetadataFactory(cmax, parent_stats.avg) - new_stats = factory.from_data(new_values) + new_factory = BitCountingMetadataFactory(val_max, parent_stats.avg) + new_stats = new_factory.from_data(new_values) + print "TRACE parent: {p}".format(p=parent_values) + print "TRACE new: {n}".format(n=new_values) print "DEBUG parent: {p}".format(p=parent_stats) print "DEBUG new: {n}".format(n=new_stats) common_max = max(parent_stats.avg, new_stats.avg) @@ -71,15 +101,16 @@ for index in range(num_tests): print "DEBUG difference: {d}%".format(d=100 * difference) classified_list = classifier.classify([parent_stats, new_stats]) if len(classified_list) < 2: - print "Test index {index}: normal (no anomaly)".format( - index=index) + print "Test test_index {test_index}: normal (no anomaly)".format( + test_index=test_index) continue anomaly = classified_list[1].metadata.classification if anomaly == "regression": - print "Test index {index}: anomaly regression".format(index=index) + print "Test test_index {test_index}: anomaly regression".format( + test_index=test_index) exit_code = 1 continue - print "Test index {index}: anomaly {anomaly}".format( - index=index, anomaly=anomaly) + print "Test test_index {test_index}: anomaly {anomaly}".format( + test_index=test_index, anomaly=anomaly) print "DEBUG exit code {code}".format(code=exit_code) sys.exit(exit_code) -- 2.16.6